Skip to content
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

Fix for Keys availability upon Session.RunAsync and Transaction.RunAsync #235

Merged
merged 3 commits into from
Sep 15, 2017

Conversation

ali-ince
Copy link
Contributor

No description provided.

Copy link
Contributor

@zhenlineo zhenlineo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also Fix the sync Result to be the same as java where we will wait for the first Success if the user calls result.Keys?

@@ -222,6 +223,10 @@ public override Task<IStatementResultCursor> RunAsync(Statement statement)
_connection.Server);
_connection.Run(statement.Text, statement.Parameters, resultBuilder);
await _connection.SendAsync().ConfigureAwait(false);

// Wait for SUCCESS/FAILURE message from the server
await _connection.ReceiveOneAsync().ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought for transaction run, if we did not provide bookmark, at line 225 we would have queued messages:

Run Begin
Pull_All
Run transaction XX
Pull_All

Then if we send and just wait for the first reply, then we only got reply for Run Begin?

I am now wondering why tests would pass...

Copy link
Contributor

@zhenlineo zhenlineo Sep 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested locally, the receiveOneAsync will only wait for Success for the firstRun Begin message. While as we batching messages and handle them together if them fit in buffer, when Success for Run Begin returns, the others messages also arrived and processed in time.

@zhenlineo zhenlineo merged commit e8f44de into neo4j:1.5 Sep 15, 2017
@ali-ince ali-ince deleted the 1.5-wait-for-success-after-runasync branch September 27, 2017 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants