[
https://issues.apache.org/jira/browse/DERBY-2432?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504168
]
Julius Stroffek commented on DERBY-2432:
----------------------------------------
Thanks Knut for spending some time with a patch.
> XATransactionState.java:
>
> - it would be good if the new methods had javadoc comments
I'll add missing javadoc.
>
> - perhaps the common code in xa_rollback() and xa_commit() could be
> factored out into a utility method, say cancelCancellationTask()?
I thought that it is too simple to be a method.
> - I think there is a possibility that CancelXATransactionTask tries to roll
> back the transaction after an explicit rollback or commit has been issued.
> This is because CancelXATransactionTask.run() and
> XATransactionState.xa_commit()/xa_rollback() synchronize on different
> objects.
Yes, it is. But it does not succeed. If I will change the
CancelXATransactionTask class to non-static just the same synchronization might
not help. It is not documented what the TimerTask.cancel method does when the
task is running (and waiting for the synchronization on run method). How can I
synchronize on the same objects if some of those might be null
(XATransactionState.timeoutTask) or might be null after synchronization
(CancelXATransactionTask.run method)? I think that maintaining a variable
whether the transaction was committed/rolled back already might be appropriate.
I can check that variable in XATransactionState.cancel method.
> - would it be more natural to have CancelXATransactionTask as a non-static
> class? Then we wouldn't need the tranState variable (it could be accessed
> with XATransactionState.this).
I copied the way of the transaction timeout from
org.apache.derby.impl.sql.conn.GenericStatementContext where the CancelTask is
made static. I can not see
> - I'm not sure I understand why scheduleTimeoutTask() can be unsynchronized
> whereas all the other methods require synchronization of some kind. Could you
> please explain?
I wanted to avoid unnecessary synchronization. scheduleTimeoutTask is called
just from EmbedXAResource.start which is synchornized on EmbedXAResource. You
can not execute any other statements/functions on behalf of that global
transaction in parallel or more precisely said all of those paralel invocations
(except one) will fail.
This should be explained in comment but looking again at this I think it is
much more transparent to add a synchronization on scheduleTimeoutTask since the
function is not heavily invoked... ;-)
> - if CancelXATransactionTask.run() catches an exception, it prints it to
> the console. I'm not sure what's the best way to handle these exceptions, but
> I don't think printing them is appropriate. Perhaps it would be better to use
> Monitor.logMessage() (or perhaps create a Monitor.logThrowable() which could
> print the stack trace)?
I had no idea about correct handling either. I made it to print it to the
console just temporary but I forgot to think about he correct handling. I'll
add a method Monitor.logThrowable as you are suggesting.
> - XATransactionState.cancel() catches SQLException and re-throws it as
> XAException. Do you think we could chain the exceptions with initCause() so
> that we preserve the original error?
Probably yes, because this exception will end in Monitor.logThrowable as you
are suggesting.
I'll prepare a new version of the patch ASAP. Thanks.
> Unimplemented transaction time out for XA transactions may cause that locks
> will not be released when client terminates outside a unit of work.
> -----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-2432
> URL: https://issues.apache.org/jira/browse/DERBY-2432
> Project: Derby
> Issue Type: New Feature
> Components: JDBC
> Reporter: Julius Stroffek
> Assignee: Julius Stroffek
> Fix For: 10.3.0.0
>
> Attachments: d2432.diff, d2432.stat, description.txt
>
>
> The XAResource interface provides function setTransactionTimeout which is
> currently not supported in derby.
> When client application uses client driver to connect to derby database and
> the application crashes outside the unit of work of XA transaction and the
> transaction is not committed or rolled back yet the locks held by the
> transaction will not be released.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
