Geronimo
  1. Geronimo
  2. GERONIMO-2349

jta 1.1 support with container manager jpa support in transaction module

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.2
    • Component/s: transaction manager
    • Security Level: public (Regular issues)
    • Labels:
      None

      Description

      We need a strategy for supporting jta 1.1 and parts of jpa in the transaction module while still passing the j2ee 1.4 signature tests. Here's a proposal:

      • put the basis for support into the current transaction module, but don't use any jee5 interfaces
      • add an additional transaction-jta11 module that uses the new interfaces and extends the appropriate classes in transaction.

      I've started along this path. I'll put the new module in sandbox and supply a patch for the changes to the existing transaction module.

      1. GERONIMO-2349-1.patch
        13 kB
        David Jencks
      2. GERONIMO-2349-2.patch
        13 kB
        David Jencks
      3. GERONIMO-2349-3.patch
        13 kB
        David Jencks
      4. GERONIMO-2349-v4-plus-2163.patch
        384 kB
        David Jencks
      5. GERONIMO-2349-v4-plus-2163-openejb.patch
        36 kB
        David Jencks
      6. GERONIMO-2349-v5.patch
        13 kB
        David Jencks
      7. persistencecontextref.zip
        21 kB
        David Jencks

        Activity

        Hide
        David Jencks added a comment -

        First snapshot of implementing jee 5 support without the interfaces

        Show
        David Jencks added a comment - First snapshot of implementing jee 5 support without the interfaces
        Hide
        David Jencks added a comment -

        Updated patch that works with current version of sandbox/jee5-jta11/geronimo-transaction-jta11. I think that the cm entitymanager implementation is as complete as I can make it for now until we have actual deployment. I'd appreciate review of this patch with particular attention to the question of whether including this implementation in the base classes is appropriate. I think it is reasonable to include, but it would also be possible to push it to a TransactionImpl subclass if we created the TransactionImpl in a protected method.

        N.B. There's a possible bug in ordering of enlisting extended entity managers into transactions compared to enlisting connections. I suspect we will need to do something like the InterposedSynch with the TransactionManagerMonitors so all the connections get enlisted first and delisted last.

        Show
        David Jencks added a comment - Updated patch that works with current version of sandbox/jee5-jta11/geronimo-transaction-jta11. I think that the cm entitymanager implementation is as complete as I can make it for now until we have actual deployment. I'd appreciate review of this patch with particular attention to the question of whether including this implementation in the base classes is appropriate. I think it is reasonable to include, but it would also be possible to push it to a TransactionImpl subclass if we created the TransactionImpl in a protected method. N.B. There's a possible bug in ordering of enlisting extended entity managers into transactions compared to enlisting connections. I suspect we will need to do something like the InterposedSynch with the TransactionManagerMonitors so all the connections get enlisted first and delisted last.
        Hide
        David Jencks added a comment -

        Updated transaction patch adapted to new module names

        Show
        David Jencks added a comment - Updated transaction patch adapted to new module names
        Hide
        David Blevins added a comment -

        +1

        Show
        David Blevins added a comment - +1
        Hide
        David Jencks added a comment -

        This is a work-in-progress view of converting the ENCConfigBuilder to use NamingBuilders. Unfortunately I haven't kept my projects separate and the patches include GERONIMO-2163 (hint: vote for it

        They work to some extent. The only bug I know about is that message-destination-refs don't work.

        I've also included a tiny sample app that shows that cm persistence-context-ref lookup works with a mock jpa provider.

        Show
        David Jencks added a comment - This is a work-in-progress view of converting the ENCConfigBuilder to use NamingBuilders. Unfortunately I haven't kept my projects separate and the patches include GERONIMO-2163 (hint: vote for it They work to some extent. The only bug I know about is that message-destination-refs don't work. I've also included a tiny sample app that shows that cm persistence-context-ref lookup works with a mock jpa provider.
        Hide
        Jacek Laskowski added a comment -

        Hey Dave, I'm lost. I was about to +1 it, but then thought to give it a try and make it to the trunk. But I don't know what to apply. Would you care to comment on it a bit?

        Show
        Jacek Laskowski added a comment - Hey Dave, I'm lost. I was about to +1 it, but then thought to give it a try and make it to the trunk. But I don't know what to apply. Would you care to comment on it a bit?
        Hide
        David Jencks added a comment -

        At the moment only the "review" part is relevant. The last patch contains the start of reorganizing the ENCConfigBuilder and the stuff it uses around the idea of pluggable naming builders. I'd really like some feedback on that part. I'm hoping to have a more complete patch in a couple days. My current version includes an initContext phase in the NamingBuilder.

        It would help me a lot if GERONIMO-2163 was committed.

        Show
        David Jencks added a comment - At the moment only the "review" part is relevant. The last patch contains the start of reorganizing the ENCConfigBuilder and the stuff it uses around the idea of pluggable naming builders. I'd really like some feedback on that part. I'm hoping to have a more complete patch in a couple days. My current version includes an initContext phase in the NamingBuilder. It would help me a lot if GERONIMO-2163 was committed.
        Hide
        David Jencks added a comment -

        This is getting out of control... I'd like to scale back this issue to the changes in the transaction module itself and deal with persistence-refs in a separate issue (GERONIMO-2383).

        Please consider this patch together with the sandbox jta11 modules and vote soon.

        Show
        David Jencks added a comment - This is getting out of control... I'd like to scale back this issue to the changes in the transaction module itself and deal with persistence-refs in a separate issue ( GERONIMO-2383 ). Please consider this patch together with the sandbox jta11 modules and vote soon.
        Hide
        Gianny Damour added a comment -

        I applied v5, which contains the changes against the transaction module. +1 to have it applied.

        Show
        Gianny Damour added a comment - I applied v5, which contains the changes against the transaction module. +1 to have it applied.
        Hide
        Kevan Miller added a comment -

        +1 A comment or two about interposedSynchronization and when it would be registered would be nice...

        Show
        Kevan Miller added a comment - +1 A comment or two about interposedSynchronization and when it would be registered would be nice...
        Hide
        Matt Hogstrom added a comment -

        +1...commit away.

        Show
        Matt Hogstrom added a comment - +1...commit away.
        Hide
        David Jencks added a comment -

        Thanks, reviewers!

        Small final patch applied rev 442384

        Show
        David Jencks added a comment - Thanks, reviewers! Small final patch applied rev 442384

          People

          • Assignee:
            David Jencks
            Reporter:
            David Jencks
          • Votes:
            3 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development