[
https://issues.apache.org/jira/browse/TINKERPOP-2719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509771#comment-17509771
]
ASF GitHub Bot commented on TINKERPOP-2719:
-------------------------------------------
divijvaidya commented on a change in pull request #1586:
URL: https://github.com/apache/tinkerpop/pull/1586#discussion_r830964395
##########
File path:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
##########
@@ -383,9 +383,14 @@ protected void handleIterator(final Context context, final
Iterator itty, final
break;
}
+ // track whether there is anything left in the iterator
because it needs to be accessed after
+ // the transaction could be closed - in that case a call
to hasNext() could open a new transaction
+ // unintentionally
+ final boolean moreInIterator = itty.hasNext();
Review comment:
Sorry, I did not understand your explanation. Let me try to rephrase it
and please let me know what am I missing here.
**The problem**
Transaction is closed in `onTraversalSuccess` function(), hence, any call to
`itty.hasNext()` after that may open a new transaction. We do have a need to
know if there is more information in the iterator so that we can decide whether
to call `iterateComplete` or not.
**Solution**
Store the state of `itty.hasNext()` before calling `onTraversalSuccess` and
use the same state variable to determine whether to call `iterateComplete` or
not.
Currently, `hasMore` is set to false when there are no more results in the
iterator. It is also used to determine whether to continue looping or not. If
we use `hasMore` at the place where you are using `moreInIterator`, we are just
setting the value of hasMore a few lines beforehand (which is completely fine).
We just don't reset it.
```
hasMore = itty.hasNext();
try {
// only need to reset the aggregation list if
there's more stuff to write
if (hasMore)
aggregate = new
ArrayList<>(resultIterationBatchSize);
else {
// iteration and serialization are both complete
which means this finished successfully. note that
// errors internal to script eval or timeout
will rollback given GremlinServer's global configurations.
// local errors will get rolledback below
because the exceptions aren't thrown in those cases to be
// caught by the GremlinExecutor for global
rollback logic. this only needs to be committed if
// there are no more items to iterate and
serialization is complete
onTraversalSuccess(graph, context);
}
} catch (Exception ex) {
// a frame may use a Bytebuf which is a countable
release - if it does not get written
// downstream it needs to be released here
if (frame != null) frame.tryRelease();
throw ex;
}
if (!hasMore) iterateComplete(nettyContext, msg, itty);
```
> So if we also use hasMore here instead, then hasMore cannot become false
any more.
Which is ok. We can remove the statement where we are setting hasMore to
false since we are now setting it a few lines earlier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
> hasNext is called on TraverserIterator after transaction is committed
> ---------------------------------------------------------------------
>
> Key: TINKERPOP-2719
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2719
> Project: TinkerPop
> Issue Type: Bug
> Components: server
> Affects Versions: 3.5.2
> Reporter: Florian G
> Priority: Major
>
> In {{{}TraversalOpProcessor{}}}, the {{hasNext()}} [function is
> called|https://github.com/apache/tinkerpop/blob/a52887d084053cd420580a0c0a80e745b49ee252/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java#L428]
> on a {{TraversalIterator}} object after the transaction [is
> committed|https://github.com/apache/tinkerpop/blob/a52887d084053cd420580a0c0a80e745b49ee252/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java#L414].
> This causes issues if the database tries to further execute the traversal
> upon the {{hasNext()}} call but has no access to the transaction anymore.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
