Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: 3.0.2-incubating
    • Fix Version/s: 3.2.1
    • Component/s: process
    • Labels:
      None

      Description

      This issue was discussed to some degree here:

      https://pony-poc.apache.org/thread.html/e6477fc9c58d37a5bdcb5938a0eaa285456ad15aa39e16446290e2ff@1444993523@%3Cdev.tinkerpop.apache.org%3E

      There's a good half-way solution in all this that balances performance with the desired capability.

        Activity

        Hide
        spmallette stephen mallette added a comment - - edited

        Did some work here to implement this and ran gremlin-benchmark ( https://gist.github.com/spmallette/ed21267f2e7e17bb3fbd5a8d1a568d2b ) :

        WITHOUT INTERRUPT
        
        Benchmark                                                                                         Mode  Cnt     Score     Error  Units
        GraphTraversalBenchmark.g_E_hasLabelXwrittenByX_whereXinV_inEXsungByX_count_isX0XX_subgraphXsgX  thrpt   20   433.002 ±  22.085  ops/s
        GraphTraversalBenchmark.g_V_label_groupCount                                                     thrpt   20  7209.703 ± 130.932  ops/s
        GraphTraversalBenchmark.g_V_localXout_out_valuesXnameX_foldX                                     thrpt   20    13.210 ±   0.524  ops/s
        GraphTraversalBenchmark.g_V_match_selectXbX_valuesXnameX                                         thrpt   20  3186.930 ± 388.578  ops/s
        GraphTraversalBenchmark.g_V_outE_inV_outE_inV_outE_inV                                           thrpt   20     0.885 ±   0.025  ops/s
        GraphTraversalBenchmark.g_V_out_localXout_out_valuesXnameX_foldX                                 thrpt   20     0.233 ±   0.053  ops/s
        GraphTraversalBenchmark.g_V_out_mapXout_out_valuesXnameX_toListX                                 thrpt   20     0.423 ±   0.009  ops/s
        GraphTraversalBenchmark.g_V_out_out_out                                                          thrpt   20     0.903 ±   0.015  ops/s
        GraphTraversalBenchmark.g_V_out_out_out_path                                                     thrpt   20     0.058 ±   0.013  ops/s
        GraphTraversalBenchmark.g_V_repeatXoutX_timesX2X                                                 thrpt   20    23.742 ±   0.717  ops/s
        GraphTraversalBenchmark.g_V_repeatXoutX_timesX3X                                                 thrpt   20     2.345 ±   0.099  ops/s
        
        WITH INTERRUPT
        
        Benchmark                                                                                         Mode  Cnt     Score     Error  Units
        GraphTraversalBenchmark.g_E_hasLabelXwrittenByX_whereXinV_inEXsungByX_count_isX0XX_subgraphXsgX  thrpt   20   424.980 ±  25.509  ops/s
        GraphTraversalBenchmark.g_V_label_groupCount                                                     thrpt   20  6920.125 ± 288.286  ops/s
        GraphTraversalBenchmark.g_V_localXout_out_valuesXnameX_foldX                                     thrpt   20    13.147 ±   0.589  ops/s
        GraphTraversalBenchmark.g_V_match_selectXbX_valuesXnameX                                         thrpt   20  3663.093 ± 242.539  ops/s
        GraphTraversalBenchmark.g_V_outE_inV_outE_inV_outE_inV                                           thrpt   20     0.914 ±   0.034  ops/s
        GraphTraversalBenchmark.g_V_out_localXout_out_valuesXnameX_foldX                                 thrpt   20     0.317 ±   0.008  ops/s
        GraphTraversalBenchmark.g_V_out_mapXout_out_valuesXnameX_toListX                                 thrpt   20     0.433 ±   0.012  ops/s
        GraphTraversalBenchmark.g_V_out_out_out                                                          thrpt   20     0.912 ±   0.024  ops/s
        GraphTraversalBenchmark.g_V_out_out_out_path                                                     thrpt   20     0.045 ±   0.009  ops/s
        GraphTraversalBenchmark.g_V_repeatXoutX_timesX2X                                                 thrpt   20    22.773 ±   0.436  ops/s
        GraphTraversalBenchmark.g_V_repeatXoutX_timesX3X                                                 thrpt   20     2.155 ±   0.131  ops/s
        
        WITHOUT INTERRUPT
        
        Benchmark                                     Mode  Cnt         Score         Error  Units
        GraphMutateBenchmark.testAddE                thrpt   20     40856.030 ±    4533.506  ops/s
        GraphMutateBenchmark.testAddEdge             thrpt   20   3264592.648 ±  258747.094  ops/s
        GraphMutateBenchmark.testAddV                thrpt   20    553033.856 ±   22083.826  ops/s
        GraphMutateBenchmark.testAddVertex           thrpt   20   5788322.626 ±  405728.238  ops/s
        GraphMutateBenchmark.testEdgeProperty        thrpt   20  10828146.027 ± 1575892.641  ops/s
        GraphMutateBenchmark.testEdgePropertyStep    thrpt   20    184517.929 ±    7717.985  ops/s
        GraphMutateBenchmark.testVertexProperty      thrpt   20   4664901.713 ±  355620.823  ops/s
        GraphMutateBenchmark.testVertexPropertyStep  thrpt   20    164545.617 ±   16218.479  ops/s
        
        WITH INTERRUPT
        
        Benchmark                                     Mode  Cnt         Score         Error  Units
        GraphMutateBenchmark.testAddE                thrpt   20     40366.064 ±    5311.549  ops/s
        GraphMutateBenchmark.testAddEdge             thrpt   20   3394429.025 ±  215705.312  ops/s
        GraphMutateBenchmark.testAddV                thrpt   20    535506.229 ±   16138.350  ops/s
        GraphMutateBenchmark.testAddVertex           thrpt   20   5685259.448 ±  492161.001  ops/s
        GraphMutateBenchmark.testEdgeProperty        thrpt   20  11337725.761 ± 1600906.128  ops/s
        GraphMutateBenchmark.testEdgePropertyStep    thrpt   20    180209.108 ±    8663.875  ops/s
        GraphMutateBenchmark.testVertexProperty      thrpt   20   4628182.249 ±  393292.238  ops/s
        GraphMutateBenchmark.testVertexPropertyStep  thrpt   20    169016.050 ±    7639.421  ops/s
        
        Show
        spmallette stephen mallette added a comment - - edited Did some work here to implement this and ran gremlin-benchmark ( https://gist.github.com/spmallette/ed21267f2e7e17bb3fbd5a8d1a568d2b ) : WITHOUT INTERRUPT Benchmark Mode Cnt Score Error Units GraphTraversalBenchmark.g_E_hasLabelXwrittenByX_whereXinV_inEXsungByX_count_isX0XX_subgraphXsgX thrpt 20 433.002 ± 22.085 ops/s GraphTraversalBenchmark.g_V_label_groupCount thrpt 20 7209.703 ± 130.932 ops/s GraphTraversalBenchmark.g_V_localXout_out_valuesXnameX_foldX thrpt 20 13.210 ± 0.524 ops/s GraphTraversalBenchmark.g_V_match_selectXbX_valuesXnameX thrpt 20 3186.930 ± 388.578 ops/s GraphTraversalBenchmark.g_V_outE_inV_outE_inV_outE_inV thrpt 20 0.885 ± 0.025 ops/s GraphTraversalBenchmark.g_V_out_localXout_out_valuesXnameX_foldX thrpt 20 0.233 ± 0.053 ops/s GraphTraversalBenchmark.g_V_out_mapXout_out_valuesXnameX_toListX thrpt 20 0.423 ± 0.009 ops/s GraphTraversalBenchmark.g_V_out_out_out thrpt 20 0.903 ± 0.015 ops/s GraphTraversalBenchmark.g_V_out_out_out_path thrpt 20 0.058 ± 0.013 ops/s GraphTraversalBenchmark.g_V_repeatXoutX_timesX2X thrpt 20 23.742 ± 0.717 ops/s GraphTraversalBenchmark.g_V_repeatXoutX_timesX3X thrpt 20 2.345 ± 0.099 ops/s WITH INTERRUPT Benchmark Mode Cnt Score Error Units GraphTraversalBenchmark.g_E_hasLabelXwrittenByX_whereXinV_inEXsungByX_count_isX0XX_subgraphXsgX thrpt 20 424.980 ± 25.509 ops/s GraphTraversalBenchmark.g_V_label_groupCount thrpt 20 6920.125 ± 288.286 ops/s GraphTraversalBenchmark.g_V_localXout_out_valuesXnameX_foldX thrpt 20 13.147 ± 0.589 ops/s GraphTraversalBenchmark.g_V_match_selectXbX_valuesXnameX thrpt 20 3663.093 ± 242.539 ops/s GraphTraversalBenchmark.g_V_outE_inV_outE_inV_outE_inV thrpt 20 0.914 ± 0.034 ops/s GraphTraversalBenchmark.g_V_out_localXout_out_valuesXnameX_foldX thrpt 20 0.317 ± 0.008 ops/s GraphTraversalBenchmark.g_V_out_mapXout_out_valuesXnameX_toListX thrpt 20 0.433 ± 0.012 ops/s GraphTraversalBenchmark.g_V_out_out_out thrpt 20 0.912 ± 0.024 ops/s GraphTraversalBenchmark.g_V_out_out_out_path thrpt 20 0.045 ± 0.009 ops/s GraphTraversalBenchmark.g_V_repeatXoutX_timesX2X thrpt 20 22.773 ± 0.436 ops/s GraphTraversalBenchmark.g_V_repeatXoutX_timesX3X thrpt 20 2.155 ± 0.131 ops/s WITHOUT INTERRUPT Benchmark Mode Cnt Score Error Units GraphMutateBenchmark.testAddE thrpt 20 40856.030 ± 4533.506 ops/s GraphMutateBenchmark.testAddEdge thrpt 20 3264592.648 ± 258747.094 ops/s GraphMutateBenchmark.testAddV thrpt 20 553033.856 ± 22083.826 ops/s GraphMutateBenchmark.testAddVertex thrpt 20 5788322.626 ± 405728.238 ops/s GraphMutateBenchmark.testEdgeProperty thrpt 20 10828146.027 ± 1575892.641 ops/s GraphMutateBenchmark.testEdgePropertyStep thrpt 20 184517.929 ± 7717.985 ops/s GraphMutateBenchmark.testVertexProperty thrpt 20 4664901.713 ± 355620.823 ops/s GraphMutateBenchmark.testVertexPropertyStep thrpt 20 164545.617 ± 16218.479 ops/s WITH INTERRUPT Benchmark Mode Cnt Score Error Units GraphMutateBenchmark.testAddE thrpt 20 40366.064 ± 5311.549 ops/s GraphMutateBenchmark.testAddEdge thrpt 20 3394429.025 ± 215705.312 ops/s GraphMutateBenchmark.testAddV thrpt 20 535506.229 ± 16138.350 ops/s GraphMutateBenchmark.testAddVertex thrpt 20 5685259.448 ± 492161.001 ops/s GraphMutateBenchmark.testEdgeProperty thrpt 20 11337725.761 ± 1600906.128 ops/s GraphMutateBenchmark.testEdgePropertyStep thrpt 20 180209.108 ± 8663.875 ops/s GraphMutateBenchmark.testVertexProperty thrpt 20 4628182.249 ± 393292.238 ops/s GraphMutateBenchmark.testVertexPropertyStep thrpt 20 169016.050 ± 7639.421 ops/s
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user spmallette opened a pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299

        TINKERPOP-946 Interrupting Traversals

        https://issues.apache.org/jira/browse/TINKERPOP-946

        This is a mostly complete implementation of traversal interruption. I think it will cover most use cases and as such makes it a candidate for merging back to master. I think the areas where it is lacking is around cancellation of jobs in giraph/hadoop and perhaps some corner cases in existing steps which might have been missed somehow. I think we can just open separate jira tickets for these things as they arise. I have at least one or two in mind already.

        I came up with methods to test interruption with unit tests (and it's easy to add new ones) but an easy way to test this out in the console is to do something like:

        ```text
        gremlin> t = new Thread({
        gremlin> try

        { gremlin> println g.V().out().out().toList() gremlin> }

        catch (Exception ex) { ex.printStackTrace() }})
        ==>Thread[Thread-91,5,main]
        gremlin> t.start()
        ==>null
        gremlin> t.interrupt()
        ==>null
        gremlin> org.apache.tinkerpop.gremlin.process.traversal.util.TraversalInterruptedException
        at org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.VertexProgramStep.processNextStart(VertexProgramStep.java:80)
        at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.hasNext(AbstractStep.java:143)
        at org.apache.tinkerpop.gremlin.process.traversal.step.util.ExpandableStepIterator.next(ExpandableStepIterator.java:50)
        at org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.ComputerResultStep.processNextStart(ComputerResultStep.java:70)
        at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.next(AbstractStep.java:128)
        at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.next(AbstractStep.java:38)
        at org.apache.tinkerpop.gremlin.process.traversal.Traversal.fill(Traversal.java:146)
        at org.apache.tinkerpop.gremlin.process.traversal.Traversal.toList(Traversal.java:103)
        at org.apache.tinkerpop.gremlin.process.traversal.Traversal$toList.call(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:117)
        at groovysh_evaluate$_run_closure1.doCall(groovysh_evaluate:5)
        at groovysh_evaluate$_run_closure1.doCall(groovysh_evaluate)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1021)
        at groovy.lang.Closure.call(Closure.java:426)
        at groovy.lang.Closure.call(Closure.java:420)
        at groovy.lang.Closure.run(Closure.java:507)
        at java.lang.Thread.run(Thread.java:745)
        ```

        The above is a test of interrupting `SparkGraphComputer` - works well and kills the remotely executing spark job.

        `RemoteGraph` interruption may need some specific attention. There is no way to remote kill a job so the best that will happen is the client thread gets released.

        Ran full integration tests to success: `mvn clean install -DskipIntegrationTests=false`

        VOTE +1

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP-946

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/incubator-tinkerpop/pull/299.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 #299


        commit cc533c7574f4c32d367a4d1ac77d3e70daef3e8b
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2016-04-18T12:50:33Z

        Added support for Traversal interruption.

        Check for Thread.interrupted() in loops within steps and throw an unchecked TraversalInterruptedException. Would be better to throw InterruptedException but that would involve a breaking change which would be nice to avoid.

        commit fd16fabd7595470bd33f30b882b1f0297d05b55a
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2016-04-19T23:09:20Z

        Added process tests for traversal interruption

        Covered GraphStep, VertexStep and PropertiesStep which should yield some coverage to provider implementations.

        commit 7a9f7de6ebed5d4293708524c772db0f0b2c3bac
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2016-04-20T11:52:01Z

        Refactored Traversal interruption tests.

        Provided better coverage and easier maintenance by making the tests parameterized.

        commit db6dfbcea68d1d698cc9eabcfd47ddaf6e4476e6
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2016-04-22T19:26:35Z

        OLAP Traversals now support traversal interruption.

        Implemented for SparkGraphComputer and TinkerGraphComputer.

        commit e235c128ee148399a385df7e4f546926bda8a6fe
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2016-04-22T19:49:29Z

        Cleaned up OptOuts around traversal interruption test for giraph/spark.

        commit fd30fc289eac25baba34a5cb88f6deff80b571c7
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2016-04-30T10:31:27Z

        Added a couple more OptOuts.

        The interruption tests are not good for RemoteGraph or HadoopGraph. The interruption models are different there given their different processing. Those might need specific testing.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/incubator-tinkerpop/pull/299 TINKERPOP-946 Interrupting Traversals https://issues.apache.org/jira/browse/TINKERPOP-946 This is a mostly complete implementation of traversal interruption. I think it will cover most use cases and as such makes it a candidate for merging back to master. I think the areas where it is lacking is around cancellation of jobs in giraph/hadoop and perhaps some corner cases in existing steps which might have been missed somehow. I think we can just open separate jira tickets for these things as they arise. I have at least one or two in mind already. I came up with methods to test interruption with unit tests (and it's easy to add new ones) but an easy way to test this out in the console is to do something like: ```text gremlin> t = new Thread({ gremlin> try { gremlin> println g.V().out().out().toList() gremlin> } catch (Exception ex) { ex.printStackTrace() }}) ==>Thread [Thread-91,5,main] gremlin> t.start() ==>null gremlin> t.interrupt() ==>null gremlin> org.apache.tinkerpop.gremlin.process.traversal.util.TraversalInterruptedException at org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.VertexProgramStep.processNextStart(VertexProgramStep.java:80) at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.hasNext(AbstractStep.java:143) at org.apache.tinkerpop.gremlin.process.traversal.step.util.ExpandableStepIterator.next(ExpandableStepIterator.java:50) at org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.ComputerResultStep.processNextStart(ComputerResultStep.java:70) at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.next(AbstractStep.java:128) at org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep.next(AbstractStep.java:38) at org.apache.tinkerpop.gremlin.process.traversal.Traversal.fill(Traversal.java:146) at org.apache.tinkerpop.gremlin.process.traversal.Traversal.toList(Traversal.java:103) at org.apache.tinkerpop.gremlin.process.traversal.Traversal$toList.call(Unknown Source) at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:117) at groovysh_evaluate$_run_closure1.doCall(groovysh_evaluate:5) at groovysh_evaluate$_run_closure1.doCall(groovysh_evaluate) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93) at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325) at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294) at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1021) at groovy.lang.Closure.call(Closure.java:426) at groovy.lang.Closure.call(Closure.java:420) at groovy.lang.Closure.run(Closure.java:507) at java.lang.Thread.run(Thread.java:745) ``` The above is a test of interrupting `SparkGraphComputer` - works well and kills the remotely executing spark job. `RemoteGraph` interruption may need some specific attention. There is no way to remote kill a job so the best that will happen is the client thread gets released. Ran full integration tests to success: `mvn clean install -DskipIntegrationTests=false` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP-946 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tinkerpop/pull/299.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 #299 commit cc533c7574f4c32d367a4d1ac77d3e70daef3e8b Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-04-18T12:50:33Z Added support for Traversal interruption. Check for Thread.interrupted() in loops within steps and throw an unchecked TraversalInterruptedException. Would be better to throw InterruptedException but that would involve a breaking change which would be nice to avoid. commit fd16fabd7595470bd33f30b882b1f0297d05b55a Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-04-19T23:09:20Z Added process tests for traversal interruption Covered GraphStep, VertexStep and PropertiesStep which should yield some coverage to provider implementations. commit 7a9f7de6ebed5d4293708524c772db0f0b2c3bac Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-04-20T11:52:01Z Refactored Traversal interruption tests. Provided better coverage and easier maintenance by making the tests parameterized. commit db6dfbcea68d1d698cc9eabcfd47ddaf6e4476e6 Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-04-22T19:26:35Z OLAP Traversals now support traversal interruption. Implemented for SparkGraphComputer and TinkerGraphComputer. commit e235c128ee148399a385df7e4f546926bda8a6fe Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-04-22T19:49:29Z Cleaned up OptOuts around traversal interruption test for giraph/spark. commit fd30fc289eac25baba34a5cb88f6deff80b571c7 Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-04-30T10:31:27Z Added a couple more OptOuts. The interruption tests are not good for RemoteGraph or HadoopGraph. The interruption models are different there given their different processing. Those might need specific testing.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkuppitz commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-215975367

        `docker/build.sh -t -i -n` succeeded.

        VOTE: +1

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkuppitz commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-215975367 `docker/build.sh -t -i -n` succeeded. VOTE: +1
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user okram commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-215977029

        I don't think you need to put as may interrupt checks. Most steps inherit from `AbstractStep` and thus, if `AbstractStep.next()` and `AbstractStep.hasNext()` have the interrupt check, then all the `processNextStart()` in subclasses don't need to check for the interrupt. This should greatly reduce the amount of classes touched and requirements on providers with their own step implementations. In short, they just need to make sure they extend `AbstractStep`. However, with that said, I still think you need the `VertexProgramStep` work in there as you have it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user okram commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-215977029 I don't think you need to put as may interrupt checks. Most steps inherit from `AbstractStep` and thus, if `AbstractStep.next()` and `AbstractStep.hasNext()` have the interrupt check, then all the `processNextStart()` in subclasses don't need to check for the interrupt. This should greatly reduce the amount of classes touched and requirements on providers with their own step implementations. In short, they just need to make sure they extend `AbstractStep`. However, with that said, I still think you need the `VertexProgramStep` work in there as you have it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user spmallette commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216502810

        I just pushed a change as you suggested @okram

        Show
        githubbot ASF GitHub Bot added a comment - Github user spmallette commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216502810 I just pushed a change as you suggested @okram
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user okram commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216542972

        You don't need it in `ComputerResultStep` either as that is a simple `AbstractStep` and thus, gets the "interrupt check" from `next()/hasNext()` of `AbstractStep`.

        VOTE +1. – very cool we finally have this. Also, super glad its not some insane reengineering of all the steps.

        Show
        githubbot ASF GitHub Bot added a comment - Github user okram commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216542972 You don't need it in `ComputerResultStep` either as that is a simple `AbstractStep` and thus, gets the "interrupt check" from `next()/hasNext()` of `AbstractStep`. VOTE +1. – very cool we finally have this. Also, super glad its not some insane reengineering of all the steps.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user spmallette commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216825501

        Removed the `ComputerResultStep` interruption check and rebased. Waiting for a clean build then will merge.

        Show
        githubbot ASF GitHub Bot added a comment - Github user spmallette commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216825501 Removed the `ComputerResultStep` interruption check and rebased. Waiting for a clean build then will merge.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user spmallette commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216893595

        Ok - changes are good with: `$ docker/build.sh -t -i -n`

        Show
        githubbot ASF GitHub Bot added a comment - Github user spmallette commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/299#issuecomment-216893595 Ok - changes are good with: `$ docker/build.sh -t -i -n`
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/incubator-tinkerpop/pull/299

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-tinkerpop/pull/299

          People

          • Assignee:
            spmallette stephen mallette
            Reporter:
            spmallette stephen mallette
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development