[Bug] copy inner-hits contexts per fetch to fix concurrent segment search corruption

[waterWang]

Description

Fixes #22868

When concurrent segment search is active (index.search.concurrent_segment_search.mode=all), a request carrying both a nested inner_hits and a top_hits aggregation corrupts _source reads and can kill the node.

Root cause: every slice thread runs InnerHitsPhase.hitExecute on the same request-level InnerHitsContext. InnerHitSubContext holds mutable per-hit state (docIdsToLoad, root id, root SourceLookup) that is re-aimed for each hit (InnerHitsPhase.hitExecute lines ~91-93). Two slice threads interleave those writes, so one thread reads through another thread's SourceLookup. SourceLookup is documented "Not thread safe" and lazily caches a single Lucene stored-fields merge instance (Lucene90CompressingStoredFieldsReader.serializedDocument seeks and resets one shared block state without a lock). The corrupted reads surface as index_out_of_bounds_exception, CorruptIndexException against checksum-perfect files, or — for the illegal type flags 6/7 under TYPE_MASK — AssertionError: Unknown type flag: N, which escapes the catch (Exception) in SourceLookup and kills the node via OpenSearchUncaughtExceptionHandler.

Fix: InnerHitsPhase.getProcessor now deep-copies the inner-hits contexts for each fetch, so every concurrent slice thread works on its own InnerHitSubContext instance and never shares mutable per-hit state. This mirrors the rest of the fetch path, which is already per-slice by construction (SubSearchContext per slice, FetchContext per fetch). The inner-hits contexts were the only piece of per-hit state that escaped that isolation, because SubSearchContext inherits innerHits() from FilteredSearchContext, which forwards to the request-level context.

Changes:

• InnerHitsContext: new copy() returning a deep copy of the definition map; new abstract InnerHitSubContext.copy(); copyTo() helper replicating fetch configuration (including recursive child inner hits).
• SubSearchContext: new copyFetchStateTo() that copies fetch configuration (from/size/sort/query/highlight/script fields/fetch source/docvalues/fetch fields) onto a freshly constructed instance, while leaving result holders and per-fetch doc-IDs state untouched.
• NestedInnerHitSubContext: implements copy().
• JoinFieldInnerHitSubContext: implements copy().
• InnerHitsPhase: getProcessor uses the deep copy instead of the shared request-level map.

Testing: InnerHitsContextTests covers the copy semantics (independent maps and definition instances, names preserved, empty context). The reproducible cluster-level scenario from the issue (nested inner_hits + terms→top_hits over high-cardinality field, mode=all, max_slice_count=8) no longer fails.

Signed-off-by: waterWang 672684719@qq.com

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Incomplete state copy copyFetchStateTo omits several fields that are set through SubSearchContext setters and used during fetch: searchAfter, minimumScore, terminateAfter, trackTotalHitsUpTo, collapse, parentInnerHits, queryBoost, and rescore. If any inner-hits builder configures these (e.g., searchAfter, highlight-related state that depends on queryBoost, or nested inner hits using parentInnerHits), the copied SubSearchContext used per fetch will behave differently from the request-level context, producing incorrect inner-hit results. Consider copying all mutable configuration fields or explicitly documenting which are intentionally excluded. protected void copyFetchStateTo(SubSearchContext target) { target.from = from; target.size = size; target.sort = sort; target.parsedQuery = parsedQuery; target.query = query; target.storedFields = storedFields; if (scriptFields != null) { for (ScriptFieldsContext.ScriptField field : scriptFields.fields()) { target.scriptFields().add(field); } } target.fetchSourceContext = fetchSourceContext; target.docValuesContext = docValuesContext; target.fetchFieldsContext = fetchFieldsContext; target.highlight = highlight; target.explain = explain; target.trackScores = trackScores; target.includeNamedQueriesScore = includeNamedQueriesScore; target.version = version; target.seqNoAndPrimaryTerm = seqNoAndPrimaryTerm; } Possible NPE InnerHitSubContext.copyTo calls super.copyFetchStateTo(target), but InnerHitSubContext extends FilteredSearchContext (via SubSearchContext), and copyFetchStateTo is declared on SubSearchContext. If any concrete InnerHitSubContext subclass is not a SubSearchContext subclass, this will not compile; if it is, verify that childInnerHits copy handles the case where a child sub-context references parent state. Also confirm setChildInnerHits accepts the map returned from copy().getInnerHits() without breaking parent references used by nested lookups. protected void copyTo(InnerHitSubContext target) { super.copyFetchStateTo(target); if (childInnerHits != null) { target.setChildInnerHits(childInnerHits.copy().getInnerHits()); } }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve original inner-hit name selection The change from name to getName() alters the sub-context name from the potentially-user-provided innerHitBuilder.getName() (falling back to typeName) to whatever getName() returns on the builder, which is a different value. This is a behavioral regression unrelated to the fix and will break inner-hits keyed by the configured name. Revert to using the locally computed name. modules/parent-join/src/main/java/org/opensearch/join/query/ParentChildInnerHitContextBuilder.java [93-99] String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : typeName; JoinFieldInnerHitSubContext joinFieldInnerHits = new JoinFieldInnerHitSubContext( - getName(), + name, context, typeName, fetchChildInnerHits, joinFieldMapper Suggestion importance[1-10]: 9 __ Why: This is a valid catch — the PR changes name to getName() in the JoinFieldInnerHitSubContext constructor call, which likely refers to the enclosing ParentChildInnerHitContextBuilder.getName() rather than the locally computed name variable derived from innerHitBuilder.getName() or typeName. This appears to be an unintended behavioral change that could break inner-hits naming.	High

[github-actions]

❌ Gradle check result for adb6503: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?