GitHub user spmallette opened a pull request:
https://github.com/apache/tinkerpop/pull/413
TINKERPOP-1442 Improved session cleanup on client close [master]
https://issues.apache.org/jira/browse/TINKERPOP-1442
see #412 for more description
All unit tests and gremlin-server integration tests are passing on this branch.
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1442-master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/413.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #413
commit e7d63381dcc0c53999f4270f0e6b9d4a56093673
Author: Stephen Mallette <spmva@genoprime.com>
Date: 2016-09-13T22:10:09Z
Improved session cleanup on client close.
While not a perfect implementation, a long run job blocking a close request from the client will now at least get an attempt at interruption rather thant consuming the thread indefinitely. TINKERPOP-1442
commit cd07845479c5e945683cb5e49b92a74277295e55
Author: Stephen Mallette <spmva@genoprime.com>
Date: 2016-09-13T22:17:23Z
Merge branch 'TINKERPOP-1442' into TINKERPOP-1442-master
Tried several different approaches to solving this - each were valiant attempts - but ultimately there's no perfect way to do this and we end up with the Graph implementation stuck with the having to clean up it's own open transactions. The problem is that a session must maintain a single thread associated with it. If you interrupt that thread in doing its work then you orphan the transaction because you can't then issue more work in that thread to rollback incomplete transactions. Perhaps this is another reason to decouple thread interruption from traversal interruption, however, thread interruption still seems like a necessary approach to killing the session because a script may have nothing to do with "traversal interruption". You may just be trying to break out of a Thread.sleep().
Anyway, I've at least got this setup so that you can issue a "close" on a session and if there's a job blocking it will attempt to kill it - that's more than what we had before. In this way, that session thread won't run into oblivion on close waiting for a timeout to trigger. The timeout could be quite high in some cases (like for an OLAP traversal for example) and if enough of those hung around it wouldn't be healthy for the server.
I was able to make the change without breaks and no new additions to the protocol. Running tests and then will issue PRs.