[wildfly-dev] JCA service/model names a.k.a WFLY-1656

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[wildfly-dev] JCA service/model names a.k.a WFLY-1656

Bartosz Baranowski
Hey

Finally I was able ( I think ) to nail the problem and forge fix. There are still some intermittent CI failures, but other than that CI looks clear.
While I polish last parts it would be good to get feedback on changes, since impact on JCA integration is quite big.

Fix removes all static/leaky code from ConnectorServices ( add relevant code to MSC service ), move away from static hashmap checks in favor of MSC service present/deployment/dependency and remove ability to create more than one activation for distinct model address.

Branch:
https://github.com/baranowb/wildfly/tree/WFLY-1656

Relevant issues:
https://issues.jboss.org/browse/WFLY-1656
https://issues.jboss.org/browse/WFLY-1492  -> https://bugzilla.redhat.com/show_bug.cgi?id=964202
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Jesper Pedersen
Hi,

On 08/06/2013 09:29 AM, Bartosz Baranowski wrote:
> Finally I was able ( I think ) to nail the problem and forge fix. There are still some intermittent CI failures, but other than that CI looks clear.
> While I polish last parts it would be good to get feedback on changes, since impact on JCA integration is quite big.
>
> Fix removes all static/leaky code from ConnectorServices ( add relevant code to MSC service ), move away from static hashmap checks in favor of MSC service present/deployment/dependency and remove ability to create more than one activation for distinct model address.
>

Amazing that a patch like this can be written without asking for input
from the existing contributors for the subsystems in question.

And, are you even sure that the all use-cases are covered ? Like
multiple activations of the same resource adapter, and other setups that
are used in production environments. Well, asking for feedback up-front
could have helped.

Anyway, Stefano can take a look at this. If you want feedback before
that you know where to find us.

Best regards,
  Jesper

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Bartosz Baranowski


----- Original Message -----

> From: "Jesper Pedersen" <[hidden email]>
> To: [hidden email]
> Sent: Tuesday, August 6, 2013 4:04:40 PM
> Subject: Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656
>
> Hi,
>
> On 08/06/2013 09:29 AM, Bartosz Baranowski wrote:
> > Finally I was able ( I think ) to nail the problem and forge fix. There are
> > still some intermittent CI failures, but other than that CI looks clear.
> > While I polish last parts it would be good to get feedback on changes,
> > since impact on JCA integration is quite big.
> >
> > Fix removes all static/leaky code from ConnectorServices ( add relevant
> > code to MSC service ), move away from static hashmap checks in favor of
> > MSC service present/deployment/dependency and remove ability to create
> > more than one activation for distinct model address.
> >
>
> Amazing that a patch like this can be written without asking for input
> from the existing contributors for the subsystems in question.

Well, I've talked with JCA guys and I've been told that it's according to design and it's all good. However, there is JIRA, which has been validated by David/Carlo. Yet only feedback, from JCA team, on  subject is:
 - "there must be public discussion before JIRA is filed" ( JIRA is public btw )
 - "WontFix" without any reason ( iirc issue has been closed twice ) - so it did not slip through fingers like a stealthy ninja, it has been ignored.

So well yeah, its amazing such patch has been created. Just to point out, such deprecatory tone in light of above, is out of place.


>
> And, are you even sure that the all use-cases are covered ? Like
> multiple activations of the same resource adapter, and other setups that
> are used in production environments. Well, asking for feedback up-front
> could have helped.
>
> Anyway, Stefano can take a look at this. If you want feedback before
> that you know where to find us.
>
> Best regards,
>   Jesper
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Jesper Pedersen
On 08/06/2013 10:48 AM, Bartosz Baranowski wrote:
> Well, I've talked with JCA guys and I've been told that it's according to design and it's all good.

I have never said there isn't an issue; providing a test case that
showed the problem could have been of benefit.

> However, there is JIRA, which has been validated by David/Carlo. Yet only feedback, from JCA team, on  subject is:
>   - "there must be public discussion before JIRA is filed" ( JIRA is public btw )

Yes, create a forum discussion for things that you aren't sure why they
are done that way such that the use-cases can be discussed.

