-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
document the search context is freed if the scroll is not extended #34739
Conversation
The `fetchPhaseShouldFreeContext` returns true when there is a scroll context but the scroll parameter is null, thus freeing the search context. https://github.com/elastic/elasticsearch/blob/183c32d4c39948e037a7fb44dccf31ab0d60d3c3/server/src/main/java/org/elasticsearch/search/SearchService.java#L491
Pinging @elastic/es-search-aggs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change the wording a little? I'm worried it won't be clear to someone who hasn't read the code.
@@ -109,7 +109,8 @@ request) tells Elasticsearch how long it should keep the search context alive. | |||
Its value (e.g. `1m`, see <<time-units>>) does not need to be long enough to | |||
process all data -- it just needs to be long enough to process the previous | |||
batch of results. Each `scroll` request (with the `scroll` parameter) sets a | |||
new expiry time. | |||
new expiry time. If the `scroll` parameter is not passed, then the next |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a scroll request doesn't pass in scroll
then we'll free the scroll context as part of that scroll request. The words you use makes it seem like the next scroll request would free the context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'll stick this in my "to merge" queue and pick it up soon.
…34739) The `fetchPhaseShouldFreeContext` returns true when there is a scroll context but the scroll parameter is null, thus freeing the search context. https://github.com/elastic/elasticsearch/blob/183c32d4c39948e037a7fb44dccf31ab0d60d3c3/server/src/main/java/org/elasticsearch/search/SearchService.java#L491
…34739) The `fetchPhaseShouldFreeContext` returns true when there is a scroll context but the scroll parameter is null, thus freeing the search context. https://github.com/elastic/elasticsearch/blob/183c32d4c39948e037a7fb44dccf31ab0d60d3c3/server/src/main/java/org/elasticsearch/search/SearchService.java#L491
Merged and forward ported to the 6.5, 6.x, and master branches. You should see the documentation update in a half hour or so. |
Thanks so much for making the doc clearer @scampi! |
…34739) The `fetchPhaseShouldFreeContext` returns true when there is a scroll context but the scroll parameter is null, thus freeing the search context. https://github.com/elastic/elasticsearch/blob/183c32d4c39948e037a7fb44dccf31ab0d60d3c3/server/src/main/java/org/elasticsearch/search/SearchService.java#L491
* master: (74 commits) XContent: Check for bad parsers (elastic#34561) Docs: Align prose with snippet (elastic#34839) document the search context is freed if the scroll is not extended (elastic#34739) Test: Lookup node versions on rest test start (elastic#34657) SQL: Return error with ORDER BY on non-grouped. (elastic#34855) Reduce channels in AbstractSimpleTransportTestCase (elastic#34863) [DOCS] Updates Elasticsearch monitoring tasks (elastic#34339) Check self references in metric agg after last doc collection (elastic#33593) (elastic#34001) [Docs] Add `indices.query.bool.max_clause_count` setting (elastic#34779) Add 6.6.0 version to master (elastic#34847) Test: ensure char[] doesn't being with prefix (elastic#34816) Remove static import from HLRC doc snippet (elastic#34834) Logging: server: clean up logging (elastic#34593) Logging: tests: clean up logging (elastic#34606) SQL: Fix edge case: `<field> IN (null)` (elastic#34802) [Test] Mute FullClusterRestartIT.testShrink() until test is fixed SQL: Introduce ODBC mode, similar to JDBC (elastic#34825) SQL: handle X-Pack or X-Pack SQL not being available in a more graceful way (elastic#34736) [Docs] Add explanation for code snippets line width (elastic#34796) CCR: Rename follow-task parameters and stats (elastic#34836) ...
…34739) The `fetchPhaseShouldFreeContext` returns true when there is a scroll context but the scroll parameter is null, thus freeing the search context. https://github.com/elastic/elasticsearch/blob/183c32d4c39948e037a7fb44dccf31ab0d60d3c3/server/src/main/java/org/elasticsearch/search/SearchService.java#L491
The
fetchPhaseShouldFreeContext
returns true when there is a scroll context but the scroll parameter is null, thus freeing the search context.elasticsearch/server/src/main/java/org/elasticsearch/search/SearchService.java
Line 491 in 183c32d