Incorporate PR review comments

Signed-off-by: Ashish Singh <ssashish@amazon.com>

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

@@ -88,8 +88,8 @@
 import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.mapper.MapperService.MergeReason;
 import org.opensearch.index.query.QueryShardContext;
-import org.opensearch.index.remote.RemoteStoreBlobPathResolver;
-import org.opensearch.index.remote.RemoteStoreBlobPathType;
+import org.opensearch.index.remote.RemoteStorePathResolver;
+import org.opensearch.index.remote.RemoteStorePathType;
 import org.opensearch.index.shard.IndexSettingProvider;
 import org.opensearch.index.translog.Translog;
 import org.opensearch.indices.IndexCreationException;
@@ -170,7 +170,7 @@ public class MetadataCreateIndexService {
     private AwarenessReplicaBalance awarenessReplicaBalance;
 
     @Nullable
-    private final RemoteStoreBlobPathResolver remoteStoreBlobPathResolver;
+    private final RemoteStorePathResolver remoteStorePathResolver;
 
     public MetadataCreateIndexService(
         final Settings settings,
@@ -203,8 +203,8 @@ public MetadataCreateIndexService(
 
         // Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction.
         createIndexTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_KEY, true);
-        remoteStoreBlobPathResolver = isRemoteDataAttributePresent(settings)
-            ? new RemoteStoreBlobPathResolver(clusterService.getClusterSettings())
+        remoteStorePathResolver = isRemoteDataAttributePresent(settings)
+            ? new RemoteStorePathResolver(clusterService.getClusterSettings())
             : null;
     }
 
@@ -554,9 +554,9 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
         tmpImdBuilder.settings(indexSettings);
         tmpImdBuilder.system(isSystem);
 
-        if (remoteStoreBlobPathResolver != null) {
-            String pathType = remoteStoreBlobPathResolver.resolveType().toString();
-            tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, Map.of(RemoteStoreBlobPathType.NAME, pathType));
+        if (remoteStorePathResolver != null) {
+            String pathType = remoteStorePathResolver.resolveType().toString();
+            tmpImdBuilder.putCustom(IndexMetadata.REMOTE_STORE_CUSTOM_KEY, Map.of(RemoteStorePathType.NAME, pathType));
         }
 
         // Set up everything, now locally create the index to see that things are ok, and apply

server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

@@ -713,7 +713,7 @@ public void apply(Settings value, Settings current, Settings previous) {
                 IoBasedAdmissionControllerSettings.SEARCH_IO_USAGE_LIMIT,
                 IoBasedAdmissionControllerSettings.INDEXING_IO_USAGE_LIMIT,
                 IndicesService.CLUSTER_INDEX_RESTRICT_REPLICATION_TYPE_SETTING,
-                IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING,
+                IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING,
 
                 // Concurrent segment search settings
                 SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING,

server/src/main/java/org/opensearch/index/remote/RemoteStoreBlobPathResolver.java → server/src/main/java/org/opensearch/index/remote/RemoteStorePathResolver.java

@@ -12,21 +12,19 @@
 import org.opensearch.indices.IndicesService;
 
 /**
- * Determines the {@link RemoteStoreBlobPathType} at the time of index metadata creation.
+ * Determines the {@link RemoteStorePathType} at the time of index metadata creation.
  *
  * @opensearch.internal
  */
-public class RemoteStoreBlobPathResolver {
+public class RemoteStorePathResolver {
 
     private final ClusterSettings clusterSettings;
 
-    public RemoteStoreBlobPathResolver(ClusterSettings clusterSettings) {
+    public RemoteStorePathResolver(ClusterSettings clusterSettings) {
         this.clusterSettings = clusterSettings;
     }
 
-    public RemoteStoreBlobPathType resolveType() {
-        return clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING)
-            ? RemoteStoreBlobPathType.HASHED_PREFIX
-            : RemoteStoreBlobPathType.FIXED;
+    public RemoteStorePathType resolveType() {
+        return clusterSettings.get(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING);
     }
 }

server/src/main/java/org/opensearch/index/remote/RemoteStoreBlobPathType.java → server/src/main/java/org/opensearch/index/remote/RemoteStorePathType.java

@@ -8,25 +8,24 @@
 
 package org.opensearch.index.remote;
 
+import java.util.Locale;
+
 /**
  * Enumerates the types of remote store paths resolution techniques supported by OpenSearch.
  * For more information, see <a href="https://github.com/opensearch-project/OpenSearch/issues/12567">Github issue #12567</a>.
  *
  * @opensearch.internal
  */
-public enum RemoteStoreBlobPathType {
+public enum RemoteStorePathType {
 
     FIXED,
     HASHED_PREFIX;
 
-    public static RemoteStoreBlobPathType parseString(String remoteStoreBlobPathType) {
+    public static RemoteStorePathType parseString(String remoteStoreBlobPathType) {
         try {
-            return RemoteStoreBlobPathType.valueOf(remoteStoreBlobPathType);
+            return RemoteStorePathType.valueOf(remoteStoreBlobPathType.toUpperCase(Locale.ROOT));
         } catch (IllegalArgumentException e) {
             throw new IllegalArgumentException("Could not parse RemoteStorePathType for [" + remoteStoreBlobPathType + "]");
-        } catch (NullPointerException npe) {
-            // return a default value for null input
-            return FIXED;
         }
     }
 

server/src/main/java/org/opensearch/indices/IndicesService.java

@@ -123,6 +123,7 @@
 import org.opensearch.index.query.QueryRewriteContext;
 import org.opensearch.index.recovery.RecoveryStats;
 import org.opensearch.index.refresh.RefreshStats;
+import org.opensearch.index.remote.RemoteStorePathType;
 import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory;
 import org.opensearch.index.search.stats.SearchStats;
 import org.opensearch.index.seqno.RetentionLeaseStats;
@@ -314,12 +315,13 @@ public class IndicesService extends AbstractLifecycleComponent
     );
 
     /**
-     * This setting is used to enable the optimisation in prefix path which helps in achieving higher throughput and lesser
-     * rate limiting by remote store providers. This setting is effective only for remote store enabled cluster.
+     * This setting is used to set the remote store blob store path prefix strategy. This setting is effective only for
+     * remote store enabled cluster.
      */
-    public static final Setting<Boolean> CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING = Setting.boolSetting(
-        "cluster.remote_store.index.path.prefix.optimised",
-        false,
+    public static final Setting<RemoteStorePathType> CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING = new Setting<>(
+        "cluster.remote_store.index.path.prefix.type",
+        RemoteStorePathType.FIXED.toString(),
+        RemoteStorePathType::parseString,
         Property.NodeScope,
         Property.Dynamic
     );

server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java

@@ -71,7 +71,7 @@
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.query.QueryShardContext;
-import org.opensearch.index.remote.RemoteStoreBlobPathType;
+import org.opensearch.index.remote.RemoteStorePathType;
 import org.opensearch.index.translog.Translog;
 import org.opensearch.indices.IndexCreationException;
 import org.opensearch.indices.IndicesService;
@@ -1587,34 +1587,32 @@ public void testBuildIndexMetadata() {
      */
     public void testRemoteCustomData() {
         // Case 1 - Remote store is not enabled
-        IndexMetadata indexMetadata = testRemoteCustomData(false, randomBoolean());
+        IndexMetadata indexMetadata = testRemoteCustomData(false, randomFrom(RemoteStorePathType.values()));
         assertNull(indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY));
 
-        // Case 2 - cluster.remote_store.index.path.prefix.optimised=false (default value)
-        indexMetadata = testRemoteCustomData(true, false);
+        // Case 2 - cluster.remote_store.index.path.prefix.optimised=fixed (default value)
+        indexMetadata = testRemoteCustomData(true, RemoteStorePathType.FIXED);
         validateRemoteCustomData(
             indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY),
-            RemoteStoreBlobPathType.NAME,
-            RemoteStoreBlobPathType.FIXED.toString()
+            RemoteStorePathType.NAME,
+            RemoteStorePathType.FIXED.toString()
         );
 
-        // Case 3 - cluster.remote_store.index.path.prefix.optimised=true
-        indexMetadata = testRemoteCustomData(true, true);
+        // Case 3 - cluster.remote_store.index.path.prefix.optimised=hashed_prefix
+        indexMetadata = testRemoteCustomData(true, RemoteStorePathType.HASHED_PREFIX);
         validateRemoteCustomData(
             indexMetadata.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY),
-            RemoteStoreBlobPathType.NAME,
-            RemoteStoreBlobPathType.HASHED_PREFIX.toString()
+            RemoteStorePathType.NAME,
+            RemoteStorePathType.HASHED_PREFIX.toString()
         );
     }
 
-    private IndexMetadata testRemoteCustomData(boolean remoteStoreEnabled, boolean optimisedPrefix) {
+    private IndexMetadata testRemoteCustomData(boolean remoteStoreEnabled, RemoteStorePathType remoteStorePathType) {
         Settings.Builder settingsBuilder = Settings.builder();
         if (remoteStoreEnabled) {
             settingsBuilder.put(NODE_ATTRIBUTES.getKey() + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "test");
         }
-        if (optimisedPrefix) {
-            settingsBuilder.put(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_OPTIMISED_SETTING.getKey(), true);
-        }
+        settingsBuilder.put(IndicesService.CLUSTER_REMOTE_STORE_PATH_PREFIX_TYPE_SETTING.getKey(), remoteStorePathType.toString());
         Settings settings = settingsBuilder.build();
 
         ClusterService clusterService = mock(ClusterService.class);