Skip to content

Fix FileSettingsServiceIT race condition #128374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 26, 2025
Merged

Fix FileSettingsServiceIT race condition #128374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b1e48df
Fix FileSettingsServiceIT race condition
jfreden May 23, 2025
50a952e
Merge remote-tracking branch 'upstream/main' into fix_fileservice_rac…
jfreden May 23, 2025
0b70c21
Unmute test
jfreden May 23, 2025
86b3368
Merge remote-tracking branch 'upstream/main' into fix_fileservice_rac…
jfreden May 23, 2025
861ec5f
Merge remote-tracking branch 'upstream/main' into fix_fileservice_rac…
jfreden May 26, 2025
64bbda4
fixup! unmute test
jfreden May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions muted-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,6 @@ tests:
- class: org.elasticsearch.packaging.test.TemporaryDirectoryConfigTests
method: test21AcceptsCustomPathInDocker
issue: https://github.com/elastic/elasticsearch/issues/128114
- class: org.elasticsearch.reservedstate.service.FileSettingsServiceIT
method: testSymlinkUpdateTriggerReload
issue: https://github.com/elastic/elasticsearch/issues/128369
- class: org.elasticsearch.xpack.esql.qa.single_node.PushQueriesIT
method: testEqualityAndOther {semantic_text}
issue: https://github.com/elastic/elasticsearch/issues/128414
Expand Down
41 changes: 19 additions & 22 deletions ...ternalClusterTest/java/org/elasticsearch/reservedstate/service/FileSettingsServiceIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,18 @@ public void clusterChanged(ClusterChangedEvent event) {
return new Tuple<>(savedClusterState, metadataVersion);
}

private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node) {
private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node, long version) {
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 && handlerMetadata.keys().contains("indices.recovery.max_bytes_per_sec")) {
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
if (reservedState != null && reservedState.version() == version) {
clusterService.removeListener(this);
metadataVersion.set(event.state().metadata().version());
savedClusterState.countDown();
}
}
});
Expand Down Expand Up @@ -258,14 +255,14 @@ public void testSettingsApplied() throws Exception {
logger.info("--> start master node");
final String masterNode = internalCluster().startMasterOnlyNode();
assertMasterNode(internalCluster().nonMasterClient(), masterNode);
var savedClusterState = setupClusterStateListener(masterNode);
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());

FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);

assertBusy(() -> assertTrue(masterFileSettingsService.watching()));
assertFalse(dataFileSettingsService.watching());

writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet());
writeJSONFile(masterNode, testJSON, logger, versionCounter.get());
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");
}

Expand All @@ -276,11 +273,11 @@ public void testSettingsAppliedOnStart() throws Exception {
FileSettingsService dataFileSettingsService = internalCluster().getInstance(FileSettingsService.class, dataNode);

assertFalse(dataFileSettingsService.watching());
var savedClusterState = setupClusterStateListener(dataNode);
var savedClusterState = setupClusterStateListener(dataNode, versionCounter.incrementAndGet());

// In internal cluster tests, the nodes share the config directory, so when we write with the data node path
// the master will pick it up on start
writeJSONFile(dataNode, testJSON, logger, versionCounter.incrementAndGet());
writeJSONFile(dataNode, testJSON, logger, versionCounter.get());

logger.info("--> start master node");
final String masterNode = internalCluster().startMasterOnlyNode();
Expand All @@ -301,14 +298,14 @@ public void testReservedStatePersistsOnRestart() throws Exception {
Settings.builder().put(INITIAL_STATE_TIMEOUT_SETTING.getKey(), "0s").build()
);
assertMasterNode(internalCluster().masterClient(), masterNode);
var savedClusterState = setupClusterStateListener(masterNode);
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());

FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);

assertBusy(() -> assertTrue(masterFileSettingsService.watching()));

logger.info("--> write some settings");
writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet());
writeJSONFile(masterNode, testJSON, logger, versionCounter.get());
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");

logger.info("--> restart master");
Expand Down Expand Up @@ -476,12 +473,12 @@ public void testSettingsAppliedOnMasterReElection() throws Exception {
assertFalse(master1FS.watching());
assertFalse(master2FS.watching());

var savedClusterState = setupClusterStateListener(masterNode);
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);

assertBusy(() -> assertTrue(masterFileSettingsService.watching()));

writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet());
writeJSONFile(masterNode, testJSON, logger, versionCounter.get());
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");

internalCluster().stopCurrentMasterNode();
Expand All @@ -501,8 +498,8 @@ public void testSettingsAppliedOnMasterReElection() throws Exception {
boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS);
assertTrue(awaitSuccessful);

savedClusterState = setupClusterStateListener(internalCluster().getMasterName());
writeJSONFile(internalCluster().getMasterName(), testJSON43mb, logger, versionCounter.incrementAndGet());
savedClusterState = setupClusterStateListener(internalCluster().getMasterName(), versionCounter.incrementAndGet());
writeJSONFile(internalCluster().getMasterName(), testJSON43mb, logger, versionCounter.get());

assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "43mb");
}
Expand All @@ -515,21 +512,21 @@ public void testSymlinkUpdateTriggerReload() throws Exception {
assertBusy(() -> assertTrue(masterFileSettingsService.watching()));

{
var savedClusterState = setupClusterStateListener(masterNode);
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
// Create the settings.json as a symlink to simulate k8 setup
// settings.json -> ..data/settings.json
// ..data -> ..TIMESTAMP_TEMP_FOLDER_1
var fileDir = Files.createDirectories(baseDir.resolve("..TIMESTAMP_TEMP_FOLDER_1"));
writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet(), fileDir.resolve("settings.json"));
writeJSONFile(masterNode, testJSON, logger, versionCounter.get(), fileDir.resolve("settings.json"));
var dataDir = Files.createSymbolicLink(baseDir.resolve("..data"), fileDir.getFileName());
Files.createSymbolicLink(baseDir.resolve("settings.json"), dataDir.getFileName().resolve("settings.json"));
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");
}
{
var savedClusterState = setupClusterStateListener(masterNode);
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
// Update ..data symlink to ..data -> ..TIMESTAMP_TEMP_FOLDER_2 to simulate kubernetes secret update
var fileDir = Files.createDirectories(baseDir.resolve("..TIMESTAMP_TEMP_FOLDER_2"));
writeJSONFile(masterNode, testJSON43mb, logger, versionCounter.incrementAndGet(), fileDir.resolve("settings.json"));
writeJSONFile(masterNode, testJSON43mb, logger, versionCounter.get(), fileDir.resolve("settings.json"));
Files.deleteIfExists(baseDir.resolve("..data"));
Files.createSymbolicLink(baseDir.resolve("..data"), fileDir.getFileName());
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "43mb");
Expand Down