OpenEJB
  1. OpenEJB
  2. OPENEJB-702

Transaction Logging not working in OpenEJB for MDBs

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 3.0
    • Component/s: container system
    • Labels:
      None
    • Environment:
      All

      Description

      I was investigating GERONIMO-3354. Found that the exception
      getting thrown was due to an XAResource getting passed to the the
      geronimo transaction manager instead of a NamedXAResource .
      To fix this we need to change the method
      public MessageEndpoint createEndpoint(XAResource xaResource) throws UnavailableException in
      org.apache.openejb.core.mdb.EndpointFactory

      In this method in the case of using the Geronimo Transaction Manager we need to pass a NamedXAResource if we need transaction logging to function.

      Directly passing a NamedXAResource will fix this problem at the openEJB side for now but we need to bring in a layer of abstraction since the NamedXAResource is a geronimo TM specific class and in OpenEJB standalone the TM must be pluggable.

      For the time being its acceptable to use NamedXAResource.

      This requires a fix in AMQ also to work . The JIRA in question is
      https://issues.apache.org/activemq/browse/AMQ-1438

      1. OPENEJB-702.patch
        5 kB
        Manu T George

        Activity

        Hide
        Manu T George added a comment -

        A fix for this is required to fix https://issues.apache.org/jira/browse/GERONIMO-3354

        Show
        Manu T George added a comment - A fix for this is required to fix https://issues.apache.org/jira/browse/GERONIMO-3354
        Hide
        Manu T George added a comment -

        this should be applied along with the corressponding AMQ patch

        Show
        Manu T George added a comment - this should be applied along with the corressponding AMQ patch
        Hide
        David Jencks added a comment -

        It would be great to get this into the upcoming openejb beta.

        Show
        David Jencks added a comment - It would be great to get this into the upcoming openejb beta.
        Hide
        Manu T George added a comment -

        David J. Being an activeMQ committer and the writer of the G Transaction manager it would be great review the fix. I have verified that the tests work against the openejb codebase at the time I created the patch but my knowledge of both the Geronimo TM and AMQ are not as good as I would like it to be.

        I can reverify and get this into openejb provided someone gets the activemq patch committed. We will also ideally need a new release of AMQ 4 with the related patch (When is the next AMQ 4.x release)

        Another approach would be to upgrade to AMQ 5.0.(I believe Dain was planning to work on that).

        Show
        Manu T George added a comment - David J. Being an activeMQ committer and the writer of the G Transaction manager it would be great review the fix. I have verified that the tests work against the openejb codebase at the time I created the patch but my knowledge of both the Geronimo TM and AMQ are not as good as I would like it to be. I can reverify and get this into openejb provided someone gets the activemq patch committed. We will also ideally need a new release of AMQ 4 with the related patch (When is the next AMQ 4.x release) Another approach would be to upgrade to AMQ 5.0.(I believe Dain was planning to work on that).
        Hide
        Manu T George added a comment -

        Missed a few words above
        ----------------------------------------
        David J. Being an activeMQ committer and the writer of the G Transaction manager it would be great if you could review the fix. I have verified that the tests work against the openejb codebase at the time I created the patch but my knowledge of both the Geronimo TM and AMQ are not as good as I would like it to be. I can reverify and post a new patch if you want.

        I can reverify and get this into openejb provided someone gets the activemq patch committed. We will also ideally need a new release of AMQ 4 with the related patch (When is the next AMQ 4.x release)

        Another approach would be to upgrade to AMQ 5.0.(I believe Dain was planning to work on that but I think not for this release).

        Show
        Manu T George added a comment - Missed a few words above ---------------------------------------- David J. Being an activeMQ committer and the writer of the G Transaction manager it would be great if you could review the fix. I have verified that the tests work against the openejb codebase at the time I created the patch but my knowledge of both the Geronimo TM and AMQ are not as good as I would like it to be. I can reverify and post a new patch if you want. I can reverify and get this into openejb provided someone gets the activemq patch committed. We will also ideally need a new release of AMQ 4 with the related patch (When is the next AMQ 4.x release) Another approach would be to upgrade to AMQ 5.0.(I believe Dain was planning to work on that but I think not for this release).
        Hide
        David Blevins added a comment -

        Since we had to respin for beta 2 I tried to get this patch in. Unfortunately I get a bunch of test failures and errors – clearly we need the activemq fixes as well (should have read closer ).

        Hopefully we can get this change coordinated between g, o, and a for our next round of releases.

        Show
        David Blevins added a comment - Since we had to respin for beta 2 I tried to get this patch in. Unfortunately I get a bunch of test failures and errors – clearly we need the activemq fixes as well (should have read closer ). Hopefully we can get this change coordinated between g, o, and a for our next round of releases.
        Hide
        David Jencks added a comment -

        This patch isn't quite right. The containerId should be used as the name, not xaResource.toString().

        The use of NamedXAResource is needed for any use of the geronimo transaction manager, not just geronimo, so the dependency may be more general than implied by the comments in the patch.

        Since transacted message delivery to mdbs depends on this patch I hope it can be included in openejb 3.0.

        Show
        David Jencks added a comment - This patch isn't quite right. The containerId should be used as the name, not xaResource.toString(). The use of NamedXAResource is needed for any use of the geronimo transaction manager, not just geronimo, so the dependency may be more general than implied by the comments in the patch. Since transacted message delivery to mdbs depends on this patch I hope it can be included in openejb 3.0.
        Hide
        David Jencks added a comment -

        I simplified the patch a bit. After some thought I realized that only the activemq vm transport is broken and switching to tcp transport lets all the openejb tests pass.

        rev 637368 applied to trunk.

        Show
        David Jencks added a comment - I simplified the patch a bit. After some thought I realized that only the activemq vm transport is broken and switching to tcp transport lets all the openejb tests pass. rev 637368 applied to trunk.

          People

          • Assignee:
            David Jencks
            Reporter:
            Manu T George
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development