From 011dead2ae10096432c0725cb054ab26d6ab2b73 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Tue, 2 Jul 2024 17:20:55 +0530 Subject: [PATCH 01/18] Implement write and read flow to upload/download shard diff file. Signed-off-by: Shailendra Singh --- .../InternalRemoteRoutingTableService.java | 66 +++- .../remote/NoopRemoteRoutingTableService.java | 28 +- .../remote/RemoteRoutingTableService.java | 48 ++- .../AbstractRemoteWritableBlobEntity.java | 4 + .../remote/ClusterStateDiffManifest.java | 52 ++- .../remote/RemoteClusterStateService.java | 102 +++++- .../remote/RemoteClusterStateUtils.java | 1 + .../RemoteIndexRoutingTableDiff.java | 258 ++++++++++++++ .../RemoteIndexShardRoutingTableDiff.java | 89 +++++ .../RemoteRoutingTableServiceTests.java | 69 ---- .../remote/ClusterMetadataManifestTests.java | 336 +----------------- .../RemoteClusterStateServiceTests.java | 207 +---------- .../model/ClusterStateDiffManifestTests.java | 2 +- 13 files changed, 627 insertions(+), 635 deletions(-) create mode 100644 server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java create mode 100644 server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java index f3f245ee9f8f0..07d1036aaf116 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java @@ -12,6 +12,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.Diff; import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; @@ -26,8 +27,10 @@ import org.opensearch.core.compress.Compressor; import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.gateway.remote.RemoteStateTransferException; +import org.opensearch.gateway.remote.model.RemoteClusterStateBlobStore; import org.opensearch.gateway.remote.model.RemoteRoutingTableBlobStore; import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable; +import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; @@ -53,12 +56,13 @@ * @opensearch.internal */ public class InternalRemoteRoutingTableService extends AbstractLifecycleComponent implements RemoteRoutingTableService { - + private static final Logger logger = LogManager.getLogger(InternalRemoteRoutingTableService.class); private final Settings settings; private final Supplier repositoriesService; private Compressor compressor; private RemoteWritableEntityStore remoteIndexRoutingTableStore; + private RemoteWritableEntityStore remoteIndexRoutingTableDiffStore; private final ClusterSettings clusterSettings; private BlobStoreRepository blobStoreRepository; private final ThreadPool threadPool; @@ -85,9 +89,10 @@ public List getIndicesRouting(RoutingTable routingTable) { /** * Returns diff between the two routing tables, which includes upserts and deletes. + * * @param before previous routing table - * @param after current routing table - * @return diff of the previous and current routing table + * @param after current routing table + * @return incremental diff of the previous and current routing table */ public DiffableUtils.MapDiff> getIndicesRoutingMapDiff( RoutingTable before, @@ -97,7 +102,7 @@ public DiffableUtils.MapDiff getAsyncIndexRoutingWriteAction( return () -> remoteIndexRoutingTableStore.writeAsync(remoteIndexRoutingTable, completionListener); } + public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( + String clusterUUID, + long term, + long version, + Map> indexRoutingTableDiff, + LatchedActionListener latchedActionListener + ) { + + RemoteIndexRoutingTableDiff remoteIndexRoutingTableDiff = new RemoteIndexRoutingTableDiff( + indexRoutingTableDiff, + clusterUUID, + compressor, + term, + version + ); + + ActionListener completionListener = ActionListener.wrap( + resp -> latchedActionListener.onResponse(remoteIndexRoutingTableDiff.getUploadedMetadata()), + ex -> latchedActionListener.onFailure( + new RemoteStateTransferException("Exception in writing index routing diff to remote store", ex) + ) + ); + return () -> remoteIndexRoutingTableDiffStore.writeAsync(remoteIndexRoutingTableDiff, completionListener); + } + /** * Combines IndicesRoutingMetadata from previous manifest and current uploaded indices, removes deleted indices. * @param previousManifest previous manifest, used to get all existing indices routing paths @@ -172,6 +202,26 @@ public CheckedRunnable getAsyncIndexRoutingReadAction( return () -> remoteIndexRoutingTableStore.readAsync(remoteIndexRoutingTable, actionListener); } + // @Override + public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( + String clusterUUID, + String uploadedFilename, + LatchedActionListener>> latchedActionListener + ) { + ActionListener actionListener = ActionListener.wrap( + response -> latchedActionListener.onResponse(response.getDiffs()), + latchedActionListener::onFailure + ); + + RemoteIndexRoutingTableDiff remoteIndexRoutingTableDiff = new RemoteIndexRoutingTableDiff( + uploadedFilename, + clusterUUID, + compressor + ); + + return () -> remoteIndexRoutingTableDiffStore.readAsync(remoteIndexRoutingTableDiff, actionListener); + } + @Override public List getUpdatedIndexRoutingTableMetadata( List updatedIndicesRouting, @@ -213,6 +263,14 @@ protected void doStart() { ThreadPool.Names.REMOTE_STATE_READ, clusterSettings ); + + this.remoteIndexRoutingTableDiffStore = new RemoteClusterStateBlobStore<>( + new BlobStoreTransferService(blobStoreRepository.blobStore(), threadPool), + blobStoreRepository, + clusterName, + threadPool, + ThreadPool.Names.REMOTE_STATE_READ + ); } @Override diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java index 4636e492df28f..b1c639d8deffa 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java @@ -9,6 +9,7 @@ package org.opensearch.cluster.routing.remote; import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.Diff; import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; @@ -35,7 +36,12 @@ public DiffableUtils.MapDiff getAsyncIndexRoutingWriteAction( return () -> {}; } + public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( + String clusterUUID, + long term, + long version, + Map> indexRoutingTableDiff, + LatchedActionListener latchedActionListener + ) { + // noop + return () -> {}; + } + @Override public List getAllUploadedIndicesRouting( ClusterMetadataManifest previousManifest, @@ -70,6 +87,15 @@ public CheckedRunnable getAsyncIndexRoutingReadAction( return () -> {}; } + public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( + String clusterUUID, + String uploadedFilename, + LatchedActionListener>> latchedActionListener + ) { + // noop + return () -> {}; + } + @Override public List getUpdatedIndexRoutingTableMetadata( List updatedIndicesRouting, 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 d319123bc2cee..f6cf2f24ba987 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 @@ -9,16 +9,20 @@ package org.opensearch.cluster.routing.remote; import org.opensearch.action.LatchedActionListener; +import org.opensearch.cluster.Diff; import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.lifecycle.LifecycleComponent; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.gateway.remote.routingtable.RemoteIndexShardRoutingTableDiff; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -28,16 +32,38 @@ * @opensearch.internal */ public interface RemoteRoutingTableService extends LifecycleComponent { - public static final DiffableUtils.NonDiffableValueSerializer CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER = - new DiffableUtils.NonDiffableValueSerializer() { + + public static final DiffableUtils.DiffableValueSerializer CUSTOM_ROUTING_TABLE_DIFFABLE_VALUE_SERIALIZER = + new DiffableUtils.DiffableValueSerializer() { + @Override + public IndexRoutingTable read(StreamInput in, String key) throws IOException { + return IndexRoutingTable.readFrom(in); + } + @Override public void write(IndexRoutingTable value, StreamOutput out) throws IOException { value.writeTo(out); } @Override - public IndexRoutingTable read(StreamInput in, String key) throws IOException { - return IndexRoutingTable.readFrom(in); + public Diff readDiff(StreamInput in, String key) throws IOException { + return IndexRoutingTable.readDiffFrom(in); + } + + @Override + public Diff diff(IndexRoutingTable currentState, IndexRoutingTable previousState) { + List diffs = new ArrayList<>(); + for (Map.Entry entry : currentState.getShards().entrySet()) { + Integer index = entry.getKey(); + IndexShardRoutingTable currentShardRoutingTable = entry.getValue(); + IndexShardRoutingTable previousShardRoutingTable = previousState.shard(index); + if (previousShardRoutingTable == null) { + diffs.add(currentShardRoutingTable); + } else if (!previousShardRoutingTable.equals(currentShardRoutingTable)) { + diffs.add(currentShardRoutingTable); + } + } + return new RemoteIndexShardRoutingTableDiff(diffs); } }; @@ -49,6 +75,12 @@ CheckedRunnable getAsyncIndexRoutingReadAction( LatchedActionListener latchedActionListener ); + CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( + String clusterUUID, + String uploadedFilename, + LatchedActionListener>> latchedActionListener + ); + List getUpdatedIndexRoutingTableMetadata( List updatedIndicesRouting, List allIndicesRouting @@ -67,6 +99,14 @@ CheckedRunnable getAsyncIndexRoutingWriteAction( LatchedActionListener latchedActionListener ); + CheckedRunnable getAsyncIndexRoutingDiffWriteAction( + String clusterUUID, + long term, + long version, + Map> indexRoutingTableDiff, + LatchedActionListener latchedActionListener + ); + List getAllUploadedIndicesRouting( ClusterMetadataManifest previousManifest, List indicesRoutingUploaded, 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 237c077cb673c..bdaa236a0950e 100644 --- a/server/src/main/java/org/opensearch/common/remote/AbstractRemoteWritableBlobEntity.java +++ b/server/src/main/java/org/opensearch/common/remote/AbstractRemoteWritableBlobEntity.java @@ -44,6 +44,10 @@ public AbstractRemoteWritableBlobEntity(final String clusterUUID, final Compress this(clusterUUID, compressor, null); } + public AbstractRemoteWritableBlobEntity() { + this(null, null, null); + } + public abstract BlobPathParameters getBlobPathParameters(); public abstract String getType(); diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java index aca53c92781e4..3ccc9446d0ea4 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java @@ -32,7 +32,6 @@ import static org.opensearch.cluster.DiffableUtils.NonDiffableValueSerializer.getAbstractInstance; import static org.opensearch.cluster.DiffableUtils.getStringKeySerializer; -import static org.opensearch.cluster.routing.remote.RemoteRoutingTableService.CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; /** @@ -53,6 +52,7 @@ public class ClusterStateDiffManifest implements ToXContentFragment, Writeable { private static final String METADATA_CUSTOM_DIFF_FIELD = "metadata_custom_diff"; private static final String UPSERTS_FIELD = "upserts"; private static final String DELETES_FIELD = "deletes"; + private static final String DIFF_FIELD = "diff"; private static final String CLUSTER_BLOCKS_UPDATED_FIELD = "cluster_blocks_diff"; private static final String DISCOVERY_NODES_UPDATED_FIELD = "discovery_nodes_diff"; private static final String ROUTING_TABLE_DIFF = "routing_table_diff"; @@ -72,11 +72,16 @@ public class ClusterStateDiffManifest implements ToXContentFragment, Writeable { private final boolean discoveryNodesUpdated; private final List indicesRoutingUpdated; private final List indicesRoutingDeleted; + private String indicesRoutingDiffPath; private final boolean hashesOfConsistentSettingsUpdated; private final List clusterStateCustomUpdated; private final List clusterStateCustomDeleted; - public ClusterStateDiffManifest(ClusterState state, ClusterState previousState) { + public ClusterStateDiffManifest( + ClusterState state, + ClusterState previousState, + DiffableUtils.MapDiff> routingTableIncrementalDiff + ) { fromStateUUID = previousState.stateUUID(); toStateUUID = state.stateUUID(); coordinationMetadataUpdated = !Metadata.isCoordinationMetadataEqual(state.metadata(), previousState.metadata()); @@ -103,17 +108,12 @@ public ClusterStateDiffManifest(ClusterState state, ClusterState previousState) customMetadataUpdated.addAll(customDiff.getUpserts().keySet()); customMetadataDeleted = customDiff.getDeletes(); - DiffableUtils.MapDiff> routingTableDiff = DiffableUtils.diff( - previousState.getRoutingTable().getIndicesRouting(), - state.getRoutingTable().getIndicesRouting(), - DiffableUtils.getStringKeySerializer(), - CUSTOM_ROUTING_TABLE_VALUE_SERIALIZER - ); - indicesRoutingUpdated = new ArrayList<>(); - routingTableDiff.getUpserts().forEach((k, v) -> indicesRoutingUpdated.add(k)); - - indicesRoutingDeleted = routingTableDiff.getDeletes(); + indicesRoutingDeleted = new ArrayList<>(); + if (routingTableIncrementalDiff != null) { + routingTableIncrementalDiff.getUpserts().forEach((k, v) -> indicesRoutingUpdated.add(k)); + indicesRoutingDeleted.addAll(routingTableIncrementalDiff.getDeletes()); + } hashesOfConsistentSettingsUpdated = !state.metadata() .hashesOfConsistentSettings() .equals(previousState.metadata().hashesOfConsistentSettings()); @@ -143,6 +143,7 @@ public ClusterStateDiffManifest( boolean discoveryNodesUpdated, List indicesRoutingUpdated, List indicesRoutingDeleted, + String indicesRoutingDiffPath, boolean hashesOfConsistentSettingsUpdated, List clusterStateCustomUpdated, List clusterStateCustomDeleted @@ -159,8 +160,9 @@ public ClusterStateDiffManifest( this.indicesDeleted = Collections.unmodifiableList(indicesDeleted); this.clusterBlocksUpdated = clusterBlocksUpdated; this.discoveryNodesUpdated = discoveryNodesUpdated; - this.indicesRoutingUpdated = Collections.unmodifiableList(indicesRoutingUpdated); - this.indicesRoutingDeleted = Collections.unmodifiableList(indicesRoutingDeleted); + this.indicesRoutingUpdated = indicesRoutingUpdated; + this.indicesRoutingDeleted = indicesRoutingDeleted; + this.indicesRoutingDiffPath = indicesRoutingDiffPath; this.hashesOfConsistentSettingsUpdated = hashesOfConsistentSettingsUpdated; this.clusterStateCustomUpdated = Collections.unmodifiableList(clusterStateCustomUpdated); this.clusterStateCustomDeleted = Collections.unmodifiableList(clusterStateCustomDeleted); @@ -181,6 +183,7 @@ public ClusterStateDiffManifest(StreamInput in) throws IOException { this.discoveryNodesUpdated = in.readBoolean(); this.indicesRoutingUpdated = in.readStringList(); this.indicesRoutingDeleted = in.readStringList(); + this.indicesRoutingDiffPath = in.readString(); this.hashesOfConsistentSettingsUpdated = in.readBoolean(); this.clusterStateCustomUpdated = in.readStringList(); this.clusterStateCustomDeleted = in.readStringList(); @@ -237,6 +240,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.value(index); } builder.endArray(); + if (indicesRoutingDiffPath != null) { + builder.field(DIFF_FIELD, indicesRoutingDiffPath); + } builder.endObject(); builder.startObject(CLUSTER_STATE_CUSTOM_DIFF_FIELD); builder.startArray(UPSERTS_FIELD); @@ -341,6 +347,9 @@ public static ClusterStateDiffManifest fromXContent(XContentParser parser) throw case DELETES_FIELD: builder.indicesRoutingDeleted(convertListToString(parser.listOrderedMap())); break; + case DIFF_FIELD: + builder.indicesRoutingDiffPath(parser.textOrNull()); + break; default: throw new XContentParseException("Unexpected field [" + currentFieldName + "]"); } @@ -456,6 +465,10 @@ public List getIndicesRoutingUpdated() { return indicesRoutingUpdated; } + public String getIndicesRoutingDiffPath() { + return indicesRoutingDiffPath; + } + public List getIndicesRoutingDeleted() { return indicesRoutingDeleted; } @@ -468,6 +481,10 @@ public List getClusterStateCustomDeleted() { return clusterStateCustomDeleted; } + public void setIndicesRoutingDiffPath(String indicesRoutingDiffPath) { + this.indicesRoutingDiffPath = indicesRoutingDiffPath; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -560,6 +577,7 @@ public static class Builder { private boolean discoveryNodesUpdated; private List indicesRoutingUpdated; private List indicesRoutingDeleted; + private String indicesRoutingDiff; private boolean hashesOfConsistentSettingsUpdated; private List clusterStateCustomUpdated; private List clusterStateCustomDeleted; @@ -650,6 +668,11 @@ public Builder indicesRoutingDeleted(List indicesRoutingDeleted) { return this; } + public Builder indicesRoutingDiffPath(String indicesRoutingDiffPath) { + this.indicesRoutingDiff = indicesRoutingDiffPath; + return this; + } + public Builder clusterStateCustomUpdated(List clusterStateCustomUpdated) { this.clusterStateCustomUpdated = clusterStateCustomUpdated; return this; @@ -676,6 +699,7 @@ public ClusterStateDiffManifest build() { discoveryNodesUpdated, indicesRoutingUpdated, indicesRoutingDeleted, + indicesRoutingDiff, hashesOfConsistentSettingsUpdated, clusterStateCustomUpdated, clusterStateCustomDeleted 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 7e7a93e1d42ec..a46067269a68e 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -14,6 +14,7 @@ import org.opensearch.action.LatchedActionListener; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.Diff; import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.block.ClusterBlocks; import org.opensearch.cluster.coordination.CoordinationMetadata; @@ -57,6 +58,7 @@ import org.opensearch.gateway.remote.model.RemoteReadResult; import org.opensearch.gateway.remote.model.RemoteTemplatesMetadata; import org.opensearch.gateway.remote.model.RemoteTransientSettingsMetadata; +import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; @@ -235,13 +237,20 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat isPublicationEnabled, isPublicationEnabled ? clusterState.customs() : Collections.emptyMap(), isPublicationEnabled, - remoteRoutingTableService.getIndicesRouting(clusterState.getRoutingTable()) + remoteRoutingTableService.getIndicesRouting(clusterState.getRoutingTable()), + null ); + ClusterStateDiffManifest clusterStateDiffManifest = new ClusterStateDiffManifest(clusterState, ClusterState.EMPTY_STATE, null); + if (uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null) { + clusterStateDiffManifest.setIndicesRoutingDiffPath( + uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() + ); + } final RemoteClusterStateManifestInfo manifestDetails = remoteManifestManager.uploadManifest( clusterState, uploadedMetadataResults, previousClusterUUID, - new ClusterStateDiffManifest(clusterState, ClusterState.EMPTY_STATE), + clusterStateDiffManifest, false ); @@ -331,10 +340,13 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( indicesToBeDeletedFromRemote.remove(indexMetadata.getIndex().getName()); } - final DiffableUtils.MapDiff> routingTableDiff = remoteRoutingTableService - .getIndicesRoutingMapDiff(previousClusterState.getRoutingTable(), clusterState.getRoutingTable()); final List indicesRoutingToUpload = new ArrayList<>(); - routingTableDiff.getUpserts().forEach((k, v) -> indicesRoutingToUpload.add(v)); + final DiffableUtils.MapDiff> routingTableIncrementalDiff = + remoteRoutingTableService.getIndicesRoutingMapDiff(previousClusterState.getRoutingTable(), clusterState.getRoutingTable()); + + Map> indexRoutingTableDiffs = routingTableIncrementalDiff.getDiffs(); + routingTableIncrementalDiff.getDiffs().forEach((k, v) -> indicesRoutingToUpload.add(clusterState.getRoutingTable().index(k))); + routingTableIncrementalDiff.getUpserts().forEach((k, v) -> indicesRoutingToUpload.add(v)); UploadedMetadataResults uploadedMetadataResults; // For migration case from codec V0 or V1 to V2, we have added null check on metadata attribute files, @@ -370,7 +382,8 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( updateTransientSettingsMetadata, clusterStateCustomsDiff.getUpserts(), updateHashesOfConsistentSettings, - indicesRoutingToUpload + indicesRoutingToUpload, + indexRoutingTableDiffs ); // update the map if the metadata was uploaded @@ -412,14 +425,24 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( uploadedMetadataResults.uploadedIndicesRoutingMetadata = remoteRoutingTableService.getAllUploadedIndicesRouting( previousManifest, uploadedMetadataResults.uploadedIndicesRoutingMetadata, - routingTableDiff.getDeletes() + routingTableIncrementalDiff.getDeletes() ); + ClusterStateDiffManifest clusterStateDiffManifest = new ClusterStateDiffManifest( + clusterState, + previousClusterState, + routingTableIncrementalDiff + ); + if (uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null) { + clusterStateDiffManifest.setIndicesRoutingDiffPath( + uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() + ); + } final RemoteClusterStateManifestInfo manifestDetails = remoteManifestManager.uploadManifest( clusterState, uploadedMetadataResults, previousManifest.getPreviousClusterUUID(), - new ClusterStateDiffManifest(clusterState, previousClusterState), + clusterStateDiffManifest, false ); @@ -489,13 +512,15 @@ UploadedMetadataResults writeMetadataInParallel( boolean uploadTransientSettingMetadata, Map clusterStateCustomToUpload, boolean uploadHashesOfConsistentSettings, - List indicesRoutingToUpload + List indicesRoutingToUpload, + Map> indexRoutingTableDiff ) throws IOException { assert Objects.nonNull(indexMetadataUploadListeners) : "indexMetadataUploadListeners can not be null"; int totalUploadTasks = indexToUpload.size() + indexMetadataUploadListeners.size() + customToUpload.size() + (uploadCoordinationMetadata ? 1 : 0) + (uploadSettingsMetadata ? 1 : 0) + (uploadTemplateMetadata ? 1 : 0) + (uploadDiscoveryNodes ? 1 : 0) + (uploadClusterBlock ? 1 : 0) + (uploadTransientSettingMetadata ? 1 : 0) - + clusterStateCustomToUpload.size() + (uploadHashesOfConsistentSettings ? 1 : 0) + indicesRoutingToUpload.size(); + + clusterStateCustomToUpload.size() + (uploadHashesOfConsistentSettings ? 1 : 0) + indicesRoutingToUpload.size() + + (indexRoutingTableDiff != null && !indexRoutingTableDiff.isEmpty() ? 1 : 0); CountDownLatch latch = new CountDownLatch(totalUploadTasks); Map> uploadTasks = new ConcurrentHashMap<>(totalUploadTasks); Map results = new ConcurrentHashMap<>(totalUploadTasks); @@ -672,6 +697,18 @@ UploadedMetadataResults writeMetadataInParallel( ) ); }); + if (indexRoutingTableDiff != null && !indexRoutingTableDiff.isEmpty()) { + uploadTasks.put( + RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE, + remoteRoutingTableService.getAsyncIndexRoutingDiffWriteAction( + clusterState.metadata().clusterUUID(), + clusterState.term(), + clusterState.version(), + indexRoutingTableDiff, + listener + ) + ); + } // start async upload of all required metadata files for (CheckedRunnable uploadTask : uploadTasks.values()) { @@ -721,6 +758,8 @@ UploadedMetadataResults writeMetadataInParallel( if (uploadedMetadata.getClass().equals(UploadedIndexMetadata.class) && uploadedMetadata.getComponent().contains(INDEX_ROUTING_METADATA_PREFIX)) { response.uploadedIndicesRoutingMetadata.add((UploadedIndexMetadata) uploadedMetadata); + } else if (RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE.equals(name)) { + response.uploadedIndicesRoutingDiffMetadata = (UploadedMetadataAttribute) uploadedMetadata; } else if (name.startsWith(CUSTOM_METADATA)) { // component name for custom metadata will look like custom-- String custom = name.split(DELIMITER)[0].split(CUSTOM_DELIMITER)[1]; @@ -990,17 +1029,19 @@ ClusterState readClusterStateInParallel( List indicesRoutingToRead, boolean readHashesOfConsistentSettings, Map clusterStateCustomToRead, + boolean readIndexRoutingTableDiff, boolean includeEphemeral ) throws IOException { int totalReadTasks = indicesToRead.size() + customToRead.size() + (readCoordinationMetadata ? 1 : 0) + (readSettingsMetadata ? 1 : 0) + (readTemplatesMetadata ? 1 : 0) + (readDiscoveryNodes ? 1 : 0) + (readClusterBlocks ? 1 : 0) + (readTransientSettingsMetadata ? 1 : 0) + (readHashesOfConsistentSettings ? 1 : 0) + clusterStateCustomToRead.size() - + indicesRoutingToRead.size(); + + indicesRoutingToRead.size() + (readIndexRoutingTableDiff ? 1 : 0); CountDownLatch latch = new CountDownLatch(totalReadTasks); List> asyncMetadataReadActions = new ArrayList<>(); List readResults = Collections.synchronizedList(new ArrayList<>()); List readIndexRoutingTableResults = Collections.synchronizedList(new ArrayList<>()); + AtomicReference readIndexRoutingTableDiffResults = new AtomicReference<>(); List exceptionList = Collections.synchronizedList(new ArrayList<>(totalReadTasks)); LatchedActionListener listener = new LatchedActionListener<>(ActionListener.wrap(response -> { @@ -1038,6 +1079,29 @@ ClusterState readClusterStateInParallel( ); } + LatchedActionListener>> routingTableDiffLatchedActionListener = new LatchedActionListener<>( + ActionListener.wrap(response -> { + logger.debug("Successfully read cluster state diff component from remote"); + readIndexRoutingTableDiffResults.set( + new RemoteIndexRoutingTableDiff(response, clusterUUID, blobStoreRepository.getCompressor()) + ); + }, ex -> { + logger.error("Failed to read cluster state diff from remote", ex); + exceptionList.add(ex); + }), + latch + ); + + if (readIndexRoutingTableDiff) { + asyncMetadataReadActions.add( + remoteRoutingTableService.getAsyncIndexRoutingTableDiffReadAction( + clusterUUID, + manifest.getDiffManifest().getIndicesRoutingDiffPath(), + routingTableDiffLatchedActionListener + ) + ); + } + for (Map.Entry entry : customToRead.entrySet()) { asyncMetadataReadActions.add( remoteGlobalMetadataManager.getAsyncMetadataReadAction( @@ -1262,6 +1326,14 @@ ClusterState readClusterStateInParallel( readIndexRoutingTableResults.forEach( indexRoutingTable -> indicesRouting.put(indexRoutingTable.getIndex().getName(), indexRoutingTable) ); + RemoteIndexRoutingTableDiff routingTableDiff = readIndexRoutingTableDiffResults.get(); + if (routingTableDiff != null) { + routingTableDiff.getDiffs().forEach((key, diff) -> { + IndexRoutingTable previousIndexRoutingTable = indicesRouting.get(key); + IndexRoutingTable updatedTable = diff.apply(previousIndexRoutingTable); + indicesRouting.put(key, updatedTable); + }); + } clusterStateBuilder.routingTable(new RoutingTable(manifest.getRoutingTableVersion(), indicesRouting)); return clusterStateBuilder.build(); @@ -1291,6 +1363,10 @@ public ClusterState getClusterStateForManifest( includeEphemeral && manifest.getHashesOfConsistentSettings() != null, includeEphemeral ? manifest.getClusterStateCustomMap() : emptyMap(), includeEphemeral + && manifest.getDiffManifest() != null + && manifest.getDiffManifest().getIndicesRoutingDiffPath() != null + && !manifest.getDiffManifest().getIndicesRoutingDiffPath().isEmpty(), + includeEphemeral ); } else { ClusterState clusterState = readClusterStateInParallel( @@ -1310,6 +1386,7 @@ public ClusterState getClusterStateForManifest( emptyList(), false, emptyMap(), + false, false ); Metadata.Builder mb = Metadata.builder(remoteGlobalMetadataManager.getGlobalMetadata(manifest.getClusterUUID(), manifest)); @@ -1366,6 +1443,9 @@ public ClusterState getClusterStateUsingDiff(ClusterMetadataManifest manifest, C updatedIndexRouting, diff.isHashesOfConsistentSettingsUpdated(), updatedClusterStateCustom, + manifest.getDiffManifest() != null + && manifest.getDiffManifest().getIndicesRoutingDiffPath() != null + && !manifest.getDiffManifest().getIndicesRoutingDiffPath().isEmpty(), true ); ClusterState.Builder clusterStateBuilder = ClusterState.builder(updatedClusterState); diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateUtils.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateUtils.java index f2b93c3784407..74cb838286961 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateUtils.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateUtils.java @@ -88,6 +88,7 @@ public static class UploadedMetadataResults { ClusterMetadataManifest.UploadedMetadataAttribute uploadedClusterBlocks; List uploadedIndicesRoutingMetadata; ClusterMetadataManifest.UploadedMetadataAttribute uploadedHashesOfConsistentSettings; + ClusterMetadataManifest.UploadedMetadataAttribute uploadedIndicesRoutingDiffMetadata; public UploadedMetadataResults( List uploadedIndexMetadata, diff --git a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java new file mode 100644 index 0000000000000..ad92f099343be --- /dev/null +++ b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java @@ -0,0 +1,258 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.gateway.remote.routingtable; + +import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.store.InputStreamDataInput; +import org.apache.lucene.store.OutputStreamDataOutput; +import org.opensearch.cluster.Diff; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.IndexShardRoutingTable; +import org.opensearch.common.io.Streams; +import org.opensearch.common.remote.AbstractRemoteWritableBlobEntity; +import org.opensearch.common.remote.BlobPathParameters; +import org.opensearch.core.common.io.stream.BufferedChecksumStreamInput; +import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput; +import org.opensearch.core.common.io.stream.InputStreamStreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; +import org.opensearch.core.compress.Compressor; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.index.remote.RemoteStoreUtils; +import org.opensearch.repositories.blobstore.ChecksumWritableBlobStoreFormat; + +import java.io.EOFException; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.opensearch.gateway.remote.RemoteClusterStateUtils.DELIMITER; + +/** + * Represents a difference between {@link IndexRoutingTable} objects that can be serialized and deserialized. + * This class is responsible for writing and reading the differences between IndexRoutingTables to and from an input/output stream. + */ +public class RemoteIndexRoutingTableDiff extends AbstractRemoteWritableBlobEntity + implements + Diff, + Writeable { + + private final Map> diffs; + + private long term; + private long version; + + public static final String INDEX_ROUTING_TABLE_DIFF = "index-routing-diff"; + + public static final String INDEX_ROUTING_DIFF_METADATA_PREFIX = "indexRoutingDiff--"; + + public static final String INDEX_ROUTING_DIFF_FILE = "index_routing_diff"; + private static final String codec = "RemoteIndexRoutingTableDiff"; + public static final String INDEX_ROUTING_DIFF_PATH_TOKEN = "index-routing-diff"; + + public static final int VERSION = 1; + + public static final ChecksumWritableBlobStoreFormat RemoteIndexRoutingTableDiffFormat = + new ChecksumWritableBlobStoreFormat<>(codec, RemoteIndexRoutingTableDiff::new); + + /** + * Constructs a new RemoteIndexRoutingTableDiff with the given differences. + * + * @param diffs a map containing the differences of {@link IndexRoutingTable}. + * @param clusterUUID the cluster UUID. + * @param compressor the compressor to be used. + * @param term the term of the routing table. + * @param version the version of the routing table. + */ + public RemoteIndexRoutingTableDiff( + Map> diffs, + String clusterUUID, + Compressor compressor, + long term, + long version + ) { + super(clusterUUID, compressor); + this.diffs = diffs; + this.term = term; + this.version = version; + } + + /** + * Constructs a new RemoteIndexRoutingTableDiff with the given differences. + * + * @param diffs a map containing the differences of {@link IndexRoutingTable}. + * @param clusterUUID the cluster UUID. + * @param compressor the compressor to be used. + */ + public RemoteIndexRoutingTableDiff(Map> diffs, String clusterUUID, Compressor compressor) { + super(clusterUUID, compressor); + this.diffs = diffs; + } + + /** + * Constructs a new RemoteIndexRoutingTableDiff with the given blob name, cluster UUID, and compressor. + * + * @param blobName the name of the blob. + * @param clusterUUID the cluster UUID. + * @param compressor the compressor to be used. + */ + public RemoteIndexRoutingTableDiff(String blobName, String clusterUUID, Compressor compressor) { + super(clusterUUID, compressor); + this.diffs = null; + this.blobName = blobName; + } + + /** + * Reads data from inputStream and creates RemoteIndexRoutingTableDiff object with the {@link Diff} + * + * @param inputStream input stream with diff data + * @throws IOException exception thrown on failing to read from stream. + */ + public RemoteIndexRoutingTableDiff(InputStream inputStream) throws IOException { + super(); + try (BufferedChecksumStreamInput in = new BufferedChecksumStreamInput(new InputStreamStreamInput(inputStream), "assertion")) { + CodecUtil.checkHeader(new InputStreamDataInput(inputStream), codec, VERSION, VERSION); + int size = in.readVInt(); + Map> diffs = new HashMap<>(); + + for (int i = 0; i < size; i++) { + String key = in.readString(); + List shardRoutingTables = new ArrayList<>(); + + // Read each IndexShardRoutingTable from the stream + int numShards = in.readVInt(); + for (int j = 0; j < numShards; j++) { + IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); + shardRoutingTables.add(shardRoutingTable); + } + + // Create a diff object for the index + Diff diff = new RemoteIndexShardRoutingTableDiff(shardRoutingTables); + + // Put the diff into the map with the key + diffs.put(key, diff); + } + verifyCheckSum(in); + this.diffs = diffs; + } catch (EOFException e) { + throw new IOException("Diffs data is corrupted", e); + } + } + + /** + * Writes {@link Diff} to the given stream + * + * @param streamOutput output stream to write + * @throws IOException exception thrown on failing to write to stream. + */ + @Override + public void writeTo(StreamOutput streamOutput) throws IOException { + try (BufferedChecksumStreamOutput out = new BufferedChecksumStreamOutput(streamOutput)) { + // Write codec header + CodecUtil.writeHeader(new OutputStreamDataOutput(streamOutput), codec, VERSION); + + out.writeVInt(diffs.size()); + for (Map.Entry> entry : diffs.entrySet()) { + out.writeString(entry.getKey()); + entry.getValue().writeTo(out); + } + + out.writeLong(out.getChecksum()); + out.flush(); + } catch (IOException e) { + throw new IOException("Failed to write Diff to stream", e); + } + } + + /** + * Applies the differences to the provided {@link IndexRoutingTable}. + * + * @param part the initial {@link IndexRoutingTable}. + * @return the modified {@link IndexRoutingTable} with applied differences. + */ + @Override + public IndexRoutingTable apply(IndexRoutingTable part) { + // Apply diffs to the provided IndexRoutingTable + for (Map.Entry> entry : diffs.entrySet()) { + part = entry.getValue().apply(part); + } + return part; + } + + /** + * Verifies the checksum of the data in the given input stream. + * + * @param in the input stream containing the data. + * @throws IOException if the checksum verification fails or if an I/O exception occurs. + */ + private void verifyCheckSum(BufferedChecksumStreamInput in) throws IOException { + long expectedChecksum = in.getChecksum(); + long readChecksum = in.readLong(); + if (readChecksum != expectedChecksum) { + throw new IOException( + "checksum verification failed - expected: 0x" + + Long.toHexString(expectedChecksum) + + ", got: 0x" + + Long.toHexString(readChecksum) + ); + } + } + + /** + * Gets the map of differences of {@link IndexRoutingTable}. + * + * @return a map containing the differences. + */ + public Map> getDiffs() { + return diffs; + } + + @Override + public BlobPathParameters getBlobPathParameters() { + return new BlobPathParameters(List.of(INDEX_ROUTING_DIFF_PATH_TOKEN), INDEX_ROUTING_DIFF_METADATA_PREFIX); + } + + @Override + public String getType() { + return INDEX_ROUTING_TABLE_DIFF; + } + + @Override + public String generateBlobFileName() { + if (blobFileName == null) { + blobFileName = String.join( + DELIMITER, + getBlobPathParameters().getFilePrefix(), + RemoteStoreUtils.invertLong(term), + RemoteStoreUtils.invertLong(version), + RemoteStoreUtils.invertLong(System.currentTimeMillis()) + ); + } + return blobFileName; + } + + @Override + public ClusterMetadataManifest.UploadedMetadata getUploadedMetadata() { + assert blobName != null; + return new ClusterMetadataManifest.UploadedMetadataAttribute(INDEX_ROUTING_DIFF_FILE, blobName); + } + + @Override + public InputStream serialize() throws IOException { + return RemoteIndexRoutingTableDiffFormat.serialize(this, generateBlobFileName(), getCompressor()).streamInput(); + } + + @Override + public RemoteIndexRoutingTableDiff deserialize(InputStream in) throws IOException { + return RemoteIndexRoutingTableDiffFormat.deserialize(blobName, Streams.readFully(in)); + } +} diff --git a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java new file mode 100644 index 0000000000000..eeea4d7d3291f --- /dev/null +++ b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java @@ -0,0 +1,89 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.gateway.remote.routingtable; + +import org.opensearch.cluster.Diff; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.IndexShardRoutingTable; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +/** + * Represents the differences between {@link IndexRoutingTable}s for uploading and downloading index routing table diff data to/from a remote store. + * + * @opensearch.internal + */ +public class RemoteIndexShardRoutingTableDiff implements Diff { + + private final List indexShardRoutingTables; + + /** + * Constructs a new RemoteIndexShardRoutingTableDiff with the given shard routing tables. + * + * @param indexShardRoutingTables the list of {@link IndexShardRoutingTable}s representing the differences. + */ + public RemoteIndexShardRoutingTableDiff(List indexShardRoutingTables) { + this.indexShardRoutingTables = indexShardRoutingTables; + } + + /** + * Applies the differences to the provided {@link IndexRoutingTable}. + * + * @param part the original {@link IndexRoutingTable}. + * @return the updated {@link IndexRoutingTable} with the applied differences. + */ + @Override + public IndexRoutingTable apply(IndexRoutingTable part) { + IndexRoutingTable.Builder builder = new IndexRoutingTable.Builder(part.getIndex()); + for (IndexShardRoutingTable shardRoutingTable : part) { + builder.addIndexShard(shardRoutingTable); // Add existing shards to builder + } + + // Apply the diff: update or add the new shard routing tables + for (IndexShardRoutingTable diffShard : indexShardRoutingTables) { + builder.addIndexShard(diffShard); + } + return builder.build(); + } + + /** + * Writes the shard routing tables to the provided {@link StreamOutput}. + * + * @param out the output stream to write to. + * @throws IOException if an I/O exception occurs while writing. + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVInt(indexShardRoutingTables.size()); + for (IndexShardRoutingTable shardRoutingTable : indexShardRoutingTables) { + IndexShardRoutingTable.Builder.writeTo(shardRoutingTable, out); + } + } + + /** + * Reads the shard routing tables from the provided {@link StreamInput}. + * + * @param in the input stream to read from. + * @return a new {@link RemoteIndexShardRoutingTableDiff} with the read shard routing tables. + * @throws IOException if an I/O exception occurs while reading. + */ + public static RemoteIndexShardRoutingTableDiff readFrom(StreamInput in) throws IOException { + int size = in.readVInt(); + List indexShardRoutingTables = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); + indexShardRoutingTables.add(shardRoutingTable); + } + return new RemoteIndexShardRoutingTableDiff(indexShardRoutingTables); + } +} 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 564c7f7aed304..952d4e63d2d8d 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 @@ -258,75 +258,6 @@ public void testGetIndicesRoutingMapDiffIndexAdded() { assertEquals(0, diff.getDeletes().size()); } - public void testGetIndicesRoutingMapDiffShardChanged() { - String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); - int noOfShards = between(1, 1000); - int noOfReplicas = randomInt(10); - final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( - Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") - .build() - ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); - - RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); - - final IndexMetadata indexMetadata2 = new IndexMetadata.Builder(indexName).settings( - Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") - .build() - ).numberOfShards(noOfShards + 1).numberOfReplicas(noOfReplicas).build(); - RoutingTable routingTable2 = RoutingTable.builder().addAsNew(indexMetadata2).build(); - - DiffableUtils.MapDiff> diff = remoteRoutingTableService - .getIndicesRoutingMapDiff(routingTable, routingTable2); - assertEquals(1, diff.getUpserts().size()); - assertNotNull(diff.getUpserts().get(indexName)); - assertEquals(noOfShards + 1, diff.getUpserts().get(indexName).getShards().size()); - assertEquals(noOfReplicas + 1, diff.getUpserts().get(indexName).getShards().get(0).getSize()); - assertEquals(0, diff.getDeletes().size()); - - final IndexMetadata indexMetadata3 = new IndexMetadata.Builder(indexName).settings( - Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") - .build() - ).numberOfShards(noOfShards + 1).numberOfReplicas(noOfReplicas + 1).build(); - RoutingTable routingTable3 = RoutingTable.builder().addAsNew(indexMetadata3).build(); - - diff = remoteRoutingTableService.getIndicesRoutingMapDiff(routingTable2, routingTable3); - assertEquals(1, diff.getUpserts().size()); - assertNotNull(diff.getUpserts().get(indexName)); - assertEquals(noOfShards + 1, diff.getUpserts().get(indexName).getShards().size()); - assertEquals(noOfReplicas + 2, diff.getUpserts().get(indexName).getShards().get(0).getSize()); - - assertEquals(0, diff.getDeletes().size()); - } - - public void testGetIndicesRoutingMapDiffShardDetailChanged() { - String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); - int noOfShards = between(1, 1000); - int noOfReplicas = randomInt(10); - final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( - Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) - .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") - .build() - ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); - - RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); - RoutingTable routingTable2 = RoutingTable.builder().addAsRecovery(indexMetadata).build(); - - DiffableUtils.MapDiff> diff = remoteRoutingTableService - .getIndicesRoutingMapDiff(routingTable, routingTable2); - assertEquals(1, diff.getUpserts().size()); - assertNotNull(diff.getUpserts().get(indexName)); - assertEquals(noOfShards, diff.getUpserts().get(indexName).getShards().size()); - assertEquals(noOfReplicas + 1, diff.getUpserts().get(indexName).getShards().get(0).getSize()); - assertEquals(0, diff.getDeletes().size()); - } - public void testGetIndicesRoutingMapDiffIndexDeleted() { String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index 256161af1a3e2..e1abe083b06e7 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -34,15 +34,11 @@ import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V0; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V1; -import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_BLOCKS; -import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.DISCOVERY_NODES; import static org.opensearch.gateway.remote.model.RemoteCoordinationMetadata.COORDINATION_METADATA; import static org.opensearch.gateway.remote.model.RemoteCustomMetadata.CUSTOM_DELIMITER; import static org.opensearch.gateway.remote.model.RemoteCustomMetadata.CUSTOM_METADATA; -import static org.opensearch.gateway.remote.model.RemoteHashesOfConsistentSettings.HASHES_OF_CONSISTENT_SETTINGS; 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; public class ClusterMetadataManifestTests extends OpenSearchTestCase { @@ -148,335 +144,6 @@ public void testClusterMetadataManifestXContent() throws IOException { } } - public void testClusterMetadataManifestSerializationEqualsHashCode() { - ClusterMetadataManifest initialManifest = ClusterMetadataManifest.builder() - .clusterTerm(1337L) - .stateVersion(7L) - .clusterUUID("HrYF3kP5SmSPWtKlWhnNSA") - .stateUUID("6By9p9G0Rv2MmFYJcPAOgA") - .opensearchVersion(Version.CURRENT) - .nodeId("B10RX1f5RJenMQvYccCgSQ") - .committed(true) - .codecVersion(ClusterMetadataManifest.CODEC_V2) - .indices(randomUploadedIndexMetadataList()) - .previousClusterUUID("yfObdx8KSMKKrXf8UyHhM") - .clusterUUIDCommitted(true) - .coordinationMetadata(new UploadedMetadataAttribute(COORDINATION_METADATA, "coordination-file")) - .settingMetadata(new UploadedMetadataAttribute(SETTING_METADATA, "setting-file")) - .templatesMetadata(new UploadedMetadataAttribute(TEMPLATES_METADATA, "templates-file")) - .customMetadataMap( - Collections.unmodifiableList( - Arrays.asList( - new UploadedMetadataAttribute( - CUSTOM_METADATA + CUSTOM_DELIMITER + RepositoriesMetadata.TYPE, - "custom--repositories-file" - ), - new UploadedMetadataAttribute( - CUSTOM_METADATA + CUSTOM_DELIMITER + IndexGraveyard.TYPE, - "custom--index_graveyard-file" - ), - new UploadedMetadataAttribute( - CUSTOM_METADATA + CUSTOM_DELIMITER + WeightedRoutingMetadata.TYPE, - "custom--weighted_routing_netadata-file" - ) - ) - ).stream().collect(Collectors.toMap(UploadedMetadataAttribute::getAttributeName, Function.identity())) - ) - .routingTableVersion(1L) - .discoveryNodesMetadata(new UploadedMetadataAttribute(DISCOVERY_NODES, "discovery-nodes-file")) - .clusterBlocksMetadata(new UploadedMetadataAttribute(CLUSTER_BLOCKS, "cluster-block-file")) - .transientSettingsMetadata(new UploadedMetadataAttribute(TRANSIENT_SETTING_METADATA, "transient-settings-file")) - .hashesOfConsistentSettings(new UploadedMetadataAttribute(HASHES_OF_CONSISTENT_SETTINGS, "hashes-of-consistent-settings-file")) - .clusterStateCustomMetadataMap(Collections.emptyMap()) - .diffManifest( - new ClusterStateDiffManifest( - RemoteClusterStateServiceTests.generateClusterStateWithOneIndex().build(), - ClusterState.EMPTY_STATE - ) - ) - .build(); - { // Mutate Cluster Term - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.clusterTerm(1338L); - return builder.build(); - } - ); - } - { // Mutate State Version - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.stateVersion(8L); - return builder.build(); - } - ); - } - { // Mutate Cluster UUID - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.clusterUUID("efOkMiPbQZCUQQgtFWdbPw"); - return builder.build(); - } - ); - } - { // Mutate State UUID - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.stateUUID("efOkMiPbQZCUQQgtFWdbPw"); - return builder.build(); - } - ); - } - { // Mutate OpenSearch Version - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.opensearchVersion(Version.V_EMPTY); - return builder.build(); - } - ); - } - { // Mutate Committed State - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.committed(false); - return builder.build(); - } - ); - } - { // Mutate Indices - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.indices(randomUploadedIndexMetadataList()); - return builder.build(); - } - ); - } - { // Mutate Previous cluster UUID - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.previousClusterUUID("vZX62DCQEOzGXlxXCrEu"); - return builder.build(); - } - ); - - } - { // Mutate cluster uuid committed - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.clusterUUIDCommitted(false); - return builder.build(); - } - ); - } - { - // Mutate Coordination metadata - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.coordinationMetadata(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - { - // Mutate setting metadata - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.settingMetadata(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - { - // Mutate template metadata - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.templatesMetadata(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - { - // Mutate custom metadata - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.customMetadataMap(Collections.emptyMap()); - return builder.build(); - } - ); - } - { - // Mutate discovery nodes - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.discoveryNodesMetadata(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - { - // Mutate cluster blocks - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.clusterBlocksMetadata(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - { - // Mutate transient settings metadata - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.transientSettingsMetadata(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - { - // Mutate diff manifest - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.diffManifest(null); - return builder.build(); - } - ); - } - { - // Mutate hashes of consistent settings - EqualsHashCodeTestUtils.checkEqualsAndHashCode( - initialManifest, - orig -> OpenSearchTestCase.copyWriteable( - orig, - new NamedWriteableRegistry(Collections.emptyList()), - ClusterMetadataManifest::new - ), - manifest -> { - ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); - builder.hashesOfConsistentSettings(randomUploadedMetadataAttribute()); - return builder.build(); - } - ); - } - } - public void testClusterMetadataManifestXContentV2() throws IOException { UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "test-uuid", "/test/upload/path"); UploadedMetadataAttribute uploadedMetadataAttribute = new UploadedMetadataAttribute("attribute_name", "testing_attribute"); @@ -523,7 +190,8 @@ public void testClusterMetadataManifestXContentV2() throws IOException { .diffManifest( new ClusterStateDiffManifest( RemoteClusterStateServiceTests.generateClusterStateWithOneIndex().build(), - ClusterState.EMPTY_STATE + ClusterState.EMPTY_STATE, + null ) ) .build(); 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 ebd3488d06007..5f219eaf3d899 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -101,7 +101,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.mockito.ArgumentMatchers; -import org.mockito.Mockito; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -149,16 +148,12 @@ 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; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; -import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -500,50 +495,6 @@ public void run() { ); } - public void testTimeoutWhileWritingManifestFile() throws IOException { - // verify update metadata manifest upload timeout - int metadataManifestUploadTimeout = 2; - Settings newSettings = Settings.builder() - .put("cluster.remote_store.state.metadata_manifest.upload_timeout", metadataManifestUploadTimeout + "s") - .build(); - clusterSettings.applySettings(newSettings); - - final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); - AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); - - ArgumentCaptor> actionListenerArgumentCaptor = ArgumentCaptor.forClass(ActionListener.class); - - doAnswer((i) -> { - // For Manifest file perform No Op, so latch in code will timeout - return null; - }).when(container).asyncBlobUpload(any(WriteContext.class), actionListenerArgumentCaptor.capture()); - - remoteClusterStateService.start(); - RemoteClusterStateService spiedService = spy(remoteClusterStateService); - when( - spiedService.writeMetadataInParallel( - any(), - anyList(), - anyMap(), - anyMap(), - anyBoolean(), - anyBoolean(), - anyBoolean(), - anyBoolean(), - anyBoolean(), - anyBoolean(), - anyMap(), - anyBoolean(), - anyList() - ) - ).thenReturn(new RemoteClusterStateUtils.UploadedMetadataResults()); - RemoteStateTransferException ex = expectThrows( - RemoteStateTransferException.class, - () -> spiedService.writeFullMetadata(clusterState, randomAlphaOfLength(10)) - ); - assertTrue(ex.getMessage().contains("Timed out waiting for transfer of manifest file to complete")); - } - public void testWriteFullMetadataInParallelFailureForIndexMetadata() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); @@ -590,153 +541,6 @@ public void testFailWriteIncrementalMetadataWhenTermChanged() { ); } - public void testWriteIncrementalMetadataSuccess() throws IOException { - final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); - mockBlobStoreObjects(); - final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); - final ClusterState previousClusterState = ClusterState.builder(ClusterName.DEFAULT) - .metadata(Metadata.builder().coordinationMetadata(coordinationMetadata)) - .build(); - - final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); - - remoteClusterStateService.start(); - final RemoteClusterStateService rcssSpy = Mockito.spy(remoteClusterStateService); - final RemoteClusterStateManifestInfo manifestInfo = rcssSpy.writeIncrementalMetadata( - previousClusterState, - clusterState, - previousManifest - ); - final ClusterMetadataManifest manifest = manifestInfo.getClusterMetadataManifest(); - final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); - final List indices = List.of(uploadedIndexMetadata); - - final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() - .indices(indices) - .clusterTerm(1L) - .stateVersion(1L) - .stateUUID("state-uuid") - .clusterUUID("cluster-uuid") - .previousClusterUUID("prev-cluster-uuid") - .build(); - - Mockito.verify(rcssSpy) - .writeMetadataInParallel( - eq(clusterState), - eq(new ArrayList(clusterState.metadata().indices().values())), - eq(Collections.singletonMap(indices.get(0).getIndexName(), null)), - eq(clusterState.metadata().customs()), - eq(true), - eq(true), - eq(true), - eq(false), - eq(false), - eq(false), - eq(Collections.emptyMap()), - eq(false), - eq(Collections.emptyList()) - ); - - assertThat(manifestInfo.getManifestFileName(), notNullValue()); - assertThat(manifest.getIndices().size(), is(1)); - assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); - assertThat(manifest.getIndices().get(0).getIndexUUID(), is(uploadedIndexMetadata.getIndexUUID())); - assertThat(manifest.getIndices().get(0).getUploadedFilename(), notNullValue()); - assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); - assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); - assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); - assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); - assertThat(manifest.getHashesOfConsistentSettings(), nullValue()); - assertThat(manifest.getDiscoveryNodesMetadata(), nullValue()); - assertThat(manifest.getClusterBlocksMetadata(), nullValue()); - assertThat(manifest.getClusterStateCustomMap(), anEmptyMap()); - assertThat(manifest.getTransientSettingsMetadata(), nullValue()); - assertThat(manifest.getTemplatesMetadata(), notNullValue()); - assertThat(manifest.getCoordinationMetadata(), notNullValue()); - assertThat(manifest.getCustomMetadataMap().size(), is(2)); - assertThat(manifest.getIndicesRouting().size(), is(0)); - } - - public void testWriteIncrementalMetadataSuccessWhenPublicationEnabled() throws IOException { - publicationEnabled = true; - Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, publicationEnabled).build(); - FeatureFlags.initializeFeatureFlags(nodeSettings); - remoteClusterStateService = new RemoteClusterStateService( - "test-node-id", - repositoriesServiceSupplier, - settings, - clusterService, - () -> 0L, - threadPool, - List.of(new RemoteIndexPathUploader(threadPool, settings, repositoriesServiceSupplier, clusterSettings)), - writableRegistry() - ); - final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); - mockBlobStoreObjects(); - final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); - final ClusterState previousClusterState = ClusterState.builder(ClusterName.DEFAULT) - .metadata(Metadata.builder().coordinationMetadata(coordinationMetadata)) - .build(); - - final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); - - remoteClusterStateService.start(); - final RemoteClusterStateService rcssSpy = Mockito.spy(remoteClusterStateService); - final RemoteClusterStateManifestInfo manifestInfo = rcssSpy.writeIncrementalMetadata( - previousClusterState, - clusterState, - previousManifest - ); - final ClusterMetadataManifest manifest = manifestInfo.getClusterMetadataManifest(); - final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); - final List indices = List.of(uploadedIndexMetadata); - - final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() - .indices(indices) - .clusterTerm(1L) - .stateVersion(1L) - .stateUUID("state-uuid") - .clusterUUID("cluster-uuid") - .previousClusterUUID("prev-cluster-uuid") - .build(); - - Mockito.verify(rcssSpy) - .writeMetadataInParallel( - eq(clusterState), - eq(new ArrayList(clusterState.metadata().indices().values())), - eq(Collections.singletonMap(indices.get(0).getIndexName(), null)), - eq(clusterState.metadata().customs()), - eq(true), - eq(true), - eq(true), - eq(true), - eq(false), - eq(false), - eq(Collections.emptyMap()), - eq(true), - Mockito.anyList() - ); - - assertThat(manifestInfo.getManifestFileName(), notNullValue()); - assertThat(manifest.getIndices().size(), is(1)); - assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); - assertThat(manifest.getIndices().get(0).getIndexUUID(), is(uploadedIndexMetadata.getIndexUUID())); - assertThat(manifest.getIndices().get(0).getUploadedFilename(), notNullValue()); - assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); - assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); - assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); - assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); - assertThat(manifest.getHashesOfConsistentSettings(), notNullValue()); - assertThat(manifest.getDiscoveryNodesMetadata(), notNullValue()); - assertThat(manifest.getClusterBlocksMetadata(), nullValue()); - assertThat(manifest.getClusterStateCustomMap(), anEmptyMap()); - assertThat(manifest.getTransientSettingsMetadata(), nullValue()); - assertThat(manifest.getTemplatesMetadata(), notNullValue()); - assertThat(manifest.getCoordinationMetadata(), notNullValue()); - assertThat(manifest.getCustomMetadataMap().size(), is(2)); - assertThat(manifest.getIndicesRouting().size(), is(1)); - } - public void testTimeoutWhileWritingMetadata() throws IOException { AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); doNothing().when(container).asyncBlobUpload(any(), any()); @@ -761,7 +565,8 @@ public void testTimeoutWhileWritingMetadata() throws IOException { true, emptyMap(), true, - emptyList() + emptyList(), + null ) ); assertTrue(exception.getMessage().startsWith("Timed out waiting for transfer of following metadata to complete")); @@ -808,6 +613,7 @@ public void testGetClusterStateForManifest_IncludeEphemeral() throws IOException eq(manifest.getIndicesRouting()), eq(true), eq(manifest.getClusterStateCustomMap()), + eq(false), eq(true) ); } @@ -853,7 +659,9 @@ public void testGetClusterStateForManifest_ExcludeEphemeral() throws IOException eq(emptyList()), eq(false), eq(emptyMap()), + eq(false), eq(false) + ); } @@ -899,6 +707,7 @@ public void testGetClusterStateFromManifest_CodecV1() throws IOException { eq(emptyList()), eq(false), eq(emptyMap()), + eq(false), eq(false) ); verify(mockedGlobalMetadataManager, times(1)).getGlobalMetadata(eq(manifest.getClusterUUID()), eq(manifest)); @@ -1222,6 +1031,7 @@ public void testReadClusterStateInParallel_TimedOut() throws IOException { emptyList(), true, emptyMap(), + false, true ) ); @@ -1253,6 +1063,7 @@ public void testReadClusterStateInParallel_ExceptionDuringRead() throws IOExcept emptyList(), true, emptyMap(), + false, true ) ); @@ -1358,6 +1169,7 @@ public void testReadClusterStateInParallel_UnexpectedResult() throws IOException emptyList(), true, newClusterStateCustoms, + false, true ) ); @@ -1642,6 +1454,7 @@ public void testReadClusterStateInParallel_Success() throws IOException { emptyList(), true, newClusterStateCustoms, + false, true ); diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java index 897b2f5eeb25d..8fe017abbc646 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java @@ -191,7 +191,7 @@ private ClusterStateDiffManifest updateAndVerifyState( } ClusterState updatedClusterState = clusterStateBuilder.metadata(metadataBuilder.build()).build(); - ClusterStateDiffManifest manifest = new ClusterStateDiffManifest(updatedClusterState, initialState); + ClusterStateDiffManifest manifest = new ClusterStateDiffManifest(updatedClusterState, initialState, null); assertEquals(indicesToAdd.stream().map(im -> im.getIndex().getName()).collect(toList()), manifest.getIndicesUpdated()); assertEquals(indicesToRemove, manifest.getIndicesDeleted()); assertEquals(new ArrayList<>(customsToAdd.keySet()), manifest.getCustomMetadataUpdated()); From 66ddb7f3185f62e573b6166124582d3f833e8a53 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Sat, 20 Jul 2024 15:11:34 +0530 Subject: [PATCH 02/18] Create RoutingTableIncrementalDiff non-remote entity. Signed-off-by: Shailendra Singh --- .../routing/RoutingTableIncrementalDiff.java | 173 ++++++++++++++++++ .../InternalRemoteRoutingTableService.java | 11 +- .../remote/NoopRemoteRoutingTableService.java | 3 +- .../remote/RemoteRoutingTableService.java | 6 +- .../AbstractRemoteWritableBlobEntity.java | 4 - .../remote/ClusterStateDiffManifest.java | 4 +- .../remote/RemoteClusterStateService.java | 15 +- .../RemoteIndexRoutingTableDiff.java | 124 +------------ .../RemoteIndexShardRoutingTableDiff.java | 89 --------- 9 files changed, 200 insertions(+), 229 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java delete mode 100644 server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java new file mode 100644 index 0000000000000..2fddaea2bcedf --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java @@ -0,0 +1,173 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.opensearch.cluster.Diff; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Represents a difference between {@link IndexRoutingTable} objects that can be serialized and deserialized. + */ +public class RoutingTableIncrementalDiff implements Diff { + private final Map> diffs; + + /** + * Constructs a new RoutingTableIncrementalDiff with the given differences. + * + * @param diffs a map containing the differences of {@link IndexRoutingTable}. + */ + public RoutingTableIncrementalDiff(Map> diffs) { + this.diffs = diffs; + } + + /** + * Gets the map of differences of {@link IndexRoutingTable}. + * + * @return a map containing the differences. + */ + public Map> getDiffs() { + return diffs; + } + + /** + * Reads a {@link RoutingTableIncrementalDiff} from the given {@link StreamInput}. + * + * @param in the input stream to read from. + * @return the deserialized RoutingTableIncrementalDiff. + * @throws IOException if an I/O exception occurs while reading from the stream. + */ + public static RoutingTableIncrementalDiff readFrom(StreamInput in) throws IOException { + int size = in.readVInt(); + Map> diffs = new HashMap<>(); + + for (int i = 0; i < size; i++) { + String key = in.readString(); + List shardRoutingTables = new ArrayList<>(); + + // Read each IndexShardRoutingTable from the stream + int numShards = in.readVInt(); + for (int j = 0; j < numShards; j++) { + IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); + shardRoutingTables.add(shardRoutingTable); + } + + // Create a diff object for the index + Diff diff = new IndexShardRoutingTableDiff(shardRoutingTables); + + // Put the diff into the map with the key + diffs.put(key, diff); + } + return new RoutingTableIncrementalDiff(diffs); + } + + /** + * Applies the differences to the provided {@link IndexRoutingTable}. + * + * @param part the original IndexRoutingTable to which the differences will be applied. + * @return the updated IndexRoutingTable with the applied differences. + */ + @Override + public IndexRoutingTable apply(IndexRoutingTable part) { + // Apply diffs to the provided IndexRoutingTable + for (Map.Entry> entry : diffs.entrySet()) { + part = entry.getValue().apply(part); + } + return part; + } + + /** + * Writes the differences to the given {@link StreamOutput}. + * + * @param out the output stream to write to. + * @throws IOException if an I/O exception occurs while writing to the stream. + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVInt(diffs.size()); + for (Map.Entry> entry : diffs.entrySet()) { + out.writeString(entry.getKey()); + entry.getValue().writeTo(out); + } + } + + /** + * Represents a difference between {@link IndexShardRoutingTable} objects that can be serialized and deserialized. + */ + public static class IndexShardRoutingTableDiff implements Diff { + + private final List indexShardRoutingTables; + + /** + * Constructs a new IndexShardRoutingTableDiff with the given shard routing tables. + * + * @param indexShardRoutingTables a list of IndexShardRoutingTable representing the differences. + */ + public IndexShardRoutingTableDiff(List indexShardRoutingTables) { + this.indexShardRoutingTables = indexShardRoutingTables; + } + + /** + * Applies the differences to the provided {@link IndexRoutingTable}. + * + * @param part the original IndexRoutingTable to which the differences will be applied. + * @return the updated IndexRoutingTable with the applied differences. + */ + @Override + public IndexRoutingTable apply(IndexRoutingTable part) { + IndexRoutingTable.Builder builder = new IndexRoutingTable.Builder(part.getIndex()); + for (IndexShardRoutingTable shardRoutingTable : part) { + builder.addIndexShard(shardRoutingTable); // Add existing shards to builder + } + + // Apply the diff: update or add the new shard routing tables + for (IndexShardRoutingTable diffShard : indexShardRoutingTables) { + builder.addIndexShard(diffShard); + } + return builder.build(); + } + + /** + * Writes the differences to the given {@link StreamOutput}. + * + * @param out the output stream to write to. + * @throws IOException if an I/O exception occurs while writing to the stream. + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVInt(indexShardRoutingTables.size()); + for (IndexShardRoutingTable shardRoutingTable : indexShardRoutingTables) { + IndexShardRoutingTable.Builder.writeTo(shardRoutingTable, out); + } + } + + /** + * Reads a {@link IndexShardRoutingTableDiff} from the given {@link StreamInput}. + * + * @param in the input stream to read from. + * @return the deserialized IndexShardRoutingTableDiff. + * @throws IOException if an I/O exception occurs while reading from the stream. + */ + public static IndexShardRoutingTableDiff readFrom(StreamInput in) throws IOException { + int size = in.readVInt(); + List indexShardRoutingTables = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); + indexShardRoutingTables.add(shardRoutingTable); + } + return new IndexShardRoutingTableDiff(indexShardRoutingTables); + } + } +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java index 07d1036aaf116..d4b8870263c62 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java @@ -16,6 +16,7 @@ import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; @@ -56,13 +57,13 @@ * @opensearch.internal */ public class InternalRemoteRoutingTableService extends AbstractLifecycleComponent implements RemoteRoutingTableService { - + private static final Logger logger = LogManager.getLogger(InternalRemoteRoutingTableService.class); private final Settings settings; private final Supplier repositoriesService; private Compressor compressor; private RemoteWritableEntityStore remoteIndexRoutingTableStore; - private RemoteWritableEntityStore remoteIndexRoutingTableDiffStore; + private RemoteWritableEntityStore remoteIndexRoutingTableDiffStore; private final ClusterSettings clusterSettings; private BlobStoreRepository blobStoreRepository; private final ThreadPool threadPool; @@ -206,10 +207,10 @@ public CheckedRunnable getAsyncIndexRoutingReadAction( public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, - LatchedActionListener>> latchedActionListener + LatchedActionListener latchedActionListener ) { - ActionListener actionListener = ActionListener.wrap( - response -> latchedActionListener.onResponse(response.getDiffs()), + ActionListener actionListener = ActionListener.wrap( + latchedActionListener::onResponse, latchedActionListener::onFailure ); diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java index b1c639d8deffa..a90b481858d6a 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java @@ -13,6 +13,7 @@ import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.gateway.remote.ClusterMetadataManifest; @@ -90,7 +91,7 @@ public CheckedRunnable getAsyncIndexRoutingReadAction( public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, - LatchedActionListener>> latchedActionListener + LatchedActionListener latchedActionListener ) { // noop return () -> {}; 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 f6cf2f24ba987..e0feeb623a87a 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 @@ -14,12 +14,12 @@ import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; import org.opensearch.common.CheckedRunnable; import org.opensearch.common.lifecycle.LifecycleComponent; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.gateway.remote.ClusterMetadataManifest; -import org.opensearch.gateway.remote.routingtable.RemoteIndexShardRoutingTableDiff; import java.io.IOException; import java.util.ArrayList; @@ -63,7 +63,7 @@ public Diff diff(IndexRoutingTable currentState, IndexRouting diffs.add(currentShardRoutingTable); } } - return new RemoteIndexShardRoutingTableDiff(diffs); + return new RoutingTableIncrementalDiff.IndexShardRoutingTableDiff(diffs); } }; @@ -78,7 +78,7 @@ CheckedRunnable getAsyncIndexRoutingReadAction( CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, - LatchedActionListener>> latchedActionListener + LatchedActionListener latchedActionListener ); List getUpdatedIndexRoutingTableMetadata( 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 bdaa236a0950e..237c077cb673c 100644 --- a/server/src/main/java/org/opensearch/common/remote/AbstractRemoteWritableBlobEntity.java +++ b/server/src/main/java/org/opensearch/common/remote/AbstractRemoteWritableBlobEntity.java @@ -44,10 +44,6 @@ public AbstractRemoteWritableBlobEntity(final String clusterUUID, final Compress this(clusterUUID, compressor, null); } - public AbstractRemoteWritableBlobEntity() { - this(null, null, null); - } - public abstract BlobPathParameters getBlobPathParameters(); public abstract String getType(); diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java index 3ccc9446d0ea4..d74ece5ca81cf 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java @@ -160,8 +160,8 @@ public ClusterStateDiffManifest( this.indicesDeleted = Collections.unmodifiableList(indicesDeleted); this.clusterBlocksUpdated = clusterBlocksUpdated; this.discoveryNodesUpdated = discoveryNodesUpdated; - this.indicesRoutingUpdated = indicesRoutingUpdated; - this.indicesRoutingDeleted = indicesRoutingDeleted; + this.indicesRoutingUpdated = Collections.unmodifiableList(indicesRoutingUpdated); + this.indicesRoutingDeleted = Collections.unmodifiableList(indicesRoutingDeleted); this.indicesRoutingDiffPath = indicesRoutingDiffPath; this.hashesOfConsistentSettingsUpdated = hashesOfConsistentSettingsUpdated; this.clusterStateCustomUpdated = Collections.unmodifiableList(clusterStateCustomUpdated); 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 a46067269a68e..4ba25c045e9f8 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -27,6 +27,7 @@ import org.opensearch.cluster.node.DiscoveryNodes.Builder; import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; import org.opensearch.cluster.routing.remote.RemoteRoutingTableService; import org.opensearch.cluster.routing.remote.RemoteRoutingTableServiceFactory; import org.opensearch.cluster.service.ClusterService; @@ -1041,7 +1042,7 @@ ClusterState readClusterStateInParallel( List> asyncMetadataReadActions = new ArrayList<>(); List readResults = Collections.synchronizedList(new ArrayList<>()); List readIndexRoutingTableResults = Collections.synchronizedList(new ArrayList<>()); - AtomicReference readIndexRoutingTableDiffResults = new AtomicReference<>(); + AtomicReference readIndexRoutingTableDiffResults = new AtomicReference<>(); List exceptionList = Collections.synchronizedList(new ArrayList<>(totalReadTasks)); LatchedActionListener listener = new LatchedActionListener<>(ActionListener.wrap(response -> { @@ -1079,14 +1080,12 @@ ClusterState readClusterStateInParallel( ); } - LatchedActionListener>> routingTableDiffLatchedActionListener = new LatchedActionListener<>( + LatchedActionListener routingTableDiffLatchedActionListener = new LatchedActionListener<>( ActionListener.wrap(response -> { - logger.debug("Successfully read cluster state diff component from remote"); - readIndexRoutingTableDiffResults.set( - new RemoteIndexRoutingTableDiff(response, clusterUUID, blobStoreRepository.getCompressor()) - ); + logger.debug("Successfully read routing table diff component from remote"); + readIndexRoutingTableDiffResults.set(response); }, ex -> { - logger.error("Failed to read cluster state diff from remote", ex); + logger.error("Failed to read routing table diff from remote", ex); exceptionList.add(ex); }), latch @@ -1326,7 +1325,7 @@ ClusterState readClusterStateInParallel( readIndexRoutingTableResults.forEach( indexRoutingTable -> indicesRouting.put(indexRoutingTable.getIndex().getName(), indexRoutingTable) ); - RemoteIndexRoutingTableDiff routingTableDiff = readIndexRoutingTableDiffResults.get(); + RoutingTableIncrementalDiff routingTableDiff = readIndexRoutingTableDiffResults.get(); if (routingTableDiff != null) { routingTableDiff.getDiffs().forEach((key, diff) -> { IndexRoutingTable previousIndexRoutingTable = indicesRouting.get(key); diff --git a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java index ad92f099343be..2b1f2be03da47 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java +++ b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java @@ -8,30 +8,19 @@ package org.opensearch.gateway.remote.routingtable; -import org.apache.lucene.codecs.CodecUtil; -import org.apache.lucene.store.InputStreamDataInput; -import org.apache.lucene.store.OutputStreamDataOutput; import org.opensearch.cluster.Diff; import org.opensearch.cluster.routing.IndexRoutingTable; -import org.opensearch.cluster.routing.IndexShardRoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; import org.opensearch.common.io.Streams; import org.opensearch.common.remote.AbstractRemoteWritableBlobEntity; import org.opensearch.common.remote.BlobPathParameters; -import org.opensearch.core.common.io.stream.BufferedChecksumStreamInput; -import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput; -import org.opensearch.core.common.io.stream.InputStreamStreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.core.common.io.stream.Writeable; import org.opensearch.core.compress.Compressor; import org.opensearch.gateway.remote.ClusterMetadataManifest; import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.repositories.blobstore.ChecksumWritableBlobStoreFormat; -import java.io.EOFException; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -41,11 +30,7 @@ * Represents a difference between {@link IndexRoutingTable} objects that can be serialized and deserialized. * This class is responsible for writing and reading the differences between IndexRoutingTables to and from an input/output stream. */ -public class RemoteIndexRoutingTableDiff extends AbstractRemoteWritableBlobEntity - implements - Diff, - Writeable { - +public class RemoteIndexRoutingTableDiff extends AbstractRemoteWritableBlobEntity { private final Map> diffs; private long term; @@ -61,8 +46,8 @@ public class RemoteIndexRoutingTableDiff extends AbstractRemoteWritableBlobEntit public static final int VERSION = 1; - public static final ChecksumWritableBlobStoreFormat RemoteIndexRoutingTableDiffFormat = - new ChecksumWritableBlobStoreFormat<>(codec, RemoteIndexRoutingTableDiff::new); + public static final ChecksumWritableBlobStoreFormat RemoteIndexRoutingTableDiffFormat = + new ChecksumWritableBlobStoreFormat<>(codec, RoutingTableIncrementalDiff::readFrom); /** * Constructs a new RemoteIndexRoutingTableDiff with the given differences. @@ -111,102 +96,6 @@ public RemoteIndexRoutingTableDiff(String blobName, String clusterUUID, Compress this.blobName = blobName; } - /** - * Reads data from inputStream and creates RemoteIndexRoutingTableDiff object with the {@link Diff} - * - * @param inputStream input stream with diff data - * @throws IOException exception thrown on failing to read from stream. - */ - public RemoteIndexRoutingTableDiff(InputStream inputStream) throws IOException { - super(); - try (BufferedChecksumStreamInput in = new BufferedChecksumStreamInput(new InputStreamStreamInput(inputStream), "assertion")) { - CodecUtil.checkHeader(new InputStreamDataInput(inputStream), codec, VERSION, VERSION); - int size = in.readVInt(); - Map> diffs = new HashMap<>(); - - for (int i = 0; i < size; i++) { - String key = in.readString(); - List shardRoutingTables = new ArrayList<>(); - - // Read each IndexShardRoutingTable from the stream - int numShards = in.readVInt(); - for (int j = 0; j < numShards; j++) { - IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); - shardRoutingTables.add(shardRoutingTable); - } - - // Create a diff object for the index - Diff diff = new RemoteIndexShardRoutingTableDiff(shardRoutingTables); - - // Put the diff into the map with the key - diffs.put(key, diff); - } - verifyCheckSum(in); - this.diffs = diffs; - } catch (EOFException e) { - throw new IOException("Diffs data is corrupted", e); - } - } - - /** - * Writes {@link Diff} to the given stream - * - * @param streamOutput output stream to write - * @throws IOException exception thrown on failing to write to stream. - */ - @Override - public void writeTo(StreamOutput streamOutput) throws IOException { - try (BufferedChecksumStreamOutput out = new BufferedChecksumStreamOutput(streamOutput)) { - // Write codec header - CodecUtil.writeHeader(new OutputStreamDataOutput(streamOutput), codec, VERSION); - - out.writeVInt(diffs.size()); - for (Map.Entry> entry : diffs.entrySet()) { - out.writeString(entry.getKey()); - entry.getValue().writeTo(out); - } - - out.writeLong(out.getChecksum()); - out.flush(); - } catch (IOException e) { - throw new IOException("Failed to write Diff to stream", e); - } - } - - /** - * Applies the differences to the provided {@link IndexRoutingTable}. - * - * @param part the initial {@link IndexRoutingTable}. - * @return the modified {@link IndexRoutingTable} with applied differences. - */ - @Override - public IndexRoutingTable apply(IndexRoutingTable part) { - // Apply diffs to the provided IndexRoutingTable - for (Map.Entry> entry : diffs.entrySet()) { - part = entry.getValue().apply(part); - } - return part; - } - - /** - * Verifies the checksum of the data in the given input stream. - * - * @param in the input stream containing the data. - * @throws IOException if the checksum verification fails or if an I/O exception occurs. - */ - private void verifyCheckSum(BufferedChecksumStreamInput in) throws IOException { - long expectedChecksum = in.getChecksum(); - long readChecksum = in.readLong(); - if (readChecksum != expectedChecksum) { - throw new IOException( - "checksum verification failed - expected: 0x" - + Long.toHexString(expectedChecksum) - + ", got: 0x" - + Long.toHexString(readChecksum) - ); - } - } - /** * Gets the map of differences of {@link IndexRoutingTable}. * @@ -248,11 +137,12 @@ public ClusterMetadataManifest.UploadedMetadata getUploadedMetadata() { @Override public InputStream serialize() throws IOException { - return RemoteIndexRoutingTableDiffFormat.serialize(this, generateBlobFileName(), getCompressor()).streamInput(); + return RemoteIndexRoutingTableDiffFormat.serialize(new RoutingTableIncrementalDiff(diffs), generateBlobFileName(), getCompressor()) + .streamInput(); } @Override - public RemoteIndexRoutingTableDiff deserialize(InputStream in) throws IOException { + public RoutingTableIncrementalDiff deserialize(InputStream in) throws IOException { return RemoteIndexRoutingTableDiffFormat.deserialize(blobName, Streams.readFully(in)); } } diff --git a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java deleted file mode 100644 index eeea4d7d3291f..0000000000000 --- a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexShardRoutingTableDiff.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.gateway.remote.routingtable; - -import org.opensearch.cluster.Diff; -import org.opensearch.cluster.routing.IndexRoutingTable; -import org.opensearch.cluster.routing.IndexShardRoutingTable; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -/** - * Represents the differences between {@link IndexRoutingTable}s for uploading and downloading index routing table diff data to/from a remote store. - * - * @opensearch.internal - */ -public class RemoteIndexShardRoutingTableDiff implements Diff { - - private final List indexShardRoutingTables; - - /** - * Constructs a new RemoteIndexShardRoutingTableDiff with the given shard routing tables. - * - * @param indexShardRoutingTables the list of {@link IndexShardRoutingTable}s representing the differences. - */ - public RemoteIndexShardRoutingTableDiff(List indexShardRoutingTables) { - this.indexShardRoutingTables = indexShardRoutingTables; - } - - /** - * Applies the differences to the provided {@link IndexRoutingTable}. - * - * @param part the original {@link IndexRoutingTable}. - * @return the updated {@link IndexRoutingTable} with the applied differences. - */ - @Override - public IndexRoutingTable apply(IndexRoutingTable part) { - IndexRoutingTable.Builder builder = new IndexRoutingTable.Builder(part.getIndex()); - for (IndexShardRoutingTable shardRoutingTable : part) { - builder.addIndexShard(shardRoutingTable); // Add existing shards to builder - } - - // Apply the diff: update or add the new shard routing tables - for (IndexShardRoutingTable diffShard : indexShardRoutingTables) { - builder.addIndexShard(diffShard); - } - return builder.build(); - } - - /** - * Writes the shard routing tables to the provided {@link StreamOutput}. - * - * @param out the output stream to write to. - * @throws IOException if an I/O exception occurs while writing. - */ - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(indexShardRoutingTables.size()); - for (IndexShardRoutingTable shardRoutingTable : indexShardRoutingTables) { - IndexShardRoutingTable.Builder.writeTo(shardRoutingTable, out); - } - } - - /** - * Reads the shard routing tables from the provided {@link StreamInput}. - * - * @param in the input stream to read from. - * @return a new {@link RemoteIndexShardRoutingTableDiff} with the read shard routing tables. - * @throws IOException if an I/O exception occurs while reading. - */ - public static RemoteIndexShardRoutingTableDiff readFrom(StreamInput in) throws IOException { - int size = in.readVInt(); - List indexShardRoutingTables = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); - indexShardRoutingTables.add(shardRoutingTable); - } - return new RemoteIndexShardRoutingTableDiff(indexShardRoutingTables); - } -} From 740e94bc1ec60a3e300d927725d974f771e4597a Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Sat, 20 Jul 2024 17:48:44 +0530 Subject: [PATCH 03/18] Address comments Signed-off-by: Shailendra Singh --- .../routing/RoutingTableIncrementalDiff.java | 12 ++++---- .../remote/RemoteRoutingTableService.java | 2 +- .../remote/ClusterMetadataManifest.java | 28 +++++++++++++++++-- .../remote/ClusterStateDiffManifest.java | 11 ++++++-- .../remote/RemoteClusterStateService.java | 26 +++++++++-------- .../model/RemoteClusterMetadataManifest.java | 7 ++++- .../remote/ClusterMetadataManifestTests.java | 1 + .../model/ClusterStateDiffManifestTests.java | 4 +-- 8 files changed, 64 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java index 2fddaea2bcedf..cdb7ef39484c3 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java @@ -65,7 +65,7 @@ public static RoutingTableIncrementalDiff readFrom(StreamInput in) throws IOExce } // Create a diff object for the index - Diff diff = new IndexShardRoutingTableDiff(shardRoutingTables); + Diff diff = new IndexRoutingTableIncrementalDiff(shardRoutingTables); // Put the diff into the map with the key diffs.put(key, diff); @@ -106,7 +106,7 @@ public void writeTo(StreamOutput out) throws IOException { /** * Represents a difference between {@link IndexShardRoutingTable} objects that can be serialized and deserialized. */ - public static class IndexShardRoutingTableDiff implements Diff { + public static class IndexRoutingTableIncrementalDiff implements Diff { private final List indexShardRoutingTables; @@ -115,7 +115,7 @@ public static class IndexShardRoutingTableDiff implements Diff indexShardRoutingTables) { + public IndexRoutingTableIncrementalDiff(List indexShardRoutingTables) { this.indexShardRoutingTables = indexShardRoutingTables; } @@ -154,20 +154,20 @@ public void writeTo(StreamOutput out) throws IOException { } /** - * Reads a {@link IndexShardRoutingTableDiff} from the given {@link StreamInput}. + * Reads a {@link IndexRoutingTableIncrementalDiff} from the given {@link StreamInput}. * * @param in the input stream to read from. * @return the deserialized IndexShardRoutingTableDiff. * @throws IOException if an I/O exception occurs while reading from the stream. */ - public static IndexShardRoutingTableDiff readFrom(StreamInput in) throws IOException { + public static IndexRoutingTableIncrementalDiff readFrom(StreamInput in) throws IOException { int size = in.readVInt(); List indexShardRoutingTables = new ArrayList<>(size); for (int i = 0; i < size; i++) { IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); indexShardRoutingTables.add(shardRoutingTable); } - return new IndexShardRoutingTableDiff(indexShardRoutingTables); + return new IndexRoutingTableIncrementalDiff(indexShardRoutingTables); } } } 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 e0feeb623a87a..3662e81e14f27 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 @@ -63,7 +63,7 @@ public Diff diff(IndexRoutingTable currentState, IndexRouting diffs.add(currentShardRoutingTable); } } - return new RoutingTableIncrementalDiff.IndexShardRoutingTableDiff(diffs); + return new RoutingTableIncrementalDiff.IndexRoutingTableIncrementalDiff(diffs); } }; diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java index 3a66419b1dc20..457bcd9d2bc5f 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java @@ -44,6 +44,7 @@ public class ClusterMetadataManifest implements Writeable, ToXContentFragment { public static final int CODEC_V2 = 2; // In Codec V2, there are separate metadata files rather than a single global metadata file, // also we introduce index routing-metadata, diff and other attributes as part of manifest // required for state publication + public static final int CODEC_V3 = 3; // In Codec V3, we have introduced new diff field in diff-manifest's routing_table_diff private static final ParseField CLUSTER_TERM_FIELD = new ParseField("cluster_term"); private static final ParseField STATE_VERSION_FIELD = new ParseField("state_version"); @@ -109,6 +110,23 @@ private static ClusterMetadataManifest.Builder manifestV2Builder(Object[] fields .clusterStateCustomMetadataMap(clusterStateCustomMetadata(fields)); } + private static ClusterMetadataManifest.Builder manifestV3Builder(Object[] fields) { + return manifestV0Builder(fields).codecVersion(codecVersion(fields)) + .coordinationMetadata(coordinationMetadata(fields)) + .settingMetadata(settingsMetadata(fields)) + .templatesMetadata(templatesMetadata(fields)) + .customMetadataMap(customMetadata(fields)) + .routingTableVersion(routingTableVersion(fields)) + .indicesRouting(indicesRouting(fields)) + .discoveryNodesMetadata(discoveryNodesMetadata(fields)) + .clusterBlocksMetadata(clusterBlocksMetadata(fields)) + .diffManifest(diffManifest(fields)) + .metadataVersion(metadataVersion(fields)) + .transientSettingsMetadata(transientSettingsMetadata(fields)) + .hashesOfConsistentSettings(hashesOfConsistentSettings(fields)) + .clusterStateCustomMetadataMap(clusterStateCustomMetadata(fields)); + } + private static long term(Object[] fields) { return (long) fields[0]; } @@ -226,12 +244,18 @@ private static ClusterStateDiffManifest diffManifest(Object[] fields) { fields -> manifestV2Builder(fields).build() ); - private static final ConstructingObjectParser CURRENT_PARSER = PARSER_V2; + private static final ConstructingObjectParser PARSER_V3 = new ConstructingObjectParser<>( + "cluster_metadata_manifest", + fields -> manifestV3Builder(fields).build() + ); + + private static final ConstructingObjectParser CURRENT_PARSER = PARSER_V3; static { declareParser(PARSER_V0, CODEC_V0); declareParser(PARSER_V1, CODEC_V1); declareParser(PARSER_V2, CODEC_V2); + declareParser(PARSER_V3, CODEC_V3); } private static void declareParser(ConstructingObjectParser parser, long codec_version) { @@ -309,7 +333,7 @@ private static void declareParser(ConstructingObjectParser ClusterStateDiffManifest.fromXContent(p), + (p, c) -> ClusterStateDiffManifest.fromXContent(p, codec_version), DIFF_MANIFEST ); } diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java index d74ece5ca81cf..177652157fcb9 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java @@ -33,6 +33,7 @@ import static org.opensearch.cluster.DiffableUtils.NonDiffableValueSerializer.getAbstractInstance; import static org.opensearch.cluster.DiffableUtils.getStringKeySerializer; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V3; /** * Manifest of diff between two cluster states @@ -80,7 +81,8 @@ public class ClusterStateDiffManifest implements ToXContentFragment, Writeable { public ClusterStateDiffManifest( ClusterState state, ClusterState previousState, - DiffableUtils.MapDiff> routingTableIncrementalDiff + DiffableUtils.MapDiff> routingTableIncrementalDiff, + String indicesRoutingDiffPath ) { fromStateUUID = previousState.stateUUID(); toStateUUID = state.stateUUID(); @@ -110,6 +112,7 @@ public ClusterStateDiffManifest( indicesRoutingUpdated = new ArrayList<>(); indicesRoutingDeleted = new ArrayList<>(); + this.indicesRoutingDiffPath = indicesRoutingDiffPath; if (routingTableIncrementalDiff != null) { routingTableIncrementalDiff.getUpserts().forEach((k, v) -> indicesRoutingUpdated.add(k)); indicesRoutingDeleted.addAll(routingTableIncrementalDiff.getDeletes()); @@ -259,7 +262,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - public static ClusterStateDiffManifest fromXContent(XContentParser parser) throws IOException { + public static ClusterStateDiffManifest fromXContent(XContentParser parser, long codec_version) throws IOException { Builder builder = new Builder(); if (parser.currentToken() == null) { // fresh parser? move to next token parser.nextToken(); @@ -348,7 +351,9 @@ public static ClusterStateDiffManifest fromXContent(XContentParser parser) throw builder.indicesRoutingDeleted(convertListToString(parser.listOrderedMap())); break; case DIFF_FIELD: - builder.indicesRoutingDiffPath(parser.textOrNull()); + if (codec_version >= CODEC_V3) { + builder.indicesRoutingDiffPath(parser.textOrNull()); + } break; default: throw new XContentParseException("Unexpected field [" + currentFieldName + "]"); 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 4ba25c045e9f8..9efe4eb9c41c0 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -241,12 +241,15 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat remoteRoutingTableService.getIndicesRouting(clusterState.getRoutingTable()), null ); - ClusterStateDiffManifest clusterStateDiffManifest = new ClusterStateDiffManifest(clusterState, ClusterState.EMPTY_STATE, null); - if (uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null) { - clusterStateDiffManifest.setIndicesRoutingDiffPath( - uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() - ); - } + + ClusterStateDiffManifest clusterStateDiffManifest = new ClusterStateDiffManifest( + clusterState, + ClusterState.EMPTY_STATE, + null, + uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null + ? uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() + : null + ); final RemoteClusterStateManifestInfo manifestDetails = remoteManifestManager.uploadManifest( clusterState, uploadedMetadataResults, @@ -432,13 +435,12 @@ public RemoteClusterStateManifestInfo writeIncrementalMetadata( ClusterStateDiffManifest clusterStateDiffManifest = new ClusterStateDiffManifest( clusterState, previousClusterState, - routingTableIncrementalDiff + routingTableIncrementalDiff, + uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null + ? uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() + : null ); - if (uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null) { - clusterStateDiffManifest.setIndicesRoutingDiffPath( - uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() - ); - } + final RemoteClusterStateManifestInfo manifestDetails = remoteManifestManager.uploadManifest( clusterState, uploadedMetadataResults, diff --git a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java index 1dc56712d4ab5..acaae3173315a 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/model/RemoteClusterMetadataManifest.java @@ -35,7 +35,7 @@ public class RemoteClusterMetadataManifest extends AbstractRemoteWritableBlobEnt public static final int SPLITTED_MANIFEST_FILE_LENGTH = 6; public static final String METADATA_MANIFEST_NAME_FORMAT = "%s"; - public static final int MANIFEST_CURRENT_CODEC_VERSION = ClusterMetadataManifest.CODEC_V2; + public static final int MANIFEST_CURRENT_CODEC_VERSION = ClusterMetadataManifest.CODEC_V3; public static final String COMMITTED = "C"; public static final String PUBLISHED = "P"; @@ -50,6 +50,9 @@ public class RemoteClusterMetadataManifest extends AbstractRemoteWritableBlobEnt public static final ChecksumBlobStoreFormat CLUSTER_METADATA_MANIFEST_FORMAT_V1 = new ChecksumBlobStoreFormat<>("cluster-metadata-manifest", METADATA_MANIFEST_NAME_FORMAT, ClusterMetadataManifest::fromXContentV1); + public static final ChecksumBlobStoreFormat CLUSTER_METADATA_MANIFEST_FORMAT_V2 = + new ChecksumBlobStoreFormat<>("cluster-metadata-manifest", METADATA_MANIFEST_NAME_FORMAT, ClusterMetadataManifest::fromXContentV2); + /** * Manifest format compatible with codec v2, where we introduced codec versions/global metadata. */ @@ -149,6 +152,8 @@ private ChecksumBlobStoreFormat getClusterMetadataManif long codecVersion = getManifestCodecVersion(); if (codecVersion == MANIFEST_CURRENT_CODEC_VERSION) { return CLUSTER_METADATA_MANIFEST_FORMAT; + } else if (codecVersion == ClusterMetadataManifest.CODEC_V2) { + return CLUSTER_METADATA_MANIFEST_FORMAT_V2; } else if (codecVersion == ClusterMetadataManifest.CODEC_V1) { return CLUSTER_METADATA_MANIFEST_FORMAT_V1; } else if (codecVersion == ClusterMetadataManifest.CODEC_V0) { diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index e1abe083b06e7..d6973706c5ea2 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -191,6 +191,7 @@ public void testClusterMetadataManifestXContentV2() throws IOException { new ClusterStateDiffManifest( RemoteClusterStateServiceTests.generateClusterStateWithOneIndex().build(), ClusterState.EMPTY_STATE, + null, null ) ) diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java index 8fe017abbc646..b9c55dfacfc5b 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java @@ -114,7 +114,7 @@ public void testClusterStateDiffManifestXContent() throws IOException { diffManifest.toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { - final ClusterStateDiffManifest parsedManifest = ClusterStateDiffManifest.fromXContent(parser); + final ClusterStateDiffManifest parsedManifest = ClusterStateDiffManifest.fromXContent(parser, 0); assertEquals(diffManifest, parsedManifest); } } @@ -191,7 +191,7 @@ private ClusterStateDiffManifest updateAndVerifyState( } ClusterState updatedClusterState = clusterStateBuilder.metadata(metadataBuilder.build()).build(); - ClusterStateDiffManifest manifest = new ClusterStateDiffManifest(updatedClusterState, initialState, null); + ClusterStateDiffManifest manifest = new ClusterStateDiffManifest(updatedClusterState, initialState, null, null); assertEquals(indicesToAdd.stream().map(im -> im.getIndex().getName()).collect(toList()), manifest.getIndicesUpdated()); assertEquals(indicesToRemove, manifest.getIndicesDeleted()); assertEquals(new ArrayList<>(customsToAdd.keySet()), manifest.getCustomMetadataUpdated()); From 8258729f63ad97755942d9e2b7de1f5a108c43c9 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Sun, 21 Jul 2024 10:20:41 +0530 Subject: [PATCH 04/18] Add removed UTs. Signed-off-by: Shailendra Singh --- .../RemoteRoutingTableServiceTests.java | 76 +++++ .../remote/ClusterMetadataManifestTests.java | 311 ++++++++++++++++++ .../RemoteClusterStateServiceTests.java | 199 +++++++++++ .../model/ClusterStateDiffManifestTests.java | 3 +- 4 files changed, 588 insertions(+), 1 deletion(-) 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 952d4e63d2d8d..3cbcccf5f9159 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 @@ -258,6 +258,82 @@ public void testGetIndicesRoutingMapDiffIndexAdded() { assertEquals(0, diff.getDeletes().size()); } + public void testGetIndicesRoutingMapDiffShardChanged() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int noOfShards = between(1, 1000); + int noOfReplicas = randomInt(10); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + + final IndexMetadata indexMetadata2 = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards + 1).numberOfReplicas(noOfReplicas).build(); + RoutingTable routingTable2 = RoutingTable.builder().addAsNew(indexMetadata2).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(0, diff.getUpserts().size()); + assertEquals(1, diff.getDiffs().size()); + assertNotNull(diff.getDiffs().get(indexName)); + assertEquals(noOfShards + 1, diff.getDiffs().get(indexName).apply(routingTable.indicesRouting().get(indexName)).shards().size()); + assertEquals( + noOfReplicas + 1, + diff.getDiffs().get(indexName).apply(routingTable.indicesRouting().get(indexName)).getShards().get(0).getSize() + ); + assertEquals(0, diff.getDeletes().size()); + + final IndexMetadata indexMetadata3 = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards + 1).numberOfReplicas(noOfReplicas + 1).build(); + RoutingTable routingTable3 = RoutingTable.builder().addAsNew(indexMetadata3).build(); + + diff = remoteRoutingTableService.getIndicesRoutingMapDiff(routingTable2, routingTable3); + assertEquals(0, diff.getUpserts().size()); + assertEquals(1, diff.getDiffs().size()); + assertNotNull(diff.getDiffs().get(indexName)); + assertEquals(noOfShards + 1, diff.getDiffs().get(indexName).apply(routingTable.indicesRouting().get(indexName)).shards().size()); + assertEquals( + noOfReplicas + 2, + diff.getDiffs().get(indexName).apply(routingTable.indicesRouting().get(indexName)).getShards().get(0).getSize() + ); + assertEquals(0, diff.getDeletes().size()); + } + + public void testGetIndicesRoutingMapDiffShardDetailChanged() { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int noOfShards = between(1, 1000); + int noOfReplicas = randomInt(10); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( + Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, "uuid") + .build() + ).numberOfShards(noOfShards).numberOfReplicas(noOfReplicas).build(); + + RoutingTable routingTable = RoutingTable.builder().addAsNew(indexMetadata).build(); + RoutingTable routingTable2 = RoutingTable.builder().addAsRecovery(indexMetadata).build(); + + DiffableUtils.MapDiff> diff = remoteRoutingTableService + .getIndicesRoutingMapDiff(routingTable, routingTable2); + assertEquals(1, diff.getDiffs().size()); + assertNotNull(diff.getDiffs().get(indexName)); + assertEquals(noOfShards, diff.getDiffs().get(indexName).apply(routingTable.indicesRouting().get(indexName)).shards().size()); + assertEquals(0, diff.getUpserts().size()); + assertEquals(0, diff.getDeletes().size()); + } + public void testGetIndicesRoutingMapDiffIndexDeleted() { String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); final IndexMetadata indexMetadata = new IndexMetadata.Builder(indexName).settings( diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index d6973706c5ea2..f2680f5e1110f 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -34,11 +34,15 @@ import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V0; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V1; +import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_BLOCKS; +import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.DISCOVERY_NODES; import static org.opensearch.gateway.remote.model.RemoteCoordinationMetadata.COORDINATION_METADATA; import static org.opensearch.gateway.remote.model.RemoteCustomMetadata.CUSTOM_DELIMITER; import static org.opensearch.gateway.remote.model.RemoteCustomMetadata.CUSTOM_METADATA; +import static org.opensearch.gateway.remote.model.RemoteHashesOfConsistentSettings.HASHES_OF_CONSISTENT_SETTINGS; 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; public class ClusterMetadataManifestTests extends OpenSearchTestCase { @@ -144,6 +148,313 @@ public void testClusterMetadataManifestXContent() throws IOException { } } + public void testClusterMetadataManifestSerializationEqualsHashCode() { + ClusterMetadataManifest initialManifest = ClusterMetadataManifest.builder() + .clusterTerm(1337L) + .stateVersion(7L) + .clusterUUID("HrYF3kP5SmSPWtKlWhnNSA") + .stateUUID("6By9p9G0Rv2MmFYJcPAOgA") + .opensearchVersion(Version.CURRENT) + .nodeId("B10RX1f5RJenMQvYccCgSQ") + .committed(true) + .codecVersion(ClusterMetadataManifest.CODEC_V3) + .indices(randomUploadedIndexMetadataList()) + .previousClusterUUID("yfObdx8KSMKKrXf8UyHhM") + .clusterUUIDCommitted(true) + .coordinationMetadata(new UploadedMetadataAttribute(COORDINATION_METADATA, "coordination-file")) + .settingMetadata(new UploadedMetadataAttribute(SETTING_METADATA, "setting-file")) + .templatesMetadata(new UploadedMetadataAttribute(TEMPLATES_METADATA, "templates-file")) + .customMetadataMap( + Collections.unmodifiableList( + Arrays.asList( + new UploadedMetadataAttribute( + CUSTOM_METADATA + CUSTOM_DELIMITER + RepositoriesMetadata.TYPE, + "custom--repositories-file" + ), + new UploadedMetadataAttribute( + CUSTOM_METADATA + CUSTOM_DELIMITER + IndexGraveyard.TYPE, + "custom--index_graveyard-file" + ), + new UploadedMetadataAttribute( + CUSTOM_METADATA + CUSTOM_DELIMITER + WeightedRoutingMetadata.TYPE, + "custom--weighted_routing_netadata-file" + ) + ) + ).stream().collect(Collectors.toMap(UploadedMetadataAttribute::getAttributeName, Function.identity())) + ) + .routingTableVersion(1L) + .discoveryNodesMetadata(new UploadedMetadataAttribute(DISCOVERY_NODES, "discovery-nodes-file")) + .clusterBlocksMetadata(new UploadedMetadataAttribute(CLUSTER_BLOCKS, "cluster-block-file")) + .transientSettingsMetadata(new UploadedMetadataAttribute(TRANSIENT_SETTING_METADATA, "transient-settings-file")) + .hashesOfConsistentSettings(new UploadedMetadataAttribute(HASHES_OF_CONSISTENT_SETTINGS, "hashes-of-consistent-settings-file")) + .clusterStateCustomMetadataMap(Collections.emptyMap()) + .build(); + { // Mutate Cluster Term + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.clusterTerm(1338L); + return builder.build(); + } + ); + } + { // Mutate State Version + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.stateVersion(8L); + return builder.build(); + } + ); + } + { // Mutate Cluster UUID + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.clusterUUID("efOkMiPbQZCUQQgtFWdbPw"); + return builder.build(); + } + ); + } + { // Mutate State UUID + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.stateUUID("efOkMiPbQZCUQQgtFWdbPw"); + return builder.build(); + } + ); + } + { // Mutate OpenSearch Version + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.opensearchVersion(Version.V_EMPTY); + return builder.build(); + } + ); + } + { // Mutate Committed State + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.committed(false); + return builder.build(); + } + ); + } + { // Mutate Indices + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.indices(randomUploadedIndexMetadataList()); + return builder.build(); + } + ); + } + { // Mutate Previous cluster UUID + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.previousClusterUUID("vZX62DCQEOzGXlxXCrEu"); + return builder.build(); + } + ); + + } + { // Mutate cluster uuid committed + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.clusterUUIDCommitted(false); + return builder.build(); + } + ); + } + { + // Mutate Coordination metadata + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.coordinationMetadata(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + { + // Mutate setting metadata + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.settingMetadata(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + { + // Mutate template metadata + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.templatesMetadata(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + { + // Mutate custom metadata + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.customMetadataMap(Collections.emptyMap()); + return builder.build(); + } + ); + } + { + // Mutate discovery nodes + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.discoveryNodesMetadata(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + { + // Mutate cluster blocks + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.clusterBlocksMetadata(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + { + // Mutate transient settings metadata + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.transientSettingsMetadata(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + { + // Mutate hashes of consistent settings + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.hashesOfConsistentSettings(randomUploadedMetadataAttribute()); + return builder.build(); + } + ); + } + } + public void testClusterMetadataManifestXContentV2() throws IOException { UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "test-uuid", "/test/upload/path"); UploadedMetadataAttribute uploadedMetadataAttribute = new UploadedMetadataAttribute("attribute_name", "testing_attribute"); 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 5f219eaf3d899..a38b5ae9dc7a1 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -101,6 +101,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -148,12 +149,16 @@ 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; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -495,6 +500,51 @@ public void run() { ); } + public void testTimeoutWhileWritingManifestFile() throws IOException { + // verify update metadata manifest upload timeout + int metadataManifestUploadTimeout = 2; + Settings newSettings = Settings.builder() + .put("cluster.remote_store.state.metadata_manifest.upload_timeout", metadataManifestUploadTimeout + "s") + .build(); + clusterSettings.applySettings(newSettings); + + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); + + ArgumentCaptor> actionListenerArgumentCaptor = ArgumentCaptor.forClass(ActionListener.class); + + doAnswer((i) -> { + // For Manifest file perform No Op, so latch in code will timeout + return null; + }).when(container).asyncBlobUpload(any(WriteContext.class), actionListenerArgumentCaptor.capture()); + + remoteClusterStateService.start(); + RemoteClusterStateService spiedService = spy(remoteClusterStateService); + when( + spiedService.writeMetadataInParallel( + any(), + anyList(), + anyMap(), + anyMap(), + anyBoolean(), + anyBoolean(), + anyBoolean(), + anyBoolean(), + anyBoolean(), + anyBoolean(), + anyMap(), + anyBoolean(), + anyList(), + anyMap() + ) + ).thenReturn(new RemoteClusterStateUtils.UploadedMetadataResults()); + RemoteStateTransferException ex = expectThrows( + RemoteStateTransferException.class, + () -> spiedService.writeFullMetadata(clusterState, randomAlphaOfLength(10)) + ); + assertTrue(ex.getMessage().contains("Timed out waiting for transfer of following metadata to complete")); + } + public void testWriteFullMetadataInParallelFailureForIndexMetadata() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); @@ -541,6 +591,155 @@ public void testFailWriteIncrementalMetadataWhenTermChanged() { ); } + public void testWriteIncrementalMetadataSuccess() throws IOException { + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + mockBlobStoreObjects(); + final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); + final ClusterState previousClusterState = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().coordinationMetadata(coordinationMetadata)) + .build(); + + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); + + remoteClusterStateService.start(); + final RemoteClusterStateService rcssSpy = Mockito.spy(remoteClusterStateService); + final RemoteClusterStateManifestInfo manifestInfo = rcssSpy.writeIncrementalMetadata( + previousClusterState, + clusterState, + previousManifest + ); + final ClusterMetadataManifest manifest = manifestInfo.getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); + final List indices = List.of(uploadedIndexMetadata); + + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(indices) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .build(); + + Mockito.verify(rcssSpy) + .writeMetadataInParallel( + eq(clusterState), + eq(new ArrayList(clusterState.metadata().indices().values())), + eq(Collections.singletonMap(indices.get(0).getIndexName(), null)), + eq(clusterState.metadata().customs()), + eq(true), + eq(true), + eq(true), + eq(false), + eq(false), + eq(false), + eq(Collections.emptyMap()), + eq(false), + eq(Collections.emptyList()), + eq(Collections.emptyMap()) + ); + + assertThat(manifestInfo.getManifestFileName(), notNullValue()); + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); + assertThat(manifest.getIndices().get(0).getIndexUUID(), is(uploadedIndexMetadata.getIndexUUID())); + assertThat(manifest.getIndices().get(0).getUploadedFilename(), notNullValue()); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getHashesOfConsistentSettings(), nullValue()); + assertThat(manifest.getDiscoveryNodesMetadata(), nullValue()); + assertThat(manifest.getClusterBlocksMetadata(), nullValue()); + assertThat(manifest.getClusterStateCustomMap(), anEmptyMap()); + assertThat(manifest.getTransientSettingsMetadata(), nullValue()); + assertThat(manifest.getTemplatesMetadata(), notNullValue()); + assertThat(manifest.getCoordinationMetadata(), notNullValue()); + assertThat(manifest.getCustomMetadataMap().size(), is(2)); + assertThat(manifest.getIndicesRouting().size(), is(0)); + } + + public void testWriteIncrementalMetadataSuccessWhenPublicationEnabled() throws IOException { + publicationEnabled = true; + Settings nodeSettings = Settings.builder().put(REMOTE_PUBLICATION_EXPERIMENTAL, publicationEnabled).build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + remoteClusterStateService = new RemoteClusterStateService( + "test-node-id", + repositoriesServiceSupplier, + settings, + clusterService, + () -> 0L, + threadPool, + List.of(new RemoteIndexPathUploader(threadPool, settings, repositoriesServiceSupplier, clusterSettings)), + writableRegistry() + ); + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + mockBlobStoreObjects(); + final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); + final ClusterState previousClusterState = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().coordinationMetadata(coordinationMetadata)) + .build(); + + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(Collections.emptyList()).build(); + + remoteClusterStateService.start(); + final RemoteClusterStateService rcssSpy = Mockito.spy(remoteClusterStateService); + final RemoteClusterStateManifestInfo manifestInfo = rcssSpy.writeIncrementalMetadata( + previousClusterState, + clusterState, + previousManifest + ); + final ClusterMetadataManifest manifest = manifestInfo.getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); + final List indices = List.of(uploadedIndexMetadata); + + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(indices) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .build(); + + Mockito.verify(rcssSpy) + .writeMetadataInParallel( + eq(clusterState), + eq(new ArrayList(clusterState.metadata().indices().values())), + eq(Collections.singletonMap(indices.get(0).getIndexName(), null)), + eq(clusterState.metadata().customs()), + eq(true), + eq(true), + eq(true), + eq(true), + eq(false), + eq(false), + eq(Collections.emptyMap()), + eq(true), + anyList(), + eq(Collections.emptyMap()) + ); + + assertThat(manifestInfo.getManifestFileName(), notNullValue()); + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getIndices().get(0).getIndexName(), is(uploadedIndexMetadata.getIndexName())); + assertThat(manifest.getIndices().get(0).getIndexUUID(), is(uploadedIndexMetadata.getIndexUUID())); + assertThat(manifest.getIndices().get(0).getUploadedFilename(), notNullValue()); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getHashesOfConsistentSettings(), notNullValue()); + assertThat(manifest.getDiscoveryNodesMetadata(), notNullValue()); + assertThat(manifest.getClusterBlocksMetadata(), nullValue()); + assertThat(manifest.getClusterStateCustomMap(), anEmptyMap()); + assertThat(manifest.getTransientSettingsMetadata(), nullValue()); + assertThat(manifest.getTemplatesMetadata(), notNullValue()); + assertThat(manifest.getCoordinationMetadata(), notNullValue()); + assertThat(manifest.getCustomMetadataMap().size(), is(2)); + assertThat(manifest.getIndicesRouting().size(), is(1)); + } + public void testTimeoutWhileWritingMetadata() throws IOException { AsyncMultiStreamBlobContainer container = (AsyncMultiStreamBlobContainer) mockBlobStoreObjects(AsyncMultiStreamBlobContainer.class); doNothing().when(container).asyncBlobUpload(any(), any()); diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java index b9c55dfacfc5b..d042a3eeb57c1 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java @@ -42,6 +42,7 @@ import static org.opensearch.cluster.ClusterState.EMPTY_STATE; import static org.opensearch.core.common.transport.TransportAddress.META_ADDRESS; import static org.opensearch.gateway.remote.model.RemoteClusterBlocksTests.randomClusterBlocks; +import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V3; public class ClusterStateDiffManifestTests extends OpenSearchTestCase { @@ -114,7 +115,7 @@ public void testClusterStateDiffManifestXContent() throws IOException { diffManifest.toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { - final ClusterStateDiffManifest parsedManifest = ClusterStateDiffManifest.fromXContent(parser, 0); + final ClusterStateDiffManifest parsedManifest = ClusterStateDiffManifest.fromXContent(parser, CODEC_V3); assertEquals(diffManifest, parsedManifest); } } From 2c9bf5033cf4cc1951283c7d4ca5f437d6bac07a Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Sun, 21 Jul 2024 15:53:33 +0530 Subject: [PATCH 05/18] Address comments. Signed-off-by: Shailendra Singh --- CHANGELOG.md | 1 + .../routing/remote/InternalRemoteRoutingTableService.java | 3 ++- .../cluster/routing/remote/NoopRemoteRoutingTableService.java | 2 ++ .../opensearch/gateway/remote/RemoteClusterStateService.java | 4 +--- .../gateway/remote/model/ClusterStateDiffManifestTests.java | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0417cc14ee86f..980f60a5287fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add matchesPluginSystemIndexPattern to SystemIndexRegistry ([#14750](https://github.com/opensearch-project/OpenSearch/pull/14750)) - Add Plugin interface for loading application based configuration templates (([#14659](https://github.com/opensearch-project/OpenSearch/issues/14659))) - Refactor remote-routing-table service inline with remote state interfaces([#14668](https://github.com/opensearch-project/OpenSearch/pull/14668)) +- Add shard-diff path to diff manifest to reduce number of read calls remote store (([#14684](https://github.com/opensearch-project/OpenSearch/pull/14684))) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java index d4b8870263c62..8997c6367cb36 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java @@ -137,6 +137,7 @@ public CheckedRunnable getAsyncIndexRoutingWriteAction( return () -> remoteIndexRoutingTableStore.writeAsync(remoteIndexRoutingTable, completionListener); } + @Override public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( String clusterUUID, long term, @@ -203,7 +204,7 @@ public CheckedRunnable getAsyncIndexRoutingReadAction( return () -> remoteIndexRoutingTableStore.readAsync(remoteIndexRoutingTable, actionListener); } - // @Override + @Override public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java index a90b481858d6a..69be4bcb094f6 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java @@ -57,6 +57,7 @@ public CheckedRunnable getAsyncIndexRoutingWriteAction( return () -> {}; } + @Override public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( String clusterUUID, long term, @@ -88,6 +89,7 @@ public CheckedRunnable getAsyncIndexRoutingReadAction( return () -> {}; } + @Override public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, 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 9efe4eb9c41c0..d16a5a4cac21f 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -246,9 +246,7 @@ public RemoteClusterStateManifestInfo writeFullMetadata(ClusterState clusterStat clusterState, ClusterState.EMPTY_STATE, null, - uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata != null - ? uploadedMetadataResults.uploadedIndicesRoutingDiffMetadata.getUploadedFilename() - : null + null ); final RemoteClusterStateManifestInfo manifestDetails = remoteManifestManager.uploadManifest( clusterState, diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java index d042a3eeb57c1..69eb6f2a9f6b8 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java @@ -41,8 +41,8 @@ import static org.opensearch.Version.CURRENT; import static org.opensearch.cluster.ClusterState.EMPTY_STATE; import static org.opensearch.core.common.transport.TransportAddress.META_ADDRESS; -import static org.opensearch.gateway.remote.model.RemoteClusterBlocksTests.randomClusterBlocks; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V3; +import static org.opensearch.gateway.remote.model.RemoteClusterBlocksTests.randomClusterBlocks; public class ClusterStateDiffManifestTests extends OpenSearchTestCase { From cef920fe096a5ef2b8359141203108945e95425e Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 00:00:16 +0530 Subject: [PATCH 06/18] Add UTs for write Signed-off-by: Shailendra Singh --- .../remote/ClusterMetadataManifestTests.java | 71 +++++++++ .../RemoteClusterStateServiceTests.java | 150 ++++++++++++++++++ 2 files changed, 221 insertions(+) diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index f2680f5e1110f..4f7f5c0d35ccf 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -10,9 +10,11 @@ import org.opensearch.Version; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.metadata.IndexGraveyard; import org.opensearch.cluster.metadata.RepositoriesMetadata; import org.opensearch.cluster.metadata.WeightedRoutingMetadata; +import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.NamedWriteableRegistry; @@ -29,9 +31,12 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.function.Function; import java.util.stream.Collectors; +import org.mockito.Mockito; + import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V0; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V1; import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_BLOCKS; @@ -518,6 +523,72 @@ public void testClusterMetadataManifestXContentV2() throws IOException { } } + public void testClusterMetadataManifestXContentV3() throws IOException { + UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "test-uuid", "/test/upload/path"); + UploadedMetadataAttribute uploadedMetadataAttribute = new UploadedMetadataAttribute("attribute_name", "testing_attribute"); + final DiffableUtils.MapDiff> routingTableIncrementalDiff = Mockito.mock( + DiffableUtils.MapDiff.class + ); + ClusterMetadataManifest originalManifest = ClusterMetadataManifest.builder() + .clusterTerm(1L) + .stateVersion(1L) + .clusterUUID("test-cluster-uuid") + .stateUUID("test-state-uuid") + .opensearchVersion(Version.CURRENT) + .nodeId("test-node-id") + .committed(false) + .codecVersion(ClusterMetadataManifest.CODEC_V3) + .indices(Collections.singletonList(uploadedIndexMetadata)) + .previousClusterUUID("prev-cluster-uuid") + .clusterUUIDCommitted(true) + .coordinationMetadata(uploadedMetadataAttribute) + .settingMetadata(uploadedMetadataAttribute) + .templatesMetadata(uploadedMetadataAttribute) + .customMetadataMap( + Collections.unmodifiableList( + Arrays.asList( + new UploadedMetadataAttribute( + CUSTOM_METADATA + CUSTOM_DELIMITER + RepositoriesMetadata.TYPE, + "custom--repositories-file" + ), + new UploadedMetadataAttribute( + CUSTOM_METADATA + CUSTOM_DELIMITER + IndexGraveyard.TYPE, + "custom--index_graveyard-file" + ), + new UploadedMetadataAttribute( + CUSTOM_METADATA + CUSTOM_DELIMITER + WeightedRoutingMetadata.TYPE, + "custom--weighted_routing_netadata-file" + ) + ) + ).stream().collect(Collectors.toMap(UploadedMetadataAttribute::getAttributeName, Function.identity())) + ) + .routingTableVersion(1L) + .indicesRouting(Collections.singletonList(uploadedIndexMetadata)) + .discoveryNodesMetadata(uploadedMetadataAttribute) + .clusterBlocksMetadata(uploadedMetadataAttribute) + .transientSettingsMetadata(uploadedMetadataAttribute) + .hashesOfConsistentSettings(uploadedMetadataAttribute) + .clusterStateCustomMetadataMap(Collections.emptyMap()) + .diffManifest( + new ClusterStateDiffManifest( + RemoteClusterStateServiceTests.generateClusterStateWithOneIndex().build(), + ClusterState.EMPTY_STATE, + routingTableIncrementalDiff, + uploadedMetadataAttribute.getUploadedFilename() + ) + ) + .build(); + final XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + originalManifest.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + + try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { + final ClusterMetadataManifest fromXContentManifest = ClusterMetadataManifest.fromXContent(parser); + assertEquals(originalManifest, fromXContentManifest); + } + } + public void testClusterMetadataManifestXContentV2WithoutEphemeral() throws IOException { UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "test-uuid", "/test/upload/path"); UploadedMetadataAttribute uploadedMetadataAttribute = new UploadedMetadataAttribute("attribute_name", "testing_attribute"); 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 a38b5ae9dc7a1..f45863f69bd78 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -2747,6 +2747,108 @@ public void testWriteIncrementalMetadataSuccessWithRoutingTable() throws IOExcep assertThat(manifest.getIndicesRouting().get(0).getUploadedFilename(), notNullValue()); } + public void testWriteIncrementalMetadataSuccessWithRoutingTableDiff() throws IOException { + initializeRoutingTable(); + final ClusterState clusterState = generateClusterStateWithOneIndex("test-index", 5, 1, false).nodes( + nodesWithLocalNodeClusterManager() + ).build(); + mockBlobStoreObjects(); + List indices = new ArrayList<>(); + final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( + "test-index", + "index-uuid", + "routing-filename", + INDEX_ROUTING_METADATA_PREFIX + ); + indices.add(uploadedIndiceRoutingMetadata); + final ClusterState previousClusterState = generateClusterStateWithOneIndex("test-index", 5, 1, true).nodes( + nodesWithLocalNodeClusterManager() + ).build(); + + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(indices).build(); + when((blobStoreRepository.basePath())).thenReturn(BlobPath.cleanPath().add("base-path")); + + remoteClusterStateService.start(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeIncrementalMetadata( + previousClusterState, + clusterState, + previousManifest + ).getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(List.of(uploadedIndexMetadata)) + .clusterTerm(clusterState.term()) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1) + .indicesRouting(List.of(uploadedIndiceRoutingMetadata)) + .build(); + + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getRoutingTableVersion(), is(expectedManifest.getRoutingTableVersion())); + assertThat(manifest.getIndicesRouting().get(0).getIndexName(), is(uploadedIndiceRoutingMetadata.getIndexName())); + assertThat(manifest.getIndicesRouting().get(0).getIndexUUID(), is(uploadedIndiceRoutingMetadata.getIndexUUID())); + assertThat(manifest.getIndicesRouting().get(0).getUploadedFilename(), notNullValue()); + assertThat(manifest.getDiffManifest().getIndicesRoutingDiffPath(), notNullValue()); + } + + public void testWriteIncrementalMetadataSuccessWithRoutingTableDiffNull() throws IOException { + initializeRoutingTable(); + final ClusterState clusterState = generateClusterStateWithOneIndex("test-index", 5, 1, false).nodes( + nodesWithLocalNodeClusterManager() + ).build(); + mockBlobStoreObjects(); + List indices = new ArrayList<>(); + final UploadedIndexMetadata uploadedIndiceRoutingMetadata = new UploadedIndexMetadata( + "test-index", + "index-uuid", + "routing-filename", + INDEX_ROUTING_METADATA_PREFIX + ); + indices.add(uploadedIndiceRoutingMetadata); + final ClusterState previousClusterState = generateClusterStateWithOneIndex("test-index2", 5, 1, false).nodes( + nodesWithLocalNodeClusterManager() + ).build(); + + final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(indices).build(); + when((blobStoreRepository.basePath())).thenReturn(BlobPath.cleanPath().add("base-path")); + + remoteClusterStateService.start(); + final ClusterMetadataManifest manifest = remoteClusterStateService.writeIncrementalMetadata( + previousClusterState, + clusterState, + previousManifest + ).getClusterMetadataManifest(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(List.of(uploadedIndexMetadata)) + .clusterTerm(clusterState.term()) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1) + .indicesRouting(List.of(uploadedIndiceRoutingMetadata)) + .build(); + + assertThat(manifest.getIndices().size(), is(1)); + assertThat(manifest.getClusterTerm(), is(expectedManifest.getClusterTerm())); + assertThat(manifest.getStateVersion(), is(expectedManifest.getStateVersion())); + assertThat(manifest.getClusterUUID(), is(expectedManifest.getClusterUUID())); + assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); + assertThat(manifest.getRoutingTableVersion(), is(expectedManifest.getRoutingTableVersion())); + assertThat(manifest.getIndicesRouting().get(0).getIndexName(), is(uploadedIndiceRoutingMetadata.getIndexName())); + assertThat(manifest.getIndicesRouting().get(0).getIndexUUID(), is(uploadedIndiceRoutingMetadata.getIndexUUID())); + assertThat(manifest.getIndicesRouting().get(0).getUploadedFilename(), notNullValue()); + assertThat(manifest.getDiffManifest().getIndicesRoutingDiffPath(), nullValue()); + } + private void initializeRoutingTable() { Settings newSettings = Settings.builder() .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "routing_repository") @@ -3219,6 +3321,54 @@ static ClusterState.Builder generateClusterStateWithOneIndex() { .routingTable(RoutingTable.builder().addAsNew(indexMetadata).version(1L).build()); } + public static ClusterState.Builder generateClusterStateWithOneIndex( + String indexName, + int primaryShards, + int replicaShards, + boolean addAsNew + ) { + + final Index index = new Index(indexName, "index-uuid"); + final Settings idxSettings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_INDEX_UUID, index.getUUID()) + .build(); + final IndexMetadata indexMetadata = new IndexMetadata.Builder(index.getName()).settings(idxSettings) + .numberOfShards(primaryShards) + .numberOfReplicas(replicaShards) + .build(); + final CoordinationMetadata coordinationMetadata = CoordinationMetadata.builder().term(1L).build(); + final Settings settings = Settings.builder().put("mock-settings", true).build(); + final TemplatesMetadata templatesMetadata = TemplatesMetadata.builder() + .put(IndexTemplateMetadata.builder("template1").settings(idxSettings).patterns(List.of("test*")).build()) + .build(); + final CustomMetadata1 customMetadata1 = new CustomMetadata1("custom-metadata-1"); + + RoutingTable.Builder routingTableBuilder = RoutingTable.builder(); + if (addAsNew) { + routingTableBuilder.addAsNew(indexMetadata); + } else { + routingTableBuilder.addAsRecovery(indexMetadata); + } + + return ClusterState.builder(ClusterName.DEFAULT) + .version(1L) + .stateUUID("state-uuid") + .metadata( + Metadata.builder() + .version(randomNonNegativeLong()) + .put(indexMetadata, true) + .clusterUUID("cluster-uuid") + .coordinationMetadata(coordinationMetadata) + .persistentSettings(settings) + .templates(templatesMetadata) + .hashesOfConsistentSettings(Map.of("key1", "value1", "key2", "value2")) + .putCustom(customMetadata1.getWriteableName(), customMetadata1) + .build() + ) + .routingTable(routingTableBuilder.version(1L).build()); + } + static ClusterState.Builder generateClusterStateWithAllAttributes() { final Index index = new Index("test-index", "index-uuid"); final Settings idxSettings = Settings.builder() From dce3b88993105f94fce82e8a3111d256b995ff12 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 03:26:46 +0530 Subject: [PATCH 07/18] Add tests for upload/download shard diff file. Signed-off-by: Shailendra Singh --- .../RemoteRoutingTableServiceTests.java | 110 +++++++ .../RemoteClusterStateServiceTests.java | 303 ++++++++++++++++++ .../model/ClusterStateDiffManifestTests.java | 30 +- 3 files changed, 439 insertions(+), 4 deletions(-) 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 3cbcccf5f9159..2bb9664e3c445 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 @@ -12,12 +12,15 @@ import org.opensearch.action.LatchedActionListener; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.Diff; import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.coordination.CoordinationMetadata; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.blobstore.BlobContainer; import org.opensearch.common.blobstore.BlobPath; @@ -50,8 +53,11 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -69,6 +75,10 @@ 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.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE; +import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_METADATA_PREFIX; +import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_PATH_TOKEN; +import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.RemoteIndexRoutingTableDiffFormat; import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64; import static org.opensearch.index.remote.RemoteStoreEnums.PathType.HASHED_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; @@ -559,6 +569,44 @@ public void testGetAsyncIndexRoutingReadAction() throws Exception { assertEquals(clusterState.getRoutingTable().getIndicesRouting().get(indexName), indexRoutingTable); } + public void testGetAsyncIndexRoutingTableDiffReadAction() throws Exception { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + ClusterState currentState = createClusterState(indexName); + + // Get the IndexRoutingTable from the current state + IndexRoutingTable indexRoutingTable = currentState.routingTable().index(indexName); + Map shardRoutingTables = indexRoutingTable.getShards(); + + RoutingTableIncrementalDiff.IndexRoutingTableIncrementalDiff indexRoutingTableDiff = + new RoutingTableIncrementalDiff.IndexRoutingTableIncrementalDiff(new ArrayList<>(shardRoutingTables.values())); + + // Create the map for RoutingTableIncrementalDiff + Map> diffs = new HashMap<>(); + diffs.put(indexName, indexRoutingTableDiff); + + RoutingTableIncrementalDiff diff = new RoutingTableIncrementalDiff(diffs); + + String uploadedFileName = String.format(Locale.ROOT, "index-routing-diff/" + indexName); + when(blobContainer.readBlob(indexName)).thenReturn( + RemoteIndexRoutingTableDiffFormat.serialize(diff, uploadedFileName, compressor).streamInput() + ); + + TestCapturingListener listener = new TestCapturingListener<>(); + CountDownLatch latch = new CountDownLatch(1); + + remoteRoutingTableService.getAsyncIndexRoutingTableDiffReadAction( + "cluster-uuid", + uploadedFileName, + new LatchedActionListener<>(listener, latch) + ).run(); + latch.await(); + + assertNull(listener.getFailure()); + assertNotNull(listener.getResult()); + RoutingTableIncrementalDiff resultDiff = listener.getResult(); + assertEquals(diff.getDiffs().size(), resultDiff.getDiffs().size()); + } + public void testGetAsyncIndexRoutingWriteAction() throws Exception { String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); ClusterState clusterState = createClusterState(indexName); @@ -611,6 +659,68 @@ public void testGetAsyncIndexRoutingWriteAction() throws Exception { assertThat(RemoteStoreUtils.invertLong(fileNameTokens[3]), lessThanOrEqualTo(System.currentTimeMillis())); } + public void testGetAsyncIndexRoutingDiffWriteAction() throws Exception { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + ClusterState currentState = createClusterState(indexName); + + // Get the IndexRoutingTable from the current state + IndexRoutingTable indexRoutingTable = currentState.routingTable().index(indexName); + Map shardRoutingTables = indexRoutingTable.getShards(); + + RoutingTableIncrementalDiff.IndexRoutingTableIncrementalDiff indexRoutingTableDiff = + new RoutingTableIncrementalDiff.IndexRoutingTableIncrementalDiff(new ArrayList<>(shardRoutingTables.values())); + + // Create the map for RoutingTableIncrementalDiff + Map> diffs = new HashMap<>(); + diffs.put(indexName, indexRoutingTableDiff); + + // RoutingTableIncrementalDiff diff = new RoutingTableIncrementalDiff(diffs); + + Iterable remotePath = new BlobPath().add("base-path") + .add( + Base64.getUrlEncoder() + .withoutPadding() + .encodeToString(currentState.getClusterName().value().getBytes(StandardCharsets.UTF_8)) + ) + .add("cluster-state") + .add(currentState.metadata().clusterUUID()) + .add(INDEX_ROUTING_DIFF_PATH_TOKEN); + + doAnswer(invocationOnMock -> { + invocationOnMock.getArgument(4, ActionListener.class).onResponse(null); + return null; + }).when(blobStoreTransferService) + .uploadBlob(any(InputStream.class), eq(remotePath), anyString(), eq(WritePriority.URGENT), any(ActionListener.class)); + + TestCapturingListener listener = new TestCapturingListener<>(); + CountDownLatch latch = new CountDownLatch(1); + + remoteRoutingTableService.getAsyncIndexRoutingDiffWriteAction( + currentState.metadata().clusterUUID(), + currentState.term(), + currentState.version(), + diffs, + new LatchedActionListener<>(listener, latch) + ).run(); + latch.await(); + assertNull(listener.getFailure()); + assertNotNull(listener.getResult()); + ClusterMetadataManifest.UploadedMetadata uploadedMetadata = listener.getResult(); + + assertEquals(INDEX_ROUTING_DIFF_FILE, uploadedMetadata.getComponent()); + String uploadedFileName = uploadedMetadata.getUploadedFilename(); + String[] pathTokens = uploadedFileName.split(PATH_DELIMITER); + assertEquals(6, pathTokens.length); + assertEquals(pathTokens[0], "base-path"); + String[] fileNameTokens = pathTokens[5].split(DELIMITER); + + assertEquals(4, fileNameTokens.length); + assertEquals(INDEX_ROUTING_DIFF_METADATA_PREFIX, fileNameTokens[0]); + assertEquals(RemoteStoreUtils.invertLong(1L), fileNameTokens[1]); + assertEquals(RemoteStoreUtils.invertLong(2L), fileNameTokens[2]); + assertThat(RemoteStoreUtils.invertLong(fileNameTokens[3]), lessThanOrEqualTo(System.currentTimeMillis())); + } + public void testGetUpdatedIndexRoutingTableMetadataWhenNoChange() { List updatedIndicesRouting = new ArrayList<>(); List indicesRouting = randomUploadedIndexMetadataList(); 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 f45863f69bd78..aa1e88e6c5906 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -1734,6 +1734,309 @@ public void testReadClusterStateInParallel_Success() throws IOException { }); } + public void testReadClusterStateInParallelWithShardDiff_Success() throws IOException { + ClusterState previousClusterState = generateClusterStateWithAllAttributes().build(); + String indexFilename = "test-index-1-file__2"; + String customMetadataFilename = "test-custom3-file__1"; + String clusterStateCustomFilename = "test-cluster-state-custom3-file__1"; + // index already present in previous state + List uploadedIndexMetadataList = new ArrayList<>( + List.of(new UploadedIndexMetadata("test-index", "test-index-uuid", "test-index-file__2")) + ); + // new index to be added + List newIndicesToRead = List.of( + new UploadedIndexMetadata("test-index-1", "test-index-1-uuid", indexFilename) + ); + uploadedIndexMetadataList.addAll(newIndicesToRead); + // existing custom metadata + Map uploadedCustomMetadataMap = new HashMap<>( + Map.of( + "custom_md_1", + new UploadedMetadataAttribute("custom_md_1", "test-custom1-file__1"), + "custom_md_2", + new UploadedMetadataAttribute("custom_md_2", "test-custom2-file__1") + ) + ); + // new custom metadata to be added + Map newCustomMetadataMap = Map.of( + "custom_md_3", + new UploadedMetadataAttribute("custom_md_3", customMetadataFilename) + ); + uploadedCustomMetadataMap.putAll(newCustomMetadataMap); + // already existing cluster state customs + Map uploadedClusterStateCustomMap = new HashMap<>( + Map.of( + "custom_1", + new UploadedMetadataAttribute("custom_1", "test-cluster-state-custom1-file__1"), + "custom_2", + new UploadedMetadataAttribute("custom_2", "test-cluster-state-custom2-file__1") + ) + ); + // new customs uploaded + Map newClusterStateCustoms = Map.of( + "custom_3", + new UploadedMetadataAttribute("custom_3", clusterStateCustomFilename) + ); + uploadedClusterStateCustomMap.putAll(newClusterStateCustoms); + + ClusterMetadataManifest manifest = generateClusterMetadataManifestWithAllAttributes().indices(uploadedIndexMetadataList) + .customMetadataMap(uploadedCustomMetadataMap) + .clusterStateCustomMetadataMap(uploadedClusterStateCustomMap) + .build(); + + IndexMetadata newIndexMetadata = new IndexMetadata.Builder("test-index-1").state(IndexMetadata.State.OPEN) + .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + CustomMetadata3 customMetadata3 = new CustomMetadata3("custom_md_3"); + CoordinationMetadata updatedCoordinationMetadata = CoordinationMetadata.builder() + .term(previousClusterState.metadata().coordinationMetadata().term() + 1) + .build(); + Settings updatedPersistentSettings = Settings.builder() + .put("random_persistent_setting_" + randomAlphaOfLength(3), randomAlphaOfLength(5)) + .build(); + Settings updatedTransientSettings = Settings.builder() + .put("random_transient_setting_" + randomAlphaOfLength(3), randomAlphaOfLength(5)) + .build(); + TemplatesMetadata updatedTemplateMetadata = getTemplatesMetadata(); + DiffableStringMap updatedHashesOfConsistentSettings = getHashesOfConsistentSettings(); + DiscoveryNodes updatedDiscoveryNodes = getDiscoveryNodes(); + ClusterBlocks updatedClusterBlocks = randomClusterBlocks(); + TestClusterStateCustom3 updatedClusterStateCustom3 = new TestClusterStateCustom3("custom_3"); + + RemoteIndexMetadataManager mockedIndexManager = mock(RemoteIndexMetadataManager.class); + RemoteGlobalMetadataManager mockedGlobalMetadataManager = mock(RemoteGlobalMetadataManager.class); + RemoteClusterStateAttributesManager mockedClusterStateAttributeManager = mock(RemoteClusterStateAttributesManager.class); + + when( + mockedIndexManager.getAsyncIndexMetadataReadAction( + eq(manifest.getClusterUUID()), + eq(indexFilename), + any(LatchedActionListener.class) + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(newIndexMetadata, INDEX, "test-index-1") + ); + }); + when( + mockedGlobalMetadataManager.getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(customMetadataFilename)), + eq("custom_md_3"), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(customMetadata3, CUSTOM_METADATA, "custom_md_3") + ); + }); + when( + mockedGlobalMetadataManager.getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(COORDINATION_METADATA_FILENAME)), + eq(COORDINATION_METADATA), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedCoordinationMetadata, COORDINATION_METADATA, COORDINATION_METADATA) + ); + }); + when( + mockedGlobalMetadataManager.getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(PERSISTENT_SETTINGS_FILENAME)), + eq(SETTING_METADATA), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedPersistentSettings, SETTING_METADATA, SETTING_METADATA) + ); + }); + when( + mockedGlobalMetadataManager.getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(TRANSIENT_SETTINGS_FILENAME)), + eq(TRANSIENT_SETTING_METADATA), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedTransientSettings, TRANSIENT_SETTING_METADATA, TRANSIENT_SETTING_METADATA) + ); + }); + when( + mockedGlobalMetadataManager.getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(TEMPLATES_METADATA_FILENAME)), + eq(TEMPLATES_METADATA), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedTemplateMetadata, TEMPLATES_METADATA, TEMPLATES_METADATA) + ); + }); + when( + mockedGlobalMetadataManager.getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(HASHES_OF_CONSISTENT_SETTINGS_FILENAME)), + eq(HASHES_OF_CONSISTENT_SETTINGS), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedHashesOfConsistentSettings, HASHES_OF_CONSISTENT_SETTINGS, HASHES_OF_CONSISTENT_SETTINGS) + ); + }); + when( + mockedClusterStateAttributeManager.getAsyncMetadataReadAction( + eq(DISCOVERY_NODES), + argThat(new BlobNameMatcher(DISCOVERY_NODES_FILENAME)), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedDiscoveryNodes, CLUSTER_STATE_ATTRIBUTE, DISCOVERY_NODES) + ); + }); + when( + mockedClusterStateAttributeManager.getAsyncMetadataReadAction( + eq(CLUSTER_BLOCKS), + argThat(new BlobNameMatcher(CLUSTER_BLOCKS_FILENAME)), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult(updatedClusterBlocks, CLUSTER_STATE_ATTRIBUTE, CLUSTER_BLOCKS) + ); + }); + when( + mockedClusterStateAttributeManager.getAsyncMetadataReadAction( + eq(String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, updatedClusterStateCustom3.getWriteableName())), + argThat(new BlobNameMatcher(clusterStateCustomFilename)), + any() + ) + ).thenAnswer(invocationOnMock -> { + LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); + return (CheckedRunnable) () -> latchedActionListener.onResponse( + new RemoteReadResult( + updatedClusterStateCustom3, + CLUSTER_STATE_ATTRIBUTE, + String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, updatedClusterStateCustom3.getWriteableName()) + ) + ); + }); + + remoteClusterStateService.start(); + remoteClusterStateService.setRemoteIndexMetadataManager(mockedIndexManager); + remoteClusterStateService.setRemoteGlobalMetadataManager(mockedGlobalMetadataManager); + remoteClusterStateService.setRemoteClusterStateAttributesManager(mockedClusterStateAttributeManager); + + ClusterState updatedClusterState = remoteClusterStateService.readClusterStateInParallel( + previousClusterState, + manifest, + manifest.getClusterUUID(), + NODE_ID, + newIndicesToRead, + newCustomMetadataMap, + true, + true, + true, + true, + true, + true, + emptyList(), + true, + newClusterStateCustoms, + false, + true + ); + + assertEquals(uploadedIndexMetadataList.size(), updatedClusterState.metadata().indices().size()); + assertTrue(updatedClusterState.metadata().indices().containsKey("test-index-1")); + assertEquals(newIndexMetadata, updatedClusterState.metadata().index(newIndexMetadata.getIndex())); + uploadedCustomMetadataMap.keySet().forEach(key -> assertTrue(updatedClusterState.metadata().customs().containsKey(key))); + assertEquals(customMetadata3, updatedClusterState.metadata().custom(customMetadata3.getWriteableName())); + assertEquals( + previousClusterState.metadata().coordinationMetadata().term() + 1, + updatedClusterState.metadata().coordinationMetadata().term() + ); + assertEquals(updatedPersistentSettings, updatedClusterState.metadata().persistentSettings()); + assertEquals(updatedTransientSettings, updatedClusterState.metadata().transientSettings()); + assertEquals(updatedTemplateMetadata.getTemplates(), updatedClusterState.metadata().templates()); + assertEquals(updatedHashesOfConsistentSettings, updatedClusterState.metadata().hashesOfConsistentSettings()); + assertEquals(updatedDiscoveryNodes.getSize(), updatedClusterState.getNodes().getSize()); + updatedDiscoveryNodes.getNodes().forEach((nodeId, node) -> assertEquals(updatedClusterState.getNodes().get(nodeId), node)); + assertEquals(updatedDiscoveryNodes.getClusterManagerNodeId(), updatedClusterState.getNodes().getClusterManagerNodeId()); + assertEquals(updatedClusterBlocks, updatedClusterState.blocks()); + uploadedClusterStateCustomMap.keySet().forEach(key -> assertTrue(updatedClusterState.customs().containsKey(key))); + assertEquals(updatedClusterStateCustom3, updatedClusterState.custom("custom_3")); + newIndicesToRead.forEach( + uploadedIndexMetadata -> verify(mockedIndexManager, times(1)).getAsyncIndexMetadataReadAction( + eq(previousClusterState.getMetadata().clusterUUID()), + eq(uploadedIndexMetadata.getUploadedFilename()), + any() + ) + ); + verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(COORDINATION_METADATA_FILENAME)), + eq(COORDINATION_METADATA), + any() + ); + verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(PERSISTENT_SETTINGS_FILENAME)), + eq(SETTING_METADATA), + any() + ); + verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(TRANSIENT_SETTINGS_FILENAME)), + eq(TRANSIENT_SETTING_METADATA), + any() + ); + verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(TEMPLATES_METADATA_FILENAME)), + eq(TEMPLATES_METADATA), + any() + ); + verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(HASHES_OF_CONSISTENT_SETTINGS_FILENAME)), + eq(HASHES_OF_CONSISTENT_SETTINGS), + any() + ); + newCustomMetadataMap.keySet().forEach(uploadedCustomMetadataKey -> { + verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( + argThat(new BlobNameMatcher(newCustomMetadataMap.get(uploadedCustomMetadataKey).getUploadedFilename())), + eq(uploadedCustomMetadataKey), + any() + ); + }); + verify(mockedClusterStateAttributeManager, times(1)).getAsyncMetadataReadAction( + eq(DISCOVERY_NODES), + argThat(new BlobNameMatcher(DISCOVERY_NODES_FILENAME)), + any() + ); + verify(mockedClusterStateAttributeManager, times(1)).getAsyncMetadataReadAction( + eq(CLUSTER_BLOCKS), + argThat(new BlobNameMatcher(CLUSTER_BLOCKS_FILENAME)), + any() + ); + newClusterStateCustoms.keySet().forEach(uploadedClusterStateCustomMetadataKey -> { + verify(mockedClusterStateAttributeManager, times(1)).getAsyncMetadataReadAction( + eq(String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, uploadedClusterStateCustomMetadataKey)), + argThat(new BlobNameMatcher(newClusterStateCustoms.get(uploadedClusterStateCustomMetadataKey).getUploadedFilename())), + any() + ); + }); + } + /* * Here we will verify the migration of manifest file from codec V0. * diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java index 69eb6f2a9f6b8..b1e3a460a83b2 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java @@ -10,6 +10,7 @@ import org.opensearch.Version; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DiffableUtils; import org.opensearch.cluster.coordination.CoordinationMetadata; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexTemplateMetadata; @@ -17,6 +18,7 @@ import org.opensearch.cluster.metadata.TemplatesMetadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.common.bytes.BytesReference; @@ -33,6 +35,8 @@ import java.util.List; import java.util.Map; +import org.mockito.Mockito; + import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -47,6 +51,9 @@ public class ClusterStateDiffManifestTests extends OpenSearchTestCase { public void testClusterStateDiffManifest() { + final DiffableUtils.MapDiff> routingTableIncrementalDiff = Mockito.mock( + DiffableUtils.MapDiff.class + ); ClusterState initialState = ClusterState.builder(EMPTY_STATE) .metadata( Metadata.builder() @@ -78,11 +85,16 @@ public void testClusterStateDiffManifest() { randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + randomBoolean(), + "indicesRoutingDiffPath", + routingTableIncrementalDiff ); } public void testClusterStateDiffManifestXContent() throws IOException { + final DiffableUtils.MapDiff> routingTableIncrementalDiff = Mockito.mock( + DiffableUtils.MapDiff.class + ); ClusterState initialState = ClusterState.builder(EMPTY_STATE) .metadata( Metadata.builder() @@ -108,7 +120,9 @@ public void testClusterStateDiffManifestXContent() throws IOException { true, true, true, - true + true, + "indicesRoutingDiffPath", + routingTableIncrementalDiff ); final XContentBuilder builder = JsonXContent.contentBuilder(); builder.startObject(); @@ -134,7 +148,9 @@ private ClusterStateDiffManifest updateAndVerifyState( boolean updateTransientSettings, boolean updateDiscoveryNodes, boolean updateClusterBlocks, - boolean updateHashesOfConsistentSettings + boolean updateHashesOfConsistentSettings, + String indicesRoutingDiffPath, + DiffableUtils.MapDiff> routingTableIncrementalDiff ) { ClusterState.Builder clusterStateBuilder = ClusterState.builder(initialState); Metadata.Builder metadataBuilder = Metadata.builder(initialState.metadata()); @@ -192,7 +208,13 @@ private ClusterStateDiffManifest updateAndVerifyState( } ClusterState updatedClusterState = clusterStateBuilder.metadata(metadataBuilder.build()).build(); - ClusterStateDiffManifest manifest = new ClusterStateDiffManifest(updatedClusterState, initialState, null, null); + ClusterStateDiffManifest manifest = new ClusterStateDiffManifest( + updatedClusterState, + initialState, + routingTableIncrementalDiff, + indicesRoutingDiffPath + ); + assertEquals(indicesRoutingDiffPath, manifest.getIndicesRoutingDiffPath()); assertEquals(indicesToAdd.stream().map(im -> im.getIndex().getName()).collect(toList()), manifest.getIndicesUpdated()); assertEquals(indicesToRemove, manifest.getIndicesDeleted()); assertEquals(new ArrayList<>(customsToAdd.keySet()), manifest.getCustomMetadataUpdated()); From 48170997ffbe05af03ed235b5b381166344e3a56 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 08:44:18 +0530 Subject: [PATCH 08/18] Remove unwanted UT. Signed-off-by: Shailendra Singh --- .../RemoteClusterStateServiceTests.java | 303 ------------------ 1 file changed, 303 deletions(-) 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 aa1e88e6c5906..f45863f69bd78 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -1734,309 +1734,6 @@ public void testReadClusterStateInParallel_Success() throws IOException { }); } - public void testReadClusterStateInParallelWithShardDiff_Success() throws IOException { - ClusterState previousClusterState = generateClusterStateWithAllAttributes().build(); - String indexFilename = "test-index-1-file__2"; - String customMetadataFilename = "test-custom3-file__1"; - String clusterStateCustomFilename = "test-cluster-state-custom3-file__1"; - // index already present in previous state - List uploadedIndexMetadataList = new ArrayList<>( - List.of(new UploadedIndexMetadata("test-index", "test-index-uuid", "test-index-file__2")) - ); - // new index to be added - List newIndicesToRead = List.of( - new UploadedIndexMetadata("test-index-1", "test-index-1-uuid", indexFilename) - ); - uploadedIndexMetadataList.addAll(newIndicesToRead); - // existing custom metadata - Map uploadedCustomMetadataMap = new HashMap<>( - Map.of( - "custom_md_1", - new UploadedMetadataAttribute("custom_md_1", "test-custom1-file__1"), - "custom_md_2", - new UploadedMetadataAttribute("custom_md_2", "test-custom2-file__1") - ) - ); - // new custom metadata to be added - Map newCustomMetadataMap = Map.of( - "custom_md_3", - new UploadedMetadataAttribute("custom_md_3", customMetadataFilename) - ); - uploadedCustomMetadataMap.putAll(newCustomMetadataMap); - // already existing cluster state customs - Map uploadedClusterStateCustomMap = new HashMap<>( - Map.of( - "custom_1", - new UploadedMetadataAttribute("custom_1", "test-cluster-state-custom1-file__1"), - "custom_2", - new UploadedMetadataAttribute("custom_2", "test-cluster-state-custom2-file__1") - ) - ); - // new customs uploaded - Map newClusterStateCustoms = Map.of( - "custom_3", - new UploadedMetadataAttribute("custom_3", clusterStateCustomFilename) - ); - uploadedClusterStateCustomMap.putAll(newClusterStateCustoms); - - ClusterMetadataManifest manifest = generateClusterMetadataManifestWithAllAttributes().indices(uploadedIndexMetadataList) - .customMetadataMap(uploadedCustomMetadataMap) - .clusterStateCustomMetadataMap(uploadedClusterStateCustomMap) - .build(); - - IndexMetadata newIndexMetadata = new IndexMetadata.Builder("test-index-1").state(IndexMetadata.State.OPEN) - .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build()) - .numberOfShards(1) - .numberOfReplicas(1) - .build(); - CustomMetadata3 customMetadata3 = new CustomMetadata3("custom_md_3"); - CoordinationMetadata updatedCoordinationMetadata = CoordinationMetadata.builder() - .term(previousClusterState.metadata().coordinationMetadata().term() + 1) - .build(); - Settings updatedPersistentSettings = Settings.builder() - .put("random_persistent_setting_" + randomAlphaOfLength(3), randomAlphaOfLength(5)) - .build(); - Settings updatedTransientSettings = Settings.builder() - .put("random_transient_setting_" + randomAlphaOfLength(3), randomAlphaOfLength(5)) - .build(); - TemplatesMetadata updatedTemplateMetadata = getTemplatesMetadata(); - DiffableStringMap updatedHashesOfConsistentSettings = getHashesOfConsistentSettings(); - DiscoveryNodes updatedDiscoveryNodes = getDiscoveryNodes(); - ClusterBlocks updatedClusterBlocks = randomClusterBlocks(); - TestClusterStateCustom3 updatedClusterStateCustom3 = new TestClusterStateCustom3("custom_3"); - - RemoteIndexMetadataManager mockedIndexManager = mock(RemoteIndexMetadataManager.class); - RemoteGlobalMetadataManager mockedGlobalMetadataManager = mock(RemoteGlobalMetadataManager.class); - RemoteClusterStateAttributesManager mockedClusterStateAttributeManager = mock(RemoteClusterStateAttributesManager.class); - - when( - mockedIndexManager.getAsyncIndexMetadataReadAction( - eq(manifest.getClusterUUID()), - eq(indexFilename), - any(LatchedActionListener.class) - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(newIndexMetadata, INDEX, "test-index-1") - ); - }); - when( - mockedGlobalMetadataManager.getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(customMetadataFilename)), - eq("custom_md_3"), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(customMetadata3, CUSTOM_METADATA, "custom_md_3") - ); - }); - when( - mockedGlobalMetadataManager.getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(COORDINATION_METADATA_FILENAME)), - eq(COORDINATION_METADATA), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedCoordinationMetadata, COORDINATION_METADATA, COORDINATION_METADATA) - ); - }); - when( - mockedGlobalMetadataManager.getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(PERSISTENT_SETTINGS_FILENAME)), - eq(SETTING_METADATA), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedPersistentSettings, SETTING_METADATA, SETTING_METADATA) - ); - }); - when( - mockedGlobalMetadataManager.getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(TRANSIENT_SETTINGS_FILENAME)), - eq(TRANSIENT_SETTING_METADATA), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedTransientSettings, TRANSIENT_SETTING_METADATA, TRANSIENT_SETTING_METADATA) - ); - }); - when( - mockedGlobalMetadataManager.getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(TEMPLATES_METADATA_FILENAME)), - eq(TEMPLATES_METADATA), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedTemplateMetadata, TEMPLATES_METADATA, TEMPLATES_METADATA) - ); - }); - when( - mockedGlobalMetadataManager.getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(HASHES_OF_CONSISTENT_SETTINGS_FILENAME)), - eq(HASHES_OF_CONSISTENT_SETTINGS), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedHashesOfConsistentSettings, HASHES_OF_CONSISTENT_SETTINGS, HASHES_OF_CONSISTENT_SETTINGS) - ); - }); - when( - mockedClusterStateAttributeManager.getAsyncMetadataReadAction( - eq(DISCOVERY_NODES), - argThat(new BlobNameMatcher(DISCOVERY_NODES_FILENAME)), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedDiscoveryNodes, CLUSTER_STATE_ATTRIBUTE, DISCOVERY_NODES) - ); - }); - when( - mockedClusterStateAttributeManager.getAsyncMetadataReadAction( - eq(CLUSTER_BLOCKS), - argThat(new BlobNameMatcher(CLUSTER_BLOCKS_FILENAME)), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult(updatedClusterBlocks, CLUSTER_STATE_ATTRIBUTE, CLUSTER_BLOCKS) - ); - }); - when( - mockedClusterStateAttributeManager.getAsyncMetadataReadAction( - eq(String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, updatedClusterStateCustom3.getWriteableName())), - argThat(new BlobNameMatcher(clusterStateCustomFilename)), - any() - ) - ).thenAnswer(invocationOnMock -> { - LatchedActionListener latchedActionListener = invocationOnMock.getArgument(2, LatchedActionListener.class); - return (CheckedRunnable) () -> latchedActionListener.onResponse( - new RemoteReadResult( - updatedClusterStateCustom3, - CLUSTER_STATE_ATTRIBUTE, - String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, updatedClusterStateCustom3.getWriteableName()) - ) - ); - }); - - remoteClusterStateService.start(); - remoteClusterStateService.setRemoteIndexMetadataManager(mockedIndexManager); - remoteClusterStateService.setRemoteGlobalMetadataManager(mockedGlobalMetadataManager); - remoteClusterStateService.setRemoteClusterStateAttributesManager(mockedClusterStateAttributeManager); - - ClusterState updatedClusterState = remoteClusterStateService.readClusterStateInParallel( - previousClusterState, - manifest, - manifest.getClusterUUID(), - NODE_ID, - newIndicesToRead, - newCustomMetadataMap, - true, - true, - true, - true, - true, - true, - emptyList(), - true, - newClusterStateCustoms, - false, - true - ); - - assertEquals(uploadedIndexMetadataList.size(), updatedClusterState.metadata().indices().size()); - assertTrue(updatedClusterState.metadata().indices().containsKey("test-index-1")); - assertEquals(newIndexMetadata, updatedClusterState.metadata().index(newIndexMetadata.getIndex())); - uploadedCustomMetadataMap.keySet().forEach(key -> assertTrue(updatedClusterState.metadata().customs().containsKey(key))); - assertEquals(customMetadata3, updatedClusterState.metadata().custom(customMetadata3.getWriteableName())); - assertEquals( - previousClusterState.metadata().coordinationMetadata().term() + 1, - updatedClusterState.metadata().coordinationMetadata().term() - ); - assertEquals(updatedPersistentSettings, updatedClusterState.metadata().persistentSettings()); - assertEquals(updatedTransientSettings, updatedClusterState.metadata().transientSettings()); - assertEquals(updatedTemplateMetadata.getTemplates(), updatedClusterState.metadata().templates()); - assertEquals(updatedHashesOfConsistentSettings, updatedClusterState.metadata().hashesOfConsistentSettings()); - assertEquals(updatedDiscoveryNodes.getSize(), updatedClusterState.getNodes().getSize()); - updatedDiscoveryNodes.getNodes().forEach((nodeId, node) -> assertEquals(updatedClusterState.getNodes().get(nodeId), node)); - assertEquals(updatedDiscoveryNodes.getClusterManagerNodeId(), updatedClusterState.getNodes().getClusterManagerNodeId()); - assertEquals(updatedClusterBlocks, updatedClusterState.blocks()); - uploadedClusterStateCustomMap.keySet().forEach(key -> assertTrue(updatedClusterState.customs().containsKey(key))); - assertEquals(updatedClusterStateCustom3, updatedClusterState.custom("custom_3")); - newIndicesToRead.forEach( - uploadedIndexMetadata -> verify(mockedIndexManager, times(1)).getAsyncIndexMetadataReadAction( - eq(previousClusterState.getMetadata().clusterUUID()), - eq(uploadedIndexMetadata.getUploadedFilename()), - any() - ) - ); - verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(COORDINATION_METADATA_FILENAME)), - eq(COORDINATION_METADATA), - any() - ); - verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(PERSISTENT_SETTINGS_FILENAME)), - eq(SETTING_METADATA), - any() - ); - verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(TRANSIENT_SETTINGS_FILENAME)), - eq(TRANSIENT_SETTING_METADATA), - any() - ); - verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(TEMPLATES_METADATA_FILENAME)), - eq(TEMPLATES_METADATA), - any() - ); - verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(HASHES_OF_CONSISTENT_SETTINGS_FILENAME)), - eq(HASHES_OF_CONSISTENT_SETTINGS), - any() - ); - newCustomMetadataMap.keySet().forEach(uploadedCustomMetadataKey -> { - verify(mockedGlobalMetadataManager, times(1)).getAsyncMetadataReadAction( - argThat(new BlobNameMatcher(newCustomMetadataMap.get(uploadedCustomMetadataKey).getUploadedFilename())), - eq(uploadedCustomMetadataKey), - any() - ); - }); - verify(mockedClusterStateAttributeManager, times(1)).getAsyncMetadataReadAction( - eq(DISCOVERY_NODES), - argThat(new BlobNameMatcher(DISCOVERY_NODES_FILENAME)), - any() - ); - verify(mockedClusterStateAttributeManager, times(1)).getAsyncMetadataReadAction( - eq(CLUSTER_BLOCKS), - argThat(new BlobNameMatcher(CLUSTER_BLOCKS_FILENAME)), - any() - ); - newClusterStateCustoms.keySet().forEach(uploadedClusterStateCustomMetadataKey -> { - verify(mockedClusterStateAttributeManager, times(1)).getAsyncMetadataReadAction( - eq(String.join(CUSTOM_DELIMITER, CLUSTER_STATE_CUSTOM, uploadedClusterStateCustomMetadataKey)), - argThat(new BlobNameMatcher(newClusterStateCustoms.get(uploadedClusterStateCustomMetadataKey).getUploadedFilename())), - any() - ); - }); - } - /* * Here we will verify the migration of manifest file from codec V0. * From 46b89eca297525e99d771e51ec184a5a684359de Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 11:50:27 +0530 Subject: [PATCH 09/18] Add UTs for Manifest diff file. Signed-off-by: Shailendra Singh --- .../remote/ClusterStateDiffManifest.java | 2 + .../RemoteClusterStateServiceTests.java | 2 +- .../model/ClusterStateDiffManifestTests.java | 90 ++++++++++++++----- 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java index 177652157fcb9..ce2d78563abb1 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java @@ -117,6 +117,7 @@ public ClusterStateDiffManifest( routingTableIncrementalDiff.getUpserts().forEach((k, v) -> indicesRoutingUpdated.add(k)); indicesRoutingDeleted.addAll(routingTableIncrementalDiff.getDeletes()); } + List indicies = indicesRoutingUpdated; hashesOfConsistentSettingsUpdated = !state.metadata() .hashesOfConsistentSettings() .equals(previousState.metadata().hashesOfConsistentSettings()); @@ -129,6 +130,7 @@ public ClusterStateDiffManifest( clusterStateCustomUpdated = new ArrayList<>(clusterStateCustomDiff.getDiffs().keySet()); clusterStateCustomUpdated.addAll(clusterStateCustomDiff.getUpserts().keySet()); clusterStateCustomDeleted = clusterStateCustomDiff.getDeletes(); + List indicie1s = indicesRoutingUpdated; } public ClusterStateDiffManifest( 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 f45863f69bd78..afa6fde5b5998 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -3448,7 +3448,7 @@ static ClusterMetadataManifest.Builder generateClusterMetadataManifestWithAllAtt ); } - static DiscoveryNodes nodesWithLocalNodeClusterManager() { + public static DiscoveryNodes nodesWithLocalNodeClusterManager() { final DiscoveryNode localNode = new DiscoveryNode("cluster-manager-id", buildNewFakeTransportAddress(), Version.CURRENT); return DiscoveryNodes.builder().clusterManagerNodeId("cluster-manager-id").localNodeId("cluster-manager-id").add(localNode).build(); } diff --git a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java index b1e3a460a83b2..f89619a09cd52 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/model/ClusterStateDiffManifestTests.java @@ -35,8 +35,6 @@ import java.util.List; import java.util.Map; -import org.mockito.Mockito; - import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -44,16 +42,16 @@ import static java.util.stream.Collectors.toList; import static org.opensearch.Version.CURRENT; import static org.opensearch.cluster.ClusterState.EMPTY_STATE; +import static org.opensearch.cluster.routing.remote.RemoteRoutingTableService.CUSTOM_ROUTING_TABLE_DIFFABLE_VALUE_SERIALIZER; import static org.opensearch.core.common.transport.TransportAddress.META_ADDRESS; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V3; +import static org.opensearch.gateway.remote.RemoteClusterStateServiceTests.generateClusterStateWithOneIndex; +import static org.opensearch.gateway.remote.RemoteClusterStateServiceTests.nodesWithLocalNodeClusterManager; import static org.opensearch.gateway.remote.model.RemoteClusterBlocksTests.randomClusterBlocks; public class ClusterStateDiffManifestTests extends OpenSearchTestCase { public void testClusterStateDiffManifest() { - final DiffableUtils.MapDiff> routingTableIncrementalDiff = Mockito.mock( - DiffableUtils.MapDiff.class - ); ClusterState initialState = ClusterState.builder(EMPTY_STATE) .metadata( Metadata.builder() @@ -85,16 +83,11 @@ public void testClusterStateDiffManifest() { randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean(), - "indicesRoutingDiffPath", - routingTableIncrementalDiff + randomBoolean() ); } public void testClusterStateDiffManifestXContent() throws IOException { - final DiffableUtils.MapDiff> routingTableIncrementalDiff = Mockito.mock( - DiffableUtils.MapDiff.class - ); ClusterState initialState = ClusterState.builder(EMPTY_STATE) .metadata( Metadata.builder() @@ -120,9 +113,7 @@ public void testClusterStateDiffManifestXContent() throws IOException { true, true, true, - true, - "indicesRoutingDiffPath", - routingTableIncrementalDiff + true ); final XContentBuilder builder = JsonXContent.contentBuilder(); builder.startObject(); @@ -134,6 +125,65 @@ public void testClusterStateDiffManifestXContent() throws IOException { } } + public void testClusterStateWithRoutingTableDiffInDiffManifestXContent() throws IOException { + ClusterState initialState = generateClusterStateWithOneIndex("test-index", 5, 1, true).nodes(nodesWithLocalNodeClusterManager()) + .build(); + + ClusterState updatedState = generateClusterStateWithOneIndex("test-index", 5, 2, false).nodes(nodesWithLocalNodeClusterManager()) + .build(); + + ClusterStateDiffManifest diffManifest = verifyRoutingTableDiffManifest(initialState, updatedState); + final XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + diffManifest.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { + final ClusterStateDiffManifest parsedManifest = ClusterStateDiffManifest.fromXContent(parser, CODEC_V3); + assertEquals(diffManifest, parsedManifest); + } + } + + public void testClusterStateWithRoutingTableDiffInDiffManifestXContent1() throws IOException { + ClusterState initialState = generateClusterStateWithOneIndex("test-index", 5, 1, true).nodes(nodesWithLocalNodeClusterManager()) + .build(); + + ClusterState updatedState = generateClusterStateWithOneIndex("test-index-1", 5, 2, false).nodes(nodesWithLocalNodeClusterManager()) + .build(); + + ClusterStateDiffManifest diffManifest = verifyRoutingTableDiffManifest(initialState, updatedState); + final XContentBuilder builder = JsonXContent.contentBuilder(); + builder.startObject(); + diffManifest.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) { + final ClusterStateDiffManifest parsedManifest = ClusterStateDiffManifest.fromXContent(parser, CODEC_V3); + assertEquals(diffManifest, parsedManifest); + } + } + + private ClusterStateDiffManifest verifyRoutingTableDiffManifest(ClusterState previousState, ClusterState currentState) { + // Create initial and updated IndexRoutingTable maps + Map initialRoutingTableMap = previousState.getRoutingTable().indicesRouting(); + Map updatedRoutingTableMap = currentState.getRoutingTable().indicesRouting(); + + DiffableUtils.MapDiff> routingTableIncrementalDiff = DiffableUtils.diff( + initialRoutingTableMap, + updatedRoutingTableMap, + DiffableUtils.getStringKeySerializer(), + CUSTOM_ROUTING_TABLE_DIFFABLE_VALUE_SERIALIZER + ); + ClusterStateDiffManifest manifest = new ClusterStateDiffManifest( + currentState, + previousState, + routingTableIncrementalDiff, + "indicesRoutingDiffPath" + ); + assertEquals("indicesRoutingDiffPath", manifest.getIndicesRoutingDiffPath()); + assertEquals(routingTableIncrementalDiff.getUpserts().size(), manifest.getIndicesRoutingUpdated().size()); + assertEquals(routingTableIncrementalDiff.getDeletes().size(), manifest.getIndicesRoutingDeleted().size()); + return manifest; + } + private ClusterStateDiffManifest updateAndVerifyState( ClusterState initialState, List indicesToAdd, @@ -148,9 +198,7 @@ private ClusterStateDiffManifest updateAndVerifyState( boolean updateTransientSettings, boolean updateDiscoveryNodes, boolean updateClusterBlocks, - boolean updateHashesOfConsistentSettings, - String indicesRoutingDiffPath, - DiffableUtils.MapDiff> routingTableIncrementalDiff + boolean updateHashesOfConsistentSettings ) { ClusterState.Builder clusterStateBuilder = ClusterState.builder(initialState); Metadata.Builder metadataBuilder = Metadata.builder(initialState.metadata()); @@ -208,13 +256,7 @@ private ClusterStateDiffManifest updateAndVerifyState( } ClusterState updatedClusterState = clusterStateBuilder.metadata(metadataBuilder.build()).build(); - ClusterStateDiffManifest manifest = new ClusterStateDiffManifest( - updatedClusterState, - initialState, - routingTableIncrementalDiff, - indicesRoutingDiffPath - ); - assertEquals(indicesRoutingDiffPath, manifest.getIndicesRoutingDiffPath()); + ClusterStateDiffManifest manifest = new ClusterStateDiffManifest(updatedClusterState, initialState, null, null); assertEquals(indicesToAdd.stream().map(im -> im.getIndex().getName()).collect(toList()), manifest.getIndicesUpdated()); assertEquals(indicesToRemove, manifest.getIndicesDeleted()); assertEquals(new ArrayList<>(customsToAdd.keySet()), manifest.getCustomMetadataUpdated()); From ba79c44f7688f0cce4dbae90c93958983b829f78 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 12:38:03 +0530 Subject: [PATCH 10/18] Add UTs for SerDe RemoteIndexRoutingTableDiff Signed-off-by: Shailendra Singh --- .../RemoteIndexRoutingTableDiffTests.java | 307 ++++++++++++++++++ 1 file changed, 307 insertions(+) create mode 100644 server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java diff --git a/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java b/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java new file mode 100644 index 0000000000000..4bba0b3764c07 --- /dev/null +++ b/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java @@ -0,0 +1,307 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.gateway.remote.routingtable; + +import org.opensearch.Version; +import org.opensearch.cluster.Diff; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.compress.DeflateCompressor; +import org.opensearch.common.remote.BlobPathParameters; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.common.io.stream.NamedWriteableRegistry; +import org.opensearch.core.compress.Compressor; +import org.opensearch.core.compress.NoneCompressor; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.index.remote.RemoteStoreUtils; +import org.opensearch.index.translog.transfer.BlobStoreTransferService; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class RemoteIndexRoutingTableDiffTests extends OpenSearchTestCase { + + private static final String TEST_BLOB_NAME = "/test-path/test-blob-name"; + private static final String TEST_BLOB_PATH = "test-path"; + private static final String TEST_BLOB_FILE_NAME = "test-blob-name"; + private static final long STATE_VERSION = 3L; + private static final long STATE_TERM = 2L; + private String clusterUUID; + private BlobStoreRepository blobStoreRepository; + private BlobStoreTransferService blobStoreTransferService; + private ClusterSettings clusterSettings; + private Compressor compressor; + + private String clusterName; + private NamedWriteableRegistry namedWriteableRegistry; + private final ThreadPool threadPool = new TestThreadPool(getClass().getName()); + + @Before + public void setup() { + clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + this.clusterUUID = "test-cluster-uuid"; + this.blobStoreTransferService = mock(BlobStoreTransferService.class); + this.blobStoreRepository = mock(BlobStoreRepository.class); + BlobPath blobPath = new BlobPath().add("/path"); + when(blobStoreRepository.basePath()).thenReturn(blobPath); + when(blobStoreRepository.getCompressor()).thenReturn(new DeflateCompressor()); + compressor = new NoneCompressor(); + namedWriteableRegistry = writableRegistry(); + this.clusterName = "test-cluster-name"; + } + + @After + public void tearDown() throws Exception { + super.tearDown(); + threadPool.shutdown(); + } + + public void testClusterUUID() { + Map> diffs = new HashMap<>(); + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + IndexMetadata indexMetadata = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + .build(); + + IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); + + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + assertEquals(remoteDiffForUpload.clusterUUID(), clusterUUID); + + RemoteIndexRoutingTableDiff remoteDiffForDownload = new RemoteIndexRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); + assertEquals(remoteDiffForDownload.clusterUUID(), clusterUUID); + } + + public void testFullBlobName() { + Map> diffs = new HashMap<>(); + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + IndexMetadata indexMetadata = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + .build(); + + IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); + + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + assertThat(remoteDiffForUpload.getFullBlobName(), nullValue()); + + RemoteIndexRoutingTableDiff remoteDiffForDownload = new RemoteIndexRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); + assertThat(remoteDiffForDownload.getFullBlobName(), is(TEST_BLOB_NAME)); + } + + public void testBlobFileName() { + Map> diffs = new HashMap<>(); + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + IndexMetadata indexMetadata = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + .build(); + + IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); + + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + assertThat(remoteDiffForUpload.getBlobFileName(), nullValue()); + + RemoteIndexRoutingTableDiff remoteDiffForDownload = new RemoteIndexRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); + assertThat(remoteDiffForDownload.getBlobFileName(), is(TEST_BLOB_FILE_NAME)); + } + + public void testBlobPathParameters() { + Map> diffs = new HashMap<>(); + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + IndexMetadata indexMetadata = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + .build(); + + IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); + + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + assertThat(remoteDiffForUpload.getBlobFileName(), nullValue()); + + BlobPathParameters params = remoteDiffForUpload.getBlobPathParameters(); + assertThat(params.getPathTokens(), is(List.of("index-routing-diff"))); + String expectedPrefix = "indexRoutingDiff--"; + assertThat(params.getFilePrefix(), is(expectedPrefix)); + } + + public void testGenerateBlobFileName() { + Map> diffs = new HashMap<>(); + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + IndexMetadata indexMetadata = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + .build(); + + IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); + + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + + String blobFileName = remoteDiffForUpload.generateBlobFileName(); + String[] nameTokens = blobFileName.split("__"); + assertEquals(nameTokens[0], "indexRoutingDiff--"); + assertEquals(nameTokens[1], RemoteStoreUtils.invertLong(STATE_TERM)); + assertEquals(nameTokens[2], RemoteStoreUtils.invertLong(STATE_VERSION)); + assertThat(RemoteStoreUtils.invertLong(nameTokens[3]), lessThanOrEqualTo(System.currentTimeMillis())); + } + + public void testGetUploadedMetadata() throws IOException { + Map> diffs = new HashMap<>(); + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + IndexMetadata indexMetadata = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + .build(); + + IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); + + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + + remoteDiffForUpload.setFullBlobName(new BlobPath().add(TEST_BLOB_PATH)); + ClusterMetadataManifest.UploadedMetadata uploadedMetadataAttribute = remoteDiffForUpload.getUploadedMetadata(); + assertEquals(INDEX_ROUTING_DIFF_FILE, uploadedMetadataAttribute.getComponent()); + } + + public void testStreamOperations() throws IOException { + String indexName = randomAlphaOfLength(randomIntBetween(1, 50)); + int numberOfShards = randomIntBetween(1, 10); + int numberOfReplicas = randomIntBetween(1, 10); + + Metadata metadata = Metadata.builder() + .put( + IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(numberOfShards) + .numberOfReplicas(numberOfReplicas) + ) + .build(); + + RoutingTable initialRoutingTable = RoutingTable.builder().addAsNew(metadata.index(indexName)).build(); + Map> diffs = new HashMap<>(); + + initialRoutingTable.getIndicesRouting().values().forEach(indexRoutingTable -> { + diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + + RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( + diffs, + clusterUUID, + compressor, + STATE_TERM, + STATE_VERSION + ); + + assertThrows(AssertionError.class, remoteDiffForUpload::getUploadedMetadata); + + try (InputStream inputStream = remoteDiffForUpload.serialize()) { + remoteDiffForUpload.setFullBlobName(BlobPath.cleanPath()); + assertThat(inputStream.available(), greaterThan(0)); + + RoutingTableIncrementalDiff routingTableIncrementalDiff = remoteDiffForUpload.deserialize(inputStream); + assertEquals(remoteDiffForUpload.getDiffs().size(), routingTableIncrementalDiff.getDiffs().size()); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } +} From 8351a4cb8134126b2a84e2df00dec7f0b57750bc Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 13:58:46 +0530 Subject: [PATCH 11/18] Remove unwanted changes Signed-off-by: Shailendra Singh --- .../gateway/remote/ClusterMetadataManifest.java | 15 +-------------- .../gateway/remote/ClusterStateDiffManifest.java | 1 - 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java index 457bcd9d2bc5f..71815b6ee324c 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterMetadataManifest.java @@ -111,20 +111,7 @@ private static ClusterMetadataManifest.Builder manifestV2Builder(Object[] fields } private static ClusterMetadataManifest.Builder manifestV3Builder(Object[] fields) { - return manifestV0Builder(fields).codecVersion(codecVersion(fields)) - .coordinationMetadata(coordinationMetadata(fields)) - .settingMetadata(settingsMetadata(fields)) - .templatesMetadata(templatesMetadata(fields)) - .customMetadataMap(customMetadata(fields)) - .routingTableVersion(routingTableVersion(fields)) - .indicesRouting(indicesRouting(fields)) - .discoveryNodesMetadata(discoveryNodesMetadata(fields)) - .clusterBlocksMetadata(clusterBlocksMetadata(fields)) - .diffManifest(diffManifest(fields)) - .metadataVersion(metadataVersion(fields)) - .transientSettingsMetadata(transientSettingsMetadata(fields)) - .hashesOfConsistentSettings(hashesOfConsistentSettings(fields)) - .clusterStateCustomMetadataMap(clusterStateCustomMetadata(fields)); + return manifestV2Builder(fields); } private static long term(Object[] fields) { diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java index ce2d78563abb1..5e14b2cdab0ac 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java @@ -117,7 +117,6 @@ public ClusterStateDiffManifest( routingTableIncrementalDiff.getUpserts().forEach((k, v) -> indicesRoutingUpdated.add(k)); indicesRoutingDeleted.addAll(routingTableIncrementalDiff.getDeletes()); } - List indicies = indicesRoutingUpdated; hashesOfConsistentSettingsUpdated = !state.metadata() .hashesOfConsistentSettings() .equals(previousState.metadata().hashesOfConsistentSettings()); From 4773bbb2af6fbbfe2ef4a52e950cb8301dfa467f Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 15:07:01 +0530 Subject: [PATCH 12/18] Add DiffManifest SerDe Test Signed-off-by: Shailendra Singh --- .../remote/ClusterStateDiffManifest.java | 8 ++++--- .../remote/ClusterMetadataManifestTests.java | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java index 5e14b2cdab0ac..ab7fa1fddf4bf 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java +++ b/server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java @@ -166,10 +166,10 @@ public ClusterStateDiffManifest( this.discoveryNodesUpdated = discoveryNodesUpdated; this.indicesRoutingUpdated = Collections.unmodifiableList(indicesRoutingUpdated); this.indicesRoutingDeleted = Collections.unmodifiableList(indicesRoutingDeleted); - this.indicesRoutingDiffPath = indicesRoutingDiffPath; this.hashesOfConsistentSettingsUpdated = hashesOfConsistentSettingsUpdated; this.clusterStateCustomUpdated = Collections.unmodifiableList(clusterStateCustomUpdated); this.clusterStateCustomDeleted = Collections.unmodifiableList(clusterStateCustomDeleted); + this.indicesRoutingDiffPath = indicesRoutingDiffPath; } public ClusterStateDiffManifest(StreamInput in) throws IOException { @@ -187,10 +187,10 @@ public ClusterStateDiffManifest(StreamInput in) throws IOException { this.discoveryNodesUpdated = in.readBoolean(); this.indicesRoutingUpdated = in.readStringList(); this.indicesRoutingDeleted = in.readStringList(); - this.indicesRoutingDiffPath = in.readString(); this.hashesOfConsistentSettingsUpdated = in.readBoolean(); this.clusterStateCustomUpdated = in.readStringList(); this.clusterStateCustomDeleted = in.readStringList(); + this.indicesRoutingDiffPath = in.readString(); } @Override @@ -512,7 +512,8 @@ public boolean equals(Object o) { && Objects.equals(indicesRoutingUpdated, that.indicesRoutingUpdated) && Objects.equals(indicesRoutingDeleted, that.indicesRoutingDeleted) && Objects.equals(clusterStateCustomUpdated, that.clusterStateCustomUpdated) - && Objects.equals(clusterStateCustomDeleted, that.clusterStateCustomDeleted); + && Objects.equals(clusterStateCustomDeleted, that.clusterStateCustomDeleted) + && Objects.equals(indicesRoutingDiffPath, that.indicesRoutingDiffPath); } @Override @@ -561,6 +562,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(hashesOfConsistentSettingsUpdated); out.writeStringCollection(clusterStateCustomUpdated); out.writeStringCollection(clusterStateCustomDeleted); + out.writeString(indicesRoutingDiffPath); } /** diff --git a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java index 4f7f5c0d35ccf..8a6dd6bc96e72 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/ClusterMetadataManifestTests.java @@ -193,6 +193,14 @@ public void testClusterMetadataManifestSerializationEqualsHashCode() { .transientSettingsMetadata(new UploadedMetadataAttribute(TRANSIENT_SETTING_METADATA, "transient-settings-file")) .hashesOfConsistentSettings(new UploadedMetadataAttribute(HASHES_OF_CONSISTENT_SETTINGS, "hashes-of-consistent-settings-file")) .clusterStateCustomMetadataMap(Collections.emptyMap()) + .diffManifest( + new ClusterStateDiffManifest( + RemoteClusterStateServiceTests.generateClusterStateWithOneIndex().build(), + ClusterState.EMPTY_STATE, + null, + "indicesRoutingDiffPath" + ) + ) .build(); { // Mutate Cluster Term EqualsHashCodeTestUtils.checkEqualsAndHashCode( @@ -442,6 +450,22 @@ public void testClusterMetadataManifestSerializationEqualsHashCode() { } ); } + { + // Mutate diff manifest + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + initialManifest, + orig -> OpenSearchTestCase.copyWriteable( + orig, + new NamedWriteableRegistry(Collections.emptyList()), + ClusterMetadataManifest::new + ), + manifest -> { + ClusterMetadataManifest.Builder builder = ClusterMetadataManifest.builder(manifest); + builder.diffManifest(null); + return builder.build(); + } + ); + } { // Mutate hashes of consistent settings EqualsHashCodeTestUtils.checkEqualsAndHashCode( From 1d521917adb236c8dac10676e328f806be4cf36d Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 17:15:58 +0530 Subject: [PATCH 13/18] Delegate responsibility to IndexShardRouting Signed-off-by: Shailendra Singh --- .../remote/RemoteRoutingTableServiceIT.java | 137 ++++++++++++++++++ .../routing/RoutingTableIncrementalDiff.java | 14 +- 2 files changed, 138 insertions(+), 13 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java new file mode 100644 index 0000000000000..a65806097f157 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java @@ -0,0 +1,137 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.gateway.remote; + +import org.junit.Before; +import org.opensearch.action.admin.cluster.state.ClusterStateRequest; +import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.settings.Settings; +import org.opensearch.gateway.remote.model.RemoteRoutingTableBlobStore; +import org.opensearch.index.remote.RemoteStoreEnums; +import org.opensearch.index.remote.RemoteStorePathStrategy; +import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase; + +import static org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService.*; +import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; +import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; + +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.nio.charset.StandardCharsets; +import java.util.*; + +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class RemoteRoutingTableServiceIT extends RemoteStoreBaseIntegTestCase { + private static String INDEX_NAME = "test-index"; + + @Before + public void setup() { + asyncUploadMockFsRepo = false; + } + + @Override + protected Settings nodeSettings(int nodeOrdinal) { + return Settings.builder().put(super.nodeSettings(nodeOrdinal)) + .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) + .put(RemoteRoutingTableBlobStore.REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING.getKey(), RemoteStoreEnums.PathType.HASHED_PREFIX.toString()) + .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, REPOSITORY_NAME) + .put(REMOTE_PUBLICATION_EXPERIMENTAL, true) + .build(); + } + + private RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.HASHED_PREFIX; + + public void testRemoteRoutingTableCreateIndex() throws Exception { + prepareCluster(1, 2, INDEX_NAME, 1, 5); + + ensureGreen(INDEX_NAME); + //createIndex("index-2", remoteStoreIndexSettings(2, 5)); + + RepositoriesService repositoriesService = internalCluster().getClusterManagerNodeInstance(RepositoriesService.class); + BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(REPOSITORY_NAME); + + BlobPath baseMetadataPath = repository.basePath() + .add( + Base64.getUrlEncoder() + .withoutPadding() + .encodeToString(getClusterState().getClusterName().value().getBytes(StandardCharsets.UTF_8)) + ) + .add("cluster-state") + .add(getClusterState().metadata().clusterUUID()); + + BlobPath indexRoutingPath = baseMetadataPath.add(INDEX_ROUTING_TABLE); + List indexRoutingTable = new ArrayList<>(getClusterState().routingTable().indicesRouting().values()); + RemoteStoreEnums.PathHashAlgorithm pathHashAlgo = RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64; + BlobPath path = pathType.path( + RemoteStorePathStrategy.PathInput.builder().basePath(indexRoutingPath).indexUUID(indexRoutingTable.get(0).getIndex().getUUID()).build(), + pathHashAlgo + ); + assertBusy(() -> { + int indexRoutingFiles = repository.blobStore().blobContainer(path).listBlobs().size(); + //assertTrue(indexRoutingFiles>0); + logger.info("number of index routing files {}", indexRoutingFiles); + }); + + String[] allNodes = internalCluster().getNodeNames(); + List routingTableVersions = new ArrayList<>(); + + // get cluster state from all the nodes + for (String node : allNodes) { + RoutingTable routingTable = internalCluster().client(node) + .admin() + .cluster() + .state(new ClusterStateRequest().local(true)) + .get().getState().routingTable(); + routingTableVersions.add(routingTable.version()); + } + assertTrue(areRoutingTableVersionsSame(routingTableVersions)); + + //updateIndexSettings(INDEX_NAME, IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2); + + assertBusy(() -> { + int indexRoutingFiles = repository.blobStore().blobContainer(path).listBlobs().size(); + //assertTrue(indexRoutingFiles>0); + logger.info("number of index routing files {}", indexRoutingFiles); + }); + } + + private void updateIndexSettings(String indexName, String settingKey, int settingValue) { + client().admin() + .indices() + .prepareUpdateSettings(indexName) + .setSettings(Settings.builder().put(settingKey, settingValue)) + .execute() + .actionGet(); + } + + private boolean areRoutingTableVersionsSame(List routingTableVersions) { + if (routingTableVersions == null || routingTableVersions.isEmpty()) { + return false; + } + + Long firstVersion = routingTableVersions.get(0); + for (Long routingTableVersion : routingTableVersions) { + if (!firstVersion.equals(routingTableVersion)) { + logger.info("Responses are not same {} {}", firstVersion, routingTableVersion); + + return false; + } + } + return true; + } +} diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java index cdb7ef39484c3..f7c1047107906 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java @@ -55,19 +55,7 @@ public static RoutingTableIncrementalDiff readFrom(StreamInput in) throws IOExce for (int i = 0; i < size; i++) { String key = in.readString(); - List shardRoutingTables = new ArrayList<>(); - - // Read each IndexShardRoutingTable from the stream - int numShards = in.readVInt(); - for (int j = 0; j < numShards; j++) { - IndexShardRoutingTable shardRoutingTable = IndexShardRoutingTable.Builder.readFrom(in); - shardRoutingTables.add(shardRoutingTable); - } - - // Create a diff object for the index - Diff diff = new IndexRoutingTableIncrementalDiff(shardRoutingTables); - - // Put the diff into the map with the key + Diff diff = IndexRoutingTableIncrementalDiff.readFrom(in); diffs.put(key, diff); } return new RoutingTableIncrementalDiff(diffs); From 5d3f31d882d815bee42e5cee4ff64293536dd9ba Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 17:27:06 +0530 Subject: [PATCH 14/18] RoutingTableIncrementalDiff implements Diff Signed-off-by: Shailendra Singh --- .../remote/RemoteRoutingTableServiceIT.java | 137 ------------------ .../routing/RoutingTableIncrementalDiff.java | 24 +-- 2 files changed, 15 insertions(+), 146 deletions(-) delete mode 100644 server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java deleted file mode 100644 index a65806097f157..0000000000000 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java +++ /dev/null @@ -1,137 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.gateway.remote; - -import org.junit.Before; -import org.opensearch.action.admin.cluster.state.ClusterStateRequest; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; -import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.routing.IndexRoutingTable; -import org.opensearch.cluster.routing.RoutingTable; -import org.opensearch.common.blobstore.BlobPath; -import org.opensearch.common.settings.Settings; -import org.opensearch.gateway.remote.model.RemoteRoutingTableBlobStore; -import org.opensearch.index.remote.RemoteStoreEnums; -import org.opensearch.index.remote.RemoteStorePathStrategy; -import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase; - -import static org.opensearch.cluster.routing.remote.InternalRemoteRoutingTableService.*; -import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; -import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE; -import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; - -import org.opensearch.repositories.RepositoriesService; -import org.opensearch.repositories.blobstore.BlobStoreRepository; -import org.opensearch.test.OpenSearchIntegTestCase; - -import java.nio.charset.StandardCharsets; -import java.util.*; - -import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; - -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) -public class RemoteRoutingTableServiceIT extends RemoteStoreBaseIntegTestCase { - private static String INDEX_NAME = "test-index"; - - @Before - public void setup() { - asyncUploadMockFsRepo = false; - } - - @Override - protected Settings nodeSettings(int nodeOrdinal) { - return Settings.builder().put(super.nodeSettings(nodeOrdinal)) - .put(REMOTE_CLUSTER_STATE_ENABLED_SETTING.getKey(), true) - .put(RemoteRoutingTableBlobStore.REMOTE_ROUTING_TABLE_PATH_TYPE_SETTING.getKey(), RemoteStoreEnums.PathType.HASHED_PREFIX.toString()) - .put("node.attr." + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, REPOSITORY_NAME) - .put(REMOTE_PUBLICATION_EXPERIMENTAL, true) - .build(); - } - - private RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.HASHED_PREFIX; - - public void testRemoteRoutingTableCreateIndex() throws Exception { - prepareCluster(1, 2, INDEX_NAME, 1, 5); - - ensureGreen(INDEX_NAME); - //createIndex("index-2", remoteStoreIndexSettings(2, 5)); - - RepositoriesService repositoriesService = internalCluster().getClusterManagerNodeInstance(RepositoriesService.class); - BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(REPOSITORY_NAME); - - BlobPath baseMetadataPath = repository.basePath() - .add( - Base64.getUrlEncoder() - .withoutPadding() - .encodeToString(getClusterState().getClusterName().value().getBytes(StandardCharsets.UTF_8)) - ) - .add("cluster-state") - .add(getClusterState().metadata().clusterUUID()); - - BlobPath indexRoutingPath = baseMetadataPath.add(INDEX_ROUTING_TABLE); - List indexRoutingTable = new ArrayList<>(getClusterState().routingTable().indicesRouting().values()); - RemoteStoreEnums.PathHashAlgorithm pathHashAlgo = RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64; - BlobPath path = pathType.path( - RemoteStorePathStrategy.PathInput.builder().basePath(indexRoutingPath).indexUUID(indexRoutingTable.get(0).getIndex().getUUID()).build(), - pathHashAlgo - ); - assertBusy(() -> { - int indexRoutingFiles = repository.blobStore().blobContainer(path).listBlobs().size(); - //assertTrue(indexRoutingFiles>0); - logger.info("number of index routing files {}", indexRoutingFiles); - }); - - String[] allNodes = internalCluster().getNodeNames(); - List routingTableVersions = new ArrayList<>(); - - // get cluster state from all the nodes - for (String node : allNodes) { - RoutingTable routingTable = internalCluster().client(node) - .admin() - .cluster() - .state(new ClusterStateRequest().local(true)) - .get().getState().routingTable(); - routingTableVersions.add(routingTable.version()); - } - assertTrue(areRoutingTableVersionsSame(routingTableVersions)); - - //updateIndexSettings(INDEX_NAME, IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2); - - assertBusy(() -> { - int indexRoutingFiles = repository.blobStore().blobContainer(path).listBlobs().size(); - //assertTrue(indexRoutingFiles>0); - logger.info("number of index routing files {}", indexRoutingFiles); - }); - } - - private void updateIndexSettings(String indexName, String settingKey, int settingValue) { - client().admin() - .indices() - .prepareUpdateSettings(indexName) - .setSettings(Settings.builder().put(settingKey, settingValue)) - .execute() - .actionGet(); - } - - private boolean areRoutingTableVersionsSame(List routingTableVersions) { - if (routingTableVersions == null || routingTableVersions.isEmpty()) { - return false; - } - - Long firstVersion = routingTableVersions.get(0); - for (Long routingTableVersion : routingTableVersions) { - if (!firstVersion.equals(routingTableVersion)) { - logger.info("Responses are not same {} {}", firstVersion, routingTableVersion); - - return false; - } - } - return true; - } -} diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java index f7c1047107906..e88fc2179b4e1 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java @@ -19,9 +19,9 @@ import java.util.Map; /** - * Represents a difference between {@link IndexRoutingTable} objects that can be serialized and deserialized. + * Represents a difference between {@link RoutingTable} objects that can be serialized and deserialized. */ -public class RoutingTableIncrementalDiff implements Diff { +public class RoutingTableIncrementalDiff implements Diff { private final Map> diffs; /** @@ -62,18 +62,24 @@ public static RoutingTableIncrementalDiff readFrom(StreamInput in) throws IOExce } /** - * Applies the differences to the provided {@link IndexRoutingTable}. + * Applies the differences to the provided {@link RoutingTable}. * - * @param part the original IndexRoutingTable to which the differences will be applied. - * @return the updated IndexRoutingTable with the applied differences. + * @param part the original RoutingTable to which the differences will be applied. + * @return the updated RoutingTable with the applied differences. */ @Override - public IndexRoutingTable apply(IndexRoutingTable part) { - // Apply diffs to the provided IndexRoutingTable + public RoutingTable apply(RoutingTable part) { + RoutingTable.Builder builder = new RoutingTable.Builder(); + for (IndexRoutingTable indexRoutingTable : part) { + builder.add(indexRoutingTable); // Add existing index routing tables to builder + } + + // Apply the diffs for (Map.Entry> entry : diffs.entrySet()) { - part = entry.getValue().apply(part); + builder.add(entry.getValue().apply(part.index(entry.getKey()))); } - return part; + + return builder.build(); } /** From ba0bf378d0019fe21bbfd542766c602a50072f4c Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Tue, 23 Jul 2024 01:11:51 +0530 Subject: [PATCH 15/18] Address comments Signed-off-by: Shailendra Singh --- .../routing/RoutingTableIncrementalDiff.java | 1 + .../InternalRemoteRoutingTableService.java | 30 ++++------ .../remote/NoopRemoteRoutingTableService.java | 7 +-- .../remote/RemoteRoutingTableService.java | 5 +- .../remote/RemoteClusterStateService.java | 6 +- ...eDiff.java => RemoteRoutingTableDiff.java} | 56 ++++++++--------- .../RemoteRoutingTableServiceTests.java | 22 +++---- .../RemoteClusterStateServiceTests.java | 3 +- .../RemoteIndexRoutingTableDiffTests.java | 60 +++++++++++-------- 9 files changed, 98 insertions(+), 92 deletions(-) rename server/src/main/java/org/opensearch/gateway/remote/routingtable/{RemoteIndexRoutingTableDiff.java => RemoteRoutingTableDiff.java} (59%) diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java index e88fc2179b4e1..3d75b22a8ed7f 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingTableIncrementalDiff.java @@ -22,6 +22,7 @@ * Represents a difference between {@link RoutingTable} objects that can be serialized and deserialized. */ public class RoutingTableIncrementalDiff implements Diff { + private final Map> diffs; /** diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java index 954a5ffe7e1aa..137453bad1344 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java @@ -17,7 +17,6 @@ import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; -import org.opensearch.common.CheckedRunnable; import org.opensearch.common.blobstore.BlobPath; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.remote.RemoteWritableEntityStore; @@ -31,7 +30,7 @@ import org.opensearch.gateway.remote.model.RemoteClusterStateBlobStore; import org.opensearch.gateway.remote.model.RemoteRoutingTableBlobStore; import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable; -import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff; +import org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; @@ -63,7 +62,7 @@ public class InternalRemoteRoutingTableService extends AbstractLifecycleComponen private final Supplier repositoriesService; private Compressor compressor; private RemoteWritableEntityStore remoteIndexRoutingTableStore; - private RemoteWritableEntityStore remoteIndexRoutingTableDiffStore; + private RemoteWritableEntityStore remoteRoutingTableDiffStore; private final ClusterSettings clusterSettings; private BlobStoreRepository blobStoreRepository; private final ThreadPool threadPool; @@ -138,16 +137,16 @@ public void getAsyncIndexRoutingWriteAction( } @Override - public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( + public void getAsyncIndexRoutingDiffWriteAction( String clusterUUID, long term, long version, Map> indexRoutingTableDiff, LatchedActionListener latchedActionListener ) { - - RemoteIndexRoutingTableDiff remoteIndexRoutingTableDiff = new RemoteIndexRoutingTableDiff( - indexRoutingTableDiff, + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(indexRoutingTableDiff); + RemoteRoutingTableDiff remoteRoutingTableDiff = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, term, @@ -155,12 +154,13 @@ public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( ); ActionListener completionListener = ActionListener.wrap( - resp -> latchedActionListener.onResponse(remoteIndexRoutingTableDiff.getUploadedMetadata()), + resp -> latchedActionListener.onResponse(remoteRoutingTableDiff.getUploadedMetadata()), ex -> latchedActionListener.onFailure( new RemoteStateTransferException("Exception in writing index routing diff to remote store", ex) ) ); - return () -> remoteIndexRoutingTableDiffStore.writeAsync(remoteIndexRoutingTableDiff, completionListener); + + remoteRoutingTableDiffStore.writeAsync(remoteRoutingTableDiff, completionListener); } /** @@ -205,7 +205,7 @@ public void getAsyncIndexRoutingReadAction( } @Override - public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( + public void getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, LatchedActionListener latchedActionListener @@ -215,13 +215,9 @@ public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( latchedActionListener::onFailure ); - RemoteIndexRoutingTableDiff remoteIndexRoutingTableDiff = new RemoteIndexRoutingTableDiff( - uploadedFilename, - clusterUUID, - compressor - ); + RemoteRoutingTableDiff remoteRoutingTableDiff = new RemoteRoutingTableDiff(uploadedFilename, clusterUUID, compressor); - return () -> remoteIndexRoutingTableDiffStore.readAsync(remoteIndexRoutingTableDiff, actionListener); + remoteRoutingTableDiffStore.readAsync(remoteRoutingTableDiff, actionListener); } @Override @@ -266,7 +262,7 @@ protected void doStart() { clusterSettings ); - this.remoteIndexRoutingTableDiffStore = new RemoteClusterStateBlobStore<>( + this.remoteRoutingTableDiffStore = new RemoteClusterStateBlobStore<>( new BlobStoreTransferService(blobStoreRepository.blobStore(), threadPool), blobStoreRepository, clusterName, diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java index 2b551dbf5883d..0793bf818c6c4 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java @@ -14,7 +14,6 @@ import org.opensearch.cluster.routing.IndexRoutingTable; import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; -import org.opensearch.common.CheckedRunnable; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.gateway.remote.ClusterMetadataManifest; @@ -57,7 +56,7 @@ public void getAsyncIndexRoutingWriteAction( } @Override - public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( + public void getAsyncIndexRoutingDiffWriteAction( String clusterUUID, long term, long version, @@ -65,7 +64,6 @@ public CheckedRunnable getAsyncIndexRoutingDiffWriteAction( LatchedActionListener latchedActionListener ) { // noop - return () -> {}; } @Override @@ -88,13 +86,12 @@ public void getAsyncIndexRoutingReadAction( } @Override - public CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( + public void getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, LatchedActionListener latchedActionListener ) { // noop - return () -> {}; } @Override 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 dbed35c480c88..de7be27ee72ee 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 @@ -15,7 +15,6 @@ import org.opensearch.cluster.routing.IndexShardRoutingTable; import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.cluster.routing.RoutingTableIncrementalDiff; -import org.opensearch.common.CheckedRunnable; import org.opensearch.common.lifecycle.LifecycleComponent; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -75,7 +74,7 @@ void getAsyncIndexRoutingReadAction( LatchedActionListener latchedActionListener ); - CheckedRunnable getAsyncIndexRoutingTableDiffReadAction( + void getAsyncIndexRoutingTableDiffReadAction( String clusterUUID, String uploadedFilename, LatchedActionListener latchedActionListener @@ -99,7 +98,7 @@ void getAsyncIndexRoutingWriteAction( LatchedActionListener latchedActionListener ); - CheckedRunnable getAsyncIndexRoutingDiffWriteAction( + void getAsyncIndexRoutingDiffWriteAction( String clusterUUID, long term, long version, 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 029e528650bad..3be3f62075edf 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -58,7 +58,7 @@ import org.opensearch.gateway.remote.model.RemoteReadResult; import org.opensearch.gateway.remote.model.RemoteTemplatesMetadata; import org.opensearch.gateway.remote.model.RemoteTransientSettingsMetadata; -import org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff; +import org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff; import org.opensearch.index.translog.transfer.BlobStoreTransferService; import org.opensearch.node.Node; import org.opensearch.node.remotestore.RemoteStoreNodeAttribute; @@ -691,7 +691,7 @@ UploadedMetadataResults writeMetadataInParallel( ); }); if (indexRoutingTableDiff != null && !indexRoutingTableDiff.isEmpty()) { - uploadTasks.add(RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE); + uploadTasks.add(RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_FILE); remoteRoutingTableService.getAsyncIndexRoutingDiffWriteAction( clusterState.metadata().clusterUUID(), clusterState.term(), @@ -746,7 +746,7 @@ UploadedMetadataResults writeMetadataInParallel( if (uploadedMetadata.getClass().equals(UploadedIndexMetadata.class) && uploadedMetadata.getComponent().contains(INDEX_ROUTING_METADATA_PREFIX)) { response.uploadedIndicesRoutingMetadata.add((UploadedIndexMetadata) uploadedMetadata); - } else if (RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE.equals(name)) { + } else if (RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_FILE.equals(name)) { response.uploadedIndicesRoutingDiffMetadata = (UploadedMetadataAttribute) 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/routingtable/RemoteIndexRoutingTableDiff.java b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteRoutingTableDiff.java similarity index 59% rename from server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java rename to server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteRoutingTableDiff.java index 2b1f2be03da47..e876d939490d0 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiff.java +++ b/server/src/main/java/org/opensearch/gateway/remote/routingtable/RemoteRoutingTableDiff.java @@ -27,60 +27,60 @@ import static org.opensearch.gateway.remote.RemoteClusterStateUtils.DELIMITER; /** - * Represents a difference between {@link IndexRoutingTable} objects that can be serialized and deserialized. - * This class is responsible for writing and reading the differences between IndexRoutingTables to and from an input/output stream. + * Represents a incremental difference between {@link org.opensearch.cluster.routing.RoutingTable} objects that can be serialized and deserialized. + * This class is responsible for writing and reading the differences between RoutingTables to and from an input/output stream. */ -public class RemoteIndexRoutingTableDiff extends AbstractRemoteWritableBlobEntity { - private final Map> diffs; +public class RemoteRoutingTableDiff extends AbstractRemoteWritableBlobEntity { + private final RoutingTableIncrementalDiff routingTableIncrementalDiff; private long term; private long version; - public static final String INDEX_ROUTING_TABLE_DIFF = "index-routing-diff"; + public static final String ROUTING_TABLE_DIFF = "routing-table-diff"; - public static final String INDEX_ROUTING_DIFF_METADATA_PREFIX = "indexRoutingDiff--"; + public static final String ROUTING_TABLE_DIFF_METADATA_PREFIX = "routingTableDiff--"; - public static final String INDEX_ROUTING_DIFF_FILE = "index_routing_diff"; - private static final String codec = "RemoteIndexRoutingTableDiff"; - public static final String INDEX_ROUTING_DIFF_PATH_TOKEN = "index-routing-diff"; + public static final String ROUTING_TABLE_DIFF_FILE = "routing_table_diff"; + private static final String codec = "RemoteRoutingTableDiff"; + public static final String ROUTING_TABLE_DIFF_PATH_TOKEN = "routing-table-diff"; public static final int VERSION = 1; - public static final ChecksumWritableBlobStoreFormat RemoteIndexRoutingTableDiffFormat = + public static final ChecksumWritableBlobStoreFormat REMOTE_ROUTING_TABLE_DIFF_FORMAT = new ChecksumWritableBlobStoreFormat<>(codec, RoutingTableIncrementalDiff::readFrom); /** - * Constructs a new RemoteIndexRoutingTableDiff with the given differences. + * Constructs a new RemoteRoutingTableDiff with the given differences. * - * @param diffs a map containing the differences of {@link IndexRoutingTable}. + * @param routingTableIncrementalDiff a RoutingTableIncrementalDiff object containing the differences of {@link IndexRoutingTable}. * @param clusterUUID the cluster UUID. * @param compressor the compressor to be used. * @param term the term of the routing table. * @param version the version of the routing table. */ - public RemoteIndexRoutingTableDiff( - Map> diffs, + public RemoteRoutingTableDiff( + RoutingTableIncrementalDiff routingTableIncrementalDiff, String clusterUUID, Compressor compressor, long term, long version ) { super(clusterUUID, compressor); - this.diffs = diffs; + this.routingTableIncrementalDiff = routingTableIncrementalDiff; this.term = term; this.version = version; } /** - * Constructs a new RemoteIndexRoutingTableDiff with the given differences. + * Constructs a new RemoteRoutingTableDiff with the given differences. * - * @param diffs a map containing the differences of {@link IndexRoutingTable}. + * @param routingTableIncrementalDiff a RoutingTableIncrementalDiff object containing the differences of {@link IndexRoutingTable}. * @param clusterUUID the cluster UUID. * @param compressor the compressor to be used. */ - public RemoteIndexRoutingTableDiff(Map> diffs, String clusterUUID, Compressor compressor) { + public RemoteRoutingTableDiff(RoutingTableIncrementalDiff routingTableIncrementalDiff, String clusterUUID, Compressor compressor) { super(clusterUUID, compressor); - this.diffs = diffs; + this.routingTableIncrementalDiff = routingTableIncrementalDiff; } /** @@ -90,9 +90,9 @@ public RemoteIndexRoutingTableDiff(Map> diffs, S * @param clusterUUID the cluster UUID. * @param compressor the compressor to be used. */ - public RemoteIndexRoutingTableDiff(String blobName, String clusterUUID, Compressor compressor) { + public RemoteRoutingTableDiff(String blobName, String clusterUUID, Compressor compressor) { super(clusterUUID, compressor); - this.diffs = null; + this.routingTableIncrementalDiff = null; this.blobName = blobName; } @@ -102,17 +102,18 @@ public RemoteIndexRoutingTableDiff(String blobName, String clusterUUID, Compress * @return a map containing the differences. */ public Map> getDiffs() { - return diffs; + assert routingTableIncrementalDiff != null; + return routingTableIncrementalDiff.getDiffs(); } @Override public BlobPathParameters getBlobPathParameters() { - return new BlobPathParameters(List.of(INDEX_ROUTING_DIFF_PATH_TOKEN), INDEX_ROUTING_DIFF_METADATA_PREFIX); + return new BlobPathParameters(List.of(ROUTING_TABLE_DIFF_PATH_TOKEN), ROUTING_TABLE_DIFF_METADATA_PREFIX); } @Override public String getType() { - return INDEX_ROUTING_TABLE_DIFF; + return ROUTING_TABLE_DIFF; } @Override @@ -132,17 +133,18 @@ public String generateBlobFileName() { @Override public ClusterMetadataManifest.UploadedMetadata getUploadedMetadata() { assert blobName != null; - return new ClusterMetadataManifest.UploadedMetadataAttribute(INDEX_ROUTING_DIFF_FILE, blobName); + return new ClusterMetadataManifest.UploadedMetadataAttribute(ROUTING_TABLE_DIFF_FILE, blobName); } @Override public InputStream serialize() throws IOException { - return RemoteIndexRoutingTableDiffFormat.serialize(new RoutingTableIncrementalDiff(diffs), generateBlobFileName(), getCompressor()) + assert routingTableIncrementalDiff != null; + return REMOTE_ROUTING_TABLE_DIFF_FORMAT.serialize(routingTableIncrementalDiff, generateBlobFileName(), getCompressor()) .streamInput(); } @Override public RoutingTableIncrementalDiff deserialize(InputStream in) throws IOException { - return RemoteIndexRoutingTableDiffFormat.deserialize(blobName, Streams.readFully(in)); + return REMOTE_ROUTING_TABLE_DIFF_FORMAT.deserialize(blobName, Streams.readFully(in)); } } 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 d15bd217a661c..0a336dde1ef06 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 @@ -75,10 +75,10 @@ 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.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE; -import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_METADATA_PREFIX; -import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_PATH_TOKEN; -import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.RemoteIndexRoutingTableDiffFormat; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.REMOTE_ROUTING_TABLE_DIFF_FORMAT; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_FILE; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_METADATA_PREFIX; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_PATH_TOKEN; import static org.opensearch.index.remote.RemoteStoreEnums.PathHashAlgorithm.FNV_1A_BASE64; import static org.opensearch.index.remote.RemoteStoreEnums.PathType.HASHED_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; @@ -586,9 +586,9 @@ public void testGetAsyncIndexRoutingTableDiffReadAction() throws Exception { RoutingTableIncrementalDiff diff = new RoutingTableIncrementalDiff(diffs); - String uploadedFileName = String.format(Locale.ROOT, "index-routing-diff/" + indexName); + String uploadedFileName = String.format(Locale.ROOT, "routing-table-diff/" + indexName); when(blobContainer.readBlob(indexName)).thenReturn( - RemoteIndexRoutingTableDiffFormat.serialize(diff, uploadedFileName, compressor).streamInput() + REMOTE_ROUTING_TABLE_DIFF_FORMAT.serialize(diff, uploadedFileName, compressor).streamInput() ); TestCapturingListener listener = new TestCapturingListener<>(); @@ -598,7 +598,7 @@ public void testGetAsyncIndexRoutingTableDiffReadAction() throws Exception { "cluster-uuid", uploadedFileName, new LatchedActionListener<>(listener, latch) - ).run(); + ); latch.await(); assertNull(listener.getFailure()); @@ -684,7 +684,7 @@ public void testGetAsyncIndexRoutingDiffWriteAction() throws Exception { ) .add("cluster-state") .add(currentState.metadata().clusterUUID()) - .add(INDEX_ROUTING_DIFF_PATH_TOKEN); + .add(ROUTING_TABLE_DIFF_PATH_TOKEN); doAnswer(invocationOnMock -> { invocationOnMock.getArgument(4, ActionListener.class).onResponse(null); @@ -701,13 +701,13 @@ public void testGetAsyncIndexRoutingDiffWriteAction() throws Exception { currentState.version(), diffs, new LatchedActionListener<>(listener, latch) - ).run(); + ); latch.await(); assertNull(listener.getFailure()); assertNotNull(listener.getResult()); ClusterMetadataManifest.UploadedMetadata uploadedMetadata = listener.getResult(); - assertEquals(INDEX_ROUTING_DIFF_FILE, uploadedMetadata.getComponent()); + assertEquals(ROUTING_TABLE_DIFF_FILE, uploadedMetadata.getComponent()); String uploadedFileName = uploadedMetadata.getUploadedFilename(); String[] pathTokens = uploadedFileName.split(PATH_DELIMITER); assertEquals(6, pathTokens.length); @@ -715,7 +715,7 @@ public void testGetAsyncIndexRoutingDiffWriteAction() throws Exception { String[] fileNameTokens = pathTokens[5].split(DELIMITER); assertEquals(4, fileNameTokens.length); - assertEquals(INDEX_ROUTING_DIFF_METADATA_PREFIX, fileNameTokens[0]); + assertEquals(ROUTING_TABLE_DIFF_METADATA_PREFIX, fileNameTokens[0]); assertEquals(RemoteStoreUtils.invertLong(1L), fileNameTokens[1]); assertEquals(RemoteStoreUtils.invertLong(2L), fileNameTokens[2]); 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 ab60dad4b14ac..59ca62dff2aa7 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -635,7 +635,8 @@ public void testWriteMetadataInParallelIncompleteUpload() throws IOException { true, clusterState.getCustoms(), true, - emptyList() + emptyList(), + null ) ); assertTrue(exception.getMessage().startsWith("Some metadata components were not uploaded successfully")); diff --git a/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java b/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java index 4bba0b3764c07..6ffa7fc5cded8 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/routingtable/RemoteIndexRoutingTableDiffTests.java @@ -39,7 +39,9 @@ import java.util.List; import java.util.Map; -import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTableDiff.INDEX_ROUTING_DIFF_FILE; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_FILE; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_METADATA_PREFIX; +import static org.opensearch.gateway.remote.routingtable.RemoteRoutingTableDiff.ROUTING_TABLE_DIFF_PATH_TOKEN; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; @@ -100,8 +102,10 @@ public void testClusterUUID() { diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); + + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -109,7 +113,7 @@ public void testClusterUUID() { ); assertEquals(remoteDiffForUpload.clusterUUID(), clusterUUID); - RemoteIndexRoutingTableDiff remoteDiffForDownload = new RemoteIndexRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); + RemoteRoutingTableDiff remoteDiffForDownload = new RemoteRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); assertEquals(remoteDiffForDownload.clusterUUID(), clusterUUID); } @@ -128,9 +132,10 @@ public void testFullBlobName() { IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -138,7 +143,7 @@ public void testFullBlobName() { ); assertThat(remoteDiffForUpload.getFullBlobName(), nullValue()); - RemoteIndexRoutingTableDiff remoteDiffForDownload = new RemoteIndexRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); + RemoteRoutingTableDiff remoteDiffForDownload = new RemoteRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); assertThat(remoteDiffForDownload.getFullBlobName(), is(TEST_BLOB_NAME)); } @@ -157,9 +162,10 @@ public void testBlobFileName() { IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -167,7 +173,7 @@ public void testBlobFileName() { ); assertThat(remoteDiffForUpload.getBlobFileName(), nullValue()); - RemoteIndexRoutingTableDiff remoteDiffForDownload = new RemoteIndexRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); + RemoteRoutingTableDiff remoteDiffForDownload = new RemoteRoutingTableDiff(TEST_BLOB_NAME, clusterUUID, compressor); assertThat(remoteDiffForDownload.getBlobFileName(), is(TEST_BLOB_FILE_NAME)); } @@ -186,9 +192,10 @@ public void testBlobPathParameters() { IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -197,8 +204,8 @@ public void testBlobPathParameters() { assertThat(remoteDiffForUpload.getBlobFileName(), nullValue()); BlobPathParameters params = remoteDiffForUpload.getBlobPathParameters(); - assertThat(params.getPathTokens(), is(List.of("index-routing-diff"))); - String expectedPrefix = "indexRoutingDiff--"; + assertThat(params.getPathTokens(), is(List.of(ROUTING_TABLE_DIFF_PATH_TOKEN))); + String expectedPrefix = ROUTING_TABLE_DIFF_METADATA_PREFIX; assertThat(params.getFilePrefix(), is(expectedPrefix)); } @@ -217,9 +224,10 @@ public void testGenerateBlobFileName() { IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -228,9 +236,9 @@ public void testGenerateBlobFileName() { String blobFileName = remoteDiffForUpload.generateBlobFileName(); String[] nameTokens = blobFileName.split("__"); - assertEquals(nameTokens[0], "indexRoutingDiff--"); - assertEquals(nameTokens[1], RemoteStoreUtils.invertLong(STATE_TERM)); - assertEquals(nameTokens[2], RemoteStoreUtils.invertLong(STATE_VERSION)); + assertEquals(ROUTING_TABLE_DIFF_METADATA_PREFIX, nameTokens[0]); + assertEquals(RemoteStoreUtils.invertLong(STATE_TERM), nameTokens[1]); + assertEquals(RemoteStoreUtils.invertLong(STATE_VERSION), nameTokens[2]); assertThat(RemoteStoreUtils.invertLong(nameTokens[3]), lessThanOrEqualTo(System.currentTimeMillis())); } @@ -249,9 +257,10 @@ public void testGetUploadedMetadata() throws IOException { IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(indexMetadata.getIndex()).initializeAsNew(indexMetadata).build(); diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -260,7 +269,7 @@ public void testGetUploadedMetadata() throws IOException { remoteDiffForUpload.setFullBlobName(new BlobPath().add(TEST_BLOB_PATH)); ClusterMetadataManifest.UploadedMetadata uploadedMetadataAttribute = remoteDiffForUpload.getUploadedMetadata(); - assertEquals(INDEX_ROUTING_DIFF_FILE, uploadedMetadataAttribute.getComponent()); + assertEquals(ROUTING_TABLE_DIFF_FILE, uploadedMetadataAttribute.getComponent()); } public void testStreamOperations() throws IOException { @@ -282,9 +291,10 @@ public void testStreamOperations() throws IOException { initialRoutingTable.getIndicesRouting().values().forEach(indexRoutingTable -> { diffs.put(indexName, indexRoutingTable.diff(indexRoutingTable)); + RoutingTableIncrementalDiff routingTableIncrementalDiff = new RoutingTableIncrementalDiff(diffs); - RemoteIndexRoutingTableDiff remoteDiffForUpload = new RemoteIndexRoutingTableDiff( - diffs, + RemoteRoutingTableDiff remoteDiffForUpload = new RemoteRoutingTableDiff( + routingTableIncrementalDiff, clusterUUID, compressor, STATE_TERM, @@ -297,7 +307,7 @@ public void testStreamOperations() throws IOException { remoteDiffForUpload.setFullBlobName(BlobPath.cleanPath()); assertThat(inputStream.available(), greaterThan(0)); - RoutingTableIncrementalDiff routingTableIncrementalDiff = remoteDiffForUpload.deserialize(inputStream); + routingTableIncrementalDiff = remoteDiffForUpload.deserialize(inputStream); assertEquals(remoteDiffForUpload.getDiffs().size(), routingTableIncrementalDiff.getDiffs().size()); } catch (IOException e) { throw new RuntimeException(e); From 183c8bca2a178d2fb9f66103e97d4bfa44886616 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Tue, 23 Jul 2024 13:25:55 +0530 Subject: [PATCH 16/18] Update ITs for routing table service Signed-off-by: Shailendra Singh --- .../remote/RemoteRoutingTableServiceIT.java | 97 ++++++++++++++++--- .../remote/RemoteRoutingTableService.java | 4 +- 2 files changed, 83 insertions(+), 18 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java index 53764c0b4d0e8..b0d046cbdf3db 100644 --- a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java @@ -8,6 +8,7 @@ package org.opensearch.gateway.remote; +import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse; import org.opensearch.action.admin.cluster.state.ClusterStateRequest; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.IndexRoutingTable; @@ -32,16 +33,19 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static org.opensearch.common.util.FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL; import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; import static org.opensearch.gateway.remote.routingtable.RemoteIndexRoutingTable.INDEX_ROUTING_TABLE; +import static org.opensearch.indices.IndicesService.CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) public class RemoteRoutingTableServiceIT extends RemoteStoreBaseIntegTestCase { private static final String INDEX_NAME = "test-index"; + private static final String INDEX_NAME_1 = "test-index-1"; BlobPath indexRoutingPath; AtomicInteger indexRoutingFiles = new AtomicInteger(); private final RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.HASHED_PREFIX; @@ -72,7 +76,13 @@ public void testRemoteRoutingTableIndexLifecycle() throws Exception { RemoteClusterStateService.class ); RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager(); - verifyUpdatesInManifestFile(remoteManifestManager); + Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + List expectedIndexNames = new ArrayList<>(); + List deletedIndexNames = new ArrayList<>(); + verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, true); List routingTableVersions = getRoutingTableFromAllNodes(); assertTrue(areRoutingTablesSame(routingTableVersions)); @@ -86,7 +96,11 @@ public void testRemoteRoutingTableIndexLifecycle() throws Exception { assertTrue(indexRoutingFilesAfterUpdate >= indexRoutingFiles.get() + 3); }); - verifyUpdatesInManifestFile(remoteManifestManager); + latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, true); routingTableVersions = getRoutingTableFromAllNodes(); assertTrue(areRoutingTablesSame(routingTableVersions)); @@ -98,6 +112,42 @@ public void testRemoteRoutingTableIndexLifecycle() throws Exception { assertTrue(areRoutingTablesSame(routingTableVersions)); } + public void testRemoteRoutingTableEmptyRoutingTableDiff() throws Exception { + prepareClusterAndVerifyRepository(); + + RemoteClusterStateService remoteClusterStateService = internalCluster().getClusterManagerNodeInstance( + RemoteClusterStateService.class + ); + RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager(); + Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + List expectedIndexNames = new ArrayList<>(); + List deletedIndexNames = new ArrayList<>(); + verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, true); + + List routingTableVersions = getRoutingTableFromAllNodes(); + assertTrue(areRoutingTablesSame(routingTableVersions)); + + // Update cluster settings + ClusterUpdateSettingsResponse response = client().admin() + .cluster() + .prepareUpdateSettings() + .setPersistentSettings(Settings.builder().put(CLUSTER_DEFAULT_INDEX_REFRESH_INTERVAL_SETTING.getKey(), 0, TimeUnit.SECONDS)) + .get(); + assertTrue(response.isAcknowledged()); + + latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, false); + + routingTableVersions = getRoutingTableFromAllNodes(); + assertTrue(areRoutingTablesSame(routingTableVersions)); + } + public void testRemoteRoutingTableIndexNodeRestart() throws Exception { BlobStoreRepository repository = prepareClusterAndVerifyRepository(); @@ -124,10 +174,16 @@ public void testRemoteRoutingTableIndexNodeRestart() throws Exception { RemoteClusterStateService.class ); RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager(); - verifyUpdatesInManifestFile(remoteManifestManager); + Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + List expectedIndexNames = new ArrayList<>(); + List deletedIndexNames = new ArrayList<>(); + verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, true); } - public void testRemoteRoutingTableIndexMasterRestart1() throws Exception { + public void testRemoteRoutingTableIndexMasterRestart() throws Exception { BlobStoreRepository repository = prepareClusterAndVerifyRepository(); List routingTableVersions = getRoutingTableFromAllNodes(); @@ -153,7 +209,13 @@ public void testRemoteRoutingTableIndexMasterRestart1() throws Exception { RemoteClusterStateService.class ); RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager(); - verifyUpdatesInManifestFile(remoteManifestManager); + Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + List expectedIndexNames = new ArrayList<>(); + List deletedIndexNames = new ArrayList<>(); + verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, true); } private BlobStoreRepository prepareClusterAndVerifyRepository() throws Exception { @@ -208,18 +270,23 @@ private BlobPath getIndexRoutingPath(BlobPath indexRoutingPath, String indexUUID ); } - private void verifyUpdatesInManifestFile(RemoteManifestManager remoteManifestManager) { - Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( - getClusterState().getClusterName().value(), - getClusterState().getMetadata().clusterUUID() - ); + private void verifyUpdatesInManifestFile( + Optional latestManifest, + List expectedIndexNames, + int expectedIndicesRoutingFilesInManifest, + List expectedDeletedIndex, + boolean isRoutingTableDiffFileExpected + ) { assertTrue(latestManifest.isPresent()); ClusterMetadataManifest manifest = latestManifest.get(); - assertTrue(manifest.getDiffManifest().getIndicesRoutingUpdated().contains(INDEX_NAME)); - assertTrue(manifest.getDiffManifest().getIndicesDeleted().isEmpty()); - assertFalse(manifest.getIndicesRouting().isEmpty()); - assertEquals(1, manifest.getIndicesRouting().size()); - assertTrue(manifest.getIndicesRouting().get(0).getUploadedFilename().contains(indexRoutingPath.buildAsString())); + + assertEquals(expectedIndexNames, manifest.getDiffManifest().getIndicesRoutingUpdated()); + assertEquals(expectedDeletedIndex, manifest.getDiffManifest().getIndicesDeleted()); + assertEquals(expectedIndicesRoutingFilesInManifest, manifest.getIndicesRouting().size()); + for (ClusterMetadataManifest.UploadedIndexMetadata uploadedFilename : manifest.getIndicesRouting()) { + assertTrue(uploadedFilename.getUploadedFilename().contains(indexRoutingPath.buildAsString())); + } + assertEquals(isRoutingTableDiffFileExpected, manifest.getDiffManifest().getIndicesRoutingDiffPath() != null); } private List getRoutingTableFromAllNodes() throws ExecutionException, InterruptedException { 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 de7be27ee72ee..e0072f1f8e4cc 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 @@ -56,9 +56,7 @@ public Diff diff(IndexRoutingTable currentState, IndexRouting Integer index = entry.getKey(); IndexShardRoutingTable currentShardRoutingTable = entry.getValue(); IndexShardRoutingTable previousShardRoutingTable = previousState.shard(index); - if (previousShardRoutingTable == null) { - diffs.add(currentShardRoutingTable); - } else if (!previousShardRoutingTable.equals(currentShardRoutingTable)) { + if (previousShardRoutingTable == null || !previousShardRoutingTable.equals(currentShardRoutingTable)) { diffs.add(currentShardRoutingTable); } } From a1506ec9304024fffe2cdb07a4ae20c56b2c1b5f Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Tue, 23 Jul 2024 16:02:00 +0530 Subject: [PATCH 17/18] Avoid reading shard diff in case of full state apply. Signed-off-by: Shailendra Singh --- .../opensearch/gateway/remote/RemoteClusterStateService.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 3be3f62075edf..674279f2251bd 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -1328,10 +1328,7 @@ public ClusterState getClusterStateForManifest( includeEphemeral ? manifest.getIndicesRouting() : emptyList(), includeEphemeral && manifest.getHashesOfConsistentSettings() != null, includeEphemeral ? manifest.getClusterStateCustomMap() : emptyMap(), - includeEphemeral - && manifest.getDiffManifest() != null - && manifest.getDiffManifest().getIndicesRoutingDiffPath() != null - && !manifest.getDiffManifest().getIndicesRoutingDiffPath().isEmpty(), + false, includeEphemeral ); } else { From 91151ad57754ff44d77d1483b8b89458bbc89715 Mon Sep 17 00:00:00 2001 From: Shailendra Singh Date: Mon, 22 Jul 2024 08:41:33 +0530 Subject: [PATCH 18/18] Delete stale indices routing diff file. Signed-off-by: Shailendra Singh --- .../InternalRemoteRoutingTableService.java | 10 ++ .../remote/NoopRemoteRoutingTableService.java | 4 + .../remote/RemoteRoutingTableService.java | 2 + .../RemoteClusterStateCleanupManager.java | 26 ++++ .../remote/RemotePersistenceStats.java | 11 ++ .../RemoteRoutingTableServiceTests.java | 22 +++ ...RemoteClusterStateCleanupManagerTests.java | 146 ++++++++++++++++++ 7 files changed, 221 insertions(+) diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java index 137453bad1344..3c578a8c5c01f 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/InternalRemoteRoutingTableService.java @@ -284,4 +284,14 @@ public void deleteStaleIndexRoutingPaths(List stalePaths) throws IOExcep throw e; } } + + public void deleteStaleIndexRoutingDiffPaths(List stalePaths) throws IOException { + try { + logger.debug(() -> "Deleting stale index routing diff files from remote - " + stalePaths); + blobStoreRepository.blobStore().blobContainer(BlobPath.cleanPath()).deleteBlobsIgnoringIfNotExists(stalePaths); + } catch (IOException e) { + logger.error(() -> new ParameterizedMessage("Failed to delete some stale index routing diff paths from {}", stalePaths), e); + throw e; + } + } } diff --git a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java index 0793bf818c6c4..1ebf3206212a1 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/remote/NoopRemoteRoutingTableService.java @@ -122,4 +122,8 @@ protected void doClose() throws IOException { public void deleteStaleIndexRoutingPaths(List stalePaths) throws IOException { // noop } + + public void deleteStaleIndexRoutingDiffPaths(List stalePaths) throws IOException { + // noop + } } 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 e0072f1f8e4cc..0811a5f3010f4 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 @@ -112,4 +112,6 @@ List getAllUploadedIndicesRouting public void deleteStaleIndexRoutingPaths(List stalePaths) throws IOException; + public void deleteStaleIndexRoutingDiffPaths(List stalePaths) throws IOException; + } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManager.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManager.java index 99235bc96bfe3..8691187c7fbfa 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManager.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManager.java @@ -179,6 +179,7 @@ void deleteClusterMetadata( Set staleGlobalMetadataPaths = new HashSet<>(); Set staleEphemeralAttributePaths = new HashSet<>(); Set staleIndexRoutingPaths = new HashSet<>(); + Set staleIndexRoutingDiffPaths = new HashSet<>(); activeManifestBlobMetadata.forEach(blobMetadata -> { ClusterMetadataManifest clusterMetadataManifest = remoteManifestManager.fetchRemoteClusterMetadataManifest( clusterName, @@ -222,6 +223,10 @@ void deleteClusterMetadata( clusterMetadataManifest.getIndicesRouting() .forEach(uploadedIndicesRouting -> filesToKeep.add(uploadedIndicesRouting.getUploadedFilename())); } + if (clusterMetadataManifest.getDiffManifest() != null + && clusterMetadataManifest.getDiffManifest().getIndicesRoutingDiffPath() != null) { + filesToKeep.add(clusterMetadataManifest.getDiffManifest().getIndicesRoutingDiffPath()); + } }); staleManifestBlobMetadata.forEach(blobMetadata -> { ClusterMetadataManifest clusterMetadataManifest = remoteManifestManager.fetchRemoteClusterMetadataManifest( @@ -264,6 +269,18 @@ void deleteClusterMetadata( } }); } + if (clusterMetadataManifest.getDiffManifest() != null + && clusterMetadataManifest.getDiffManifest().getIndicesRoutingDiffPath() != null) { + if (!filesToKeep.contains(clusterMetadataManifest.getDiffManifest().getIndicesRoutingDiffPath())) { + staleIndexRoutingDiffPaths.add(clusterMetadataManifest.getDiffManifest().getIndicesRoutingDiffPath()); + logger.debug( + () -> new ParameterizedMessage( + "Indices routing diff paths in stale manifest: {}", + clusterMetadataManifest.getDiffManifest().getIndicesRoutingDiffPath() + ) + ); + } + } clusterMetadataManifest.getIndices().forEach(uploadedIndexMetadata -> { String fileName = RemoteClusterStateUtils.getFormattedIndexFileName(uploadedIndexMetadata.getUploadedFilename()); @@ -316,6 +333,15 @@ void deleteClusterMetadata( ); remoteStateStats.indexRoutingFilesCleanupAttemptFailed(); } + try { + remoteRoutingTableService.deleteStaleIndexRoutingDiffPaths(new ArrayList<>(staleIndexRoutingDiffPaths)); + } catch (IOException e) { + logger.error( + () -> new ParameterizedMessage("Error while deleting stale index routing diff files {}", staleIndexRoutingDiffPaths), + e + ); + remoteStateStats.indicesRoutingDiffFileCleanupAttemptFailed(); + } } catch (IllegalStateException e) { logger.error("Error while fetching Remote Cluster Metadata manifests", e); } catch (IOException e) { diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemotePersistenceStats.java b/server/src/main/java/org/opensearch/gateway/remote/RemotePersistenceStats.java index 36d107a99d258..efd73e11e46b5 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemotePersistenceStats.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemotePersistenceStats.java @@ -20,15 +20,18 @@ public class RemotePersistenceStats extends PersistedStateStats { static final String CLEANUP_ATTEMPT_FAILED_COUNT = "cleanup_attempt_failed_count"; static final String INDEX_ROUTING_FILES_CLEANUP_ATTEMPT_FAILED_COUNT = "index_routing_files_cleanup_attempt_failed_count"; + static final String INDICES_ROUTING_DIFF_FILES_CLEANUP_ATTEMPT_FAILED_COUNT = "indices_routing_diff_files_cleanup_attempt_failed_count"; static final String REMOTE_UPLOAD = "remote_upload"; private AtomicLong cleanupAttemptFailedCount = new AtomicLong(0); private AtomicLong indexRoutingFilesCleanupAttemptFailedCount = new AtomicLong(0); + private AtomicLong indicesRoutingDiffFilesCleanupAttemptFailedCount = new AtomicLong(0); public RemotePersistenceStats() { super(REMOTE_UPLOAD); addToExtendedFields(CLEANUP_ATTEMPT_FAILED_COUNT, cleanupAttemptFailedCount); addToExtendedFields(INDEX_ROUTING_FILES_CLEANUP_ATTEMPT_FAILED_COUNT, indexRoutingFilesCleanupAttemptFailedCount); + addToExtendedFields(INDICES_ROUTING_DIFF_FILES_CLEANUP_ATTEMPT_FAILED_COUNT, indicesRoutingDiffFilesCleanupAttemptFailedCount); } public void cleanUpAttemptFailed() { @@ -46,4 +49,12 @@ public void indexRoutingFilesCleanupAttemptFailed() { public long getIndexRoutingFilesCleanupAttemptFailedCount() { return indexRoutingFilesCleanupAttemptFailedCount.get(); } + + public void indicesRoutingDiffFileCleanupAttemptFailed() { + indexRoutingFilesCleanupAttemptFailedCount.incrementAndGet(); + } + + public long getIndicesRoutingDiffFileCleanupAttemptFailedCount() { + return indexRoutingFilesCleanupAttemptFailedCount.get(); + } } 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 0a336dde1ef06..74254f1a1987f 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 @@ -804,4 +804,26 @@ public void testDeleteStaleIndexRoutingPathsThrowsIOException() throws IOExcepti verify(blobContainer).deleteBlobsIgnoringIfNotExists(stalePaths); } + public void testDeleteStaleIndexRoutingDiffPaths() throws IOException { + doNothing().when(blobContainer).deleteBlobsIgnoringIfNotExists(any()); + when(blobStore.blobContainer(any())).thenReturn(blobContainer); + List stalePaths = Arrays.asList("path1", "path2"); + remoteRoutingTableService.doStart(); + remoteRoutingTableService.deleteStaleIndexRoutingDiffPaths(stalePaths); + verify(blobContainer).deleteBlobsIgnoringIfNotExists(stalePaths); + } + + public void testDeleteStaleIndexRoutingDiffPathsThrowsIOException() throws IOException { + when(blobStore.blobContainer(any())).thenReturn(blobContainer); + List stalePaths = Arrays.asList("path1", "path2"); + // Simulate an IOException + doThrow(new IOException("test exception")).when(blobContainer).deleteBlobsIgnoringIfNotExists(Mockito.anyList()); + + remoteRoutingTableService.doStart(); + IOException thrown = assertThrows(IOException.class, () -> { + remoteRoutingTableService.deleteStaleIndexRoutingDiffPaths(stalePaths); + }); + assertEquals("test exception", thrown.getMessage()); + verify(blobContainer).deleteBlobsIgnoringIfNotExists(stalePaths); + } } diff --git a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerTests.java b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerTests.java index ec7e3c1ce81d3..b86f23f3d37aa 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateCleanupManagerTests.java @@ -50,6 +50,7 @@ import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V1; import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V2; +import static org.opensearch.gateway.remote.ClusterMetadataManifest.CODEC_V3; import static org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedIndexMetadata; import static org.opensearch.gateway.remote.ClusterMetadataManifest.UploadedMetadataAttribute; import static org.opensearch.gateway.remote.RemoteClusterStateCleanupManager.AsyncStaleFileDeletion; @@ -296,6 +297,74 @@ public void testDeleteClusterMetadata() throws IOException { verify(remoteRoutingTableService).deleteStaleIndexRoutingPaths(List.of(index3Metadata.getUploadedFilename())); } + public void testDeleteStaleIndicesRoutingDiffFile() throws IOException { + String clusterUUID = "clusterUUID"; + String clusterName = "test-cluster"; + List inactiveBlobs = Arrays.asList(new PlainBlobMetadata("manifest1.dat", 1L)); + List activeBlobs = Arrays.asList(new PlainBlobMetadata("manifest2.dat", 1L)); + + UploadedMetadataAttribute coordinationMetadata = new UploadedMetadataAttribute(COORDINATION_METADATA, "coordination_metadata"); + UploadedMetadataAttribute templateMetadata = new UploadedMetadataAttribute(TEMPLATES_METADATA, "template_metadata"); + UploadedMetadataAttribute settingMetadata = new UploadedMetadataAttribute(SETTING_METADATA, "settings_metadata"); + UploadedMetadataAttribute coordinationMetadataUpdated = new UploadedMetadataAttribute( + COORDINATION_METADATA, + "coordination_metadata_updated" + ); + + UploadedIndexMetadata index1Metadata = new UploadedIndexMetadata("index1", "indexUUID1", "index_metadata1__2"); + UploadedIndexMetadata index2Metadata = new UploadedIndexMetadata("index2", "indexUUID2", "index_metadata2__2"); + List indicesRouting1 = List.of(index1Metadata); + List indicesRouting2 = List.of(index2Metadata); + ClusterStateDiffManifest diffManifest1 = ClusterStateDiffManifest.builder().indicesRoutingDiffPath("index1RoutingDiffPath").build(); + ClusterStateDiffManifest diffManifest2 = ClusterStateDiffManifest.builder().indicesRoutingDiffPath("index2RoutingDiffPath").build(); + + ClusterMetadataManifest manifest1 = ClusterMetadataManifest.builder() + .indices(List.of(index1Metadata)) + .coordinationMetadata(coordinationMetadataUpdated) + .templatesMetadata(templateMetadata) + .settingMetadata(settingMetadata) + .clusterTerm(1L) + .stateVersion(1L) + .codecVersion(CODEC_V3) + .stateUUID(randomAlphaOfLength(10)) + .clusterUUID(clusterUUID) + .nodeId("nodeA") + .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID(ClusterState.UNKNOWN_UUID) + .committed(true) + .routingTableVersion(0L) + .indicesRouting(indicesRouting1) + .diffManifest(diffManifest1) + .build(); + ClusterMetadataManifest manifest2 = ClusterMetadataManifest.builder(manifest1) + .indices(List.of(index2Metadata)) + .indicesRouting(indicesRouting2) + .diffManifest(diffManifest2) + .build(); + + BlobContainer blobContainer = mock(BlobContainer.class); + doThrow(IOException.class).when(blobContainer).delete(); + when(blobStore.blobContainer(any())).thenReturn(blobContainer); + BlobPath blobPath = new BlobPath().add("random-path"); + when((blobStoreRepository.basePath())).thenReturn(blobPath); + remoteClusterStateCleanupManager.start(); + when(remoteManifestManager.getManifestFolderPath(eq(clusterName), eq(clusterUUID))).thenReturn( + new BlobPath().add(encodeString(clusterName)).add(CLUSTER_STATE_PATH_TOKEN).add(clusterUUID).add(MANIFEST) + ); + when(remoteManifestManager.fetchRemoteClusterMetadataManifest(eq(clusterName), eq(clusterUUID), any())).thenReturn( + manifest2, + manifest1 + ); + remoteClusterStateCleanupManager = new RemoteClusterStateCleanupManager( + remoteClusterStateService, + clusterService, + remoteRoutingTableService + ); + remoteClusterStateCleanupManager.start(); + remoteClusterStateCleanupManager.deleteClusterMetadata(clusterName, clusterUUID, activeBlobs, inactiveBlobs); + verify(remoteRoutingTableService).deleteStaleIndexRoutingDiffPaths(List.of("index1RoutingDiffPath")); + } + public void testDeleteClusterMetadataNoOpsRoutingTableService() throws IOException { String clusterUUID = "clusterUUID"; String clusterName = "test-cluster"; @@ -515,6 +584,83 @@ public void testIndexRoutingFilesCleanupFailureStats() throws Exception { }); } + public void testIndicesRoutingDiffFilesCleanupFailureStats() throws Exception { + String clusterUUID = "clusterUUID"; + String clusterName = "test-cluster"; + List inactiveBlobs = Arrays.asList(new PlainBlobMetadata("manifest1.dat", 1L)); + List activeBlobs = Arrays.asList(new PlainBlobMetadata("manifest2.dat", 1L)); + + UploadedMetadataAttribute coordinationMetadata = new UploadedMetadataAttribute(COORDINATION_METADATA, "coordination_metadata"); + UploadedMetadataAttribute templateMetadata = new UploadedMetadataAttribute(TEMPLATES_METADATA, "template_metadata"); + UploadedMetadataAttribute settingMetadata = new UploadedMetadataAttribute(SETTING_METADATA, "settings_metadata"); + UploadedMetadataAttribute coordinationMetadataUpdated = new UploadedMetadataAttribute( + COORDINATION_METADATA, + "coordination_metadata_updated" + ); + + UploadedIndexMetadata index1Metadata = new UploadedIndexMetadata("index1", "indexUUID1", "index_metadata1__2"); + UploadedIndexMetadata index2Metadata = new UploadedIndexMetadata("index2", "indexUUID2", "index_metadata2__2"); + List indicesRouting1 = List.of(index1Metadata); + List indicesRouting2 = List.of(index2Metadata); + ClusterStateDiffManifest diffManifest1 = ClusterStateDiffManifest.builder().indicesRoutingDiffPath("index1RoutingDiffPath").build(); + ClusterStateDiffManifest diffManifest2 = ClusterStateDiffManifest.builder().indicesRoutingDiffPath("index2RoutingDiffPath").build(); + + ClusterMetadataManifest manifest1 = ClusterMetadataManifest.builder() + .indices(List.of(index1Metadata)) + .coordinationMetadata(coordinationMetadataUpdated) + .templatesMetadata(templateMetadata) + .settingMetadata(settingMetadata) + .clusterTerm(1L) + .stateVersion(1L) + .codecVersion(CODEC_V3) + .stateUUID(randomAlphaOfLength(10)) + .clusterUUID(clusterUUID) + .nodeId("nodeA") + .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID(ClusterState.UNKNOWN_UUID) + .committed(true) + .routingTableVersion(0L) + .indicesRouting(indicesRouting1) + .diffManifest(diffManifest1) + .build(); + ClusterMetadataManifest manifest2 = ClusterMetadataManifest.builder(manifest1) + .indices(List.of(index2Metadata)) + .indicesRouting(indicesRouting2) + .diffManifest(diffManifest2) + .build(); + + BlobContainer blobContainer = mock(BlobContainer.class); + doThrow(IOException.class).when(blobContainer).delete(); + when(blobStore.blobContainer(any())).thenReturn(blobContainer); + + BlobPath blobPath = new BlobPath().add("random-path"); + when((blobStoreRepository.basePath())).thenReturn(blobPath); + remoteClusterStateCleanupManager.start(); + when(remoteManifestManager.getManifestFolderPath(eq(clusterName), eq(clusterUUID))).thenReturn( + new BlobPath().add(encodeString(clusterName)).add(CLUSTER_STATE_PATH_TOKEN).add(clusterUUID).add(MANIFEST) + ); + when(remoteManifestManager.fetchRemoteClusterMetadataManifest(eq(clusterName), eq(clusterUUID), any())).thenReturn( + manifest1, + manifest2 + ); + doNothing().when(remoteRoutingTableService).deleteStaleIndexRoutingDiffPaths(any()); + + remoteClusterStateCleanupManager.deleteClusterMetadata(clusterName, clusterUUID, activeBlobs, inactiveBlobs); + assertBusy(() -> { + // wait for stats to get updated + assertNotNull(remoteClusterStateCleanupManager.getStats()); + assertEquals(0, remoteClusterStateCleanupManager.getStats().getIndicesRoutingDiffFileCleanupAttemptFailedCount()); + }); + + doThrow(IOException.class).when(remoteRoutingTableService).deleteStaleIndexRoutingPaths(any()); + remoteClusterStateCleanupManager.deleteClusterMetadata(clusterName, clusterUUID, activeBlobs, inactiveBlobs); + assertBusy(() -> { + // wait for stats to get updated + assertNotNull(remoteClusterStateCleanupManager.getStats()); + assertEquals(1, remoteClusterStateCleanupManager.getStats().getIndicesRoutingDiffFileCleanupAttemptFailedCount()); + }); + } + public void testSingleConcurrentExecutionOfStaleManifestCleanup() throws Exception { BlobContainer blobContainer = mock(BlobContainer.class); when(blobStore.blobContainer(any())).thenReturn(blobContainer);