Skip to content

Commit

Permalink
upload committed voting config to remote in state commit phase
Browse files Browse the repository at this point in the history
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
  • Loading branch information
shiv0408 committed Aug 20, 2024
1 parent 5979358 commit af6ebf7
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.coordination.PersistedStateRegistry;
import org.opensearch.common.blobstore.BlobPath;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.gateway.GatewayMetaState;
import org.opensearch.gateway.remote.model.RemoteClusterMetadataManifest;
import org.opensearch.indices.recovery.RecoverySettings;
import org.opensearch.remotestore.RemoteStoreBaseIntegTestCase;
Expand Down Expand Up @@ -41,9 +43,6 @@
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 org.opensearch.cluster.coordination.PersistedStateRegistry;
import org.opensearch.gateway.GatewayMetaState;

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;
Expand Down
15 changes: 12 additions & 3 deletions server/src/main/java/org/opensearch/gateway/GatewayMetaState.java
Original file line number Diff line number Diff line change
Expand Up @@ -786,10 +786,14 @@ public void markLastAcceptedStateAsCommitted() {
assert lastAcceptedState != null : "Last accepted state is not present";
assert lastAcceptedManifest != null : "Last accepted manifest is not present";
ClusterState clusterState = lastAcceptedState;
Metadata.Builder metadataBuilder = commitVotingConfiguration(lastAcceptedState);
boolean shouldCommitVotingConfig = shouldCommitVotingConfig();

if (lastAcceptedState.metadata().clusterUUID().equals(Metadata.UNKNOWN_CLUSTER_UUID) == false
&& lastAcceptedState.metadata().clusterUUIDCommitted() == false) {
if (metadataBuilder == null) {
Metadata.Builder metadataBuilder;
if (shouldCommitVotingConfig) {
metadataBuilder = commitVotingConfiguration(lastAcceptedState);
} else {
metadataBuilder = Metadata.builder(lastAcceptedState.metadata());
}
metadataBuilder.clusterUUIDCommitted(true);
Expand All @@ -798,7 +802,8 @@ public void markLastAcceptedStateAsCommitted() {
if (clusterState.getNodes().isLocalNodeElectedClusterManager()) {
final RemoteClusterStateManifestInfo committedManifestDetails = remoteClusterStateService.markLastStateAsCommitted(
clusterState,
lastAcceptedManifest
lastAcceptedManifest,
shouldCommitVotingConfig
);
assert committedManifestDetails != null;
setLastAcceptedManifest(committedManifestDetails.getClusterMetadataManifest());
Expand All @@ -817,6 +822,10 @@ public void close() throws IOException {
remoteClusterStateService.close();
}

private boolean shouldCommitVotingConfig() {
return !lastAcceptedState.getLastAcceptedConfiguration().equals(lastAcceptedState.getLastCommittedConfiguration());
}

private void handleExceptionOnWrite(Exception e) {
throw ExceptionsHelper.convertToRuntime(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,18 +850,41 @@ public RemoteClusterStateCleanupManager getCleanupManager() {
}

@Nullable
public RemoteClusterStateManifestInfo markLastStateAsCommitted(ClusterState clusterState, ClusterMetadataManifest previousManifest)
throws IOException {
public RemoteClusterStateManifestInfo markLastStateAsCommitted(
ClusterState clusterState,
ClusterMetadataManifest previousManifest,
boolean commitVotingConfig
) throws IOException {
assert clusterState != null : "Last accepted cluster state is not set";
if (clusterState.nodes().isLocalNodeElectedClusterManager() == false) {
logger.error("Local node is not elected cluster manager. Exiting");
return null;
}
assert previousManifest != null : "Last cluster metadata manifest is not set";
UploadedMetadataAttribute uploadedCoordinationMetadata = previousManifest.getCoordinationMetadata();
if (commitVotingConfig) {
// update the coordination metadata if voting config is committed
uploadedCoordinationMetadata = writeMetadataInParallel(
clusterState,
emptyList(),
emptyMap(),
emptyMap(),
true,
false,
false,
false,
false,
false,
emptyMap(),
false,
emptyList(),
emptyMap()
).uploadedCoordinationMetadata;
}
UploadedMetadataResults uploadedMetadataResults = new UploadedMetadataResults(
previousManifest.getIndices(),
previousManifest.getCustomMetadataMap(),
previousManifest.getCoordinationMetadata(),
uploadedCoordinationMetadata,
previousManifest.getSettingsMetadata(),
previousManifest.getTemplatesMetadata(),
previousManifest.getTransientSettingsMetadata(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -977,16 +978,15 @@ public void testHandlePrePublishAndCommitWhenRemoteStateEnabled() throws IOExcep
Mockito.verify(remoteClusterStateService, Mockito.times(1)).writeFullMetadata(clusterState, previousClusterUUID);
assertThat(persistedStateRegistry.getPersistedState(PersistedStateType.REMOTE).getLastAcceptedState(), equalTo(clusterState));

when(remoteClusterStateService.markLastStateAsCommitted(any(), any())).thenReturn(
when(remoteClusterStateService.markLastStateAsCommitted(any(), any(), eq(false))).thenReturn(
new RemoteClusterStateManifestInfo(manifest, "path/to/manifest")
);
coordinationState.handlePreCommit();
ClusterState committedClusterState = ClusterState.builder(clusterState)
.metadata(Metadata.builder(clusterState.metadata()).clusterUUIDCommitted(true).build())
.build();
// Mockito.verify(remoteClusterStateService, Mockito.times(1)).markLastStateAsCommitted(committedClusterState, manifest);
ArgumentCaptor<ClusterState> clusterStateCaptor = ArgumentCaptor.forClass(ClusterState.class);
verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(clusterStateCaptor.capture(), any());
verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(clusterStateCaptor.capture(), any(), eq(false));
assertThat(clusterStateCaptor.getValue().metadata().indices(), equalTo(committedClusterState.metadata().indices()));
assertThat(clusterStateCaptor.getValue().metadata().clusterUUID(), equalTo(committedClusterState.metadata().clusterUUID()));
assertThat(clusterStateCaptor.getValue().stateUUID(), equalTo(committedClusterState.stateUUID()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -773,11 +774,11 @@ public void testRemotePersistedState() throws IOException {
assertThat(remotePersistedState.getLastAcceptedState(), equalTo(secondClusterState));
assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm));

when(remoteClusterStateService.markLastStateAsCommitted(Mockito.any(), Mockito.any())).thenReturn(
when(remoteClusterStateService.markLastStateAsCommitted(Mockito.any(), Mockito.any(), eq(false))).thenReturn(
new RemoteClusterStateManifestInfo(manifest, "path/to/manifest")
);
remotePersistedState.markLastAcceptedStateAsCommitted();
Mockito.verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(Mockito.any(), Mockito.any());
Mockito.verify(remoteClusterStateService, times(1)).markLastStateAsCommitted(Mockito.any(), Mockito.any(), eq(false));

assertThat(remotePersistedState.getLastAcceptedState(), equalTo(secondClusterState));
assertThat(remotePersistedState.getCurrentTerm(), equalTo(clusterTerm));
Expand Down Expand Up @@ -840,7 +841,7 @@ public void testRemotePersistentState_FollowerNode() throws IOException {
assertFalse(remotePersistedState.getLastAcceptedManifest().isCommitted());

remotePersistedState.markLastAcceptedStateAsCommitted();
Mockito.verify(remoteClusterStateService, never()).markLastStateAsCommitted(Mockito.any(), Mockito.any());
Mockito.verify(remoteClusterStateService, never()).markLastStateAsCommitted(Mockito.any(), Mockito.any(), eq(false));

assertEquals(secondClusterState, remotePersistedState.getLastAcceptedState());
assertEquals(clusterTerm, remotePersistedState.getCurrentTerm());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2466,7 +2466,7 @@ public void testMarkLastStateAsCommittedSuccess() throws IOException {
List<UploadedIndexMetadata> indices = List.of(uploadedIndexMetadata);
final ClusterMetadataManifest previousManifest = ClusterMetadataManifest.builder().indices(indices).build();

final ClusterMetadataManifest manifest = remoteClusterStateService.markLastStateAsCommitted(clusterState, previousManifest)
final ClusterMetadataManifest manifest = remoteClusterStateService.markLastStateAsCommitted(clusterState, previousManifest, false)
.getClusterMetadataManifest();

final ClusterMetadataManifest expectedManifest = ClusterMetadataManifest.builder()
Expand Down

0 comments on commit af6ebf7

Please sign in to comment.