Add stored fields prefetch support for tiered storage module.

[GeekGlider]

Description

Implements stored fields prefetch for the tiered storage module, building on the skeleton structure from #21017

Adds a StoredFieldsPrefetch search operation listener that proactively prefetches stored fields from segment readers during the pre-fetch phase. This reduces fetch latency on tiered storage indices by warming data ahead of access. Nested documents are handled by resolving root documents via bitset filters.

Also introduces TieredStoragePrefetchSettings with dynamic cluster-scoped settings (stored_fields.enabled toggle and read_ahead.block_count) and wires everything into TieredStoragePlugin behind the WRITABLE_WARM_INDEX_EXPERIMENTAL_FLAG feature flag.

Related Issues

Part of the tiered-storage open source plan.

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit f523e5d)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

📝 TODO sections modules/tiered-storage/src/main/java/org/opensearch/storage/slowlogs/TieredStorageQueryMetricService.java [160]: Has to emit as part of node stats

🔀 Multiple PR themes Sub-PR theme: Stored fields prefetch implementation and settings Relevant files: modules/tiered-storage/src/main/java/org/opensearch/storage/prefetch/StoredFieldsPrefetch.java modules/tiered-storage/src/main/java/org/opensearch/storage/prefetch/TieredStoragePrefetchSettings.java modules/tiered-storage/src/main/java/org/opensearch/storage/TieredStoragePlugin.java modules/tiered-storage/src/test/java/org/opensearch/storage/prefetch/StoredFieldsPrefetchTests.java modules/tiered-storage/src/test/java/org/opensearch/storage/TieredStoragePluginTests.java Sub-PR theme: Tiering service skeleton and transport actions Relevant files: modules/tiered-storage/src/main/java/org/opensearch/storage/tiering/TieringService.java modules/tiered-storage/src/main/java/org/opensearch/storage/tiering/HotToWarmTieringService.java modules/tiered-storage/src/main/java/org/opensearch/storage/tiering/WarmToHotTieringService.java modules/tiered-storage/src/main/java/org/opensearch/storage/common/tiering/TieringUtils.java modules/tiered-storage/src/main/java/org/opensearch/storage/action/tiering/status/model/TieringStatus.java modules/tiered-storage/src/main/java/org/opensearch/storage/action/tiering/status/transport/TransportGetTieringStatusAction.java modules/tiered-storage/src/main/java/org/opensearch/storage/action/tiering/status/transport/TransportListTieringStatusAction.java modules/tiered-storage/src/main/java/org/opensearch/storage/action/tiering/TransportCancelTierAction.java modules/tiered-storage/src/main/java/org/opensearch/storage/action/tiering/HotToWarmTierAction.java Sub-PR theme: Metrics, slow log, and index input skeleton classes Relevant files: modules/tiered-storage/src/main/java/org/opensearch/storage/slowlogs/TieredStorageQueryMetricService.java modules/tiered-storage/src/main/java/org/opensearch/storage/slowlogs/TieredStorageSearchSlowLog.java modules/tiered-storage/src/main/java/org/opensearch/storage/slowlogs/PrefetchStats.java modules/tiered-storage/src/main/java/org/opensearch/storage/indexinput/SwitchableIndexInput.java modules/tiered-storage/src/main/java/org/opensearch/storage/indexinput/BlockFetchRequest.java

⚡ Recommended focus areas for review Double Prefetch In executePrefetch, when a nested document is found (rootDocId != -1), both rootDocId and subDocId are prefetched. However, if subDocId itself is the root document, this results in a redundant double prefetch of the same document. The logic should guard against prefetching subDocId when it equals rootDocId. if (rootDocId != -1) { currentReader.prefetch(rootDocId); } currentReader.prefetch(subDocId); Exception Swallowing Exceptions thrown during prefetch are converted and re-thrown via ExceptionsHelper.convertToOpenSearchException, which will abort the entire fetch phase for all documents if any single document's prefetch fails. Consider logging and continuing rather than propagating, since prefetch is an optimization and failures should be non-fatal. } catch (Exception e) { throw ExceptionsHelper.convertToOpenSearchException(e); } Setting Scope Mismatch READ_AHEAD_BLOCK_COUNT and STORED_FIELDS_PREFETCH_ENABLED_SETTING are defined with Setting.Property.NodeScope, but the PR description states these are "dynamic cluster-scoped settings". NodeScope settings are per-node and not cluster-wide. If cluster-scoped behavior is intended, the property should be ClusterScope instead. public static final Setting<Integer> READ_AHEAD_BLOCK_COUNT = Setting.intSetting( "tiering.service.prefetch.read_ahead.block_count", DEFAULT_READ_AHEAD_BLOCK_COUNT, 0, Setting.Property.Dynamic, Setting.Property.NodeScope ); /** Setting to enable or disable stored fields prefetch. */ public static final Setting<Boolean> STORED_FIELDS_PREFETCH_ENABLED_SETTING = Setting.boolSetting( "tiering.service.prefetch.stored_fields.enabled", true, Setting.Property.Dynamic, Setting.Property.NodeScope ); Misleading Test testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch has the same body as testOnPreFetchPhase_WhenPrefetchEnabled_WithSegmentReader and does not actually test the non-segment-reader skip behavior. The test name is misleading and the scenario is not properly validated. public void testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch() throws IOException { setupSearchContext(new int[] { 0 }, false); storedFieldsPrefetch.onPreFetchPhase(searchContext); verify(searchContext, atLeastOnce()).docIdsToLoadSize(); } Singleton State TieredStorageQueryMetricService is a singleton with mutable shared state (metricCollectors, taskIdToQueryPhaseCollectorMap, taskIdToFetchPhaseCollectorMap). Most methods throw UnsupportedOperationException, meaning the singleton is partially initialized and could be accessed in a broken state if used before implementation is complete. This is risky in a production code path. private static final TieredStorageQueryMetricService INSTANCE = new TieredStorageQueryMetricService(); /** * Map of thread ID to active collector, providing a way to retrieve the currently active metric collector on a given thread. * The same thread can create multiple collectors over the lifetime of a given query, both for the same query or for other queries. * However, only one collector will be active for a given thread at a given time, inactive metric collectors for which the query * is still running are tracked in taskIdToFetchPhaseCollectorMap and taskIdToQueryPhaseCollectorMap */ protected final ConcurrentMap<Long, TieredStoragePerQueryMetric> metricCollectors = new ConcurrentHashMap<>(); /** * Map of task id + shard id to set of collectors, providing a way to look up all collectors for a given task/shard. We need both * as the same parent task may have multiple shards on the same node. For concurrent segment search there will be multiple collectors * per task/shard combination as each slice (thread) creates its own collector. The same thread can process multiple slices for the same * or for different queries so it does not come into the picture here. */ protected final ConcurrentMap<String, Set<TieredStoragePerQueryMetric>> taskIdToQueryPhaseCollectorMap = new ConcurrentHashMap<>(); /** Map of task ID and shard ID to set of fetch phase metric collectors. */ protected final ConcurrentMap<String, Set<TieredStoragePerQueryMetric>> taskIdToFetchPhaseCollectorMap = new ConcurrentHashMap<>(); private final PrefetchStatsHolder prefetchStats = new PrefetchStatsHolder();

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f523e5d
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Fix test to actually exercise non-segment reader path This test is named testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch but its body is identical to testOnPreFetchPhase_WhenPrefetchEnabled_WithSegmentReader, using the real indexReader (which contains segment readers). It does not actually test the non-segment-reader skip behavior. The test should use a wrapped non-segment reader (like NonSegmentReaderDirectoryReader) to properly validate the skip path. modules/tiered-storage/src/test/java/org/opensearch/storage/prefetch/StoredFieldsPrefetchTests.java [159-163] public void testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch() throws IOException { - setupSearchContext(new int[] { 0 }, false); - storedFieldsPrefetch.onPreFetchPhase(searchContext); - verify(searchContext, atLeastOnce()).docIdsToLoadSize(); + DirectoryReader realReader = DirectoryReader.open(directory); + DirectoryReader wrappedReader = new NonSegmentReaderDirectoryReader(realReader); + try { + when(searchContext.docIdsToLoadSize()).thenReturn(1); + when(searchContext.docIdsToLoad()).thenReturn(new int[] { 0 }); + when(searchContext.docIdsToLoadFrom()).thenReturn(0); + ContextIndexSearcher searcher = mock(ContextIndexSearcher.class); + when(searchContext.searcher()).thenReturn(searcher); + when(searcher.getIndexReader()).thenReturn(wrappedReader); + IndexShard indexShard = mock(IndexShard.class); + when(searchContext.indexShard()).thenReturn(indexShard); + when(indexShard.shardId()).thenReturn(new ShardId("test-index", "uuid", 0)); + MapperService mapperService = mock(MapperService.class); + when(searchContext.mapperService()).thenReturn(mapperService); + when(mapperService.hasNested()).thenReturn(false); + storedFieldsPrefetch.onPreFetchPhase(searchContext); + verify(searchContext, atLeastOnce()).docIdsToLoadSize(); + } finally { + wrappedReader.close(); + } } Suggestion importance[1-10]: 7 __ Why: The test testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch is indeed identical to testOnPreFetchPhase_WhenPrefetchEnabled_WithSegmentReader and does not test the intended non-segment-reader skip behavior. This is a real test coverage gap that should be fixed.	Medium
	Fix inconsistent field access modifier The tieredStoragePrefetchSettingsSupplier field is declared public while all other fields in the class use protected or private access modifiers. This breaks encapsulation and exposes the field directly. It should be changed to protected or private to be consistent with the other fields. modules/tiered-storage/src/main/java/org/opensearch/storage/indexinput/OnDemandPrefetchBlockSnapshotIndexInput.java [33-34] /** Supplier for prefetch settings. */ -public Supplier<TieredStoragePrefetchSettings> tieredStoragePrefetchSettingsSupplier; +protected Supplier<TieredStoragePrefetchSettings> tieredStoragePrefetchSettingsSupplier; Suggestion importance[1-10]: 5 __ Why: The tieredStoragePrefetchSettingsSupplier field is declared public while all other fields use protected or private, breaking encapsulation. This is a valid style/consistency issue, though the impact is limited since this is a placeholder class with implementation deferred to a future PR.	Low
Possible issue	Avoid redundant prefetch of nested child documents When a nested document is found (rootDocId != -1), the code prefetches both the root doc and the sub-doc. However, for a nested child document, the sub-doc itself may not have stored fields (only the root/parent doc does in Lucene's nested model). Prefetching the child subDocId when it is a nested child (i.e., rootDocId != -1) is unnecessary and may cause errors or wasted I/O. Consider only prefetching the root doc when a nested parent is found. modules/tiered-storage/src/main/java/org/opensearch/storage/prefetch/StoredFieldsPrefetch.java [91-95] final int rootDocId = findRootDocumentIfNested(context, currentReaderContext, subDocId); if (rootDocId != -1) { currentReader.prefetch(rootDocId); +} else { + currentReader.prefetch(subDocId); } -currentReader.prefetch(subDocId); Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern about prefetching both the root doc and the sub-doc when a nested child is found. However, the behavior depends on the specific Lucene stored fields model and whether child docs have stored fields. The fix is logically sound but may not be strictly necessary in all cases.	Low
	Add null validation for required builder fields The constructor accesses builder.directory.getDirectory() without a null check on builder.directory. If directory is not set on the builder before calling build(), this will throw a NullPointerException. Add a null check or validation in the build() method to ensure required fields are set. modules/tiered-storage/src/main/java/org/opensearch/storage/indexinput/BlockFetchRequest.java [31-38] private BlockFetchRequest(Builder builder) { + if (builder.directory == null) { + throw new IllegalArgumentException("directory must not be null"); + } + if (builder.fileName == null) { + throw new IllegalArgumentException("fileName must not be null"); + } + if (builder.blockFileName == null) { + throw new IllegalArgumentException("blockFileName must not be null"); + } this.fileName = builder.fileName; this.blockFileName = builder.blockFileName; this.filePath = builder.directory.getDirectory().resolve(blockFileName); this.directory = builder.directory; this.blockSize = builder.blockSize; this.blockStart = builder.blockStart; } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a potential NullPointerException if builder.directory is null. Adding validation in the constructor or build() method is a good defensive practice, though this is a minor robustness improvement.	Low
	Clarify timeout constant unit to prevent misuse The constant TIMEOUT_ONE_HOUR is set to 1, but without a clear unit this is ambiguous and likely incorrect — timeout APIs typically expect milliseconds or seconds, not hours as a raw integer. The value should either be expressed in the correct unit (e.g., TimeUnit.HOURS.toMillis(1)) or the constant name should reflect the actual unit used. modules/tiered-storage/src/main/java/org/opensearch/storage/common/BlockTransferManager.java [34] -private static final int TIMEOUT_ONE_HOUR = 1; +private static final long TIMEOUT_ONE_HOUR_MS = java.util.concurrent.TimeUnit.HOURS.toMillis(1); Suggestion importance[1-10]: 4 __ Why: The TIMEOUT_ONE_HOUR constant value of 1 is ambiguous without a clear unit, which could lead to bugs when the constant is used. However, since this is a skeleton/placeholder class with implementation deferred to a future PR, the impact is currently limited.	Low

---

Previous suggestions

Suggestions up to commit 61e68a8

Category	Suggestion	Impact
General	Fix duplicate test to actually test non-segment reader path This test is named testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch but uses the same setup as testOnPreFetchPhase_WhenPrefetchEnabled_WithSegmentReader (a real DirectoryReader which contains SegmentReader leaves). The test does not actually test the non-segment-reader path and is a duplicate. It should use a wrapped non-segment reader (like NonSegmentReaderDirectoryReader) to properly test the intended behavior. modules/tiered-storage/src/test/java/org/opensearch/storage/prefetch/StoredFieldsPrefetchTests.java [159-163] public void testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch() throws IOException { - setupSearchContext(new int[] { 0 }, false); - storedFieldsPrefetch.onPreFetchPhase(searchContext); - verify(searchContext, atLeastOnce()).docIdsToLoadSize(); + DirectoryReader realReader = DirectoryReader.open(directory); + DirectoryReader wrappedReader = new NonSegmentReaderDirectoryReader(realReader); + try { + when(searchContext.docIdsToLoadSize()).thenReturn(1); + when(searchContext.docIdsToLoad()).thenReturn(new int[] { 0 }); + when(searchContext.docIdsToLoadFrom()).thenReturn(0); + ContextIndexSearcher searcher = mock(ContextIndexSearcher.class); + when(searchContext.searcher()).thenReturn(searcher); + when(searcher.getIndexReader()).thenReturn(wrappedReader); + IndexShard indexShard = mock(IndexShard.class); + when(searchContext.indexShard()).thenReturn(indexShard); + when(indexShard.shardId()).thenReturn(new ShardId("test-index", "uuid", 0)); + MapperService mapperService = mock(MapperService.class); + when(searchContext.mapperService()).thenReturn(mapperService); + when(mapperService.hasNested()).thenReturn(false); + storedFieldsPrefetch.onPreFetchPhase(searchContext); + verify(searchContext, atLeastOnce()).docIdsToLoadSize(); + } finally { + wrappedReader.close(); + } } Suggestion importance[1-10]: 7 __ Why: The test testOnPreFetchPhase_WithNonSegmentReader_SkipsPrefetch is indeed a duplicate of testOnPreFetchPhase_WhenPrefetchEnabled_WithSegmentReader and does not test the intended non-segment-reader behavior. The suggested fix correctly uses NonSegmentReaderDirectoryReader to properly exercise the intended code path.	Medium
	Enforce singleton pattern with private constructor Unlike ListTieringStatusAction and other action types in this PR which use a private constructor to enforce the singleton pattern, GetTieringStatusAction has a public constructor. This allows multiple instances to be created, breaking the singleton guarantee. The constructor should be private. modules/tiered-storage/src/main/java/org/opensearch/storage/action/tiering/status/GetTieringStatusAction.java [23-25] /** Constructs a new GetTieringStatusAction. */ -public GetTieringStatusAction() { +private GetTieringStatusAction() { super(NAME, GetTieringStatusResponse::new); } Suggestion importance[1-10]: 6 __ Why: Unlike other action types in this PR (e.g., ListTieringStatusAction, HotToWarmTierAction) that use private constructors to enforce the singleton pattern, GetTieringStatusAction has a public constructor, which allows multiple instances to be created and breaks the singleton guarantee.	Low
	Fix inconsistent field access modifier The tieredStoragePrefetchSettingsSupplier field is declared public while all other fields in the class are protected or private. This breaks encapsulation and exposes internal state directly. It should be declared protected or private to be consistent with the other fields. modules/tiered-storage/src/main/java/org/opensearch/storage/indexinput/OnDemandPrefetchBlockSnapshotIndexInput.java [33-34] /** Supplier for prefetch settings. */ -public Supplier<TieredStoragePrefetchSettings> tieredStoragePrefetchSettingsSupplier; +protected Supplier<TieredStoragePrefetchSettings> tieredStoragePrefetchSettingsSupplier; Suggestion importance[1-10]: 5 __ Why: The tieredStoragePrefetchSettingsSupplier field is public while all other fields are protected or private, which breaks encapsulation. Changing it to protected would be consistent with the other fields in the class.	Low
	Clarify timeout constant unit to prevent misuse The constant TIMEOUT_ONE_HOUR is set to 1, but without a clear unit this is ambiguous and likely incorrect if used directly with time APIs that expect milliseconds or seconds. The value should either be named to include the unit (e.g., TIMEOUT_ONE_HOUR_IN_HOURS) or be expressed in the intended unit (e.g., TimeUnit.HOURS.toMillis(1)). modules/tiered-storage/src/main/java/org/opensearch/storage/common/BlockTransferManager.java [34] -private static final int TIMEOUT_ONE_HOUR = 1; +private static final long TIMEOUT_ONE_HOUR_MS = TimeUnit.HOURS.toMillis(1); Suggestion importance[1-10]: 4 __ Why: The constant TIMEOUT_ONE_HOUR = 1 is ambiguous without a unit, but since the implementation is not yet added (placeholder class), the actual usage context is unknown. The suggestion is valid for clarity but has limited immediate impact since the constant isn't used yet.	Low
Possible issue	Avoid double-prefetching nested and root documents When a nested document is found (rootDocId != -1), the code prefetches both the root doc and the sub-doc. However, for nested child documents, prefetching the child doc itself may be unnecessary or incorrect since the stored fields are stored on the root document. Consider only prefetching the root document when a nested parent is found, and only prefetching subDocId when it is itself a root (non-nested) document. modules/tiered-storage/src/main/java/org/opensearch/storage/prefetch/StoredFieldsPrefetch.java [91-95] final int rootDocId = findRootDocumentIfNested(context, currentReaderContext, subDocId); if (rootDocId != -1) { currentReader.prefetch(rootDocId); +} else { + currentReader.prefetch(subDocId); } -currentReader.prefetch(subDocId); Suggestion importance[1-10]: 6 __ Why: The current code prefetches both rootDocId and subDocId when a nested document is found, but for nested child docs the stored fields are on the root document, making the child prefetch potentially wasteful or incorrect. The suggested fix is logically sound and improves correctness.	Low
	Add null validation before building request object The constructor accesses builder.directory.getDirectory() without null-checking builder.directory, which will throw a NullPointerException if directory was not set on the builder. Add a null check or validation in the build() method to ensure required fields are set before constructing the object. modules/tiered-storage/src/main/java/org/opensearch/storage/indexinput/BlockFetchRequest.java [31-38] -private BlockFetchRequest(Builder builder) { - this.fileName = builder.fileName; - this.blockFileName = builder.blockFileName; - this.filePath = builder.directory.getDirectory().resolve(blockFileName); - this.directory = builder.directory; - this.blockSize = builder.blockSize; - this.blockStart = builder.blockStart; +public BlockFetchRequest build() { + if (directory == null) { + throw new IllegalStateException("directory must be set"); + } + if (fileName == null) { + throw new IllegalStateException("fileName must be set"); + } + if (blockFileName == null) { + throw new IllegalStateException("blockFileName must be set"); + } + return new BlockFetchRequest(this); } Suggestion importance[1-10]: 5 __ Why: The constructor dereferences builder.directory without a null check, which would cause a NullPointerException if directory is not set. The suggestion to add validation in build() is valid, though the improved_code modifies build() rather than the existing_code constructor snippet, making it slightly misaligned with the existing_code field.	Low

[github-actions]

Persistent review updated to latest commit f523e5d

[github-actions]

✅ Gradle check result for f523e5d: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 20.76125% with 458 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.08%. Comparing base (0f8cc13) to head (f523e5d).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...age/action/tiering/status/model/TieringStatus.java	0.00%	48 Missing ⚠️
...earch/storage/indexinput/SwitchableIndexInput.java	0.00%	38 Missing ⚠️
...rage/slowlogs/TieredStorageQueryMetricService.java	0.00%	35 Missing ⚠️
...ensearch/storage/indexinput/BlockFetchRequest.java	0.00%	26 Missing ⚠️
.../tiering/status/model/GetTieringStatusRequest.java	0.00%	19 Missing ⚠️
...rage/common/tiering/TieringRejectionException.java	0.00%	17 Missing ⚠️
...org/opensearch/storage/slowlogs/PrefetchStats.java	0.00%	15 Missing ⚠️
...org/opensearch/storage/tiering/TieringService.java	0.00%	14 Missing ⚠️
...tiering/status/model/ListTieringStatusRequest.java	0.00%	13 Missing ⚠️
...h/storage/action/tiering/CancelTieringRequest.java	0.00%	11 Missing ⚠️
... and 38 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21066      +/-   ##
============================================
- Coverage     73.20%   73.08%   -0.12%     
- Complexity    72751    73018     +267     
============================================
  Files          5871     5967      +96     
  Lines        332688   334020    +1332     
  Branches      48017    48108      +91     
============================================
+ Hits         243543   244128     +585     
- Misses        69625    70352     +727     
- Partials      19520    19540      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.