Skip to content

Cursor with multiple batches is closed twice #533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aburmeis opened this issue Jan 2, 2024 · 2 comments · Fixed by #534
Closed

Cursor with multiple batches is closed twice #533

aburmeis opened this issue Jan 2, 2024 · 2 comments · Fixed by #534
Assignees
Labels

Comments

@aburmeis
Copy link

aburmeis commented Jan 2, 2024

When watching documents from the server using a cursor, this works fine if not reading to the end or the result size is below the batch size.

When fetching multiple bulks including the last, it seems the server closes the cursor itself and ArangoCursorImpl.close() tries this again resulting in a server error. It seems the allowRetry set to true in the constructor leads to an unused close()-call even it the result has no more documents.

Relates to DE-511 #505

If the enforced close is needed for retry, maybe passing the flag to the constructor directly from the options.getOptions().getAllowRetry() in ArangoDatabaseImpl.createCursor() would have less side effects.

@aburmeis aburmeis changed the title Cursor with multiple batches is closed duplicate Cursor with multiple batches is closed twice Jan 2, 2024
@aburmeis
Copy link
Author

aburmeis commented Jan 2, 2024

sorry, just saw, this seems to be a duplicate of #528

But anyway, @rashtao wouldn't it be better to force close of the cursor only if the query option is set (just to optimise useless server roundtrips)?

@aburmeis aburmeis closed this as completed Jan 2, 2024
@rashtao rashtao self-assigned this Jan 5, 2024
@rashtao rashtao reopened this Jan 5, 2024
@rashtao
Copy link
Collaborator

rashtao commented Jan 5, 2024

Hi @aburmeis ,
thanks for reporting this.

When closing the cursor, the closing request is sent only if the following condition is met:

if (getId() != null && (allowRetry || iterator.result.getHasMore())) {

and I think this is OK.

But the real problem is the (wrong) way used to derive allowRetry:

this.allowRetry = result.getNextBatchId() != null;

since also non-retriable AQL cursors could return the nextBatchId attribute.

To fix it, allowRetry should be set based on the AQL cursor creation options.

This needs to be fixed both in ArangoCursorImpl and ArangoCursorAsyncImpl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants