SQL: Replace the scroll with PIT for data batching #62488
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit migrates the ScrollCursor and CompositeAggCursor cursors and their dependencies away from using the scroll API to using the new PIT feature.
REST-closing a cursor after resultset exhaustion no longer needs to be done "manually", this is taken care by the cursor implementation.
Implicit grouping queries are one-shot requests, no pagination every needed, so don't start a PIT for them.
PIT won't (can't) keep the state of last served document when paginating with search_after. In order to correctly paginate also in cases when the sorting order is not done by a strictly monotone criterion (missing or repeating values, script-sorting), we need to have the _doc part of the (returned/reused) sorting key.
Fix test after upstream merge.
Rework ResultCursor class to correct the tracking of the "after" map for comp. aggs.
PIT doesn't seem to bind the user, as the scroll does -- disable the test supposed to fail on user A employing the PIT generated by user B.
|
@elasticmachine update branch |
s/@Ignore/@AwaitsFix
- further translation tests fixes. - remove PIT keep_alive value from first SQL request PIT object.
The resuming of an order request needs to take into account the _index as well for the case the queried "table" is an alias.
|
@elasticmachine update branch |
Update translation tests with new sorting source, _index.
Unlike the scroll, the PIT can be shared by multiple users, so this test becomes obsolete.
|
@elasticmachine update branch |
|
Pinging @elastic/es-ql (:Query Languages/SQL) |
elasticmachine
added
the
Team:QL (Deprecated)
bpintea
commented
Sep 21, 2020
Comment on lines
-1028
to
-1034
| Map<String, Object> response = runSql( | ||
| new StringEntity(cursor(cursor).toString(), ContentType.APPLICATION_JSON), | ||
| "/close", | ||
| Mode.PLAIN.toString() | ||
| ); | ||
| assertEquals(true, response.get("succeeded")); | ||
|
|
There was a problem hiding this comment.
This is no longer required, the SearchHit cursor wasn't properly auto-closed on result set exhaustion, but it is now.
bpintea
commented
Sep 21, 2020
| @@ -49,7 +49,7 @@ static int remainingData(boolean hasNextPage, int size, int limit) { | |||
| if (hasNextPage == false) { | |||
| return 0; | |||
| } else { | |||
| int remainingLimit = (limit == -1) ? limit : ((limit - size) >= 0 ? (limit - size) : 0); | |||
| int remainingLimit = limit == -1 ? limit : Math.max(limit - size, 0); | |||
There was a problem hiding this comment.
Change done to compact the expression and match the similar construct in the constructor (above).
PointInTimeBuilder is now a stand-alone class.
|
Hi @bpintea, I've created a changelog YAML for you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will replace usage of the scroll API in the SQL module with the new point-in-time API.
Closes #61873