Ah…. an InterruptStrategy! … I'm liking that. Matt, can you say a bit more about such a design? If you can wrap an entire traversal in its own isolated thread and then be able to kill it when you want, that would be pretty sweet --- however, does that mess up threaded transactions and the one-thead-per-transaction-model that TP3 current supports?
Marko. http://markorodriguez.com On Oct 16, 2015, at 9:08 AM, Matt Frantz <matthew.h.fra...@gmail.com> wrote: > Perhaps a strategy could insert interrupt checks in appropriate places, > avoiding inner loops, or perhaps refactoring them into two-level loops > where we check for interrupt on the outer level. > > On Fri, Oct 16, 2015 at 6:58 AM, Marko Rodriguez <okramma...@gmail.com> > wrote: > >> Hi, >> >> This is a scary body of work as processNextStarts() is not just in the >> base steps MapStep/FlatMapStep/BranchStep/etc., but littered throughout >> where extensions to the base steps are not easily done (e.g. barriers, >> etc.). Next, benchmark --- for every next there is an interrupt check >> (eek!). >> >> The idea of being able to interrupt a Traversal is nice, but the >> implementation and performance details are probably going to prove daunting. >> >> Marko. >> >> http://markorodriguez.com >> >> On Oct 16, 2015, at 5:31 AM, Daniel Kuppitz <m...@gremlin.guru> wrote: >> >>> This is a nice one. Once we have that in place, we can probably/hopefully >>> also provide a hotkey to gracefully cancel queries in the REPL (w/o >> having >>> to exit the console itself). >>> >>> Cheers, >>> Daniel >>> >>> >>> On Fri, Oct 16, 2015 at 1:05 PM, Stephen Mallette <spmalle...@gmail.com> >>> wrote: >>> >>>> I'm going to frame this in the context of Gremlin Server but I think >> this >>>> issue generally applies to any threaded application development done >> with >>>> TinkerPop. >>>> >>>> Gremlin Server does a number of things to protect itself from crazy >> scripts >>>> (i.e. long run scripts that may have been accidentally or maliciously >> sent >>>> to it). The one thing it doesn't do perfectly is properly kill running >>>> scripts in the midst of a long run traversal. It attempts to stop a >>>> running script by interrupting the thread that is processing it, but if >> the >>>> thread is at a point in the script that doesn't respect >> Thread.interrupt(), >>>> it basically just chews up that thread until it reaches a spot that >> does. >>>> >>>> I think Traversal could do a better job respecting interrupts. Put in >> code >>>> speak, we should look to get this test to pass for all steps: >>>> >>>> @Test >>>> public void shouldRespectThreadInterruption() throws Exception { >>>> final AtomicBoolean exceptionThrown = new AtomicBoolean(false); >>>> final CountDownLatch startedIterating = new CountDownLatch(1000); >>>> final List<Integer> l = IntStream.range(0, >>>> 1000000).boxed().collect(Collectors.toList()); >>>> >>>> final Thread t = new Thread(() -> { >>>> try { >>>> __.inject(l).unfold().sideEffect(i -> >>>> startedIterating.countDown()).iterate(); >>>> fail("Should have respected the thread interrupt"); >>>> } catch (Exception ie) { >>>> exceptionThrown.set(ie instanceof RuntimeException); >>>> } >>>> }); >>>> >>>> t.start(); >>>> >>>> startedIterating.await(); >>>> t.interrupt(); >>>> >>>> t.join(); >>>> assertThat(exceptionThrown.get(), CoreMatchers.is(true)); >>>> } >>>> >>>> Two things to note...First, the changes to make this test pass >> shouldn't be >>>> "big". I got this test to pass with the addition of this line of code >> in >>>> FlatMapStep.processNextStart() on the first line after the start of the >>>> while(true). >>>> >>>> if(Thread.interrupted) throw new RuntimeException(); >>>> >>>> That single line inserted in the right places should get interrupt >> working >>>> properly across Traversal. >>>> >>>> Second, note that I'm throwing/asserting RuntimeException. I probably >>>> should be throwing a InterruptedException but that is a checked >> exception >>>> and that would really pollute up our Traversal code. So, my suggestion >> is >>>> to create our own "InterruptedRuntimeException" that we can trap >> separately >>>> that would potentially wrap an actual InterruptedException. >>>> >>>> I'd like to tack this issue in to 3.1.0 if possible - please let me know >>>> your thoughts if you have any. >>>> >> >>
