Skip to content

Commit

Permalink
[TEST] Make FileSettingsService test more reliable (#89468) (#89521)
Browse files Browse the repository at this point in the history
  • Loading branch information
@grcevski
grcevski committed Aug 22, 2022
1 parent b9bf023 commit ca0f449
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 32 deletions.
44 changes: 26 additions & 18 deletions ...ternalClusterTest/java/org/elasticsearch/reservedstate/service/FileSettingsServiceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Strings;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.reservedstate.action.ReservedClusterSettingsAction;
import org.elasticsearch.test.ESIntegTestCase;

Expand All @@ -35,7 +36,6 @@
import static org.elasticsearch.indices.recovery.RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING;
import static org.elasticsearch.test.NodeRoles.dataOnlyNode;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
Expand Down Expand Up @@ -92,33 +92,36 @@ private void writeJSONFile(String node, String json) throws Exception {
Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE);
}

private CountDownLatch setupClusterStateListener(String node) {
private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node) {
ClusterService clusterService = internalCluster().clusterService(node);
CountDownLatch savedClusterState = new CountDownLatch(1);
AtomicLong metadataVersion = new AtomicLong(-1);
clusterService.addListener(new ClusterStateListener() {
@Override
public void clusterChanged(ClusterChangedEvent event) {
ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
if (reservedState != null) {
ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedClusterSettingsAction.NAME);
if (handlerMetadata == null) {
fail("Should've found cluster settings in this metadata");
if (handlerMetadata != null && handlerMetadata.keys().contains("indices.recovery.max_bytes_per_sec")) {
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
assertThat(handlerMetadata.keys(), contains("indices.recovery.max_bytes_per_sec"));
clusterService.removeListener(this);
savedClusterState.countDown();
}
}
});

return savedClusterState;
return new Tuple<>(savedClusterState, metadataVersion);
}

private void assertClusterStateSaveOK(CountDownLatch savedClusterState) throws Exception {
private void assertClusterStateSaveOK(CountDownLatch savedClusterState, AtomicLong metadataVersion) throws Exception {
boolean awaitSuccessful = savedClusterState.await(20, TimeUnit.SECONDS);
assertTrue(awaitSuccessful);

final ClusterStateResponse clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet();
final ClusterStateResponse clusterStateResponse = client().admin()
.cluster()
.state(new ClusterStateRequest().waitForMetadataVersion(metadataVersion.get()))
.actionGet();

assertThat(
clusterStateResponse.getState().metadata().persistentSettings().get(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey()),
Expand Down Expand Up @@ -155,7 +158,7 @@ public void testSettingsApplied() throws Exception {
assertFalse(dataFileSettingsService.watching());

writeJSONFile(masterNode, testJSON);
assertClusterStateSaveOK(savedClusterState);
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2());
}

public void testSettingsAppliedOnStart() throws Exception {
Expand All @@ -180,37 +183,42 @@ public void testSettingsAppliedOnStart() throws Exception {
assertTrue(masterFileSettingsService.watching());
assertFalse(dataFileSettingsService.watching());

assertClusterStateSaveOK(savedClusterState);
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2());
}

private CountDownLatch setupClusterStateListenerForError(String node) {
private Tuple<CountDownLatch, AtomicLong> setupClusterStateListenerForError(String node) {
ClusterService clusterService = internalCluster().clusterService(node);
CountDownLatch savedClusterState = new CountDownLatch(1);
AtomicLong metadataVersion = new AtomicLong(-1);
clusterService.addListener(new ClusterStateListener() {
@Override
public void clusterChanged(ClusterChangedEvent event) {
ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
if (reservedState != null) {
if (reservedState != null && reservedState.errorMetadata() != null) {
assertEquals(ReservedStateErrorMetadata.ErrorKind.PARSING, reservedState.errorMetadata().errorKind());
assertThat(reservedState.errorMetadata().errors(), allOf(notNullValue(), hasSize(1)));
assertThat(
reservedState.errorMetadata().errors().get(0),
containsString("Missing handler definition for content key [not_cluster_settings]")
);
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
}
});

return savedClusterState;
return new Tuple<>(savedClusterState, metadataVersion);
}

private void assertClusterStateNotSaved(CountDownLatch savedClusterState) throws Exception {
private void assertClusterStateNotSaved(CountDownLatch savedClusterState, AtomicLong metadataVersion) throws Exception {
boolean awaitSuccessful = savedClusterState.await(20, TimeUnit.SECONDS);
assertTrue(awaitSuccessful);

final ClusterStateResponse clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet();
final ClusterStateResponse clusterStateResponse = client().admin()
.cluster()
.state(new ClusterStateRequest().waitForMetadataVersion(metadataVersion.get()))
.actionGet();

assertThat(clusterStateResponse.getState().metadata().persistentSettings().get("search.allow_expensive_queries"), nullValue());

Expand Down Expand Up @@ -240,6 +248,6 @@ public void testErrorSaved() throws Exception {
assertFalse(dataFileSettingsService.watching());

writeJSONFile(masterNode, testErrorJSON);
assertClusterStateNotSaved(savedClusterState);
assertClusterStateNotSaved(savedClusterState.v1(), savedClusterState.v2());
}
}
37 changes: 23 additions & 14 deletions ...alClusterTest/java/org/elasticsearch/reservedstate/service/SnaphotsAndFileSettingsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Strings;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.reservedstate.action.ReservedClusterSettingsAction;
import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase;
import org.elasticsearch.snapshots.SnapshotState;
Expand Down Expand Up @@ -85,9 +86,10 @@ private void writeJSONFile(String node, String json) throws Exception {
Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE);
}

private CountDownLatch setupClusterStateListener(String node) {
private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node) {
ClusterService clusterService = internalCluster().clusterService(node);
CountDownLatch savedClusterState = new CountDownLatch(1);
AtomicLong metadataVersion = new AtomicLong(-1);
clusterService.addListener(new ClusterStateListener() {
@Override
public void clusterChanged(ClusterChangedEvent event) {
Expand All @@ -99,20 +101,21 @@ public void clusterChanged(ClusterChangedEvent event) {
}
if (handlerMetadata.keys().contains("indices.recovery.max_bytes_per_sec")) {
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
}
}
});

return savedClusterState;
return new Tuple<>(savedClusterState, metadataVersion);
}

private ClusterStateResponse assertClusterStateSaveOK(CountDownLatch savedClusterState) throws Exception {
private ClusterStateResponse assertClusterStateSaveOK(CountDownLatch savedClusterState, AtomicLong metadataVersion) throws Exception {
boolean awaitSuccessful = savedClusterState.await(20, TimeUnit.SECONDS);
assertTrue(awaitSuccessful);

return clusterAdmin().state(new ClusterStateRequest()).actionGet();
return clusterAdmin().state(new ClusterStateRequest().waitForMetadataVersion(metadataVersion.get())).actionGet();
}

public void testRestoreWithRemovedFileSettings() throws Exception {
Expand All @@ -138,7 +141,7 @@ public void testRestoreWithRemovedFileSettings() throws Exception {

logger.info("--> write some file based settings, putting some reserved state");
writeJSONFile(masterNode, testFileSettingsJSON);
final ClusterStateResponse savedStateResponse = assertClusterStateSaveOK(savedClusterState);
final ClusterStateResponse savedStateResponse = assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2());
assertThat(
savedStateResponse.getState().metadata().persistentSettings().get(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey()),
equalTo("50mb")
Expand Down Expand Up @@ -195,26 +198,29 @@ public void testRestoreWithRemovedFileSettings() throws Exception {
}
}

private CountDownLatch removedReservedClusterStateListener(String node) {
private Tuple<CountDownLatch, AtomicLong> removedReservedClusterStateListener(String node) {
ClusterService clusterService = internalCluster().clusterService(node);
CountDownLatch savedClusterState = new CountDownLatch(1);
AtomicLong metadataVersion = new AtomicLong(-1);
clusterService.addListener(new ClusterStateListener() {
@Override
public void clusterChanged(ClusterChangedEvent event) {
ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
if (reservedState != null && reservedState.version() == 0L) {
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
}
});

return savedClusterState;
return new Tuple<>(savedClusterState, metadataVersion);
}

private CountDownLatch cleanedClusterStateListener(String node) {
private Tuple<CountDownLatch, AtomicLong> cleanedClusterStateListener(String node) {
ClusterService clusterService = internalCluster().clusterService(node);
CountDownLatch savedClusterState = new CountDownLatch(1);
AtomicLong metadataVersion = new AtomicLong(-1);
clusterService.addListener(new ClusterStateListener() {
@Override
public void clusterChanged(ClusterChangedEvent event) {
Expand All @@ -226,13 +232,14 @@ public void clusterChanged(ClusterChangedEvent event) {
}
if (handlerMetadata.keys().isEmpty()) {
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
}
}
});

return savedClusterState;
return new Tuple<>(savedClusterState, metadataVersion);
}

public void testRestoreWithPersistedFileSettings() throws Exception {
Expand All @@ -258,7 +265,7 @@ public void testRestoreWithPersistedFileSettings() throws Exception {

logger.info("--> write some file based settings, putting some reserved state");
writeJSONFile(masterNode, testFileSettingsJSON);
final ClusterStateResponse savedStateResponse = assertClusterStateSaveOK(savedClusterState);
final ClusterStateResponse savedStateResponse = assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2());
assertThat(
savedStateResponse.getState().metadata().persistentSettings().get(INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey()),
equalTo("50mb")
Expand Down Expand Up @@ -289,12 +296,14 @@ public void testRestoreWithPersistedFileSettings() throws Exception {
// cluster state for file based settings, but instead we reset the version to 0 and 'touch' the operator file
// so that it gets re-processed.
logger.info("--> reserved state version will be reset to 0, because of snapshot restore");
assertTrue(removedReservedState.await(20, TimeUnit.SECONDS));
assertTrue(removedReservedState.v1().await(20, TimeUnit.SECONDS));

logger.info("--> reserved state would be restored");
assertTrue(restoredReservedState.await(20, TimeUnit.SECONDS));
assertTrue(restoredReservedState.v1().await(20, TimeUnit.SECONDS));

final ClusterStateResponse clusterStateResponse = clusterAdmin().state(new ClusterStateRequest().metadata(true)).actionGet();
final ClusterStateResponse clusterStateResponse = clusterAdmin().state(
new ClusterStateRequest().metadata(true).waitForMetadataVersion(restoredReservedState.v2().get())
).actionGet();

assertNotNull(clusterStateResponse.getState().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE));

Expand All @@ -313,7 +322,7 @@ public void testRestoreWithPersistedFileSettings() throws Exception {

logger.info("--> clear the file based settings");
writeJSONFile(masterNode, emptyFileSettingsJSON);
assertClusterStateSaveOK(cleanupReservedState);
assertClusterStateSaveOK(cleanupReservedState.v1(), cleanupReservedState.v2());
} finally {
// cleanup
assertAcked(
Expand Down

0 comments on commit ca0f449

Please sign in to comment.