>   - "WontFix" without any reason ( iirc issue has been closed twice ) - so it did not slip through fingers like a stealthy ninja, it has been ignored.
>

See above. Having a clear documented path on how things should work, and
potentially why they don't leads to clear and precise JIRAs.

> So well yeah, its amazing such patch has been created. Just to point out, such deprecatory tone in light of above, is out of place.

Welcome to the open source world.

Best regards,
  Jesper

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

jtgreene
Administrator

On Aug 6, 2013, at 10:06 AM, Jesper Pedersen <[hidden email]> wrote:

> On 08/06/2013 10:48 AM, Bartosz Baranowski wrote:
>> Well, I've talked with JCA guys and I've been told that it's according to design and it's all good.
>
> I have never said there isn't an issue; providing a test case that
> showed the problem could have been of benefit.


I remember glancing at this one with Carlo when we were in Newcastle. The problem is that when there are duplicate names an id is generated. However the problem is that that ID is generated using a static map. It looks like everything is fine in standalone because there is only one subsystem to parse. However domain mode has multiple profiles, so you have multiple JCA subsystems that get parsed, and therefore you get nondeterministic generation. Also I believe I spotted a problem with the serialization code.

The general things that need to happen are

1. Get rid of static state, and instead store the state relative to the subsystem block
2. Double check the marshaling
3. Console needs to be updated to take an "id" on creation (HAL issue needed for that one)

--
Jason T. Greene
WildFly Lead / JBoss EAP Platform Architect
JBoss, a division of Red Hat


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Jesper Pedersen
On 08/06/2013 11:55 AM, Jason Greene wrote:
> I remember glancing at this one with Carlo when we were in Newcastle. The problem is that when there are duplicate names an id is generated. However the problem is that that ID is generated using a static map. It looks like everything is fine in standalone because there is only one subsystem to parse. However domain mode has multiple profiles, so you have multiple JCA subsystems that get parsed, and therefore you get nondeterministic generation. Also I believe I spotted a problem with the serialization code.
>
> The general things that need to happen are
>
> 1. Get rid of static state, and instead store the state relative to the subsystem block
> 2. Double check the marshaling
> 3. Console needs to be updated to take an "id" on creation (HAL issue needed for that one)
>

Ok. However, there is a customer request precisely on this attribute
that needs to be taken into account.

Best regards,
  Jesper


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Bartosz Baranowski


----- Original Message -----

> From: "Jesper Pedersen" <[hidden email]>
> To: [hidden email]
> Sent: Wednesday, August 7, 2013 11:04:40 AM
> Subject: Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656
>
> On 08/06/2013 11:55 AM, Jason Greene wrote:
> > I remember glancing at this one with Carlo when we were in Newcastle. The
> > problem is that when there are duplicate names an id is generated. However
> > the problem is that that ID is generated using a static map. It looks like
> > everything is fine in standalone because there is only one subsystem to

Not quite. If you add activation of RA in config it will change IDs of subsequent services/model names. Also if you do this from CLI/GUI level
you cant determine which ID will be assigned ( ie. if one of activation in chain has been removed )

> > parse. However domain mode has multiple profiles, so you have multiple JCA
> > subsystems that get parsed, and therefore you get nondeterministic
> > generation. Also I believe I spotted a problem with the serialization
> > code.

True, its even worse if you consider what I've written above.

> >
> > The general things that need to happen are
> >
> > 1. Get rid of static state, and instead store the state relative to the
> > subsystem block
> > 2. Double check the marshaling
> > 3. Console needs to be updated to take an "id" on creation (HAL issue
> > needed for that one)

Console fix is there, Heiko is just waiting for this issue to be fixed. AS7 has the ID parameter and from discussion Ive had, its should be perfectly fine to enforce use of it.

> >
>
> Ok. However, there is a customer request precisely on this attribute
> that needs to be taken into account.
>
> Best regards,
>   Jesper
>
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Stefano Maestri
In reply to this post by Bartosz Baranowski
Hi,

