I just did a global Intellij search in the Sqlg project. HSQLDB has 13 catch (InterruptedException e) clauses. All of them swallows the exception and none resets the interrupt flag.
Postgresql jdbc driver has 3 catch (InterruptedException e) clauses. 2 swallows the exception without resetting the interrupt flag and one throws an exception. The rest, logback, 7 catch (InterruptedException e) 1 resets the flag while the rest swallow the exception without resetting the interrupt flag google guava about 25 catch (InterruptedException e) all resets the interrupt flag hazelcast 85 catch (InterruptedException e) too many to count but some resets the interrupt flag and some don't mchange c3po pool 7 catch (InterruptedException e), 4 throws exception without resetting the interrupt flag and 3 swallow the exception without resetting the interrupt flag. mchange common 8 catch (InterruptedException e), 2 throws an exception without resetting the interrult flag and 6 complete swallow without resetting. commons-io 8 catch (InterruptedException e) 1 reset of the interrupt flag, 7 swallow the exception without resetting the interrupt flag jline 3 catch (InterruptedException e) all swallow the exception without resetting the flag. All and all I don't think using interrupt will be a reliable strategy to use. http://stackoverflow.com/questions/10401947/methods-that-clear-the-thread-interrupt-flag says that it is good practise to always reset the flag. It might be good but it is not common. >From the above rather quick search only google guava respected that good practice. AbstractStep code if (Thread.interrupted()) throw new TraversalInterruptedException(); will also reset the interrupt flag potentially making someone else's Thread.interrupted() check fail. All that said I do not have a solution for GremlinServer not having access to the traversal. Thanks Pieter On 21/07/2016 17:09, Stephen Mallette wrote: > I don't recall all the issues with doing traversal interruption with a > flag. I suppose it could work in the same way that thread interruption > works now. I will say that I'm hesitant to say that we should change this > on the basis of this being a problem general to databases as we've only > seen in so far in HSQLDB. If it was shown to be a problem in other graphs > i'd be more amplified to see a change. Not sure if any other graph > providers out there can attest to a problem with the thread interruption > approach but it would be nice to hear so if there did. > > Of course, I think you alluded to the bigger problem, which is that Gremlin > Server uses thread interruption to kill script executions and iterations > that exceed timeouts. So, the problem there is that, if someone submits a > script like this: > > t = g.V() > x = t.toList() > > that script gets pushed into a ScriptEngine.eval() method. That method > blocks until it is complete. Under that situation, Gremlin Server doesn't > have access to the Traversal to call interrupt on it. "t" is iterating via > toList() and there is no way to stop it. Not sure what we could do about > situations like that. > > On Wed, Jul 20, 2016 at 4:00 PM, pieter-gmail <pieter.mar...@gmail.com> > wrote: > >> The current interrupt implementation is failing on Sqlg's HSQLDB >> implementation. >> The reason for this is that HSQLDB itself relies on Thread.interrupt() >> for its own internal logic. When TinkerPop interrupts the thread it >> thinks it has to do with its own logic and as a result the interrupt >> flag is reset and no exception is thrown. >> >> Reading the Thread.interrupt javadocs it says that wait(), join() and >> sleep() will all reset the interrupt flag throw an InterruptedException. >> This makes TinkerPop's reliance on the flag being set somewhat fragile. >> All of those methods I suspect are common with database io code and >> TinkerPop being a high level database layer makes it susceptible to 3rd >> party interpretations of interrupt semantics. >> >> In some ways the TraversalInterruptionTest itself had to carefully reset >> the flag with its usage of Thread.sleep(). >> >> My proposal is to mark the traversal itself as interrupted rather than >> the thread and keep the logic contained to TinkerPop's space. >> >> Another benefit is that the traversal.interrupt() can raise an event >> that implementations can listen to. On receipt of the event they would >> then be able to send a separate request to the database to cancel a >> particular query. In my case would be a nice way for Sqlg to tell >> Postgresql or HSQLDB to cancel a particular query (the latest one the >> traversal executed). >> >> In many ways the semantics are the same. Currently for client code >> wanting to interrupt a particular traversal it needs to have a reference >> to the thread the traversal is executing in. Now instead it needs to >> keep a reference to executing traversals and interrupt them directly. >> >> Add Traversal.interrupt() and Traversal.isInterrupted(boolean >> ClearInterrupted) >> >> Caveat, I am not familiar with GremlinServer nor the complications >> around interrupt there so perhaps I am missing something. >> >> Thanks >> Pieter >>
