Revert "Add faster scaling composite hash value encoding for remote path"

[peternied]

Description

Reverts #13155

This pull request breaks the public contract [1] without a changelog entry or associated documentation

Comparing source compatibility of  against 
***! MODIFIED ENUM: PUBLIC ABSTRACT STATIC org.opensearch.index.remote.RemoteStoreEnums$PathHashAlgorithm  (field removed)
	===  CLASS FILE FORMAT VERSION: 55.0 <- 55.0
	---! REMOVED FIELD: PUBLIC(-) STATIC(-) FINAL(-) org.opensearch.index.remote.RemoteStoreEnums$PathHashAlgorithm FNV_1A
	+++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) org.opensearch.index.remote.RemoteStoreEnums$PathHashAlgorithm FNV_1A_COMPOSITE_1
	+++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) org.opensearch.index.remote.RemoteStoreEnums$PathHashAlgorithm FNV_1A_BASE64
***! MODIFIED CLASS: PUBLIC org.opensearch.indices.IndicesService  (not serializable)
	===  CLASS FILE FORMAT VERSION: 55.0 <- 55.0
	---! REMOVED FIELD: PUBLIC(-) STATIC(-) FINAL(-) org.opensearch.common.settings.Setting<org.opensearch.index.remote.RemoteStoreEnums$PathType> CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING
	+++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) org.opensearch.common.settings.Setting<org.opensearch.index.remote.RemoteStoreEnums$PathHashAlgorithm> CLUSTER_REMOTE_STORE_PATH_HASH_ALGORITHM_SETTING
	+++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) org.opensearch.common.settings.Setting<org.opensearch.index.remote.RemoteStoreEnums$PathType> CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING

• [1] https://github.com/opensearch-project/OpenSearch/actions/runs/8692579563/job/23837516724

Related Issues

• Related Add faster scaling composite hash value encoding for remote path #13155

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)
• 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.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[peternied]

FYI - @sachinpkale @gbbafna

[peternied]

Detect Breaking Changes / detect-breaking-change Failure ❌

The detect breaking changes check will continue to fail because the change being reverted impacted the public API surface.

[github-actions]

✅ Gradle check result for df3d79c: SUCCESS

[gbbafna]

Hi @peternied , This changes are not yet part of 2.x . Hence public contract is not broken yet . That is the reason we have overrode the gradle check . The other option is to mark these classes Experimental as well, as they are in development phase as of now .

[ashking94]

@peternied As @gbbafna mentioned, none of these changes have been backported to 2.x branches. Adding a change log entry or documentation at this stage would not be suitable. These are minor refactoring in the static field names that are not released in any version. In hindsight, I did think about adding a note about the breaking changes (that are false positive at this stage). I also want to understand why The detect breaking changes check will continue to fail because the change being reverted impacted the public API surface.?

[Bukhtawar]

I think it's fine to break, however I agree with @peternied that we need the right documentation or change log entry so that plugin developers are well aware of what is expected to break in 3.0 and accordingly make modifications as deemed necessary. The IndicesService class has long existed and it won't be apt to mark it Experimental after it has been made public

Edit: The removed fields were never added to 2.x in the first place, nor did it make it to any released version, so ideally there is nothing breaking

Maybe we need to fix the detection logic, @peternied to compare against released versions rather than two commits in the same release cycle

[reta]

Maybe we need to fix the detection logic, @peternied to compare against released versions rather than two commits in the same release cycle

I think the right fix in this case would be to switch RemoteStoreEnums to @ExperimentalApi, the initial commit (#12986) was incorrectly applied @PublicApi(since = "2.14.0") to the types in question.

[peternied]

I know this might have come as a surprise, luckily reverts are cheap and resubmitting the PR will likely be quick - I see that its already been merged with modifications .

Maybe we need to fix the detection logic, @peternied to compare against released versions rather than two commits in the same release cycle

Could you open an issue / PR with a proposal?