I've started to review this patch and at a first look can't work as is.
It needs at a first look at least changes to resource-adapters xsd,
because it bases service names on "id" attribute, but actually this
attribute is not mandatory in xsd. In case you define a resource-adapter
configuration in standalone.xml for a rar having an internal config with
ironjacamar.xml It drives to runtime error like this:

14:47:27,561 ERROR [org.jboss.msc.service.fail] (MSC service thread 1-7)
MSC000001: Failed to start service jboss.ra.deployment."ra16outij.rar":
org.jboss.msc.service.StartException in service
jboss.ra.deployment."ra16outij.rar":
org.jboss.msc.service.DuplicateServiceException: Service
jboss.ra.ra16outij is already registered

Or even defining 2 resource-adapter in standaole xml on the same rar
archive, w/o defining id as xsd permit:
14:49:59,574 ERROR [org.jboss.as.controller.management-operation]
(ServerService Thread Pool -- 11) JBAS014613: Operation ("add") failed -
address: ([
     ("subsystem" => "resource-adapters"),
     ("resource-adapter" => "ra16outij.rar")
]) - failure description: "JBAS014803: Duplicate resource [
     (\"subsystem\" => \"resource-adapters\"),
     (\"resource-adapter\" => \"ra16outij.rar\")
]"

Both this use case are normal and very frequent because resource-adapter
defined in standalone.xml should be considered multiple configuration
(activation in jca slang) on the same deployed rar. I still have to
cover all use cases, but those 2 at least needs to be fixed. Probably
the easiest solution is to make "id" mandatory, but I have to double
check it is sufficient for programmatic activations (explicit call of
service like HornetQ is doing) and ironjacamar.xml ones.
If you agree I'll take the issue and I'll merge your patch on my branch
and rework it a bit to cover all those use cases and possibly to solve
also WFLY-1776 (the problem here could be that xsd changes can't be
backported and it is a EAP case)

regards
S.


On 08/06/2013 03:29 PM, Bartosz Baranowski wrote:

> Hey
>
> Finally I was able ( I think ) to nail the problem and forge fix. There are still some intermittent CI failures, but other than that CI looks clear.
> While I polish last parts it would be good to get feedback on changes, since impact on JCA integration is quite big.
>
> Fix removes all static/leaky code from ConnectorServices ( add relevant code to MSC service ), move away from static hashmap checks in favor of MSC service present/deployment/dependency and remove ability to create more than one activation for distinct model address.
>
> Branch:
> https://github.com/baranowb/wildfly/tree/WFLY-1656
>
> Relevant issues:
> https://issues.jboss.org/browse/WFLY-1656
> https://issues.jboss.org/browse/WFLY-1492  -> https://bugzilla.redhat.com/show_bug.cgi?id=964202
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Bartosz Baranowski
Hi,

----- Original Message -----

> From: "Stefano Maestri" <[hidden email]>
> To: [hidden email]
> Sent: Monday, August 19, 2013 3:09:41 PM
> Subject: Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656
>
> Hi,
>
> I've started to review this patch and at a first look can't work as is.
> It needs at a first look at least changes to resource-adapters xsd,
> because it bases service names on "id" attribute, but actually this
> attribute is not mandatory in xsd. In case you define a resource-adapter
> configuration in standalone.xml for a rar having an internal config with
> ironjacamar.xml It drives to runtime error like this:
>
> 14:47:27,561 ERROR [org.jboss.msc.service.fail] (MSC service thread 1-7)
> MSC000001: Failed to start service jboss.ra.deployment."ra16outij.rar":
> org.jboss.msc.service.StartException in service
> jboss.ra.deployment."ra16outij.rar":
> org.jboss.msc.service.DuplicateServiceException: Service
> jboss.ra.ra16outij is already registered
>

 Thats expected error, since such configuration/setup leads to same resource being spawned at runtime.

> Or even defining 2 resource-adapter in standaole xml on the same rar
> archive, w/o defining id as xsd permit:
> 14:49:59,574 ERROR [org.jboss.as.controller.management-operation]
> (ServerService Thread Pool -- 11) JBAS014613: Operation ("add") failed -
> address: ([
>      ("subsystem" => "resource-adapters"),
>      ("resource-adapter" => "ra16outij.rar")
> ]) - failure description: "JBAS014803: Duplicate resource [
>      (\"subsystem\" => \"resource-adapters\"),
>      (\"resource-adapter\" => \"ra16outij.rar\")
> ]"
>
> Both this use case are normal and very frequent because resource-adapter
> defined in standalone.xml should be considered multiple configuration
> (activation in jca slang) on the same deployed rar.

Actually I wasnt able to grasp this to full extent - if this kind of anonymous resource
definition is a requirement from specs or something forged in IJ or AS7/8 itself.

I still have to
> cover all use cases, but those 2 at least needs to be fixed. Probably
> the easiest solution is to make "id" mandatory, but I have to double
> check it is sufficient for programmatic activations (explicit call of
> service like HornetQ is doing) and ironjacamar.xml ones.

If its not covered in testsuite or it did not make server explode, I might missed it.

> If you agree I'll take the issue and I'll merge your patch on my branch
> and rework it a bit to cover all those use cases and possibly to solve
> also WFLY-1776 (the problem here could be that xsd changes can't be
> backported and it is a EAP case)

What I was led to believe is that this step is not required. It is perfectly
fine if user has "anonymous" activation, without id, as long as there is only one.
In case there is need for more than one activation, users need to provide proper configuration, that is
provide ids.
WRT hornetq, if Im not wrong, each activation from hornetq subsystem has unique name, since
each config entry has it unique( I may be off here though ).

Also, Im not sure about MDB annotation, this part of integration looked a bit fuzzy.
Existing tests pass, but Im not quite sure how this should look like for "IDed" configs.

If you want to redo something feel free to mold it.

Thanks.

>
> regards
> S.
>
>
> On 08/06/2013 03:29 PM, Bartosz Baranowski wrote:
> > Hey
> >
> > Finally I was able ( I think ) to nail the problem and forge fix. There are
> > still some intermittent CI failures, but other than that CI looks clear.
> > While I polish last parts it would be good to get feedback on changes,
> > since impact on JCA integration is quite big.
> >
> > Fix removes all static/leaky code from ConnectorServices ( add relevant
> > code to MSC service ), move away from static hashmap checks in favor of
> > MSC service present/deployment/dependency and remove ability to create
> > more than one activation for distinct model address.
> >
> > Branch:
> > https://github.com/baranowb/wildfly/tree/WFLY-1656
> >
> > Relevant issues:
> > https://issues.jboss.org/browse/WFLY-1656
> > https://issues.jboss.org/browse/WFLY-1492  ->
> > https://bugzilla.redhat.com/show_bug.cgi?id=964202
> > _______________________________________________
> > wildfly-dev mailing list
> > [hidden email]
> > https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656

Stefano Maestri
On 08/21/2013 08:35 AM, Bartosz Baranowski wrote:

> Hi,
>
> ----- Original Message -----
>> From: "Stefano Maestri" <[hidden email]>
>> To: [hidden email]
>> Sent: Monday, August 19, 2013 3:09:41 PM
>> Subject: Re: [wildfly-dev] JCA service/model names a.k.a WFLY-1656
>>
>> Hi,
>>
>> I've started to review this patch and at a first look can't work as is.
>> It needs at a first look at least changes to resource-adapters xsd,
>> because it bases service names on "id" attribute, but actually this
>> attribute is not mandatory in xsd. In case you define a resource-adapter
>> configuration in standalone.xml for a rar having an internal config with
>> ironjacamar.xml It drives to runtime error like this:
>>
>> 14:47:27,561 ERROR [org.jboss.msc.service.fail] (MSC service thread 1-7)
>> MSC000001: Failed to start service jboss.ra.deployment."ra16outij.rar":
>> org.jboss.msc.service.StartException in service
>> jboss.ra.deployment."ra16outij.rar":
>> org.jboss.msc.service.DuplicateServiceException: Service
>> jboss.ra.ra16outij is already registered
>>
>   Thats expected error, since such configuration/setup leads to same resource being spawned at runtime.
And it's exactly reason because it would be better to make ID mandatory
IMHO, since it's a perfectly valid configuration.

>
>> Or even defining 2 resource-adapter in standaole xml on the same rar
>> archive, w/o defining id as xsd permit:
>> 14:49:59,574 ERROR [org.jboss.as.controller.management-operation]
>> (ServerService Thread Pool -- 11) JBAS014613: Operation ("add") failed -
>> address: ([
>>       ("subsystem" => "resource-adapters"),
>>       ("resource-adapter" => "ra16outij.rar")
>> ]) - failure description: "JBAS014803: Duplicate resource [
>>       (\"subsystem\" => \"resource-adapters\"),
>>       (\"resource-adapter\" => \"ra16outij.rar\")
>> ]"
>>
>> Both this use case are normal and very frequent because resource-adapter
>> defined in standalone.xml should be considered multiple configuration
>> (activation in jca slang) on the same deployed rar.
> Actually I wasnt able to grasp this to full extent - if this kind of anonymous resource
> definition is a requirement from specs or something forged in IJ or AS7/8 itself.
Spec require multiple activation, ID is a concept added in AS. I have to
double check, but should be fine to make it mandatory.

>
> I still have to
>> cover all use cases, but those 2 at least needs to be fixed. Probably
>> the easiest solution is to make "id" mandatory, but I have to double
>> check it is sufficient for programmatic activations (explicit call of
>> service like HornetQ is doing) and ironjacamar.xml ones.
> If its not covered in testsuite or it did not make server explode, I might missed it.
I think it is covered by test suite, but only adding it by DMR command
(so no anonymous ID since resource name become ID). So in fact 2
anonymous RA isn't covered by test suite.
Anyway, again, making ID mandatory should fix it.
>
>> If you agree I'll take the issue and I'll merge your patch on my branch
>> and rework it a bit to cover all those use cases and possibly to solve
>> also WFLY-1776 (the problem here could be that xsd changes can't be
>> backported and it is a EAP case)
> What I was led to believe is that this step is not required. It is perfectly
> fine if user has "anonymous" activation, without id, as long as there is only one.
> In case there is need for more than one activation, users need to provide proper configuration, that is
> provide ids.
Well, we should at least give a more expressive error. A service start
error could be confusing (in case of IJ.xml above for example), and
maybe easier to have ID mandatory.
> WRT hornetq, if Im not wrong, each activation from hornetq subsystem has unique name, since
> each config entry has it unique( I may be off here though ).
>
> Also, Im not sure about MDB annotation, this part of integration looked a bit fuzzy.
I'll take care to check them. I've already reassigned issue to me.
>  
> Existing tests pass, but Im not quite sure how this should look like for "IDed" configs.
The point is that current tests is doing everything using DMR commands,
so they are always using ID (see above). Probably the most consistent
behaviour is to make ID mandatory. I'm going in this direction, even if
I need to double check something.
>
> If you want to redo something feel free to mold it.
>
> Thanks.
Oki, I'll ping you for a review. Thanks for interest and the patch.

>> regards
>> S.
>>
>>
>> On 08/06/2013 03:29 PM, Bartosz Baranowski wrote:
>>> Hey
>>>
>>> Finally I was able ( I think ) to nail the problem and forge fix. There are
>>> still some intermittent CI failures, but other than that CI looks clear.
>>> While I polish last parts it would be good to get feedback on changes,
>>> since impact on JCA integration is quite big.
>>>
>>> Fix removes all static/leaky code from ConnectorServices ( add relevant
>>> code to MSC service ), move away from static hashmap checks in favor of
>>> MSC service present/deployment/dependency and remove ability to create
>>> more than one activation for distinct model address.
>>>
>>> Branch:
>>> https://github.com/baranowb/wildfly/tree/WFLY-1656
>>>
>>> Relevant issues:
>>> https://issues.jboss.org/browse/WFLY-1656
>>> https://issues.jboss.org/browse/WFLY-1492  ->
>>> https://bugzilla.redhat.com/show_bug.cgi?id=964202
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev