Skip to content

Conversation

lutovich
Copy link
Contributor

All result stream should be buffered/consumed when result summary is requested or when session is closed. This means underlying channel should read everything that's available. It did not do so when summary was requested after number of buffered records exceeded high watermark.

This PR makes fetching of summary or failure enable auto-read on the underlying channel. So SUCCESS and FAILURE messages will be read without waiting for all records to be consumed.

All result stream should be buffered/consumed when result summary is
requested or when session is closed. This means underlying channel
should read everything that's available. It did not do so when summary
was requested after number of buffered records exceeded high watermark.

This commit makes fetching of summary or failure enable auto-read on
the underlying channel. So SUCCESS and FAILURE messages will be read
without waiting for all records to be consumed.
@lutovich lutovich requested a review from ali-ince November 30, 2017 13:35
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

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

Changes look ok for me. One thing I found myself thinking is whether we should by-pass queuing recods in PullAllResponseHandler when StatementResultCursor#consumeAsync is called. WDYT?

@lutovich
Copy link
Contributor Author

Yeah, I think it's possible. Currently PullAllResponseHandler exposes a fairly small set of methods and #consume() builds on them. We could move it to PullAllResponseHandler and skip queueing. Similar optimization can be made for #listAsync() - we could just return a copy of the records queue.

@lutovich lutovich merged commit 026e684 into neo4j:1.5 Nov 30, 2017
@lutovich lutovich deleted the 1.5-auto-read-issue branch November 30, 2017 14:29
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.

2 participants