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

Allow simple/async sessions to request data as needed #662

Merged

Conversation

gjmwoods
Copy link
Contributor

Optimizing record buffer for AutoPullResponseHandler. Simple and Async sessions will now buffer records in a similar way to the reactive sessions by only requesting more records when the Query's consumer is close to catching up. This prevents excessive records being buffered by the driver/client.

The driver will request additional records when the current buffer contains less than 30% of the fetchSize (LOW_RECORD_WATERMARK). When the driver has buffered more than 70% of the fetchSize (HIGH_RECORD_WATERMARK), the driver will stop requesting more records until it drops below the LOW_RECORD_WATERMARK. A fetchSize of zero effectively disables this buffering.

@zhenlineo
Copy link
Contributor

Can you have a test for ResultCursor#listAsync, where we define fetchSize = 10?
I am thinking in this situation. we will pre-pull 10 records locally. However as it exceed the HIGH_RECORD_WATERMARK, then we might not be able to send PULL(n=-1) until the user has consumed any records locally. The user will not be able to consume records locally as the user is waiting for all records arrive into a list.

@gjmwoods gjmwoods force-pushed the 4.0-apply-backpressure-simple-async-sessions branch from d4708c5 to 0f147ca Compare December 16, 2019 10:06
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.

A small case where fetchSize = 1 may not work as expected.

if ( fetchSize == UNLIMITED_FETCH_SIZE )
{
this.HIGH_RECORD_WATERMARK = Long.MAX_VALUE;
this.LOW_RECORD_WATERMARK = Long.MAX_VALUE;
Copy link
Contributor

Choose a reason for hiding this comment

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

The vars are not static, so they do not need to be capitalised.

records.add( record );
}

private Record dequeueRecord()
{
if ( records.size() < LOW_RECORD_WATERMARK )
Copy link
Contributor

Choose a reason for hiding this comment

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

"<=" for the case when fetchSize = 1. (or any case where the HIGH_RECORD_WATERMARK == LOW_RECORD_WATERMARK)

@gjmwoods gjmwoods force-pushed the 4.0-apply-backpressure-simple-async-sessions branch from 0f147ca to 001e843 Compare December 16, 2019 15:25
@zhenlineo zhenlineo merged commit 983146a into neo4j:4.0 Dec 17, 2019
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