Add period at the start of the hashed prefix

Signed-off-by: Ashish Singh <[email protected]>

server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteRestoreSnapshotIT.java

@@ -55,6 +55,7 @@
 import java.util.stream.Stream;
 
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
+import static org.opensearch.index.remote.RemoteStoreCustomMetadataResolver.PATH_TYPE_NODE_ATTR_KEY;
 import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.SEGMENTS;
 import static org.opensearch.index.remote.RemoteStoreEnums.DataCategory.TRANSLOG;
 import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA;
@@ -215,8 +216,9 @@ public void testRestoreOperationsShallowCopyEnabled() throws Exception {
      * on snapshot restore.
      */
     public void testRemoteStoreCustomDataOnIndexCreationAndRestore() {
-        String clusterManagerNode = internalCluster().startClusterManagerOnlyNode();
-        internalCluster().startDataOnlyNode();
+        Settings pathTypeSetting = Settings.builder().put(PATH_TYPE_NODE_ATTR_KEY, true).build();
+        String clusterManagerNode = internalCluster().startClusterManagerOnlyNode(pathTypeSetting);
+        internalCluster().startDataOnlyNode(pathTypeSetting);
         String indexName1 = "testindex1";
         String indexName2 = "testindex2";
         String snapshotRepoName = "test-restore-snapshot-repo";

server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreUploadIndexPathIT.java

@@ -23,6 +23,7 @@
 import java.util.concurrent.ExecutionException;
 
 import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING;
+import static org.opensearch.index.remote.RemoteStoreCustomMetadataResolver.PATH_TYPE_NODE_ATTR_KEY;
 import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
@@ -33,7 +34,11 @@ public class RemoteStoreUploadIndexPathIT extends RemoteStoreBaseIntegTestCase {
 
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
-        return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true).build();
+        return Settings.builder()
+            .put(super.nodeSettings(nodeOrdinal))
+            .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true)
+            .put(PATH_TYPE_NODE_ATTR_KEY, true)
+            .build();
     }
 
     /**

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

@@ -215,7 +215,13 @@ public MetadataCreateIndexService(
         createIndexTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_KEY, true);
         Supplier<Version> minNodeVersionSupplier = () -> clusterService.state().nodes().getMinNodeVersion();
         remoteStoreCustomMetadataResolver = isRemoteDataAttributePresent(settings)
-            ? new RemoteStoreCustomMetadataResolver(remoteStoreSettings, minNodeVersionSupplier, repositoriesServiceSupplier, settings)
+            ? new RemoteStoreCustomMetadataResolver(
+                remoteStoreSettings,
+                clusterService,
+                minNodeVersionSupplier,
+                repositoriesServiceSupplier,
+                settings
+            )
             : null;
     }
 
@@ -256,7 +262,8 @@ public void validateIndexName(String index, ClusterState state) {
 
     /**
      * Validates (if this index has a dot-prefixed name) whether it follows the rules for dot-prefixed indices.
-     * @param index The name of the index in question
+     *
+     * @param index    The name of the index in question
      * @param isHidden Whether or not this is a hidden index
      */
     public boolean validateDotIndex(String index, @Nullable Boolean isHidden) {
@@ -323,7 +330,7 @@ public static void validateIndexOrAliasName(String index, BiFunction<String, Str
      * the timeout, then {@link CreateIndexClusterStateUpdateResponse#isShardsAcknowledged()} will
      * return true, otherwise if the operation timed out, then it will return false.
      *
-     * @param request the index creation cluster state update request
+     * @param request  the index creation cluster state update request
      * @param listener the listener on which to send the index creation cluster state update response
      */
     public void createIndex(
@@ -474,14 +481,15 @@ public ClusterState applyCreateIndexRequest(ClusterState currentState, CreateInd
      * Given the state and a request as well as the metadata necessary to build a new index,
      * validate the configuration with an actual index service as return a new cluster state with
      * the index added (and rerouted)
-     * @param currentState the current state to base the new state off of
-     * @param request the create index request
-     * @param silent a boolean for whether logging should be at a lower or higher level
-     * @param sourceMetadata when recovering from an existing index, metadata that should be copied to the new index
-     * @param temporaryIndexMeta metadata for the new index built from templates, source metadata, and request settings
-     * @param mappings a list of all mapping definitions to apply, in order
-     * @param aliasSupplier a function that takes the real {@link IndexService} and returns a list of {@link AliasMetadata} aliases
-     * @param templatesApplied a list of the names of the templates applied, for logging
+     *
+     * @param currentState        the current state to base the new state off of
+     * @param request             the create index request
+     * @param silent              a boolean for whether logging should be at a lower or higher level
+     * @param sourceMetadata      when recovering from an existing index, metadata that should be copied to the new index
+     * @param temporaryIndexMeta  metadata for the new index built from templates, source metadata, and request settings
+     * @param mappings            a list of all mapping definitions to apply, in order
+     * @param aliasSupplier       a function that takes the real {@link IndexService} and returns a list of {@link AliasMetadata} aliases
+     * @param templatesApplied    a list of the names of the templates applied, for logging
      * @param metadataTransformer if provided, a function that may alter cluster metadata in the same cluster state update that
      *                            creates the index
      * @return a new cluster state with the index added
@@ -1002,9 +1010,10 @@ static Settings aggregateIndexSettings(
     /**
      * Updates index settings to set replication strategy by default based on cluster level settings or remote store
      * node attributes
-     * @param settingsBuilder index settings builder to be updated with relevant settings
-     * @param requestSettings settings passed in during index create request
-     * @param clusterSettings cluster level settings
+     *
+     * @param settingsBuilder          index settings builder to be updated with relevant settings
+     * @param requestSettings          settings passed in during index create request
+     * @param clusterSettings          cluster level settings
      * @param combinedTemplateSettings combined template settings which satisfy the index
      */
     public static void updateReplicationStrategy(
@@ -1041,11 +1050,12 @@ public static void updateReplicationStrategy(
 
     /**
      * Updates index settings to enable remote store by default based on node attributes
+     *
      * @param settingsBuilder index settings builder to be updated with relevant settings
-     * @param clusterState state of cluster
+     * @param clusterState    state of cluster
      * @param clusterSettings cluster level settings
-     * @param nodeSettings node level settings
-     * @param indexName name of index
+     * @param nodeSettings    node level settings
+     * @param indexName       name of index
      */
     public static void updateRemoteStoreSettings(
         Settings.Builder settingsBuilder,
@@ -1413,7 +1423,7 @@ private static List<String> validatePrivateSettingsNotExplicitlySet(Settings set
     /**
      * Validates that the configured index data path (if any) is a sub-path of the configured shared data path (if any)
      *
-     * @param settings the index configured settings
+     * @param settings       the index configured settings
      * @param sharedDataPath the configured `path.shared_data` (if any)
      * @return a list containing validaton errors or an empty list if there aren't any errors
      */

server/src/main/java/org/opensearch/index/remote/RemoteStoreCustomMetadataResolver.java

@@ -9,16 +9,22 @@
 package org.opensearch.index.remote;
 
 import org.opensearch.Version;
+import org.opensearch.cluster.node.DiscoveryNode;
+import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm;
 import org.opensearch.index.remote.RemoteStoreEnums.PathType;
 import org.opensearch.indices.RemoteStoreSettings;
+import org.opensearch.node.Node;
 import org.opensearch.repositories.RepositoriesService;
 import org.opensearch.repositories.Repository;
 import org.opensearch.repositories.RepositoryMissingException;
 import org.opensearch.repositories.blobstore.BlobStoreRepository;
 
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
 import java.util.function.Supplier;
 
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.getRemoteStoreTranslogRepo;
@@ -31,18 +37,24 @@
 @ExperimentalApi
 public class RemoteStoreCustomMetadataResolver {
 
+    public final static String PATH_TYPE_ATTRIBUTE_KEY = "optimised_remote_store_enable";
+    public final static String PATH_TYPE_NODE_ATTR_KEY = Node.NODE_ATTRIBUTES.getKey() + PATH_TYPE_ATTRIBUTE_KEY;
+
     private final RemoteStoreSettings remoteStoreSettings;
+    private final ClusterService clusterService;
     private final Supplier<Version> minNodeVersionSupplier;
     private final Supplier<RepositoriesService> repositoriesServiceSupplier;
     private final Settings settings;
 
     public RemoteStoreCustomMetadataResolver(
         RemoteStoreSettings remoteStoreSettings,
+        ClusterService clusterService,
         Supplier<Version> minNodeVersionSupplier,
         Supplier<RepositoriesService> repositoriesServiceSupplier,
         Settings settings
     ) {
         this.remoteStoreSettings = remoteStoreSettings;
+        this.clusterService = Objects.requireNonNull(clusterService);
         this.minNodeVersionSupplier = minNodeVersionSupplier;
         this.repositoriesServiceSupplier = repositoriesServiceSupplier;
         this.settings = settings;
@@ -52,12 +64,29 @@ public RemoteStorePathStrategy getPathStrategy() {
         PathType pathType;
         PathHashAlgorithm pathHashAlgorithm;
         // Min node version check ensures that we are enabling the new prefix type only when all the nodes understand it.
-        pathType = Version.V_2_14_0.compareTo(minNodeVersionSupplier.get()) <= 0 ? remoteStoreSettings.getPathType() : PathType.FIXED;
+        pathType = isPathTypeEnabled() ? remoteStoreSettings.getPathType() : PathType.FIXED;
         // If the path type is fixed, hash algorithm is not applicable.
         pathHashAlgorithm = pathType == PathType.FIXED ? null : remoteStoreSettings.getPathHashAlgorithm();
         return new RemoteStorePathStrategy(pathType, pathHashAlgorithm);
     }
 
+    private boolean isPathTypeEnabled() {
+        Map<String, DiscoveryNode> nodesMap = Collections.unmodifiableMap(clusterService.state().nodes().getNodes());
+
+        if (nodesMap.isEmpty()) {
+            return false;
+        }
+
+        for (String node : nodesMap.keySet()) {
+            DiscoveryNode nodeDiscovery = nodesMap.get(node);
+            Map<String, String> nodeAttributes = nodeDiscovery.getAttributes();
+            if (!nodeAttributes.containsKey(PATH_TYPE_ATTRIBUTE_KEY)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     public boolean isTranslogMetadataEnabled() {
         Repository repository;
         try {

server/src/main/java/org/opensearch/index/remote/RemoteStoreEnums.java

@@ -36,6 +36,8 @@
 @ExperimentalApi
 public class RemoteStoreEnums {
 
+    static final char REMOTE_STORE_DEDICATED_PREFIX_START_CHAR = '.';
+
     /**
      * Categories of the data in Remote store.
      */
@@ -112,7 +114,7 @@ boolean requiresHashAlgorithm() {
             public BlobPath generatePath(PathInput pathInput, PathHashAlgorithm hashAlgorithm) {
                 assert Objects.nonNull(hashAlgorithm) : "hashAlgorithm is expected to be non-null";
                 return BlobPath.cleanPath()
-                    .add(hashAlgorithm.hash(pathInput))
+                    .add(REMOTE_STORE_DEDICATED_PREFIX_START_CHAR + hashAlgorithm.hash(pathInput))
                     .add(pathInput.basePath())
                     .add(pathInput.indexUUID())
                     .add(pathInput.shardId())

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

@@ -74,7 +74,7 @@ public class RemoteStoreSettings {
     @ExperimentalApi
     public static final Setting<RemoteStoreEnums.PathType> CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING = new Setting<>(
         "cluster.remote_store.index.path.type",
-        RemoteStoreEnums.PathType.FIXED.toString(),
+        RemoteStoreEnums.PathType.HASHED_PREFIX.toString(),
         RemoteStoreEnums.PathType::parseString,
         Property.NodeScope,
         Property.Dynamic

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

@@ -74,6 +74,7 @@
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.query.QueryShardContext;
+import org.opensearch.index.remote.RemoteStoreCustomMetadataResolver;
 import org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm;
 import org.opensearch.index.remote.RemoteStoreEnums.PathType;
 import org.opensearch.index.translog.Translog;
@@ -1754,8 +1755,14 @@ private IndexMetadata testRemoteCustomData(boolean remoteStoreEnabled, PathType
         Metadata metadata = Metadata.builder()
             .transientSettings(Settings.builder().put(Metadata.DEFAULT_REPLICA_COUNT_SETTING.getKey(), 1).build())
             .build();
+        DiscoveryNodes discoveryNodes = mock(DiscoveryNodes.class);
+        DiscoveryNode discoveryNode = mock(DiscoveryNode.class);
+        when(discoveryNode.getAttributes()).thenReturn(Map.of(RemoteStoreCustomMetadataResolver.PATH_TYPE_ATTRIBUTE_KEY, "true"));
+        when(discoveryNodes.getNodes()).thenReturn(Map.of("node-1", discoveryNode));
+        when(discoveryNodes.getMinNodeVersion()).thenReturn(Version.V_2_15_0);
         ClusterState clusterState = ClusterState.builder(org.opensearch.cluster.ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY))
             .metadata(metadata)
+            .nodes(discoveryNodes)
             .build();
         ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
         when(clusterService.getSettings()).thenReturn(settings);