ESQL: Aggressive release of shard contexts #129454
Merged
Conversation
This file contains hidden or 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
Keep better track of shard contexts using `RefCounted`, so they can be released more aggressively during operator processing. For example, during TopN, we can potentially release some contexts if they don't pass the limit filter. This is done in preparation of TopN fetch optimization, which will delay the fetching of additional columns to the data node coordinator, instead of doing it in each individual worker, thereby reducing IO. Since the node coordinator would need to maintain the shard contexts for a potentially longer duration, it is important we try to release what we can eariler. An even more advanced optimization is to delay fetching to the main cluster coordinator, but that would be more involved, since we need to first figure out how to transport the shard contexts between nodes.
GalLalouche added >enhancement Team:Analytics 
| Hi @GalLalouche, I've created a changelog YAML for you. |
GalLalouche commented Jun 15, 2025
| public SearchContext createSearchContext(ShardSearchRequest request, TimeValue timeout) throws IOException { | ||
| SearchContext searchContext = super.createSearchContext(request, timeout); | ||
| onPutContext.accept(searchContext.readerContext()); | ||
| try { |
There was a problem hiding this comment.
This was done after confirming with @dnhatn that onPutContext here can be replaced with onCreateSearchContext. The try/catch clause was copy pasted from above.
| * A source operator whose output is the given tuple values. This operator produces pages | ||
| * with two Blocks. The returned pages preserve the order of values as given in the in initial list. | ||
| */ | ||
| public abstract class TupleAbstractBlockSourceOperator<T, S> extends AbstractBlockSourceOperator { |
There was a problem hiding this comment.
I've generalized the already existing TupleBlockSourceOperator to support more than just a tuple of two longs.
nik9000 reviewed Jun 16, 2025
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/ShardRefCounted.java Show resolved Hide resolved
...in/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java Show resolved Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java Show resolved Hide resolved
...in/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsPhysicalOperationProviders.java Show resolved Hide resolved
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/DocVector.java Outdated Show resolved Hide resolved
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/DocVector.java Show resolved Hide resolved
| If you want me to review more let me know. Or I can wait until you remove |
Thanks @nik9000! I'm chasing down test failures right now, and will go over your comments in the meantime. I'll re-request a review when I'm done. |
GalLalouche force-pushed the feature/shard_ref_count branch 2 times, most recently from 0591cf6 to b5f56f8 Compare GalLalouche force-pushed the feature/shard_ref_count branch from b5f56f8 to 1ad8eb4 Compare | Pinging @elastic/es-analytical-engine (Team:Analytics) |
nik9000 reviewed Jun 23, 2025
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/DocVector.java Outdated Show resolved Hide resolved
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneOperator.java Outdated Show resolved Hide resolved
.../esql/compute/src/main/java/org/elasticsearch/compute/operator/OrdinalsGroupingOperator.java Show resolved Hide resolved
.../plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/TopNOperator.java Show resolved Hide resolved
nik9000 approved these changes Jun 23, 2025
.../esql/compute/src/main/java/org/elasticsearch/compute/operator/OrdinalsGroupingOperator.java Show resolved Hide resolved
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/DocVector.java Outdated Show resolved Hide resolved
…ticsearch into feature/shard_ref_count
GalLalouche force-pushed the feature/shard_ref_count branch from c9bedcd to c8af2c6 Compare Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
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.
Keep better track of shard contexts using
RefCounted, so they can be released more aggressively during operator processing. For example, during TopN, we can potentially release some contexts if they don't pass the limit filter.This is done in preparation of TopN fetch optimization, which will delay the fetching of additional columns to the data node coordinator, instead of doing it in each individual worker, thereby reducing IO. Since the node coordinator would need to maintain the shard contexts for a potentially longer duration, it is important we try to release what we can eariler.
An even more advanced optimization is to delay fetching to the main cluster coordinator, but that would be more involved, since we need to first figure out how to transport the shard contexts between nodes.
Summary of main changes:
DocVectornow maintains aRefCountedinstance per shard.DocVectors (e.g.,LuceneSourceOperator,TopNOperator), can also holdRefCountedinstances, so they can pass them toDocVectorand also ensure contexts aren't released if they can still be potentially used later.Driver's main loop iteration (runSingleLoopIteration), now closes its operators even between different operator processing. This is extra aggressive, and was mostly done to improve testability.TopNOperatorand a new integration testEsqlTopNShardManagementIT, which uses the pausable plugin framework to check thatTopNOperatorreleases things as early as possible..