From dd565b144c63f8d92b30c6596ef431f44291f6ba Mon Sep 17 00:00:00 2001 From: Rajiv Kumar Vaidyanathan Date: Mon, 26 Aug 2024 19:10:13 +0530 Subject: [PATCH] using remote cluster-state as fallback Signed-off-by: Rajiv Kumar Vaidyanathan --- CHANGELOG.md | 1 + .../RemoteClusterStateTermVersionIT.java | 212 ++++++++++++++++++ .../state/TransportClusterStateAction.java | 6 +- .../TransportClusterManagerNodeAction.java | 59 ++++- .../term/GetTermVersionResponse.java | 20 ++ .../term/TransportGetTermVersionAction.java | 28 ++- .../cluster/coordination/Coordinator.java | 9 + .../cluster/service/ClusterApplier.java | 6 + .../service/ClusterApplierService.java | 16 +- .../cluster/service/ClusterService.java | 8 + .../common/settings/FeatureFlagSettings.java | 3 +- .../opensearch/common/util/FeatureFlags.java | 9 +- .../remote/RemoteClusterStateCache.java | 58 +++++ .../remote/RemoteClusterStateService.java | 36 ++- .../gateway/remote/RemoteManifestManager.java | 23 +- ...ransportClusterManagerNodeActionTests.java | 92 ++++++++ .../term/ClusterTermVersionIT.java | 108 ++++++++- .../coordination/NoOpClusterApplier.java | 5 + .../RemoteClusterStateServiceTests.java | 169 +++++++++++++- .../snapshots/SnapshotResiliencyTests.java | 6 +- 20 files changed, 853 insertions(+), 21 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateTermVersionIT.java create mode 100644 server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCache.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 43f35a9119c12..39bb9742adf3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added - Adding WithFieldName interface for QueryBuilders with fieldName ([#15705](https://github.com/opensearch-project/OpenSearch/pull/15705)) - MultiTermQueries in keyword fields now default to `indexed` approach and gated behind cluster setting ([#15637](https://github.com/opensearch-project/OpenSearch/pull/15637)) +- Fallback to Remote cluster-state on Term-Version check mismatch - ([#15424](https://github.com/opensearch-project/OpenSearch/pull/15424)) ### Dependencies - Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https://github.com/opensearch-project/OpenSearch/pull/15578)) diff --git a/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateTermVersionIT.java b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateTermVersionIT.java new file mode 100644 index 0000000000000..256c2ef44b078 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteClusterStateTermVersionIT.java @@ -0,0 +1,212 @@ +/* + * 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.opensearch.action.admin.cluster.state.ClusterStateAction; +import org.opensearch.action.admin.cluster.state.ClusterStateRequest; +import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.action.support.clustermanager.term.GetTermVersionAction; +import org.opensearch.action.support.clustermanager.term.GetTermVersionResponse; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.coordination.ClusterStateTermVersion; +import org.opensearch.cluster.coordination.PublicationTransportHandler; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.blobstore.BlobPath; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.transport.TransportResponse; +import org.opensearch.gateway.remote.model.RemoteRoutingTableBlobStore; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.index.remote.RemoteStoreEnums; +import org.opensearch.plugins.Plugin; +import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase; +import org.opensearch.repositories.RepositoriesService; +import org.opensearch.repositories.blobstore.BlobStoreRepository; +import org.opensearch.test.OpenSearchIntegTestCase; +import org.opensearch.test.transport.MockTransportService; +import org.opensearch.transport.TransportService; +import org.junit.Before; + +import java.nio.file.Path; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING; +import static org.opensearch.gateway.remote.RemoteClusterStateService.REMOTE_PUBLICATION_SETTING_KEY; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.hamcrest.Matchers.is; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class RemoteClusterStateTermVersionIT extends RemoteStoreBaseIntegTestCase { + private static final String INDEX_NAME = "test-index"; + private static final String INDEX_NAME_1 = "test-index-1"; + List indexRoutingPaths; + AtomicInteger indexRoutingFiles = new AtomicInteger(); + private final RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.HASHED_PREFIX; + + @Before + public void setup() { + asyncUploadMockFsRepo = false; + } + + protected Collection> nodePlugins() { + return List.of(MockTransportService.TestPlugin.class); + } + + @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, REMOTE_ROUTING_TABLE_REPO) + .put(REMOTE_PUBLICATION_SETTING_KEY, true) + .build(); + } + + public void testRemoteClusterStateFallback() throws Exception { + BlobStoreRepository repository = prepareClusterAndVerifyRepository(); + + RemoteClusterStateService remoteClusterStateService = internalCluster().getClusterManagerNodeInstance( + RemoteClusterStateService.class + ); + + RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager(); + Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + + String[] dataNodes = internalCluster().getDataNodeNames().toArray(String[]::new); + MockTransportService primaryService = (MockTransportService) internalCluster().getInstance(TransportService.class, dataNodes[0]); + + String cm = internalCluster().getClusterManagerName(); + primaryService.addRequestHandlingBehavior( + PublicationTransportHandler.COMMIT_STATE_ACTION_NAME, + (handler, request, channel, task) -> { + // not committing the state + logger.info("ignoring the commit from cluster-manager {}", request); + channel.sendResponse(TransportResponse.Empty.INSTANCE); + } + ); + + String index = "index_1"; + createIndex( + index, + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), Long.MAX_VALUE) + .build() + ); + logger.info("created index {}", index); + Map callCounters = Map.ofEntries( + Map.entry(ClusterStateAction.NAME, new AtomicInteger()), + Map.entry(GetTermVersionAction.NAME, new AtomicInteger()) + ); + + addCallCountInterceptor(cm, callCounters); + + ClusterStateResponse stateResponseM = client(cm).admin().cluster().state(new ClusterStateRequest()).actionGet(); + + ClusterStateResponse stateResponseD = client(dataNodes[0]).admin().cluster().state(new ClusterStateRequest()).actionGet(); + assertEquals(stateResponseM, stateResponseD); + assertThat(callCounters.get(ClusterStateAction.NAME).get(), is(0)); + assertThat(callCounters.get(GetTermVersionAction.NAME).get(), is(1)); + + } + + public void testNoRemoteClusterStateFound() throws Exception { + BlobStoreRepository repository = prepareClusterAndVerifyRepository(); + + RemoteClusterStateService remoteClusterStateService = internalCluster().getClusterManagerNodeInstance( + RemoteClusterStateService.class + ); + + RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager(); + Optional latestManifest = remoteManifestManager.getLatestClusterMetadataManifest( + getClusterState().getClusterName().value(), + getClusterState().getMetadata().clusterUUID() + ); + + String[] dataNodes = internalCluster().getDataNodeNames().toArray(String[]::new); + MockTransportService primaryService = (MockTransportService) internalCluster().getInstance(TransportService.class, dataNodes[0]); + primaryService.addRequestHandlingBehavior( + PublicationTransportHandler.COMMIT_STATE_ACTION_NAME, + (handler, request, channel, task) -> { + // not committing the state + logger.info("ignoring the commit from cluster-manager {}", request); + channel.sendResponse(TransportResponse.Empty.INSTANCE); + } + ); + + ClusterState state = internalCluster().clusterService().state(); + String cm = internalCluster().getClusterManagerName(); + MockTransportService cmservice = (MockTransportService) internalCluster().getInstance(TransportService.class, cm); + cmservice.addRequestHandlingBehavior(GetTermVersionAction.NAME, (handler, request, channel, task) -> { + channel.sendResponse( + new GetTermVersionResponse(new ClusterStateTermVersion(state.getClusterName(), state.stateUUID(), -1, -1), true) + ); + }); + + Map callCounters = Map.ofEntries( + Map.entry(ClusterStateAction.NAME, new AtomicInteger()), + Map.entry(GetTermVersionAction.NAME, new AtomicInteger()) + ); + + addCallCountInterceptor(cm, callCounters); + + ClusterStateResponse stateResponseM = client(cm).admin().cluster().state(new ClusterStateRequest()).actionGet(); + ClusterStateResponse stateResponseD = client(dataNodes[0]).admin().cluster().state(new ClusterStateRequest()).actionGet(); + assertEquals(stateResponseM, stateResponseD); + assertThat(callCounters.get(ClusterStateAction.NAME).get(), is(1)); + assertThat(callCounters.get(GetTermVersionAction.NAME).get(), is(1)); + + } + + private void addCallCountInterceptor(String nodeName, Map callCounters) { + MockTransportService primaryService = (MockTransportService) internalCluster().getInstance(TransportService.class, nodeName); + for (var ctrEnty : callCounters.entrySet()) { + primaryService.addRequestHandlingBehavior(ctrEnty.getKey(), (handler, request, channel, task) -> { + ctrEnty.getValue().incrementAndGet(); + logger.info("--> {} response redirect", ctrEnty.getKey()); + handler.messageReceived(request, channel, task); + }); + } + } + + private BlobStoreRepository prepareClusterAndVerifyRepository() throws Exception { + clusterSettingsSuppliedByTest = true; + Path segmentRepoPath = randomRepoPath(); + Path translogRepoPath = randomRepoPath(); + Path remoteRoutingTableRepoPath = randomRepoPath(); + Settings settings = buildRemoteStoreNodeAttributes( + REPOSITORY_NAME, + segmentRepoPath, + REPOSITORY_2_NAME, + translogRepoPath, + REMOTE_ROUTING_TABLE_REPO, + remoteRoutingTableRepoPath, + false + ); + prepareCluster(1, 3, INDEX_NAME, 1, 5, settings); + ensureGreen(INDEX_NAME); + + RepositoriesService repositoriesService = internalCluster().getClusterManagerNodeInstance(RepositoriesService.class); + BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(REMOTE_ROUTING_TABLE_REPO); + + return repository; + } + +} diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/state/TransportClusterStateAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/state/TransportClusterStateAction.java index 13ea7eaa43bf8..da91d273f3bce 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/TransportClusterStateAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/state/TransportClusterStateAction.java @@ -46,10 +46,12 @@ import org.opensearch.cluster.metadata.Metadata.Custom; import org.opensearch.cluster.routing.RoutingTable; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.Nullable; import org.opensearch.common.inject.Inject; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.node.NodeClosedException; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; @@ -80,7 +82,8 @@ public TransportClusterStateAction( ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, - IndexNameExpressionResolver indexNameExpressionResolver + IndexNameExpressionResolver indexNameExpressionResolver, + @Nullable RemoteClusterStateService remoteClusterStateService ) { super( ClusterStateAction.NAME, @@ -93,6 +96,7 @@ public TransportClusterStateAction( indexNameExpressionResolver ); this.localExecuteSupported = true; + this.remoteClusterStateService = remoteClusterStateService; } @Override diff --git a/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java b/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java index 4e869f29878cd..819e09312a0df 100644 --- a/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java @@ -49,6 +49,7 @@ import org.opensearch.cluster.ClusterStateObserver; import org.opensearch.cluster.NotClusterManagerException; import org.opensearch.cluster.block.ClusterBlockException; +import org.opensearch.cluster.coordination.ClusterStateTermVersion; import org.opensearch.cluster.coordination.FailedToCommitClusterStateException; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.ProcessClusterEventTimeoutException; @@ -63,6 +64,8 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.Writeable; import org.opensearch.discovery.ClusterManagerNotDiscoveredException; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.node.NodeClosedException; import org.opensearch.ratelimitting.admissioncontrol.enums.AdmissionControlActionType; import org.opensearch.tasks.Task; @@ -74,6 +77,7 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.util.Optional; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Predicate; @@ -95,6 +99,8 @@ public abstract class TransportClusterManagerNodeAction clusterMetadataManifest = remoteClusterStateService + .getClusterMetadataManifestByTermVersion( + clusterStateTermVersion.getClusterName().value(), + clusterStateTermVersion.getClusterUUID(), + clusterStateTermVersion.getTerm(), + clusterStateTermVersion.getVersion() + ); + if (clusterMetadataManifest.isEmpty()) { + logger.trace("could not find manifest in remote-store for ClusterStateTermVersion {}", termVersion); + return null; + } + ClusterState clusterStateFromRemote = remoteClusterStateService.getClusterStateForManifest( + appliedState.getClusterName().value(), + clusterMetadataManifest.get(), + appliedState.nodes().getLocalNode().getId(), + true + ); + + if (clusterStateFromRemote != null) { + logger.trace("Using the remote cluster-state fetched from local node, ClusterStateTermVersion {}", termVersion); + return clusterStateFromRemote; + } + } catch (Exception e) { + logger.trace("Error while fetching from remote cluster state", e); + } + } + return null; + } + private boolean checkForBlock(Request request, ClusterState localClusterState) { final ClusterBlockException blockException = checkBlock(request, localClusterState); if (blockException != null) { diff --git a/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionResponse.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionResponse.java index 0906abe57d547..cf8e6085d4b32 100644 --- a/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionResponse.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionResponse.java @@ -8,6 +8,7 @@ package org.opensearch.action.support.clustermanager.term; +import org.opensearch.Version; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.coordination.ClusterStateTermVersion; import org.opensearch.core.action.ActionResponse; @@ -25,18 +26,34 @@ public class GetTermVersionResponse extends ActionResponse { private final ClusterStateTermVersion clusterStateTermVersion; + private final boolean isStatePresentInRemote; + public GetTermVersionResponse(ClusterStateTermVersion clusterStateTermVersion) { this.clusterStateTermVersion = clusterStateTermVersion; + this.isStatePresentInRemote = false; + } + + public GetTermVersionResponse(ClusterStateTermVersion clusterStateTermVersion, boolean canDownloadFromRemote) { + this.clusterStateTermVersion = clusterStateTermVersion; + this.isStatePresentInRemote = canDownloadFromRemote; } public GetTermVersionResponse(StreamInput in) throws IOException { super(in); this.clusterStateTermVersion = new ClusterStateTermVersion(in); + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + this.isStatePresentInRemote = in.readOptionalBoolean(); + } else { + this.isStatePresentInRemote = false; + } } @Override public void writeTo(StreamOutput out) throws IOException { clusterStateTermVersion.writeTo(out); + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + out.writeOptionalBoolean(isStatePresentInRemote); + } } public ClusterStateTermVersion getClusterStateTermVersion() { @@ -47,4 +64,7 @@ public boolean matches(ClusterState clusterState) { return clusterStateTermVersion != null && clusterStateTermVersion.equals(new ClusterStateTermVersion(clusterState)); } + public boolean isStatePresentInRemote() { + return isStatePresentInRemote; + } } diff --git a/server/src/main/java/org/opensearch/action/support/clustermanager/term/TransportGetTermVersionAction.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/TransportGetTermVersionAction.java index 4752a99c910e4..1cab739a20838 100644 --- a/server/src/main/java/org/opensearch/action/support/clustermanager/term/TransportGetTermVersionAction.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/term/TransportGetTermVersionAction.java @@ -15,11 +15,14 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.block.ClusterBlockException; import org.opensearch.cluster.coordination.ClusterStateTermVersion; +import org.opensearch.cluster.coordination.Coordinator; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.discovery.Discovery; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; @@ -34,13 +37,18 @@ public class TransportGetTermVersionAction extends TransportClusterManagerNodeRe private final Logger logger = LogManager.getLogger(getClass()); + private final Discovery discovery; + + private boolean usePreCommitState = false; + @Inject public TransportGetTermVersionAction( TransportService transportService, ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, - IndexNameExpressionResolver indexNameExpressionResolver + IndexNameExpressionResolver indexNameExpressionResolver, + Discovery discovery ) { super( GetTermVersionAction.NAME, @@ -52,6 +60,8 @@ public TransportGetTermVersionAction( GetTermVersionRequest::new, indexNameExpressionResolver ); + this.usePreCommitState = FeatureFlags.isEnabled(FeatureFlags.TERM_VERSION_PRECOMMIT_ENABLE_SETTING); + this.discovery = discovery; } @Override @@ -76,10 +86,22 @@ protected void clusterManagerOperation( ClusterState state, ActionListener listener ) throws Exception { - ActionListener.completeWith(listener, () -> buildResponse(request, state)); + if (usePreCommitState) { + ActionListener.completeWith(listener, () -> buildResponse(request, clusterService.preCommitState())); + } else { + ActionListener.completeWith(listener, () -> buildResponse(request, state)); + } + } private GetTermVersionResponse buildResponse(GetTermVersionRequest request, ClusterState state) { - return new GetTermVersionResponse(new ClusterStateTermVersion(state)); + ClusterStateTermVersion termVersion = new ClusterStateTermVersion(state); + if (discovery instanceof Coordinator) { + Coordinator coordinator = (Coordinator) discovery; + if (coordinator.isRemotePublicationEnabled()) { + return new GetTermVersionResponse(termVersion, coordinator.isRemotePublicationEnabled()); + } + } + return new GetTermVersionResponse(termVersion); } } diff --git a/server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java b/server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java index 9aaaa77bcbb23..13a57d93f03f0 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java @@ -386,6 +386,8 @@ private void handleApplyCommit(ApplyCommitRequest applyCommitRequest, ActionList coordinationState.get().handleCommit(applyCommitRequest); final ClusterState committedState = hideStateIfNotRecovered(coordinationState.get().getLastAcceptedState()); applierState = mode == Mode.CANDIDATE ? clusterStateWithNoClusterManagerBlock(committedState) : committedState; + clusterApplier.setPreCommitState(applierState); + if (applyCommitRequest.getSourceNode().equals(getLocalNode())) { // cluster-manager node applies the committed state at the end of the publication process, not here. applyListener.onResponse(null); @@ -1862,4 +1864,11 @@ protected void sendApplyCommit( public static boolean isZen1Node(DiscoveryNode discoveryNode) { return Booleans.isTrue(discoveryNode.getAttributes().getOrDefault("zen1", "false")); } + + public boolean isRemotePublicationEnabled() { + if (coordinationState.get() != null) { + return coordinationState.get().isRemotePublicationEnabled(); + } + return false; + } } diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterApplier.java b/server/src/main/java/org/opensearch/cluster/service/ClusterApplier.java index 5b3f7f1001779..df31b0e94d734 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterApplier.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterApplier.java @@ -49,6 +49,12 @@ public interface ClusterApplier { */ void setInitialState(ClusterState initialState); + /** + * Sets the pre-commit state for the applier. + * @param clusterState state that has been committed by coordinator to store + */ + void setPreCommitState(ClusterState clusterState); + /** * Method to invoke when a new cluster state is available to be applied * diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java index b2548a8976c73..47080cfbde692 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java @@ -119,7 +119,7 @@ public class ClusterApplierService extends AbstractLifecycleComponent implements private final Collection clusterStateListeners = new CopyOnWriteArrayList<>(); private final Map timeoutClusterStateListeners = new ConcurrentHashMap<>(); - + private final AtomicReference preCommitState = new AtomicReference<>(); // last state which is yet to be applied private final AtomicReference state; // last applied state private final String nodeName; @@ -750,4 +750,18 @@ protected long currentTimeInMillis() { protected boolean applicationMayFail() { return false; } + + /** + * Pre-commit State of the cluster-applier + * @return ClusterState + */ + public ClusterState preCommitState() { + return preCommitState.get(); + } + + @Override + public void setPreCommitState(ClusterState clusterState) { + preCommitState.set(clusterState); + } + } diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java index c3c48dd8b87ef..1a79161d223e2 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java @@ -183,6 +183,14 @@ public ClusterState state() { return clusterApplierService.state(); } + /** + * The state that is persisted to store but may not be applied to cluster. + * @return ClusterState + */ + public ClusterState preCommitState() { + return clusterApplierService.preCommitState(); + } + /** * Adds a high priority applier of updated cluster states. */ diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java index c8d00f65bda10..73375f9eaa413 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -39,6 +39,7 @@ protected FeatureFlagSettings( FeatureFlags.PLUGGABLE_CACHE_SETTING, FeatureFlags.STAR_TREE_INDEX_SETTING, FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING, - FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING + FeatureFlags.READER_WRITER_SPLIT_EXPERIMENTAL_SETTING, + FeatureFlags.TERM_VERSION_PRECOMMIT_ENABLE_SETTING ); } diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index 49ecbb0a7069d..a5acea004c3b2 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -128,6 +128,12 @@ public class FeatureFlags { false, Property.NodeScope ); + public static final String TERM_VERSION_PRECOMMIT_ENABLE = "opensearch.experimental.optimization.termversion.precommit.enabled"; + public static final Setting TERM_VERSION_PRECOMMIT_ENABLE_SETTING = Setting.boolSetting( + TERM_VERSION_PRECOMMIT_ENABLE, + false, + Property.NodeScope + ); private static final List> ALL_FEATURE_FLAG_SETTINGS = List.of( REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, @@ -139,7 +145,8 @@ public class FeatureFlags { PLUGGABLE_CACHE_SETTING, STAR_TREE_INDEX_SETTING, APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING, - READER_WRITER_SPLIT_EXPERIMENTAL_SETTING + READER_WRITER_SPLIT_EXPERIMENTAL_SETTING, + TERM_VERSION_PRECOMMIT_ENABLE_SETTING ); /** diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCache.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCache.java new file mode 100644 index 0000000000000..de36ac4429302 --- /dev/null +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateCache.java @@ -0,0 +1,58 @@ +/* + * 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.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.coordination.ClusterStateTermVersion; +import org.opensearch.common.collect.Tuple; + +import java.util.concurrent.atomic.AtomicReference; + +/** + * Cache to Remote Cluster State based on term-version check. The current implementation + * caches the last highest version of cluster-state that was downloaded from cache. + * + * @opensearch.internal + */ +public class RemoteClusterStateCache { + + private final AtomicReference> clusterStateFromCache = new AtomicReference<>(); + + public ClusterState getState(String clusterName, ClusterMetadataManifest manifest) { + Tuple cache = clusterStateFromCache.get(); + if (cache != null) { + ClusterStateTermVersion manifestStateTermVersion = new ClusterStateTermVersion( + new ClusterName(clusterName), + manifest.getClusterUUID(), + manifest.getClusterTerm(), + manifest.getStateVersion() + ); + if (cache.v1().equals(manifestStateTermVersion)) { + return cache.v2(); + } + } + return null; + } + + public void putState(final ClusterState newState) { + if (newState.metadata() == null || newState.coordinationMetadata() == null) { + // ensure the remote cluster state has coordination metadata set + return; + } + + ClusterStateTermVersion cacheStateTermVersion = new ClusterStateTermVersion( + new ClusterName(newState.getClusterName().value()), + newState.metadata().clusterUUID(), + newState.term(), + newState.version() + ); + clusterStateFromCache.set(new Tuple<>(cacheStateTermVersion, newState)); + } +} 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 e504c5abb46d3..12d10fd908b44 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java @@ -33,6 +33,7 @@ import org.opensearch.cluster.routing.remote.RemoteRoutingTableServiceFactory; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Nullable; +import org.opensearch.common.annotation.InternalApi; import org.opensearch.common.blobstore.BlobContainer; import org.opensearch.common.blobstore.BlobStore; import org.opensearch.common.settings.ClusterSettings; @@ -117,6 +118,7 @@ * * @opensearch.internal */ +@InternalApi public class RemoteClusterStateService implements Closeable { private static final Logger logger = LogManager.getLogger(RemoteClusterStateService.class); @@ -233,6 +235,7 @@ public static RemoteClusterStateValidationMode parseString(String mode) { private final boolean isPublicationEnabled; private final String remotePathPrefix; + private final RemoteClusterStateCache remoteClusterStateCache; // ToXContent Params with gateway mode. // We are using gateway context mode to persist all custom metadata. public static final ToXContent.Params FORMAT_PARAMS; @@ -282,6 +285,7 @@ public RemoteClusterStateService( ClusterName.CLUSTER_NAME_SETTING.get(settings).value() ); remoteClusterStateCleanupManager = new RemoteClusterStateCleanupManager(this, clusterService, remoteRoutingTableService); + remoteClusterStateCache = new RemoteClusterStateCache(); } /** @@ -1032,6 +1036,15 @@ public ClusterMetadataManifest getClusterMetadataManifestByFileName(String clust return remoteManifestManager.getRemoteClusterMetadataManifestByFileName(clusterUUID, fileName); } + public Optional getClusterMetadataManifestByTermVersion( + String clusterName, + String clusterUUID, + long term, + long version + ) { + return remoteManifestManager.getClusterMetadataManifestByTermVersion(clusterName, clusterUUID, term, version); + } + @Override public void close() throws IOException { remoteClusterStateCleanupManager.close(); @@ -1442,6 +1455,11 @@ public ClusterState getClusterStateForManifest( String localNodeId, boolean includeEphemeral ) throws IOException { + ClusterState stateFromCache = remoteClusterStateCache.getState(clusterName, manifest); + if (stateFromCache != null) { + return stateFromCache; + } + final ClusterState clusterState; final long startTimeNanos = relativeTimeNanosSupplier.getAsLong(); if (manifest.onOrAfterCodecVersion(CODEC_V2)) { @@ -1498,7 +1516,10 @@ public ClusterState getClusterStateForManifest( final long durationMillis = TimeValue.nsecToMSec(relativeTimeNanosSupplier.getAsLong() - startTimeNanos); remoteStateStats.stateFullDownloadSucceeded(); remoteStateStats.stateFullDownloadTook(durationMillis); - + if (includeEphemeral) { + // cache only if the entire cluster-state is present + remoteClusterStateCache.putState(clusterState); + } return clusterState; } @@ -1506,6 +1527,8 @@ public ClusterState getClusterStateUsingDiff(ClusterMetadataManifest manifest, C assert manifest.getDiffManifest() != null : "Diff manifest null which is required for downloading cluster state"; final long startTimeNanos = relativeTimeNanosSupplier.getAsLong(); ClusterStateDiffManifest diff = manifest.getDiffManifest(); + boolean includeEphemeral = true; + List updatedIndices = diff.getIndicesUpdated().stream().map(idx -> { Optional uploadedIndexMetadataOptional = manifest.getIndices() .stream() @@ -1554,7 +1577,7 @@ public ClusterState getClusterStateUsingDiff(ClusterMetadataManifest manifest, C manifest.getDiffManifest() != null && manifest.getDiffManifest().getIndicesRoutingDiffPath() != null && !manifest.getDiffManifest().getIndicesRoutingDiffPath().isEmpty(), - true + includeEphemeral ); ClusterState.Builder clusterStateBuilder = ClusterState.builder(updatedClusterState); Metadata.Builder metadataBuilder = Metadata.builder(updatedClusterState.metadata()); @@ -1588,7 +1611,6 @@ public ClusterState getClusterStateUsingDiff(ClusterMetadataManifest manifest, C .metadata(metadataBuilder) .routingTable(new RoutingTable(manifest.getRoutingTableVersion(), indexRoutingTables)) .build(); - if (!remoteClusterStateValidationMode.equals(RemoteClusterStateValidationMode.NONE) && manifest.getClusterStateChecksum() != null) { validateClusterStateFromChecksum(manifest, clusterState, previousState.getClusterName().value(), localNodeId, false); } @@ -1596,6 +1618,9 @@ public ClusterState getClusterStateUsingDiff(ClusterMetadataManifest manifest, C remoteStateStats.stateDiffDownloadSucceeded(); remoteStateStats.stateDiffDownloadTook(durationMillis); + assert includeEphemeral == true; + // newState includes all the fields of cluster-state (includeEphemeral=true always) + remoteClusterStateCache.putState(clusterState); return clusterState; } @@ -1989,4 +2014,9 @@ public void fullDownloadFailed() { public void diffDownloadFailed() { remoteStateStats.stateDiffDownloadFailed(); } + + RemoteClusterStateCache getRemoteClusterStateCache() { + return remoteClusterStateCache; + } + } diff --git a/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java b/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java index 47c847b5dc32a..b243269fe323e 100644 --- a/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java +++ b/server/src/main/java/org/opensearch/gateway/remote/RemoteManifestManager.java @@ -194,6 +194,17 @@ public Optional getLatestClusterMetadataManifest(String return latestManifestFileName.map(s -> fetchRemoteClusterMetadataManifest(clusterName, clusterUUID, s)); } + public Optional getClusterMetadataManifestByTermVersion( + String clusterName, + String clusterUUID, + long term, + long version + ) { + String prefix = RemoteManifestManager.getManifestFilePrefixForTermVersion(term, version); + Optional latestManifestFileName = getManifestFileNameByPrefix(clusterName, clusterUUID, prefix); + return latestManifestFileName.map(s -> fetchRemoteClusterMetadataManifest(clusterName, clusterUUID, s)); + } + public ClusterMetadataManifest getRemoteClusterMetadataManifestByFileName(String clusterUUID, String filename) throws IllegalStateException { try { @@ -293,7 +304,7 @@ private List getManifestFileNames(String clusterName, String clust } } - static String getManifestFilePrefixForTermVersion(long term, long version) { + public static String getManifestFilePrefixForTermVersion(long term, long version) { return String.join( DELIMITER, RemoteClusterMetadataManifest.MANIFEST, @@ -322,4 +333,14 @@ private Optional getLatestManifestFileName(String clusterName, String cl logger.info("No manifest file present in remote store for cluster name: {}, cluster UUID: {}", clusterName, clusterUUID); return Optional.empty(); } + + private Optional getManifestFileNameByPrefix(String clusterName, String clusterUUID, String filePrefix) + throws IllegalStateException { + List manifestFilesMetadata = getManifestFileNames(clusterName, clusterUUID, filePrefix, 1); + if (manifestFilesMetadata != null && !manifestFilesMetadata.isEmpty()) { + return Optional.of(manifestFilesMetadata.get(0).name()); + } + logger.info("No manifest file present in remote store for cluster name: {}, cluster UUID: {}", clusterName, clusterUUID); + return Optional.empty(); + } } diff --git a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java index 3c1c84653b384..00198364fc8d7 100644 --- a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java @@ -21,6 +21,7 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.PlainActionFuture; import org.opensearch.action.support.ThreadedActionListener; +import org.opensearch.action.support.clustermanager.term.GetTermVersionResponse; import org.opensearch.action.support.replication.ClusterStateCreationUtils; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; @@ -30,6 +31,8 @@ import org.opensearch.cluster.block.ClusterBlockException; import org.opensearch.cluster.block.ClusterBlockLevel; import org.opensearch.cluster.block.ClusterBlocks; +import org.opensearch.cluster.coordination.ClusterStateTermVersion; +import org.opensearch.cluster.coordination.CoordinationMetadata; import org.opensearch.cluster.coordination.FailedToCommitClusterStateException; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.Metadata; @@ -54,6 +57,8 @@ import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.rest.RestStatus; import org.opensearch.discovery.ClusterManagerNotDiscoveredException; +import org.opensearch.gateway.remote.ClusterMetadataManifest; +import org.opensearch.gateway.remote.RemoteClusterStateService; import org.opensearch.node.NodeClosedException; import org.opensearch.node.remotestore.RemoteStoreNodeService; import org.opensearch.snapshots.EmptySnapshotsInfoService; @@ -77,6 +82,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CyclicBarrier; @@ -84,8 +90,11 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.mockito.Mockito; + import static org.opensearch.index.remote.RemoteMigrationIndexMetadataUpdaterTests.createIndexMetadataWithRemoteStoreSettings; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; @@ -94,6 +103,8 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; public class TransportClusterManagerNodeActionTests extends OpenSearchTestCase { private static ThreadPool threadPool; @@ -209,6 +220,8 @@ public void writeTo(StreamOutput out) throws IOException { } class Action extends TransportClusterManagerNodeAction { + private boolean localExecuteSupported = false; + Action(String actionName, TransportService transportService, ClusterService clusterService, ThreadPool threadPool) { super( actionName, @@ -221,6 +234,18 @@ class Action extends TransportClusterManagerNodeAction { ); } + Action( + String actionName, + TransportService transportService, + ClusterService clusterService, + ThreadPool threadPool, + RemoteClusterStateService clusterStateService + ) { + this(actionName, transportService, clusterService, threadPool); + this.remoteClusterStateService = clusterStateService; + this.localExecuteSupported = true; + } + @Override protected void doExecute(Task task, final Request request, ActionListener listener) { // remove unneeded threading by wrapping listener with SAME to prevent super.doExecute from wrapping it with LISTENER @@ -247,6 +272,10 @@ protected void clusterManagerOperation(Request request, ClusterState state, Acti protected ClusterBlockException checkBlock(Request request, ClusterState state) { return null; // default implementation, overridden in specific tests } + + public boolean localExecuteSupportedByAction() { + return localExecuteSupported; + } } public void testLocalOperationWithoutBlocks() throws ExecutionException, InterruptedException { @@ -715,6 +744,69 @@ protected void masterOperation(Task task, Request request, ClusterState state, A assertFalse(exception.get()); } + public void testFetchFromRemoteStore() throws InterruptedException, BrokenBarrierException, ExecutionException, IOException { + Map attributes = new HashMap<>(); + attributes.put(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "repo1"); + attributes.put(REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY, "repo2"); + + localNode = new DiscoveryNode( + "local_node", + buildNewFakeTransportAddress(), + attributes, + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Version.CURRENT + ); + remoteNode = new DiscoveryNode( + "remote_node", + buildNewFakeTransportAddress(), + attributes, + Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE), + Version.CURRENT + ); + allNodes = new DiscoveryNode[] { localNode, remoteNode }; + + setState(clusterService, ClusterStateCreationUtils.state(localNode, remoteNode, allNodes)); + + ClusterState state = clusterService.state(); + RemoteClusterStateService remoteClusterStateService = Mockito.mock(RemoteClusterStateService.class); + ClusterMetadataManifest manifest = ClusterMetadataManifest.builder() + .clusterTerm(state.term() + 1) + .stateVersion(state.version() + 1) + .build(); + when( + remoteClusterStateService.getClusterMetadataManifestByTermVersion( + eq(state.getClusterName().value()), + eq(state.metadata().clusterUUID()), + eq(state.term() + 1), + eq(state.version() + 1) + ) + ).thenReturn(Optional.of(manifest)); + when(remoteClusterStateService.getClusterStateForManifest(state.getClusterName().value(), manifest, localNode.getId(), true)) + .thenReturn(buildClusterState(state, state.term() + 1, state.version() + 1)); + + PlainActionFuture listener = new PlainActionFuture<>(); + Request request = new Request(); + Action action = new Action("internal:testAction", transportService, clusterService, threadPool, remoteClusterStateService); + action.execute(request, listener); + + CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0]; + // mismatch term and version + GetTermVersionResponse termResp = new GetTermVersionResponse( + new ClusterStateTermVersion(state.getClusterName(), state.metadata().clusterUUID(), state.term() + 1, state.version() + 1), + true + ); + transport.handleResponse(capturedRequest.requestId, termResp); + // no more transport calls + assertThat(transport.capturedRequests().length, equalTo(1)); + assertTrue(listener.isDone()); + } + + private ClusterState buildClusterState(ClusterState state, long term, long version) { + CoordinationMetadata.Builder coordMetadataBuilder = CoordinationMetadata.builder().term(term); + Metadata newMetadata = Metadata.builder().coordinationMetadata(coordMetadataBuilder.build()).build(); + return ClusterState.builder(state).version(version).metadata(newMetadata).build(); + } + public void testDontAllowSwitchingToStrictCompatibilityModeForMixedCluster() { // request to change cluster compatibility mode to STRICT Settings currentCompatibilityModeSettings = Settings.builder() diff --git a/server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionIT.java b/server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionIT.java index 7b783e025a575..7ab9da231896c 100644 --- a/server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionIT.java +++ b/server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionIT.java @@ -11,9 +11,19 @@ import org.opensearch.action.admin.cluster.state.ClusterStateAction; import org.opensearch.action.admin.cluster.state.ClusterStateRequest; import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.action.admin.indices.create.CreateIndexResponse; +import org.opensearch.client.Client; +import org.opensearch.cluster.ClusterChangedEvent; import org.opensearch.cluster.ClusterName; +import org.opensearch.cluster.ClusterStateApplier; import org.opensearch.cluster.coordination.ClusterStateTermVersion; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.action.ActionFuture; +import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.index.mapper.MapperService; import org.opensearch.plugins.Plugin; import org.opensearch.test.OpenSearchIntegTestCase; import org.opensearch.test.transport.MockTransportService; @@ -22,6 +32,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import static org.hamcrest.Matchers.is; @@ -100,12 +111,107 @@ public void testDatanodeOutOfSync() throws Exception { assertThat(stateResponse.getState().nodes().getSize(), is(internalCluster().getNodeNames().length)); } + public void testDatanodeWithSlowClusterApplierFallbackToPublish() throws Exception { + List masters = internalCluster().startClusterManagerOnlyNodes( + 3, + Settings.builder().put(FeatureFlags.TERM_VERSION_PRECOMMIT_ENABLE, "true").build() + ); + List datas = internalCluster().startDataOnlyNodes(3); + + Map callCounters = Map.ofEntries( + Map.entry(ClusterStateAction.NAME, new AtomicInteger()), + Map.entry(GetTermVersionAction.NAME, new AtomicInteger()) + ); + ensureGreen(); + + String master = internalCluster().getClusterManagerName(); + + AtomicBoolean processState = new AtomicBoolean(); + ClusterService cmClsService = internalCluster().getInstance(ClusterService.class, datas.get(0)); + cmClsService.addStateApplier(new ClusterStateApplier() { + @Override + public void applyClusterState(ClusterChangedEvent event) { + logger.info("Slow applier started"); + while (processState.get()) { + try { + logger.info("Sleeping for 1s"); + Thread.sleep(1000); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + logger.info("Slow applier ended"); + } + }); + + ensureGreen(); + + GetTermVersionResponse respBeforeUpdate = internalCluster().getInstance(Client.class, master) + .execute(GetTermVersionAction.INSTANCE, new GetTermVersionRequest()) + .get(); + + processState.set(true); + String index = "index_1"; + ActionFuture startCreateIndex1 = prepareCreate(index).setSettings( + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), Long.MAX_VALUE) + .build() + ).execute(); + + ActionFuture startCreateIndex2 = prepareCreate("index_2").setSettings( + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), Long.MAX_VALUE) + .build() + ).execute(); + + // wait for cluster-manager to publish new state + waitUntil(() -> { + try { + // node is yet to ack commit to cluster-manager , only the state-update corresponding to index_1 should have been published + GetTermVersionResponse respAfterUpdate = internalCluster().getInstance(Client.class, master) + .execute(GetTermVersionAction.INSTANCE, new GetTermVersionRequest()) + .get(); + logger.info( + "data has latest , {} , {}", + respAfterUpdate.getClusterStateTermVersion().getTerm(), + respAfterUpdate.getClusterStateTermVersion().getVersion() + ); + return respBeforeUpdate.getClusterStateTermVersion().getVersion() + 1 == respAfterUpdate.getClusterStateTermVersion() + .getVersion(); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + addCallCountInterceptor(master, callCounters); + ClusterStateResponse stateResponseD = internalCluster().getInstance(Client.class, datas.get(0)) + .admin() + .cluster() + .state(new ClusterStateRequest()) + .actionGet(); + logger.info("data has the version , {} , {}", stateResponseD.getState().term(), stateResponseD.getState().version()); + assertTrue(respBeforeUpdate.getClusterStateTermVersion().getVersion() + 1 == stateResponseD.getState().version()); + + processState.set(false); + + AtomicInteger clusterStateCallsOnMaster = callCounters.get(ClusterStateAction.NAME); + AtomicInteger termCallsOnMaster = callCounters.get(GetTermVersionAction.NAME); + startCreateIndex1.get(); + startCreateIndex2.get(); + assertThat(clusterStateCallsOnMaster.get(), is(0)); + assertThat(termCallsOnMaster.get(), is(1)); + } + private void addCallCountInterceptor(String nodeName, Map callCounters) { MockTransportService primaryService = (MockTransportService) internalCluster().getInstance(TransportService.class, nodeName); for (var ctrEnty : callCounters.entrySet()) { primaryService.addRequestHandlingBehavior(ctrEnty.getKey(), (handler, request, channel, task) -> { ctrEnty.getValue().incrementAndGet(); - logger.info("--> {} response redirect", ClusterStateAction.NAME); + logger.info("--> {} response redirect", ctrEnty.getKey()); handler.messageReceived(request, channel, task); }); } diff --git a/server/src/test/java/org/opensearch/cluster/coordination/NoOpClusterApplier.java b/server/src/test/java/org/opensearch/cluster/coordination/NoOpClusterApplier.java index 9b865ace3b082..6e0d86540cf99 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/NoOpClusterApplier.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/NoOpClusterApplier.java @@ -42,6 +42,11 @@ public void setInitialState(ClusterState initialState) { } + @Override + public void setPreCommitState(ClusterState clusterState) { + + } + @Override public void onNewClusterState(String source, Supplier clusterStateSupplier, ClusterApplyListener listener) { listener.onSuccess(source); 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 e875b1c5dc64e..b11d5e48bec06 100644 --- a/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java +++ b/server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateServiceTests.java @@ -46,7 +46,6 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; -import org.opensearch.core.ParseField; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; @@ -62,6 +61,7 @@ import org.opensearch.gateway.remote.model.RemoteReadResult; import org.opensearch.gateway.remote.model.RemoteTransientSettingsMetadata; import org.opensearch.index.remote.RemoteIndexPathUploader; +import org.opensearch.index.remote.RemoteStoreUtils; import org.opensearch.indices.DefaultRemoteStoreSettings; import org.opensearch.indices.IndicesModule; import org.opensearch.repositories.FilterRepository; @@ -2322,6 +2322,73 @@ public void testReadLatestMetadataManifestSuccessButNoIndexMetadata() throws IOE ); } + public void testReadLatestClusterStateFromCache() throws IOException { + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + remoteClusterStateService.start(); + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(List.of()) + .clusterTerm(1L) + .stateVersion(12) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .codecVersion(MANIFEST_CURRENT_CODEC_VERSION) + .coordinationMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(COORDINATION_METADATA, "mock-coordination-file")) + .settingMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(SETTING_METADATA, "mock-setting-file")) + .templatesMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(TEMPLATES_METADATA, "mock-templates-file")) + .put( + IndexGraveyard.TYPE, + new ClusterMetadataManifest.UploadedMetadataAttribute(IndexGraveyard.TYPE, "mock-custom-" + IndexGraveyard.TYPE + "-file") + ) + .nodeId("nodeA") + .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1) + .indicesRouting(List.of()) + .build(); + + Metadata expectedMetadata = Metadata.builder() + .clusterUUID("cluster-uuid") + .version(12) + .coordinationMetadata(CoordinationMetadata.builder().term(1).build()) + .persistentSettings(Settings.builder().put("readonly", true).build()) + .build(); + mockBlobContainerForGlobalMetadata(mockBlobStoreObjects(), expectedManifest, expectedMetadata); + + ClusterState state = remoteClusterStateService.getLatestClusterState( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID(), + true + ); + + ClusterState stateFromCache = remoteClusterStateService.getRemoteClusterStateCache() + .getState(clusterState.getClusterName().value(), expectedManifest); + assertEquals(stateFromCache.getMetadata(), state.getMetadata()); + + final ClusterMetadataManifest notExistMetadata = ClusterMetadataManifest.builder() + .indices(List.of()) + .clusterTerm(1L) + .stateVersion(13) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .codecVersion(MANIFEST_CURRENT_CODEC_VERSION) + .coordinationMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(COORDINATION_METADATA, "mock-coordination-file")) + .settingMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(SETTING_METADATA, "mock-setting-file")) + .templatesMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(TEMPLATES_METADATA, "mock-templates-file")) + .put( + IndexGraveyard.TYPE, + new ClusterMetadataManifest.UploadedMetadataAttribute(IndexGraveyard.TYPE, "mock-custom-" + IndexGraveyard.TYPE + "-file") + ) + .nodeId("nodeA") + .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .previousClusterUUID("prev-cluster-uuid") + .routingTableVersion(1) + .indicesRouting(List.of()) + .build(); + ClusterState notInCacheState = remoteClusterStateService.getRemoteClusterStateCache() + .getState(clusterState.getClusterName().value(), notExistMetadata); + assertNull(notInCacheState); + } + public void testReadLatestMetadataManifestSuccessButIndexMetadataFetchIOException() throws IOException { final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename__2"); @@ -2388,9 +2455,46 @@ public void testReadLatestMetadataManifestSuccess() throws IOException { assertThat(manifest.getStateUUID(), is(expectedManifest.getStateUUID())); } + public void testReadLatestMetadataManifestSuccessByTermVersion() throws IOException { + final ClusterState clusterState = generateClusterStateWithOneIndex().nodes(nodesWithLocalNodeClusterManager()).build(); + final UploadedIndexMetadata uploadedIndexMetadata = new UploadedIndexMetadata("test-index", "index-uuid", "metadata-filename"); + final List indices = List.of(uploadedIndexMetadata); + + final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder() + .indices(indices) + .clusterTerm(1L) + .stateVersion(1L) + .stateUUID("state-uuid") + .clusterUUID("cluster-uuid") + .nodeId("nodeA") + .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) + .codecVersion(CODEC_V2) + .previousClusterUUID("prev-cluster-uuid") + .build(); + + mockBlobContainer(mockBlobStoreObjects(), expectedManifest, new HashMap<>(), CODEC_V2, 1, 1); + remoteClusterStateService.start(); + final ClusterMetadataManifest manifest = remoteClusterStateService.getClusterMetadataManifestByTermVersion( + clusterState.getClusterName().value(), + clusterState.metadata().clusterUUID(), + 1, + 1 + ).get(); + + 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())); + } + public void testReadGlobalMetadata() throws IOException { - when(blobStoreRepository.getNamedXContentRegistry()).thenReturn(new NamedXContentRegistry( - List.of(new NamedXContentRegistry.Entry(Metadata.Custom.class, new ParseField(IndexGraveyard.TYPE), IndexGraveyard::fromXContent)))); + // when(blobStoreRepository.getNamedXContentRegistry()).thenReturn(new NamedXContentRegistry( + // List.of(new NamedXContentRegistry.Entry(Metadata.Custom.class, new ParseField(IndexGraveyard.TYPE), + // IndexGraveyard::fromXContent)))); final ClusterState clusterState = generateClusterStateWithGlobalMetadata().nodes(nodesWithLocalNodeClusterManager()).build(); remoteClusterStateService.start(); @@ -2405,7 +2509,10 @@ public void testReadGlobalMetadata() throws IOException { .coordinationMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(COORDINATION_METADATA, "mock-coordination-file")) .settingMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(SETTING_METADATA, "mock-setting-file")) .templatesMetadata(new ClusterMetadataManifest.UploadedMetadataAttribute(TEMPLATES_METADATA, "mock-templates-file")) - .put(IndexGraveyard.TYPE, new ClusterMetadataManifest.UploadedMetadataAttribute(IndexGraveyard.TYPE, "mock-custom-" +IndexGraveyard.TYPE+ "-file")) + .put( + IndexGraveyard.TYPE, + new ClusterMetadataManifest.UploadedMetadataAttribute(IndexGraveyard.TYPE, "mock-custom-" + IndexGraveyard.TYPE + "-file") + ) .nodeId("nodeA") .opensearchVersion(VersionUtils.randomOpenSearchVersion(random())) .previousClusterUUID("prev-cluster-uuid") @@ -2413,7 +2520,10 @@ public void testReadGlobalMetadata() throws IOException { .indicesRouting(List.of()) .build(); - Metadata expectedMetadata = Metadata.builder().clusterUUID("cluster-uuid").persistentSettings(Settings.builder().put("readonly", true).build()).build(); + Metadata expectedMetadata = Metadata.builder() + .clusterUUID("cluster-uuid") + .persistentSettings(Settings.builder().put("readonly", true).build()) + .build(); mockBlobContainerForGlobalMetadata(mockBlobStoreObjects(), expectedManifest, expectedMetadata); ClusterState newClusterState = remoteClusterStateService.getLatestClusterState( @@ -3939,6 +4049,55 @@ private void mockBlobContainer( }); } + private void mockBlobContainer( + BlobContainer blobContainer, + ClusterMetadataManifest clusterMetadataManifest, + Map indexMetadataMap, + int codecVersion, + long term, + long version + ) throws IOException { + String manifestFileName = codecVersion >= CODEC_V1 + ? "manifest__manifestFileName__abcd__abcd__abcd__" + codecVersion + : "manifestFileName"; + BlobMetadata blobMetadata = new PlainBlobMetadata(manifestFileName, 1); + + String manifestPrefix = String.join(DELIMITER, "manifest", RemoteStoreUtils.invertLong(term), RemoteStoreUtils.invertLong(version)) + + DELIMITER; + when(blobContainer.listBlobsByPrefixInSortedOrder(manifestPrefix, 1, BlobContainer.BlobNameSortOrder.LEXICOGRAPHIC)).thenReturn( + Arrays.asList(blobMetadata) + ); + + BytesReference bytes = RemoteClusterMetadataManifest.CLUSTER_METADATA_MANIFEST_FORMAT.serialize( + clusterMetadataManifest, + manifestFileName, + blobStoreRepository.getCompressor(), + FORMAT_PARAMS + ); + when(blobContainer.readBlob(manifestFileName)).thenReturn(new ByteArrayInputStream(bytes.streamInput().readAllBytes())); + + clusterMetadataManifest.getIndices().forEach(uploadedIndexMetadata -> { + try { + IndexMetadata indexMetadata = indexMetadataMap.get(uploadedIndexMetadata.getIndexUUID()); + if (indexMetadata == null) { + return; + } + String fileName = uploadedIndexMetadata.getUploadedFilename(); + when(blobContainer.readBlob(getFormattedIndexFileName(fileName))).thenAnswer((invocationOnMock) -> { + BytesReference bytesIndexMetadata = INDEX_METADATA_FORMAT.serialize( + indexMetadata, + fileName, + blobStoreRepository.getCompressor(), + FORMAT_PARAMS + ); + return new ByteArrayInputStream(bytesIndexMetadata.streamInput().readAllBytes()); + }); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + private void mockBlobContainerForGlobalMetadata( BlobContainer blobContainer, ClusterMetadataManifest clusterMetadataManifest, diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java index c439ef209e049..350c6f9ae8f6b 100644 --- a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java @@ -2413,7 +2413,8 @@ public void onFailure(final Exception e) { clusterService, threadPool, actionFilters, - indexNameExpressionResolver + indexNameExpressionResolver, + null ) ); actions.put( @@ -2455,7 +2456,8 @@ public void onFailure(final Exception e) { clusterService, threadPool, actionFilters, - indexNameExpressionResolver + indexNameExpressionResolver, + null ) );