From da32e95332aa4af33d5b3fe1e8a06fe812da1f90 Mon Sep 17 00:00:00 2001 From: Arpit Bandejiya Date: Tue, 9 Jul 2024 14:50:56 +0530 Subject: [PATCH] Address comments Signed-off-by: Arpit Bandejiya --- .../routing/remote/RemoteRoutingTableService.java | 2 +- .../remote/AbstractRemoteWritableBlobEntity.java | 4 ++++ .../gateway/remote/RemoteClusterStateService.java | 4 ++-- .../remote/model/RemoteClusterStateBlobStore.java | 12 ++++++++---- .../remote/model/RemoteRoutingTableBlobStore.java | 7 +++---- .../remote/routingtable/RemoteIndexRoutingTable.java | 12 ++++-------- .../remote/RemoteRoutingTableServiceTests.java | 6 ++++-- .../remote/RemoteClusterStateServiceTests.java | 8 ++++---- .../routingtable/RemoteIndexRoutingTableTests.java | 11 +++++------ 9 files changed, 35 insertions(+), 31 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 index 828c65353cdd9..6e4108bc69d69 100644 --- 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 CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER = + public static final DiffableUtils.NonDiffableValueSerializer 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 index 23fc9d3ad77cb..237c077cb673c 100644 --- 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 index a64aabc4f8306..f756ed1949a13 100644 --- 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-- 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 index 27a9adf35f8a3..7fdc7ee4196e2 100644 --- 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> implements RemoteWritableEntityStore { - 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 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 index ecadb3159bb9f..11e53e57bc9f0 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteRoutingTableBlobStore.java +++ b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteRoutingTableBlobStore.java @@ -21,6 +21,7 @@ import org.opensearch.repositories.blobstore.BlobStoreRepository; import org.opensearch.threadpool.ThreadPool; +import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CLUSTER_STATE_PATH_TOKEN; import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE; /** @@ -77,10 +78,8 @@ public RemoteRoutingTableBlobStore( @Override public BlobPath getBlobPathForUpload(final AbstractRemoteWritableBlobEntity 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 index cc1b19fa45195..b656386c360fe 100644 --- 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/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java index e287fb9799d1d..da057dfac4c5c 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/remote/RemoteRoutingTableServiceTests.java +++ b/server/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,7 +579,7 @@ 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); @@ -585,7 +587,7 @@ public void testGetAsyncIndexRoutingWriteAction() throws Exception { 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 index 3683c93cc99e4..1ea8e3beb0c64 100644 --- 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/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableTests.java b/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableTests.java index dee32c64e1327..29d4ffa978851 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableTests.java +++ b/server/src/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) {