Fix storage integration test failures across recovery, snapshots, and index creation

[ask-kamal-nayan]

Description

Fixes multiple storage integration test failures spanning search execution, remote store uploads, recovery, snapshots, shard allocation, and index creation.

Changes:

• MetadataCreateIndexService: Disable derived source for non-optimized indices to avoid DerivedSource is not supported mapping errors
• QueryPhase: Fix execution order — check queryPlanIR() == null before accessing DataFusion.
• RemoteStoreRefreshListener: Propagate userData to underlying SegmentInfos during remote metadata upload, fixing global checkpoint regression on primary failover
• RemoteStoreUploaderService: Re-enable afterSyncToRemote callback for FileCache eviction eligibility after remote upload
• Store: Use index UUID + shard ID for temp shard paths to avoid collisions; remove unused convertCatalogSnapshotToSegmentInfos dead code
• IndexShard: Use null-safe getIndexingThrottlerOrNull() in indexingStats(); propagate IOException from getFormatAwareSegmentMetadataMap for proper failure detection

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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 405d431)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Update VSRManager test constructor calls to include index name parameter Relevant files: modules/parquet-data-format/src/test/java/com/parquet/parquetdataformat/vsr/VSRManagerTests.java Sub-PR theme: Fix remote store upload, recovery, and shard path collision issues Relevant files: server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java server/src/main/java/org/opensearch/index/shard/RemoteStoreUploaderService.java server/src/main/java/org/opensearch/index/store/Store.java server/src/main/java/org/opensearch/index/shard/IndexShard.java Sub-PR theme: Fix index creation derived source and query phase execution order Relevant files: server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java server/src/main/java/org/opensearch/search/query/QueryPhase.java

⚡ Recommended focus areas for review Unsafe Cast The directory unwrapping uses a double FilterDirectory cast chain without null checks or type validation. If storeDirectory is not a FilterDirectory wrapping another FilterDirectory, this will throw a ClassCastException at runtime. There is no guard to verify the actual type before casting. final Directory directory = isOptimizedIndex ? null : ((FilterDirectory) (((FilterDirectory) storeDirectory).getDelegate())).getDelegate(); Derived Source Logic The condition disables derived source when OPTIMIZED_INDEX_ENABLED_SETTING is null OR equals "false". This means any index that doesn't explicitly set the optimized index setting will have derived source disabled, which could be an overly broad change affecting indices that never intended to use derived source at all. The logic should be validated to ensure it doesn't inadvertently break existing index creation behavior. if (indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()) == null || indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()).equals("false")) { indexSettingsBuilder.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false); } Direct Field Access The code directly accesses SegmentInfos.userData as a public field rather than using a setter method. This is a Lucene internal field and direct mutation bypasses any encapsulation or validation that Lucene may enforce, and could break with future Lucene upgrades. ((SegmentInfosCatalogSnapshot) catalogSnapshotCloned).getSegmentInfos().userData = userData; Null Dereference Risk The code calls searchContext.request().source().queryPlanIR() without null-checking source(). If searchContext.request().source() returns null, this will throw a NullPointerException. The previous code had the same pattern but the fix should ensure null safety. if (searchContext.request().source().queryPlanIR() == null) {

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 405d431
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Safely unwrap directory to avoid ClassCastException The double FilterDirectory unwrapping assumes a specific directory wrapping structure that may not always hold, causing a ClassCastException at runtime. Consider using a safer approach by checking the type before casting, or using a utility method to unwrap to the desired type. If the structure is not as expected, the code should handle it gracefully rather than throwing an unchecked exception. server/src/main/java/org/opensearch/index/shard/RemoteStoreUploaderService.java [84-86] -final Directory directory = isOptimizedIndex - ? null - : ((FilterDirectory) (((FilterDirectory) storeDirectory).getDelegate())).getDelegate(); +final Directory directory; +if (isOptimizedIndex) { + directory = null; +} else { + Directory delegate = storeDirectory; + while (delegate instanceof FilterDirectory) { + Directory inner = ((FilterDirectory) delegate).getDelegate(); + if (inner instanceof CompositeDirectory) { + delegate = inner; + break; + } + delegate = inner; + } + directory = delegate; +} Suggestion importance[1-10]: 7 __ Why: The double FilterDirectory unwrapping with hard casts is fragile and will throw a ClassCastException if the directory structure doesn't match exactly. The suggested loop-based unwrapping is more robust and handles varying directory wrapping depths.	Medium
General	Avoid overriding explicitly set derived source setting The condition explicitly sets INDEX_DERIVED_SOURCE_SETTING to false whenever the optimized index setting is absent or "false", which may override a user-explicitly-set value for INDEX_DERIVED_SOURCE_SETTING. You should also check whether INDEX_DERIVED_SOURCE_SETTING is already set before overriding it to avoid unintentionally overwriting user configuration. server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java [1081-1083] -if (indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()) == null || indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()).equals("false")) { +if ((indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()) == null || indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()).equals("false")) + && indexSettingsBuilder.get(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey()) == null) { indexSettingsBuilder.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false); } Suggestion importance[1-10]: 6 __ Why: The current code unconditionally overrides INDEX_DERIVED_SOURCE_SETTING to false even if a user explicitly set it, which could silently override user configuration. Adding a null check before overriding prevents unintended behavior.	Low
	Use setter method instead of direct field assignment Directly assigning to userData field of SegmentInfos bypasses any encapsulation or validation that setUserData might provide, and may cause inconsistency if setUserData performs additional logic. It would be safer to use the setUserData method on SegmentInfos if available, or ensure this direct field assignment is intentional and documented. server/src/main/java/org/opensearch/index/shard/RemoteStoreRefreshListener.java [490-492] if (catalogSnapshotCloned instanceof SegmentInfosCatalogSnapshot) { - ((SegmentInfosCatalogSnapshot) catalogSnapshotCloned).getSegmentInfos().userData = userData; + // Directly set userData on SegmentInfos to ensure consistency with catalogSnapshot userData + SegmentInfos segmentInfos = ((SegmentInfosCatalogSnapshot) catalogSnapshotCloned).getSegmentInfos(); + segmentInfos.setUserData(userData, false); } Suggestion importance[1-10]: 5 __ Why: Directly assigning to the userData public field of SegmentInfos bypasses any encapsulation; using setUserData is safer and more consistent with how userData is set elsewhere in the codebase.	Low

---

Previous suggestions

Suggestions up to commit 1a081e5

Category	Suggestion	Impact
Possible issue	Safely unwrap directory to avoid ClassCastException The double FilterDirectory unwrapping assumes a specific directory wrapping structure that may not always hold, causing a ClassCastException at runtime. Consider using a safer approach by checking the type before casting, or using a utility method to unwrap to the target type. Additionally, if storeDirectory is not a FilterDirectory, this will throw a ClassCastException without a meaningful error message. server/src/main/java/org/opensearch/index/shard/RemoteStoreUploaderService.java [84-86] -final Directory directory = isOptimizedIndex - ? null - : ((FilterDirectory) (((FilterDirectory) storeDirectory).getDelegate())).getDelegate(); +Directory unwrapped = storeDirectory; +while (unwrapped instanceof FilterDirectory) { + unwrapped = ((FilterDirectory) unwrapped).getDelegate(); + if (unwrapped instanceof CompositeDirectory) { + break; + } +} +final Directory directory = isOptimizedIndex ? null : unwrapped; Suggestion importance[1-10]: 7 __ Why: The double FilterDirectory unwrapping with hard casts is fragile and will throw a ClassCastException if the directory structure doesn't match exactly. The suggested loop-based unwrapping is more robust and defensive.	Medium
	Guard against null source before accessing queryPlanIR The call to searchContext.request().source() may return null if no source is set, causing a NullPointerException. The original code had the same issue, but since this is being restructured, it's worth adding a null check for searchContext.request().source() before calling queryPlanIR(). server/src/main/java/org/opensearch/search/query/QueryPhase.java [168-176] -if (searchContext.request().source().queryPlanIR() == null) { +if (searchContext.request().source() == null || searchContext.request().source().queryPlanIR() == null) { boolean rescore = executeInternal(searchContext, queryPhaseSearcher); if (rescore) { // only if we do a regular search rescoreProcessor.process(searchContext); } suggestProcessor.process(searchContext); } else if (searchContext.getDFResults() != null) { SearchEngineResultConversionUtils.convertDFResultGeneric(searchContext); } Suggestion importance[1-10]: 5 __ Why: If searchContext.request().source() returns null, calling queryPlanIR() on it would cause a NullPointerException. Adding a null check is a valid defensive improvement, though this may be an unlikely scenario in practice.	Low
General	Avoid overriding explicitly set derived source setting The condition sets INDEX_DERIVED_SOURCE_SETTING to false whenever the optimized index setting is null or "false", but this will unconditionally override any explicitly set value for INDEX_DERIVED_SOURCE_SETTING. You should also check if INDEX_DERIVED_SOURCE_SETTING is already explicitly set before overriding it. server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java [1081-1083] -if (indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()) == null || indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()).equals("false")) { +if (indexSettingsBuilder.get(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey()) == null + && (indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()) == null + || indexSettingsBuilder.get(IndexSettings.OPTIMIZED_INDEX_ENABLED_SETTING.getKey()).equals("false"))) { indexSettingsBuilder.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false); } Suggestion importance[1-10]: 6 __ Why: The current code unconditionally overrides INDEX_DERIVED_SOURCE_SETTING even if it was explicitly set by the user. Adding a null check for the derived source setting before overriding prevents unintended behavior.	Low

[github-actions]

❌ Gradle check result for 1a081e5: 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?

[github-actions]

Persistent review updated to latest commit 405d431

[github-actions]

❌ Gradle check result for 405d431: 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?