On 07/11/2016 01:38 PM, Jesper Pedersen wrote:
Hi,
On 07/08/2016 12:24 PM, Scott Marlow wrote:
> The NoSQL prototype now has transaction enlistment for Neo4j (one phase
> wrapper).
>
> Test example link [1] shows a CMT test bean method
You should add a comment that tx.success() / tx.failure() is tied to the
out-come of the JTA transaction. And tx.close() is done in afterCompletion.
Comment added, thanks for noticing the absence. :)
> and [2] shows a BMT test bean method.
>
This test case highlights the problem of checking for enlistment in 2
places, as the Session is obtained before the transaction is started,
and there is no way to get the session reference from the transaction
instance.
So, you can manage the transaction lifecycle, but not the session
lifecycle.
In [1], when we get the Neo4j Session, we enlist it within the JTA
transaction. We also enlist the Neo4j transaction at the same time
(during the call to Driver.session() inside of a JTA transaction.) For
the duration of the JTA transaction, attempts to get the
Session/Transaction for the named Neo4j profile, will return the already
JTA enlisted Session/Transaction.
> Currently, in a JTA transaction the Neo4j Session survives ending the
> JTA transaction. It might be better to close the Neo4j Session when the
> JTA transaction ends, which would require the application to get a new
> Session, this would ensure that applications don't forget to close the
> session, with the down side that with BMT, application code would need
> to call the Neo4j Driver.session(), to get a new Session instance after
> each JTA transaction ends. After a session is closed, there is no
> guarantee that another session can be obtained, as the next
> Driver.session() may time out if the internal pool of Driver sessions,
> is empty and the configured time limit
> (neo4j.driver.acquireSessionTimeout) is exceeded (defaults to 30
> seconds). So, there are costs to automatically closing Neo4j Sessions
> at JTA transaction end time. Although, you could say that closing the
> Session, after each transaction, gives other threads a change to use the
> Session that isn't really closed, but instead is really returned to the
> internal Driver sessions pool.
As you are enlisting the Neo4J resources into the transaction, I think
that you should define driver.session() as the enlistment barrier, and
therefore also close the session in afterCompletion.
I pushed an update [1] that includes auto closing the session, when the
transaction ends.
I didn't add a Synchronization yet, to handle closing the session when
the transaction manager doesn't call XAResourceWrapper.commit/rollback.
I agree that will introduce an extra layer of safety to ensure that
resources are closed.
That makes it more clear in what scenarios the resources are managed.
In JDBC you inject the DataSource instance, and obtain a Connection, so
it would be parallel to this case Driver vs. Session.
This means that people needs to be more careful in the BMT case to get
the Session within an active transaction. JCA has
CachedConnectionManager to verify these cases, so you could add a
similar concept.
Attempts to call Neo4j transaction/session "close", within the JTA
transaction are ignored. How about calls to "close" after the JTA
transaction ends, should they also be ignored? Is there a JCA
equivalent of ignoring "close" after the transaction ends? I think that
Hibernate ORM is or was, closing DataSource's after the transaction
ends, so I assume that JCA ignores "close" after the JTA transaction ends.
Thanks again for the feedback!
Scott
[1]
https://github.com/scottmarlow/wildfly/commit/bfecbe881e6a16548b9712b3e52...