-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Description
Background
As part of #139124, FetchPhase grew a streaming (chunked) execution path alongside the two existing synchronous overloads:
public void execute(SearchContext context, int[] docIdsToLoad, RankDocShardInfo rankDocs)
public void execute(SearchContext context, int[] docIdsToLoad, RankDocShardInfo rankDocs, @Nullable IntConsumer memoryChecker)To keep the scope of that PR manageable, these overloads were intentionally left in place. This issue tracks the follow-up investigation.
Problem
Maintaining two separate code paths, one synchronous, one chunked, increases complexity and makes future changes to fetch logic more expensive. Ideally we converge on a single implementation.
Call sites to investigate
There are four production call sites using the synchronous overloads. Each needs to be assessed independently for whether it can be migrated to the chunked path:
-
SearchService.executeFetchPhase, the single-shard optimisation path. WhennumberOfShards() == 1, query and fetch run in one shot on the same data node with no separate coordinator RPC. Currently passesnullformemoryChecker, so default CB accumulation applies. -
SearchService.executeRankFeatureFetch, fetches documents for re-ranking, consumed immediately and locally byRankFeatureShardPhase.processFetch. Also passesnull. -
InnerHitsPhase.hitExecute, a sub-phase that fetches nested/parent-child documents and embeds them into the parentSearchHit. Passesnull. -
TopHitsAggregator.runFetchPhase, the only caller that supplies a non-nullmemoryChecker(this::addRequestCircuitBreakerBytes), because the aggregation manages its own circuit breaker accounting separately from the main fetch path.
Goal
Converge on a single fetch execution path (if possible) by migrating all four call sites to the chunked API and removing the synchronous overloads.