#701 Better handling of client timeouts

v2.x
open
momo
None
1
2013-06-03
2013-05-22
Sam
No

We have recently run into a serious issue with the JTDS driver. We were using JTDS with iBatis and were using timeouts to bail out of some long running queries. Since we were also using connection pooling this was leading to serious issues with stored procedures. The root cause of the issue on SQLServer is explained in the linked page but in short if we abort a request the connection should be considered "broken" and not returned to the pool.

http://weblogs.sqlteam.com/dang/archive/2007/10/20/Use-Caution-with-Explicit-Transactions-in-Stored-Procedures.aspx

When we had a timeout, we would send the cancel request to the server and then that connection would be returned to the pool. Unfortunatley SQLServer doesn't rollback the internal transactions it had created in the stored procedures and all the rest of our commands on that connection were basically stuck inside that transaction and that led to very strange deadlock like behaviors.

I have created a patch against the 1.2.5 codebase (which we are still using). It applies cleanly to 1.2.5, 1.2.7 and 1.3.0.

Changes made:

  • Throws SQLTimeoutException instead of generic SQLException on timeout
  • Attempts rollback (regardless of autoCommit settings) on timeout
  • Closes connection
  • Includes checking of System.getProperty() for "jtds.dontRollbackOnTimeout" and "jtds.dontCloseOnTimeout" to optionally disable rollback or close behaviors (they default to enabled)
1 Attachments

Discussion

  • momo

    momo - 2013-06-02
    • status: open --> closed
    • assigned_to: momo
     
  • momo

    momo - 2013-06-02

    Hi Sam,

    thanks for your feedback. But I cannot agree with all of your points. First, throwing an SQLTimeoutException instead of a normal SQLException seems to be a good idea and I changed that in SVN with revision [1246]. But rolling back the open transaction or even closing the connection on timeout clearly is the application's responsibility. If we'd do that in the driver, we could easily break application logic and cause serious performance problems for existing code. The assumption that after a timeout the running chain of DB operations is void and could be aborted simply doesn't hold true in any case. The server is able to continue with the open connection/transaction and so should be a client, if desired. I agree this might be a problem if an application's DB logic isn't designed carefully but still, if this is causing problems then it's an application-level error, not the driver's fault.

    Cheers,
    momo

     
    Last edit: momo 2013-06-02
  • Sam

    Sam - 2013-06-03

    I agree that this is mostly an application issue, the application should be responsible for rolling back transactions but there are a few complicating factors that I think necessitate some level of support in the driver.

    Most importantly an explicit rollback is also necessary even if running in autoCommit mode. Most "higher level" libraries assume a rollback during autoCommit is unnecessary. Even if the application attempted a rollback; the JTDS library ignores rollback requests if autoCommit is enabled. That was the reason for the temporary unsetting of the autoCommit flag in the patch so a rollback is always executed. Without this rollback a partially executed stored procedure will "hang around" on the connection and cause the nasty and difficult transaction leaks. I don't think it would be appropriate to expect the application or client library to know that it needs to unset autoCommit on a timeout exception if the driver is JTDS, that would break the JDBC abstraction a bit.

    There is also the issue of unnecessarily "smart" client libraries (iBatis in this case) that make the assumption that a select statement will never need a rollback since its not affecting data. This assumption is not true in a system that makes heavy use of stored procedures since even a "simple select" statement may actually be acquiring expensive read locks which need to dropped on a timeout (since the client can't handle them).

    In any case after some further soak testing we found the closeOnTimeout to be unnecessary to workaround our issues so we have disabled that by default. I was unhappy with the System.getProperty mode of configuring the driver so I made a better patch that uses the standard url string to set options.

    Perhaps if the ROLLBACKONTIMEOUT option is defaulted to false and requires explicitly enabling that might make this patch more palatable? It is not an ideal workaround and ideally we would not be in this situation (timing out stored procs) but my company is probably not the only one suffering from similar issues. I would argue that the ms-sql timeout behavior is not the standard in the JDBC world and so therefore the best place to handle those differences is in the driver?

     
  • momo

    momo - 2013-06-03

    Hi Sam,

    well, okay. I agree we might have a problem here, so maybe we should split that into 2 scenarios:

    1: autocommit == true:

    When running in autocommit mode, the driver should really be able to roll back all transactions opened in the aborted command without any hassle. So my proposal would be to implement an automatic rollback for that case (no matter what type of command has been issued). And I think there is no reason for making that configurable in the driver - as far as I see that cannot do any harm (except a minimal performance impact caused by the additional command, which should be negligible because in my optinion a cancel request shouldn't ever be performance critial, anyway).

    2: autocommit == false

    In that case, things are completely different. We already have an open transaction (@@TRANCOUNT == 1) and by issuing a "ROLLBACK TRAN", that transaction is rolled back completely. If a called procedure now opened one or more new transactions (@@TRANCOUNT >= 2) and is aborted, I don't see a way to just roll back the "inner"/"nested" transaction(s), which could otherwise be a viable solution. But now we are facing the problem I mentioned in my first reply - simply rolling back a running transaction just because one statement has been aborted might easily break application logic. And I wouldn't expect such behaviour as an application developer, btw. That is something like 2 INSERT statements, where the second fails due to whatever and that implicitly also drops the data inserted by the first one.

    Maybe we could consider setting a savepoint before executing any command, but I think that might have a quite huge performance impact and maybe wouldn't even be correct in all cases... so if we really choose to do that, it cannot be implemented as the default behaviour.

    Any thoughts? Would the automatic rollback in autocommit mode solve your problems?

    Cheers,
    momo

     
  • momo

    momo - 2013-06-03
    • status: closed --> open
     
  • Sam

    Sam - 2013-06-03

    Yeah, the more I think about it the main issue may be the lack of rollback on autoCommit. I believe the libraries we are using correctly handle doing a rollback if autoCommit == false. Since we run exclusively with autoCommit my test suites don't try with autoCommit == false so I'm not sure if that part was necessary. I have some co-workers who are also investigating this issue, I'll try to gather their opinions as well.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks