Deprecate mode

[MrFlap]

Note: running for bwc tests. Not ready yet

Description

Deprecates the mode parameter (in_memory, on_disk) for knn_vector field mappings, starting in OpenSearch 3.7.0. Storage mode is now derived automatically from compression_level, making mode redundant as a user-facing input.

Changes:

• Mode enum marked @Deprecated(since = "3.7.0", forRemoval = true) — will be removed in 4.0
• When mode is explicitly provided on an index created at version 3.7.0+, a deprecation warning is logged
• When only compression_level is provided (no mode), mode is derived automatically: 1x/2x -> in_memory, 4x and above -> on_disk
• Mode.deriveMode(CompressionLevel) added as the canonical derivation method
• Existing indices that stored an explicit mode value continue to work unchanged (BwC)
• BwC tests added for both rolling-upgrade and restart-upgrade scenarios covering: explicit mode indices surviving upgrade, and compression-only indices resolving correctly on the upgraded cluster

Related Issues

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

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[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

🔀 Multiple PR themes Sub-PR theme: Core mode deprecation logic and unit tests Relevant files: src/main/java/org/opensearch/knn/index/mapper/Mode.java src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java Sub-PR theme: BwC integration tests for mode deprecation Relevant files: qa/rolling-upgrade/src/test/java/org/opensearch/knn/bwc/ModeDeprecationIT.java qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/ModeDeprecationIT.java

⚡ Recommended focus areas for review Wrong Log Level The deprecation warning uses log.warn(...) instead of a proper OpenSearch deprecation logger (e.g., deprecationLogger.deprecate(...)). Using a standard warn log means the deprecation warning won't appear in the dedicated deprecation log file and won't be surfaced to users via the Deprecation API, which is the standard OpenSearch mechanism for communicating deprecations. if (Mode.isConfigured(userMode) && parserContext.indexVersionCreated().onOrAfter(Version.V_3_7_0)) { log.warn( "[Deprecation] The \"mode\" parameter is deprecated as of OpenSearch 3.7.0 and will be removed in 4.0. " + "Use \"compression_level\" instead. \"mode\" will be derived automatically from \"compression_level\"." ); } Missing Mode Pass-Through When the user explicitly provides mode (without compression_level), resolvedMode is set to userMode, but userCompression remains NOT_CONFIGURED. The KNNMethodConfigContext is then built with the user-supplied mode but no compression. It is unclear whether downstream resolution logic correctly handles this case (mode set, compression not set) after the refactor, since previously both were passed directly from originalParameters. Mode resolvedMode = userMode; if (Mode.isConfigured(userMode) == false && CompressionLevel.isConfigured(userCompression)) { resolvedMode = Mode.deriveMode(userCompression); } // Setup the initial configuration that is used to help resolve parameters. builder.setKnnMethodConfigContext( KNNMethodConfigContext.builder() .vectorDataType(builder.originalParameters.getVectorDataType()) .versionCreated(parserContext.indexVersionCreated()) .dimension(builder.originalParameters.getDimension()) .mode(resolvedMode) .compressionLevel(userCompression) .build() Deprecated Enum Usage The Mode enum itself is annotated @Deprecated(since = "3.7.0", forRemoval = true), but deriveMode and isConfigured are static methods on this deprecated enum that are actively called in new production code paths. This creates a contradiction: new code is being added to a type marked for removal. Consider whether these utility methods should be moved elsewhere (e.g., into a separate utility class) to avoid coupling new logic to a deprecated type. public static Mode deriveMode(CompressionLevel compressionLevel) { if (CompressionLevel.isConfigured(compressionLevel) == false) { return NOT_CONFIGURED; } switch (compressionLevel) { case x1: case x2: return IN_MEMORY; default: return ON_DISK; } } Missing Log Verification Scenario 5 (testExplicitModeAndCompression_onUpgradedCluster_thenSucceed) states "A deprecation warning is expected in the server logs" but the test does not assert that the deprecation warning was actually emitted. Without this assertion, the test cannot verify the deprecation logging behavior described in the PR. public void testExplicitModeAndCompression_onUpgradedCluster_thenSucceed() throws Exception { if (isRunningAgainstOldCluster()) { return; } String indexName = testIndex + "-explicit-both"; createOnDiskIndex(indexName, "32x"); addKNNDocs(indexName, TEST_FIELD, DIMENSION, 0, NUM_DOCS); forceMergeKnnIndex(indexName); validateKNNSearch(indexName, TEST_FIELD, DIMENSION, NUM_DOCS, K); deleteKNNIndex(indexName); }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use dedicated deprecation logger for warnings Using log.warn for a deprecation notice is non-standard in OpenSearch; deprecation warnings should be emitted via the dedicated DeprecationLogger so they appear in the deprecation log and can be surfaced to the client. Replace the log.warn call with deprecationLogger.deprecate(...) using an appropriate key. src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java [568-573] if (Mode.isConfigured(userMode) && parserContext.indexVersionCreated().onOrAfter(Version.V_3_7_0)) { - log.warn( - "[Deprecation] The \"mode\" parameter is deprecated as of OpenSearch 3.7.0 and will be removed in 4.0. " + deprecationLogger.deprecate( + "knn_mode_parameter", + "The \"mode\" parameter is deprecated as of OpenSearch 3.7.0 and will be removed in 4.0. " + "Use \"compression_level\" instead. \"mode\" will be derived automatically from \"compression_level\"." ); } Suggestion importance[1-10]: 8 __ Why: Using log.warn for deprecation notices is non-standard in OpenSearch; the dedicated DeprecationLogger is the correct mechanism to surface deprecation warnings to clients and in the deprecation log. This is a meaningful correctness issue for the deprecation feature being implemented.	Medium
	Warn on conflicting mode and compression values When both userMode and userCompression are not configured, resolvedMode remains NOT_CONFIGURED, which is the correct fallback. However, when userMode IS configured (deprecated path), the code passes the user-supplied mode directly without any validation that it is consistent with the supplied userCompression. This could silently allow contradictory combinations (e.g., mode=in_memory with compression_level=32x) to pass through without warning. Consider adding a consistency check or at least a warning when both are configured and they conflict. src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java [577-580] Mode resolvedMode = userMode; if (Mode.isConfigured(userMode) == false && CompressionLevel.isConfigured(userCompression)) { resolvedMode = Mode.deriveMode(userCompression); +} else if (Mode.isConfigured(userMode) && CompressionLevel.isConfigured(userCompression)) { + Mode derivedMode = Mode.deriveMode(userCompression); + if (derivedMode != userMode) { + log.warn( + "Provided mode={} is inconsistent with compression_level={}; derived mode would be {}. " + + "The explicit mode value will be used.", + userMode, userCompression, derivedMode + ); + } } Suggestion importance[1-10]: 5 __ Why: The suggestion to warn on inconsistent mode and compression_level combinations is a valid improvement for user experience, but it's a minor enhancement rather than a critical bug fix. The current behavior of honoring the explicit userMode is documented and intentional for BwC.	Low
	Handle non-SNAPSHOT version suffixes robustly The version string may contain additional suffixes beyond -SNAPSHOT (e.g., -alpha1, -beta2, -rc1) that Version.fromString cannot parse, causing an exception at runtime. A more robust approach is to strip everything after the first - or use a regex to extract only the numeric version part. qa/restart-upgrade/src/test/java/org/opensearch/knn/bwc/ModeDeprecationIT.java [273-279] private boolean isModeSupported(java.util.Optional<String> bwcVersion) { if (bwcVersion.isEmpty()) { return false; } - String versionString = bwcVersion.get().replace("-SNAPSHOT", ""); + String versionString = bwcVersion.get().replaceAll("-.*$", ""); return Version.fromString(versionString).onOrAfter(Version.V_2_17_0); } Suggestion importance[1-10]: 4 __ Why: The suggestion to use replaceAll("-.*$", "") instead of replace("-SNAPSHOT", "") is a reasonable robustness improvement for test infrastructure, though non-SNAPSHOT suffixes are uncommon in practice for BWC test versions.	Low

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.39%. Comparing base (af6d18d) to head (b009cca).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...ain/java/org/opensearch/knn/index/mapper/Mode.java	66.66%	1 Missing and 1 partial ⚠️
...nsearch/knn/index/mapper/KNNVectorFieldMapper.java	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3304      +/-   ##
============================================
- Coverage     83.48%   83.39%   -0.10%     
+ Complexity     4269     4267       -2     
============================================
  Files           450      450              
  Lines         15509    15521      +12     
  Branches       2006     2009       +3     
============================================
- Hits          12948    12943       -5     
- Misses         1768     1781      +13     
- Partials        793      797       +4     

☔ 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.