[jira] [Commented] (TINKERPOP-2538) traversal is not always closed in Tinkerpop test suites

["Stephen Mallette (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TINKERPOP-2538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17306126#comment-17306126 ] 

Stephen Mallette commented on TINKERPOP-2538:
---------------------------------------------

I had thought that we had handled this problem a long time ago, but I can't find evidence for it. This was definitely an issue for orientdb at one point and I think an issue with Titan/JanusGraph. I can't help but wonder if I'm mixing this problem of closing {{Traversal}} with calling {{close()}} on the {{GraphProvider}} between suite runs as I do see some hooks in there for that, but I guess that doesn't really help in this case, especially in the situation you linked to in {{IoCustomTest}}. 

While I was writing ideas for how to fix this, it occurred to me that Divij has long ago added this:

https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/iterator/StoreIteratorCounter.java

it was meant to find iterator leaks and is used in TinkerGraph as an example which has helped find many such problems in the test suite. Here is the TinkerGraph implementation of {{Iterator}} that gets used to do this:

https://github.com/apache/tinkerpop/blob/master/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphIterator.java

I suppose it is worth noting that the test suite doesn't call {{close()}] explicitly much because iterating to the end of a {{Traversal}} should have the effect of releasing resources - i.e. {{close()}} gets called. You can see such things happen in {{next()}} for example:

https://github.com/apache/tinkerpop/blob/7f7d3a485c7f100f98047b71672a0c2c9ab855b4/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java#L230

Why did the iterator leak thing divij built not catch this leak? I think it's because {{TinkerGraphIterator}} has a {{tryComputeNext()}} to prefetch the next value and therefore a call to {{next()}} could trigger the end of a traversal. 

That said, I'm not sure what the fix should be here. Going through the test suite to explicitly add {{close()}} everywhere is a lot of busy work and is then no guarantee that new ones won't slip in later. We could build registries for traversals in {{GraphProvider}} or elsewhere to track all the traversals created so that they could be closed, but that's probably an equal amount of refactoring or worse. Perhaps graph providers should look at {{TinkerGraphIterator}} to solve the problem? Maybe for test suite purposes a pre-fetch could be enabled to allow the release of resources?

> traversal is not always closed in Tinkerpop test suites
> -------------------------------------------------------
>
>                 Key: TINKERPOP-2538
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-2538
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: test-suite
>    Affects Versions: 3.4.10
>            Reporter: Norio Akagi
>            Priority: Minor
>
> Tinkerpop offers test suites so that GraphProviders can utilize and test their own Graph database implementation.
> [http://tinkerpop.apache.org/docs/3.4.10-SNAPSHOT/dev/provider/#validating-with-gremlin-test]
> The concept is that a graph provider can pass GraphProvider and Graph class to inject their own implementation. GraphProvider needs to implement *clear* method to clean up resources after each test case is complete.
> However, depending on the design GraphProvider's implementation requires GraphTraversal to be closed after a query is complete. Right now there is no way for Graph instance to clean up running traversals.
> This leads to the issue that in unit test case some resource won't be released.
> One example can be seen here:
> [https://github.com/apache/tinkerpop/blob/0d266da3e5c274afa9306367263e5c9098bedd93/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoCustomTest.java#L151]
> In the code above, we run one traversal to get a Vertex but the traversal `g1.traversal()` is not closed.
> Ideally Tinkerpop should take this use-case into account and clean up traversals in test-suites (or at least provide a way to do that for providers without touching the test-suite code base).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)