Qpid
  1. Qpid
  2. QPID-3990

Multiple XAResources isSameRM behavior

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Implemented
    • Affects Version/s: 0.14, 0.17
    • Fix Version/s: 0.17
    • Component/s: Java Client, JCA
    • Labels:
      None
    • Environment:

      All supported OS platforms. All supported JEE platforms.

      Description

      In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RM's were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

      According to Gordon, this may be a discrepancy between XA and the AMQP .10 specification. This has inadvertently worked in the JCA code due to bug in the isSameRM method and the use of the QpidRAXAResourceWrapper.

      While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

      I have a fix for this issue that allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        Activity

        Hide
        Weston M. Price added a comment -

        Closing this issue and creating a new JIRA as suggested.

        Show
        Weston M. Price added a comment - Closing this issue and creating a new JIRA as suggested.
        Hide
        Weston M. Price added a comment -

        Got it. Thanks for the heads up. I will do as suggested.

        Show
        Weston M. Price added a comment - Got it. Thanks for the heads up. I will do as suggested.
        Hide
        Robbie Gemmell added a comment -

        This JIRAs changes were already released in 0.18, and 0.20 has since gone out as well. We should really create a new JIRA for the additional aspect noted and relate it to this JIRA, so that people can properly track what was actually fixed when.

        Show
        Robbie Gemmell added a comment - This JIRAs changes were already released in 0.18, and 0.20 has since gone out as well. We should really create a new JIRA for the additional aspect noted and relate it to this JIRA, so that people can properly track what was actually fixed when.
        Hide
        Weston M. Price added a comment -

        The original fix does not account for failure scenarios. As a result, multiple siblings are kept in the chain causing state issues on the broker. I am reopening this issue as a result.

        Show
        Weston M. Price added a comment - The original fix does not account for failure scenarios. As a result, multiple siblings are kept in the chain causing state issues on the broker. I am reopening this issue as a result.
        Hide
        Weston M. Price added a comment -

        Fixed with latest additions to trunk.

        Show
        Weston M. Price added a comment - Fixed with latest additions to trunk.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/#review7859
        -----------------------------------------------------------

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java
        <https://reviews.apache.org/r/5079/#comment17192>

        The log statement will currently print every time round the loop. Given what it is saying, might it be best located before the loop?

        • Robbie

        On 2012-05-13 18:29:08, Weston Price wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5079/

        -----------------------------------------------------------

        (Updated 2012-05-13 18:29:08)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Summary

        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.

        https://issues.apache.org/jira/browse/QPID-3990

        Diffs

        -----

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1337888

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1337888

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1337888

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAXAResourceTest.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/test-profiles/JavaPre010Excludes 1337888

        Diff: https://reviews.apache.org/r/5079/diff

        Testing

        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/#review7859 ----------------------------------------------------------- http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java < https://reviews.apache.org/r/5079/#comment17192 > The log statement will currently print every time round the loop. Given what it is saying, might it be best located before the loop? Robbie On 2012-05-13 18:29:08, Weston Price wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-13 18:29:08) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs ----- http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAXAResourceTest.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/test-profiles/JavaPre010Excludes 1337888 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-14 19:47:19, Robbie Gemmell wrote:

        > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java, lines 172-180

        > <https://reviews.apache.org/r/5079/diff/4/?file=108698#file108698line172>

        >

        > The log statement will currently print every time round the loop. Given what it is saying, might it be best located before the loop?

        Ugh. Thanks for catching this. I actually just committed the fix, but I can quickly update this.

        • Weston

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/#review7859
        -----------------------------------------------------------

        On 2012-05-13 18:29:08, Weston Price wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5079/

        -----------------------------------------------------------

        (Updated 2012-05-13 18:29:08)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Summary

        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.

        https://issues.apache.org/jira/browse/QPID-3990

        Diffs

        -----

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1337888

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1337888

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1337888

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAXAResourceTest.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/test-profiles/JavaPre010Excludes 1337888

        Diff: https://reviews.apache.org/r/5079/diff

        Testing

        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-14 19:47:19, Robbie Gemmell wrote: > http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java , lines 172-180 > < https://reviews.apache.org/r/5079/diff/4/?file=108698#file108698line172 > > > The log statement will currently print every time round the loop. Given what it is saying, might it be best located before the loop? Ugh. Thanks for catching this. I actually just committed the fix, but I can quickly update this. Weston ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/#review7859 ----------------------------------------------------------- On 2012-05-13 18:29:08, Weston Price wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-13 18:29:08) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs ----- http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAXAResourceTest.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/test-profiles/JavaPre010Excludes 1337888 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/
        -----------------------------------------------------------

        (Updated 2012-05-13 18:29:08.476635)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Changes
        -------

        Updated diff to remove whitespace, add a new JCA system test, update XAResourceTest and add the new JCA system test to the JavaPre010Excludes file as this does include testing XA functionality.

        Summary
        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.
        https://issues.apache.org/jira/browse/QPID-3990

        Diffs (updated)


        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1337888
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1337888
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1337888
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAXAResourceTest.java PRE-CREATION
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/test-profiles/JavaPre010Excludes 1337888

        Diff: https://reviews.apache.org/r/5079/diff

        Testing
        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-13 18:29:08.476635) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Changes ------- Updated diff to remove whitespace, add a new JCA system test, update XAResourceTest and add the new JCA system test to the JavaPre010Excludes file as this does include testing XA functionality. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs (updated) http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1337888 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/ra/QpidRAXAResourceTest.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/test-profiles/JavaPre010Excludes 1337888 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-12 00:01:58, rajith attapattu wrote:

        > Based on the description of the problem and the proposed solution the code appears to be doing whats expected.

        > There are a few whitespace errors that could be removed. On eclipse ctr-a and ctr-i should take care of it provided you have set tabs to whitespace.

        >

        > For my own understand, could you please explain if there could be any implications/corner cases when a particular XAResource calls start and end on the other XAResources participating in the transaction even though they are all managed by the same Resource Manager. In particular is there some sort of ordering that should be preserved when this is called? How about error handling? What if one of the XAResources fail/throw exception when start or end is called ?

        > (I assume under normal circumstances the RM would call these methods and would be in a position to handle these situations. In the above case the XAResource (all though being called by an RM) would act as a proxy for it's siblings). Is my understanding correct ? If not are you able to briefly explain the flow here ?

        Weston Price wrote:

        Good question. It's important to keep in mind in a typical XA flow, an XAResource never calls anything on another XAResource as the TransactionManager does this. However, as the original description mentions, the issue here is that because the XAResources in question are on the same transaction branch, the TM only calls start/end on the first XAResource discovered in the protocol and as a result the other XAResources are not allowed to complete correctly. Another way to put this is that doing correctly support joining multiple XAResources in the same transaction from the same ResourceManager (Broker).

        The isSameRM method provides a convenient place to 'intercept' the XA flow being that the TM always calls this when another XAResource is being enlisted in the transaction. Because of this fact, the correct order will always be maintained, however, a good test case would be more than two XAResources. I will add this.

        Again, this 'fix' is simply to address what is either a discrepancy between the XA and AMQP specs, or an incorrect implementation on the Broker (both Java and C++). It's not ideal, but currently if we don't provide some sort of solution, and code involving multiple XAResources from the same Broker will not work at all which, in my opinion has severe consequences beyond introducing a fix in the JMS/JCA code.

        Also, I will remove the whitespace. I typically use Vi which does this automatically but I happened to use Eclipse for this one task. I will take care of it and thanks for pointing it out.

        • Weston

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/#review7820
        -----------------------------------------------------------

        On 2012-05-09 17:41:59, Weston Price wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5079/

        -----------------------------------------------------------

        (Updated 2012-05-09 17:41:59)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Summary

        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.

        https://issues.apache.org/jira/browse/QPID-3990

        Diffs

        -----

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275

        Diff: https://reviews.apache.org/r/5079/diff

        Testing

        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-12 00:01:58, rajith attapattu wrote: > Based on the description of the problem and the proposed solution the code appears to be doing whats expected. > There are a few whitespace errors that could be removed. On eclipse ctr-a and ctr-i should take care of it provided you have set tabs to whitespace. > > For my own understand, could you please explain if there could be any implications/corner cases when a particular XAResource calls start and end on the other XAResources participating in the transaction even though they are all managed by the same Resource Manager. In particular is there some sort of ordering that should be preserved when this is called? How about error handling? What if one of the XAResources fail/throw exception when start or end is called ? > (I assume under normal circumstances the RM would call these methods and would be in a position to handle these situations. In the above case the XAResource (all though being called by an RM) would act as a proxy for it's siblings). Is my understanding correct ? If not are you able to briefly explain the flow here ? Weston Price wrote: Good question. It's important to keep in mind in a typical XA flow, an XAResource never calls anything on another XAResource as the TransactionManager does this. However, as the original description mentions, the issue here is that because the XAResources in question are on the same transaction branch, the TM only calls start/end on the first XAResource discovered in the protocol and as a result the other XAResources are not allowed to complete correctly. Another way to put this is that doing correctly support joining multiple XAResources in the same transaction from the same ResourceManager (Broker). The isSameRM method provides a convenient place to 'intercept' the XA flow being that the TM always calls this when another XAResource is being enlisted in the transaction. Because of this fact, the correct order will always be maintained, however, a good test case would be more than two XAResources. I will add this. Again, this 'fix' is simply to address what is either a discrepancy between the XA and AMQP specs, or an incorrect implementation on the Broker (both Java and C++). It's not ideal, but currently if we don't provide some sort of solution, and code involving multiple XAResources from the same Broker will not work at all which, in my opinion has severe consequences beyond introducing a fix in the JMS/JCA code. Also, I will remove the whitespace. I typically use Vi which does this automatically but I happened to use Eclipse for this one task. I will take care of it and thanks for pointing it out. Weston ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/#review7820 ----------------------------------------------------------- On 2012-05-09 17:41:59, Weston Price wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-09 17:41:59) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs ----- http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-12 00:01:58, rajith attapattu wrote:

        > Based on the description of the problem and the proposed solution the code appears to be doing whats expected.

        > There are a few whitespace errors that could be removed. On eclipse ctr-a and ctr-i should take care of it provided you have set tabs to whitespace.

        >

        > For my own understand, could you please explain if there could be any implications/corner cases when a particular XAResource calls start and end on the other XAResources participating in the transaction even though they are all managed by the same Resource Manager. In particular is there some sort of ordering that should be preserved when this is called? How about error handling? What if one of the XAResources fail/throw exception when start or end is called ?

        > (I assume under normal circumstances the RM would call these methods and would be in a position to handle these situations. In the above case the XAResource (all though being called by an RM) would act as a proxy for it's siblings). Is my understanding correct ? If not are you able to briefly explain the flow here ?

        Good question. It's important to keep in mind in a typical XA flow, an XAResource never calls anything on another XAResource as the TransactionManager does this. However, as the original description mentions, the issue here is that because the XAResources in question are on the same transaction branch, the TM only calls start/end on the first XAResource discovered in the protocol and as a result the other XAResources are not allowed to complete correctly. Another way to put this is that doing correctly support joining multiple XAResources in the same transaction from the same ResourceManager (Broker).

        The isSameRM method provides a convenient place to 'intercept' the XA flow being that the TM always calls this when another XAResource is being enlisted in the transaction. Because of this fact, the correct order will always be maintained, however, a good test case would be more than two XAResources. I will add this.

        Again, this 'fix' is simply to address what is either a discrepancy between the XA and AMQP specs, or an incorrect implementation on the Broker (both Java and C++). It's not ideal, but currently if we don't provide some sort of solution, and code involving multiple XAResources from the same Broker will not work at all which, in my opinion has severe consequences beyond introducing a fix in the JMS/JCA code.

        • Weston

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/#review7820
        -----------------------------------------------------------

        On 2012-05-09 17:41:59, Weston Price wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5079/

        -----------------------------------------------------------

        (Updated 2012-05-09 17:41:59)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Summary

        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.

        https://issues.apache.org/jira/browse/QPID-3990

        Diffs

        -----

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275

        Diff: https://reviews.apache.org/r/5079/diff

        Testing

        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-12 00:01:58, rajith attapattu wrote: > Based on the description of the problem and the proposed solution the code appears to be doing whats expected. > There are a few whitespace errors that could be removed. On eclipse ctr-a and ctr-i should take care of it provided you have set tabs to whitespace. > > For my own understand, could you please explain if there could be any implications/corner cases when a particular XAResource calls start and end on the other XAResources participating in the transaction even though they are all managed by the same Resource Manager. In particular is there some sort of ordering that should be preserved when this is called? How about error handling? What if one of the XAResources fail/throw exception when start or end is called ? > (I assume under normal circumstances the RM would call these methods and would be in a position to handle these situations. In the above case the XAResource (all though being called by an RM) would act as a proxy for it's siblings). Is my understanding correct ? If not are you able to briefly explain the flow here ? Good question. It's important to keep in mind in a typical XA flow, an XAResource never calls anything on another XAResource as the TransactionManager does this. However, as the original description mentions, the issue here is that because the XAResources in question are on the same transaction branch, the TM only calls start/end on the first XAResource discovered in the protocol and as a result the other XAResources are not allowed to complete correctly. Another way to put this is that doing correctly support joining multiple XAResources in the same transaction from the same ResourceManager (Broker). The isSameRM method provides a convenient place to 'intercept' the XA flow being that the TM always calls this when another XAResource is being enlisted in the transaction. Because of this fact, the correct order will always be maintained, however, a good test case would be more than two XAResources. I will add this. Again, this 'fix' is simply to address what is either a discrepancy between the XA and AMQP specs, or an incorrect implementation on the Broker (both Java and C++). It's not ideal, but currently if we don't provide some sort of solution, and code involving multiple XAResources from the same Broker will not work at all which, in my opinion has severe consequences beyond introducing a fix in the JMS/JCA code. Weston ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/#review7820 ----------------------------------------------------------- On 2012-05-09 17:41:59, Weston Price wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-09 17:41:59) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs ----- http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/#review7820
        -----------------------------------------------------------

        Based on the description of the problem and the proposed solution the code appears to be doing whats expected.
        There are a few whitespace errors that could be removed. On eclipse ctr-a and ctr-i should take care of it provided you have set tabs to whitespace.

        For my own understand, could you please explain if there could be any implications/corner cases when a particular XAResource calls start and end on the other XAResources participating in the transaction even though they are all managed by the same Resource Manager. In particular is there some sort of ordering that should be preserved when this is called? How about error handling? What if one of the XAResources fail/throw exception when start or end is called ?
        (I assume under normal circumstances the RM would call these methods and would be in a position to handle these situations. In the above case the XAResource (all though being called by an RM) would act as a proxy for it's siblings). Is my understanding correct ? If not are you able to briefly explain the flow here ?

        • rajith

        On 2012-05-09 17:41:59, Weston Price wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5079/

        -----------------------------------------------------------

        (Updated 2012-05-09 17:41:59)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Summary

        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.

        https://issues.apache.org/jira/browse/QPID-3990

        Diffs

        -----

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275

        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275

        Diff: https://reviews.apache.org/r/5079/diff

        Testing

        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/#review7820 ----------------------------------------------------------- Based on the description of the problem and the proposed solution the code appears to be doing whats expected. There are a few whitespace errors that could be removed. On eclipse ctr-a and ctr-i should take care of it provided you have set tabs to whitespace. For my own understand, could you please explain if there could be any implications/corner cases when a particular XAResource calls start and end on the other XAResources participating in the transaction even though they are all managed by the same Resource Manager. In particular is there some sort of ordering that should be preserved when this is called? How about error handling? What if one of the XAResources fail/throw exception when start or end is called ? (I assume under normal circumstances the RM would call these methods and would be in a position to handle these situations. In the above case the XAResource (all though being called by an RM) would act as a proxy for it's siblings). Is my understanding correct ? If not are you able to briefly explain the flow here ? rajith On 2012-05-09 17:41:59, Weston Price wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-09 17:41:59) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs ----- http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/
        -----------------------------------------------------------

        (Updated 2012-05-09 17:41:59.301972)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Changes
        -------

        Sorry, did the SVN diff incorrectly and forgot to add new file qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java. New diff has missing file included. Please use this as the definitive diff.

        Summary
        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.
        https://issues.apache.org/jira/browse/QPID-3990

        Diffs (updated)


        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275

        Diff: https://reviews.apache.org/r/5079/diff

        Testing
        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-09 17:41:59.301972) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Changes ------- Sorry, did the SVN diff incorrectly and forgot to add new file qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java. New diff has missing file included. Please use this as the definitive diff. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs (updated) http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQXAResource.java PRE-CREATION http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/
        -----------------------------------------------------------

        (Updated 2012-05-09 17:37:24.510238)

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Changes
        -------

        Diff for QPID-3990

        Summary
        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.
        https://issues.apache.org/jira/browse/QPID-3990

        Diffs (updated)


        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275

        Diff: https://reviews.apache.org/r/5079/diff

        Testing
        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- (Updated 2012-05-09 17:37:24.510238) Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Changes ------- Diff for QPID-3990 Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs (updated) http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/5079/
        -----------------------------------------------------------

        Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey.

        Summary
        -------

        In fixing QPID-3263, an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource.

        While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc.

        The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete.

        This addresses bug QPID-3990.
        https://issues.apache.org/jira/browse/QPID-3990

        Diffs


        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275
        http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275

        Diff: https://reviews.apache.org/r/5079/diff

        Testing
        -------

        Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile.

        Thanks,

        Weston

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5079/ ----------------------------------------------------------- Review request for Ted Ross, Robbie Gemmell, rajith attapattu, and Rob Godfrey. Summary ------- In fixing QPID-3263 , an issue has cropped up when two XAResources have the same underlying ResourceManager(RM). Originally the isSameRM method returned false regardless of whether or not the RMs were the same. Since we now return true when two or more XAResources are involved, we get an ILLEGAL_STATE exception being that the DtxStart/DtxEnd calls are not paired for each XAResource because the TransactionManager (TM) treats the XAResource(s) as being on the same transaction branch. As a result, DtxEnd is only called on one resource. While it might make sense to simply roll back to the old behavior, this is not going to perform as well being that most TM's can optimize underlying transaction state with isSameRM is true. This has performance implications for the TM in terms of logging, management etc. The following patch/diff allows XAResources to be aware of each other in a transaction chain. When DtxEnd is called on one resource, the sibling XAResources.end method is called thereby matching the DtxStart/DtxEnd invocations appropriately allowing the transaction to complete. This addresses bug QPID-3990 . https://issues.apache.org/jira/browse/QPID-3990 Diffs http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XAResourceImpl.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/jca/src/main/java/org/apache/qpid/ra/QpidRAXAResource.java 1336275 http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/jms/xa/XAResourceTest.java 1336275 Diff: https://reviews.apache.org/r/5079/diff Testing ------- Added system test to XAResourceTest to exercise new behavior. Tested against both the Java and C++ Brokers using the 0.10 profile. Thanks, Weston

          People

          • Assignee:
            Weston M. Price
            Reporter:
            Weston M. Price
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development