Re: Re: [pgjdbc] XADataSource support for resource sharing & interleaving. (#47) - Mailing list pgsql-jdbc

I still can't get very excited about this. The XADataSource
implementation shouldn't have to maintain a connection pool just to
support some obscure JTA features. It doesn't make sense to require
suspend/resume and interleaving support when the underlying protocol
doesn't support them. The point of JTA is two-phase commit, and you can
do two-phase commit just as well without those features.

On the whole, I think it would be best if Glassfish developers would add
an option to not rely on interleaving - all other major Transaction
Managers do. They will need that to support other XA implementations
that don't support suspend/resume and interleaving, anyway.

If we must have this anyway, I think we should try to implement it as a
generic wrapper that could be used with any XADataSource implementation.

I also wonder if we could grab some low-hanging fruit without doing the
whole pooling thing. For example, if you try to call (2nd phase)
commit() on an connection that's currently busy with another
transaction, instead of throwing an error we could open a new temporary
connection, issue the COMMIT PREPARED, and disconnect. Sure, that would
be slow, so you'd definitely still want to configure the TM to not do
that, but at least it would work.

On 15.03.2013 19:14, Dave Cramer wrote:
> I'm looking for opinions on this patch.
>
> Heikki ? Kevin Grittner ?
>
> Dave Cramer
>
> dave.cramer(at)credativ(dot)ca
> http://www.credativ.ca
>
>
> On Fri, Mar 15, 2013 at 12:45 PM, Bryan Varner<notifications@github.com>wrote:
>
>> This reimplementation of the PGXADataSource (and supporting classes)
>> provides more complete fulfillment of the JTA 1.0.1 specification.
>>
>> It properly implements resource sharing (3.4.6) allowing all connections
>> to the same underlying XAResource to control the global transactions
>> created by other instances of that XAResource.
>>
>> It addresses proper interleaving and isolation of local&  global
>> transaction scopes. (3.4.7)
>>
>> The support for interleaving and sharing is implemented by (potentially,
>> only when needed) returning 'logical' connections from the XADataSource,
>> and opening new physical connections to the backend server (when necessary)
>> to handle preforming work on behalf of a global TX or local TX.
>>
>> The behavior of opening new physical backends (implementing interleaving)
>> can be disabled by setting the XAAcquireTimeout = 0, in which case the
>> behavior of the old XADataSource implementation is retained, with a minimal
>> performance impact.
>>
>> It tries hard to only open as many physical connections as necessary to
>> service the logical connections, and prunes extraneous connections created
>> as a result of supporting interleaving at each logical connection handle
>> close().
>>
>> In our production environment, where we have allocated our pool sizes to
>> the actual observed in-use highwater mark + 30%, we are seeing zero
>> connection churn, as there are enough free connections at any given time to
>> properly service the checked-out, in-use logical connections and the
>> interleaving&  resource sharing being invoked by the container.
>>
>> This implementation passes:
>>
>>     - all the existing XADataSource test cases
>>     - the many we've added to test corner cases of XA handling in
>>     concurrent and pooled workloads.
>>     - the JBoss narayana Transaction Manager jdbc unit tests. (
>>     https://github.com/jbosstm/narayana)
>>
>> In addition to the XADataSource changes
>>
>>     - the build.xml has been update to respect (fully) the srcdir property
>>     - AbstractJdbc23PooledConnection exposes getBackingConnection() to
>>     sub-classes, allowing subclasses access to the underlying Connection object
>>     without needing to cache the instance in their constructors.
>>
>> We have been successfully using this snapshot of code in our production
>> environment (GlassFish 3.1.2.2) with PostgreSQL 9.1, and are pleased to
>> report that it is working very well at this time.
>> ------------------------------
>> You can merge this Pull Request by running
>>
>>    git pull https://github.com/polarislabs/pgjdbc POLARIS_XA
>>
>> Or view, comment on, or merge it at:
>>
>>    https://github.com/pgjdbc/pgjdbc/pull/47
>> Commit Summary
>>
>>     - This is an in-progress refactor attempt with the express goals of
>>     providing a complete, and robust JTA compliant XA Implementation.
>>     - Stubbing out / making progress on this.
>>     - Ok, whoops.
>>     - I'm not totally sold on logicalConnectionId at this point, but it
>>     might work.
>>     - Started stubbing out / reconciling original precondition /
>>     deficiences, this should help cement the types of things we'll need the
>>     XADataSource to handle.
>>     - "A broken build is a sign of progress!"
>>     - implementation should be somewhat possible to execute.
>>     - That shouldn't be there.
>>     - helps to remember to unignore things.
>>     - updated the gitignore
>>     - I realize this this may mean some of our work does not get accepted
>>     upstream, but I can't abide not having the source in a source directory
>>     anymore, where netbeans (i know, i know) can actually resolve the proper
>>     package names and what-not. It's amazing I lasted this long without doing
>>     this. Seriously, I'm tempted to make this a maven build.
>>     - Some cleanup, and getting the build.xml to behave a little better.
>>     - Forgot I moved the getPhysicalConnection(), wow I should back off
>>     the coffee.
>>     - This is actually starting to pass test cases.
>>     - FUNCTIONAL PARITY.
>>     - Structuring the build to renable the test suite.
>>     - Minor changes, not really sure what...
>>     - Adding the nbignore...
>>     - added idea files to gitignore
>>     - Updating a few things.
>>     - we just passed the old interleaving tests.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Added docs, reformatted some stuff, cleaned up some code
>>     - Merged from Bryan's chanages to pass all the tests.
>>     - Implements Referenceable, Serializable.
>>     - Tweaking these to be more aggressive by default.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Code paranoia
>>     - Added some documentation
>>     - Abstracted connection creation, yes I actually have a use-case for
>>     this.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Moved logical connection if checks to the top to throw a
>>     IllegalArgumentException since it is required for the method.
>>     - Added some debug logging
>>     - Don't commit the out folder
>>     - Added idea's out folder to the ignore
>>     - Moved the autocommit bits to the logical connection part. I think
>>     this may help.
>>     - Updated docs and wrapped debug statement to avoid performance hit
>>     when logging not desired
>>     - Logger might as well be local if it's only going to be used in the
>>     one place.
>>     - Changing up the physical pooling policies and connection cleanup
>>     cycle.
>>     - Disassociate so we have a shot at closing things.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Cleaning up some leftovers.
>>     - Improved the first interleaving test, to exercise isolation checks
>>     in the midst of interleaving.
>>     - Added a test case for join, which is exposing an actual bug.
>>     - Added a few tests cases which are exposing some problems centered
>>     around connection.close(), and the handling of things on the backends.
>>     - Added a lot of debug logging to PGXADataSource and
>>     PhysicalXAConnection.
>>     - Added lib folder to gitignore to allow ant builds directly from idea
>>     without committing the junit jar.
>>     - Added a TODO to indicate improvement needed on Logger creation.
>>     - Added some extra logging and documentation.
>>     - Fixed typo
>>     - Fixed local thread test. The thread wasn't being joined before
>>     checking the error value. So, the error value was being checked before the
>>     query had a chance to complete.
>>     - Cleaned up imports.
>>     - Getting the physical connection-based logger would require too much
>>     rework at this point, so just use the Driver-base logger instead.
>>     - Fixed null pointer exception since the fake xid class doesn't null
>>     check in its equals method
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - synchronized a section of code that might cause a race condition
>>     while trying to get a logical connection that doesn't have a transaction
>>     started yet in order to issue a commit prepared.
>>     - This test can expose a couple of race conditions. Though because of
>>     the nature of them, the test passes ... sometimes.
>>     - Blunt-Edged, Brute-Force approach. Lock all the things!
>>     - Might as well go all Reentrant
>>     - Now start to rein it in
>>     - Completely re-implemented.
>>     - Clean up / documentation of a failure point.
>>     - Removed all the rediculous local TX tracking in the logical
>>     connection side. Really we need to care about the autocommit of the
>>     physical TX, which we should only be mangling when we associate /
>>     disassociate to an XID. A logical connection should always defer it's
>>     autocommit status to that of the physical connection servicing it.
>>     - If a physical connection is in local TX mode, try to keep it to one
>>     logical connection per physical...
>>     - Added a simple test of a transaction commit on a pooled connection
>>     to simulate app servers connection pooling. This reproduces the
>>     "double-close" issue we have been seeing.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Close behavior now waits for the logical connection to close, rather
>>     than the handle.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Keep the username / password around, so that we can allocate new
>>     physical connections if all of them are closed and a logical connection
>>     tries to do something.
>>     - Commiting changes to the wait timeouts and strategy if you set the
>>     timeout to 0.
>>     - Added threaded pool test that exhausts the pool.
>>     - Merge branch 'POLARIS_XA' of github.com:polarislabs/pgjdbc into
>>     POLARIS_XA
>>     - Allow for disabling interleaving support. We'll probably want to do
>>     this by default, and let people enable it when they need it.
>>     - Hey, we weren't using that logger...
>>     - Hey, we weren't using that logger...
>>     - Better support for people who want to disable interleaving, and have
>>     a hard&  fast 1 - 1 logical to physical mapping.
>>     - Added a test case that exercises (intermittently, however) the race
>>     condition between prepare&  commit / close.
>>     - These test cases are duplicating the exact type of intermittent
>>     failure we're seeing in our deployed environment.
>>     - More updates, we can close anything IDLE. But not OPEN or marked for
>>     ROLLBACK (at least not without allowing the rollback first!).
>>     - I haven't run this against the JBOSS TM QA tests yet, but at this
>>     point, it's passing all the tests we have (including the race-condition
>>     tests for close() logic) that we have.
>>     - Whoops. build.xml changes crept in.
>>     - Rollback only disassociates the xid, not the entire physical
>>     connection. In the case of a Connection pegged to a thread, the next
>>     invocation could end up royally messing with the state of the pool.
>>     - Passes the test where the last logical connection closed closes any
>>     non-suspended TX.
>>     - Verify that we end up on the same LocalTX transaction (backend pg
>>     PID) as we did when we started.
>>     - Moving things back to the original location.
>>     - Cleaning up build.xml changes
>>     - More cleanup, finer-grained locking in a few spots.
>>     - Added 'debugtest' target which sets up a dt_socket server for
>>     debugging test cases.
>>     - Broke resource sharing, where the XAResource can outlive the logical
>>     connection (apparently).
>>     - Fixed a few issues with close before rollback.
>>     - Eager association of logicals to physicals was causing connection
>>     bloat.
>>     - Greatly reduced connection usage, by improving re-use, and only
>>     allocating physical backends when it's necessary to provide a reasonable
>>     level of performance.
>>     - Added a test case to verify that we weren't closing connections we
>>     shouldn't be.
>>     - Even more tests about connection association...
>>     - Fixed the wait strategy to wait up to xaAcquireTimeout milliseconds
>>     (in multiple passes if necessary) before allocating a new physical
>>     connection.
>>     - RecoveredXid is blowing up when GlassFish tries to ping the
>>     XAResource with null xids.
>>     - Merge remote-tracking branch 'upstream/master' into POLARIS_XA
>>     - Adding a test case that I think will duplicate the prepared
>>     statement / cursor issue we're seeing with connection closing.
>>     - Uh, we don't need 10k of these..
>>     - Updated test case to use cursors.
>>     - Cleaned up / simplified some of the close() code.
>>
>> File Changes
>>
>>     - *M* .gitignore<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-0>(5)
>>     - *M* build.properties<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-1>(1)
>>     - *M* build.xml<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-2>(97)
>>     - *M*
org/postgresql/ds/jdbc23/AbstractJdbc23PooledConnection.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-3>(9)
>>     - *A*
org/postgresql/test/xa/PooledXADataSourceTest.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-4>(250)
>>     - *M* org/postgresql/test/xa/XADataSourceTest.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-5>(859)
>>     - *M* org/postgresql/test/xa/XATestSuite.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-6>(1)
>>     - *M* org/postgresql/xa/.gitignore<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-7>(2)
>>     - *R* org/postgresql/xa/AbstractPGXADataSource.java.in<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-8>(3)
>>     - *M* org/postgresql/xa/PGXAConnection.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-9>(710)
>>     - *A* org/postgresql/xa/PGXADataSource.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-10>(815)
>>     - *M* org/postgresql/xa/PGXADataSourceFactory.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-11>(20)
>>     - *A* org/postgresql/xa/PhysicalXAConnection.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-12>(385)
>>     - *M* org/postgresql/xa/RecoveredXid.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-13>(46)
>>     - *M*
org/postgresql/xa/jdbc3/AbstractJdbc3XADataSource.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-14>(29)
>>     - *M*
org/postgresql/xa/jdbc4/AbstractJdbc4XADataSource.java<https://github.com/pgjdbc/pgjdbc/pull/47/files#diff-15>(2)
>>
>> Patch Links:
>>
>>     - https://github.com/pgjdbc/pgjdbc/pull/47.patch
>>     - https://github.com/pgjdbc/pgjdbc/pull/47.diff
>>
>>
>


--
- Heikki