Address comments

Signed-off-by: Arpit Bandejiya <abandeji@amazon.com>
opensearch-project · Jul 10, 2024 · bce40bb · bce40bb
1 parent 545ce67
commit bce40bb
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 32 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableService.java
@@ -28,7 +28,7 @@
  * @opensearch.internal
  */
 public interface RemoteRoutingTableService extends LifecycleComponent {
-    DiffableUtils.NonDiffableValueSerializer<String, IndexRoutingTable> CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER =
+    public static final DiffableUtils.NonDiffableValueSerializer<String, IndexRoutingTable> CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER =
         new DiffableUtils.NonDiffableValueSerializer<>() {
             @Override
             public void write(IndexRoutingTable value, StreamOutput out) throws IOException {

diff --git a/server/src/main/java/org/opensearch/common/remote/AbstractRemoteWritableBlobEntity.java b/server/src/main/java/org/opensearch/common/remote/AbstractRemoteWritableBlobEntity.java
@@ -40,6 +40,10 @@ public AbstractRemoteWritableBlobEntity(
         this.namedXContentRegistry = namedXContentRegistry;
     }
 
+    public AbstractRemoteWritableBlobEntity(final String clusterUUID, final Compressor compressor) {
+        this(clusterUUID, compressor, null);
+    }
+
     public abstract BlobPathParameters getBlobPathParameters();
 
     public abstract String getType();

diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java
@@ -104,8 +104,8 @@
 import static org.opensearch.gateway.remote.model.RemotePersistentSettingsMetadata.SETTING_METADATA;
 import static org.opensearch.gateway.remote.model.RemoteTemplatesMetadata.TEMPLATES_METADATA;
 import static org.opensearch.gateway.remote.model.RemoteTransientSettingsMetadata.TRANSIENT_SETTING_METADATA;
+import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_METADATA_PREFIX;
 import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE;
-import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE_PREFIX;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.isRemoteStoreClusterStateEnabled;
 
 /**
@@ -714,7 +714,7 @@ UploadedMetadataResults writeMetadataInParallel(
         UploadedMetadataResults response = new UploadedMetadataResults();
         results.forEach((name, uploadedMetadata) -> {
             if (uploadedMetadata.getClass().equals(UploadedIndexMetadata.class)
-                && uploadedMetadata.getComponent().contains(INDEX_ROUTING_TABLE_PREFIX)) {
+                && uploadedMetadata.getComponent().contains(INDEX_ROUTING_METADATA_PREFIX)) {
                 response.uploadedIndicesRoutingMetadata.add((UploadedIndexMetadata) uploadedMetadata);
             } else if (name.startsWith(CUSTOM_METADATA)) {
                 // component name for custom metadata will look like custom--<metadata-attribute>

diff --git a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterStateBlobStore.java b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterStateBlobStore.java
@@ -23,6 +23,8 @@
 import java.io.InputStream;
 import java.util.concurrent.ExecutorService;
 
+import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CLUSTER_STATE_PATH_TOKEN;
+
 /**
  * Abstract class for a blob type storage
  *
@@ -31,7 +33,7 @@
  */
 public class RemoteClusterStateBlobStore<T, U extends AbstractRemoteWritableBlobEntity<T>> implements RemoteWritableEntityStore<T, U> {
 
-    protected final BlobStoreTransferService transferService;
+    private final BlobStoreTransferService transferService;
     private final BlobStoreRepository blobStoreRepository;
     private final String clusterName;
     private final ExecutorService executorService;
@@ -96,10 +98,12 @@ public BlobPath getBasePath() {
         return blobStoreRepository.basePath();
     }
 
+    public BlobPath getBlobPathPrefix(String clusterUUID) {
+        return getBasePath().add(RemoteClusterStateUtils.encodeString(getClusterName())).add(CLUSTER_STATE_PATH_TOKEN).add(clusterUUID);
+    }
+
     public BlobPath getBlobPathForUpload(final AbstractRemoteWritableBlobEntity<T> obj) {
-        BlobPath blobPath = getBasePath().add(RemoteClusterStateUtils.encodeString(getClusterName()))
-            .add("cluster-state")
-            .add(obj.clusterUUID());
+        BlobPath blobPath = getBlobPathPrefix(obj.clusterUUID());
         for (String token : obj.getBlobPathParameters().getPathTokens()) {
             blobPath = blobPath.add(token);
         }

diff --git a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteRoutingTableBlobStore.java b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteRoutingTableBlobStore.java
@@ -13,7 +13,6 @@
 import org.opensearch.common.remote.RemoteWriteableEntity;
 import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Setting;
-import org.opensearch.gateway.remote.RemoteClusterStateUtils;
 import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable;
 import org.opensearch.index.remote.RemoteStoreEnums;
 import org.opensearch.index.remote.RemoteStorePathStrategy;
@@ -77,10 +76,8 @@ public RemoteRoutingTableBlobStore(
     @Override
     public BlobPath getBlobPathForUpload(final AbstractRemoteWritableBlobEntity<IndexRoutingTable> obj) {
         assert obj.getBlobPathParameters().getPathTokens().size() == 1 : "Unexpected tokens in RemoteRoutingTableObject";
-        BlobPath indexRoutingPath = getBasePath().add(RemoteClusterStateUtils.encodeString(getClusterName()))
-            .add("cluster-state")
-            .add(obj.clusterUUID())
-            .add(INDEX_ROUTING_TABLE);
+        BlobPath indexRoutingPath = getBlobPathPrefix(obj.clusterUUID()).add(INDEX_ROUTING_TABLE);
+
         BlobPath path = pathType.path(
             RemoteStorePathStrategy.PathInput.builder()
                 .basePath(indexRoutingPath)

diff --git a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTable.java b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTable.java
@@ -47,7 +47,7 @@ public RemoteIndexRoutingTable(
         long term,
         long version
     ) {
-        super(clusterUUID, compressor, null);
+        super(clusterUUID, compressor);
         this.index = indexRoutingTable.getIndex();
         this.indexRoutingTable = indexRoutingTable;
         this.term = term;
@@ -61,24 +61,20 @@ public RemoteIndexRoutingTable(
      * @param compressor Compressor object
      */
     public RemoteIndexRoutingTable(String blobName, String clusterUUID, Compressor compressor) {
-        super(clusterUUID, compressor, null);
+        super(clusterUUID, compressor);
         this.index = null;
         this.term = -1;
         this.version = -1;
         this.blobName = blobName;
     }
 
-    public IndexRoutingTable getIndexRoutingTable() {
-        return indexRoutingTable;
-    }
-
     public Index getIndex() {
         return index;
     }
 
     @Override
     public BlobPathParameters getBlobPathParameters() {
-        return new BlobPathParameters(List.of(indexRoutingTable.getIndex().getUUID()));
+        return new BlobPathParameters(List.of(indexRoutingTable.getIndex().getUUID()), INDEX_ROUTING_FILE);
     }
 
     @Override
@@ -91,7 +87,7 @@ public String generateBlobFileName() {
         if (blobFileName == null) {
             blobFileName = String.join(
                 DELIMITER,
-                INDEX_ROUTING_FILE,
+                getBlobPathParameters().getFilePrefix(),
                 RemoteStoreUtils.invertLong(term),
                 RemoteStoreUtils.invertLong(version),
                 RemoteStoreUtils.invertLong(System.currentTimeMillis())

diff --git a/...r/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/...r/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java
@@ -63,6 +63,8 @@
 import static org.opensearch.gateway.remote.ClusterMetadataManifestTests.randomUploadedIndexMetadataList;
 import static org.opensearch.gateway.remote.RemoteClusterStateUtils.DELIMITER;
 import static org.opensearch.gateway.remote.RemoteClusterStateUtils.PATH_DELIMITER;
+import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_FILE;
+import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_METADATA_PREFIX;
 import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE;
 import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE_FORMAT;
 import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE_PREFIX;
@@ -577,15 +579,15 @@ public void testGetAsyncIndexRoutingWriteAction() throws Exception {
         assertNotNull(listener.getResult());
         ClusterMetadataManifest.UploadedMetadata uploadedMetadata = listener.getResult();
 
-        assertEquals(INDEX_ROUTING_TABLE_PREFIX + indexName, uploadedMetadata.getComponent());
+        assertEquals(INDEX_ROUTING_METADATA_PREFIX + indexName, uploadedMetadata.getComponent());
         String uploadedFileName = uploadedMetadata.getUploadedFilename();
         String[] pathTokens = uploadedFileName.split(PATH_DELIMITER);
         assertEquals(8, pathTokens.length);
         assertEquals(pathTokens[1], "base-path");
         String[] fileNameTokens = pathTokens[7].split(DELIMITER);
 
         assertEquals(4, fileNameTokens.length);
-        assertEquals(fileNameTokens[0], INDEX_ROUTING_TABLE);
+        assertEquals(fileNameTokens[0], INDEX_ROUTING_FILE);
         assertEquals(fileNameTokens[1], RemoteStoreUtils.invertLong(1L));
         assertEquals(fileNameTokens[2], RemoteStoreUtils.invertLong(2L));
         assertThat(RemoteStoreUtils.invertLong(fileNameTokens[3]), lessThanOrEqualTo(System.currentTimeMillis()));

diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java
@@ -142,9 +142,9 @@
 import static org.opensearch.gateway.remote.model.RemotePersistentSettingsMetadata.SETTING_METADATA;
 import static org.opensearch.gateway.remote.model.RemoteTemplatesMetadata.TEMPLATES_METADATA;
 import static org.opensearch.gateway.remote.model.RemoteTemplatesMetadata.TEMPLATES_METADATA_FORMAT;
-import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE;
 import static org.opensearch.gateway.remote.model.RemoteTemplatesMetadataTests.getTemplatesMetadata;
 import static org.opensearch.gateway.remote.model.RemoteTransientSettingsMetadata.TRANSIENT_SETTING_METADATA;
+import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_METADATA_PREFIX;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_SETTINGS_ATTRIBUTE_KEY_PREFIX;
 import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_REPOSITORY_TYPE_ATTRIBUTE_KEY_FORMAT;
@@ -2605,7 +2605,7 @@ public void testWriteFullMetadataSuccessWithRoutingTable() throws IOException {
             "test-index",
             "index-uuid",
             "routing-filename",
-            INDEX_ROUTING_TABLE + CUSTOM_DELIMITER
+            INDEX_ROUTING_METADATA_PREFIX
         );
         final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder()
             .indices(List.of(uploadedIndexMetadata))
@@ -2656,7 +2656,7 @@ public void testWriteFullMetadataInParallelSuccessWithRoutingTable() throws IOEx
             "test-index",
             "index-uuid",
             "routing-filename",
-            INDEX_ROUTING_TABLE + CUSTOM_DELIMITER
+            INDEX_ROUTING_METADATA_PREFIX
         );
 
         final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder()
@@ -2711,7 +2711,7 @@ public void testWriteIncrementalMetadataSuccessWithRoutingTable() throws IOExcep
             "test-index",
             "index-uuid",
             "routing-filename",
-            INDEX_ROUTING_TABLE + CUSTOM_DELIMITER
+            INDEX_ROUTING_METADATA_PREFIX
         );
         final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder()
             .indices(List.of(uploadedIndexMetadata))

diff --git a/...rc/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableTests.java b/...rc/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableTests.java
@@ -36,9 +36,8 @@
 import java.io.InputStream;
 import java.util.List;
 
-import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CUSTOM_DELIMITER;
-import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE;
-import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE_PREFIX;
+import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_FILE;
+import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_METADATA_PREFIX;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
@@ -206,7 +205,7 @@ public void testBlobPathParameters() {
 
             BlobPathParameters params = remoteObjectForUpload.getBlobPathParameters();
             assertThat(params.getPathTokens(), is(List.of(indexRoutingTable.getIndex().getUUID())));
-            String expectedPrefix = "";
+            String expectedPrefix = INDEX_ROUTING_FILE;
             assertThat(params.getFilePrefix(), is(expectedPrefix));
         });
     }
@@ -237,7 +236,7 @@ public void testGenerateBlobFileName() {
 
             String blobFileName = remoteObjectForUpload.generateBlobFileName();
             String[] nameTokens = blobFileName.split(RemoteClusterStateUtils.DELIMITER);
-            assertEquals(nameTokens[0], INDEX_ROUTING_TABLE_PREFIX);
+            assertEquals(nameTokens[0], INDEX_ROUTING_FILE);
             assertEquals(nameTokens[1], RemoteStoreUtils.invertLong(STATE_TERM));
             assertEquals(nameTokens[2], RemoteStoreUtils.invertLong(STATE_VERSION));
             assertThat(RemoteStoreUtils.invertLong(nameTokens[3]), lessThanOrEqualTo(System.currentTimeMillis()));
@@ -273,7 +272,7 @@ public void testGetUploadedMetadata() throws IOException {
             try (InputStream inputStream = remoteObjectForUpload.serialize()) {
                 remoteObjectForUpload.setFullBlobName(new BlobPath().add(TEST_BLOB_PATH));
                 ClusterMetadataManifest.UploadedMetadata uploadedMetadata = remoteObjectForUpload.getUploadedMetadata();
-                String expectedPrefix = String.join(CUSTOM_DELIMITER, INDEX_ROUTING_TABLE, indexRoutingTable.getIndex().getName());
+                String expectedPrefix = INDEX_ROUTING_METADATA_PREFIX + indexRoutingTable.getIndex().getName();
                 assertThat(uploadedMetadata.getComponent(), is(expectedPrefix));
                 assertThat(uploadedMetadata.getUploadedFilename(), is(remoteObjectForUpload.getFullBlobName()));
             } catch (IOException e) {