Skip to content

Commit 74d025e

Browse files
jfredenjfreden
authored
Fix FileSettingsServiceIT race condition (#128374)
Same issue as elastic/elasticsearch-serverless#3900 Instead of using handler keys as condition that reserved state has been updated, use the version. Resolves: #128369
1 parent 8554e19 commit 74d025e

File tree

2 files changed

+19
-25
lines changed

2 files changed

+19
-25
lines changed

‎muted-tests.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,9 +474,6 @@ tests:
474474
- class: org.elasticsearch.packaging.test.TemporaryDirectoryConfigTests
475475
method: test21AcceptsCustomPathInDocker
476476
issue: https://github.com/elastic/elasticsearch/issues/128114
477-
- class: org.elasticsearch.reservedstate.service.FileSettingsServiceIT
478-
method: testSymlinkUpdateTriggerReload
479-
issue: https://github.com/elastic/elasticsearch/issues/128369
480477
- class: org.elasticsearch.xpack.esql.qa.single_node.PushQueriesIT
481478
method: testEqualityAndOther {semantic_text}
482479
issue: https://github.com/elastic/elasticsearch/issues/128414

‎server/src/internalClusterTest/java/org/elasticsearch/reservedstate/service/FileSettingsServiceIT.java

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -196,21 +196,18 @@ public void clusterChanged(ClusterChangedEvent event) {
196196
return new Tuple<>(savedClusterState, metadataVersion);
197197
}
198198

199-
private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node) {
199+
private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node, long version) {
200200
ClusterService clusterService = internalCluster().clusterService(node);
201201
CountDownLatch savedClusterState = new CountDownLatch(1);
202202
AtomicLong metadataVersion = new AtomicLong(-1);
203203
clusterService.addListener(new ClusterStateListener() {
204204
@Override
205205
public void clusterChanged(ClusterChangedEvent event) {
206206
ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
207-
if (reservedState != null) {
208-
ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedClusterSettingsAction.NAME);
209-
if (handlerMetadata != null && handlerMetadata.keys().contains("indices.recovery.max_bytes_per_sec")) {
210-
clusterService.removeListener(this);
211-
metadataVersion.set(event.state().metadata().version());
212-
savedClusterState.countDown();
213-
}
207+
if (reservedState != null && reservedState.version() == version) {
208+
clusterService.removeListener(this);
209+
metadataVersion.set(event.state().metadata().version());
210+
savedClusterState.countDown();
214211
}
215212
}
216213
});
@@ -258,14 +255,14 @@ public void testSettingsApplied() throws Exception {
258255
logger.info("--> start master node");
259256
final String masterNode = internalCluster().startMasterOnlyNode();
260257
assertMasterNode(internalCluster().nonMasterClient(), masterNode);
261-
var savedClusterState = setupClusterStateListener(masterNode);
258+
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
262259

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

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

268-
writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet());
265+
writeJSONFile(masterNode, testJSON, logger, versionCounter.get());
269266
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");
270267
}
271268

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

278275
assertFalse(dataFileSettingsService.watching());
279-
var savedClusterState = setupClusterStateListener(dataNode);
276+
var savedClusterState = setupClusterStateListener(dataNode, versionCounter.incrementAndGet());
280277

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

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

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

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

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

314311
logger.info("--> restart master");
@@ -476,12 +473,12 @@ public void testSettingsAppliedOnMasterReElection() throws Exception {
476473
assertFalse(master1FS.watching());
477474
assertFalse(master2FS.watching());
478475

479-
var savedClusterState = setupClusterStateListener(masterNode);
476+
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
480477
FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);
481478

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

484-
writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet());
481+
writeJSONFile(masterNode, testJSON, logger, versionCounter.get());
485482
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");
486483

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

504-
savedClusterState = setupClusterStateListener(internalCluster().getMasterName());
505-
writeJSONFile(internalCluster().getMasterName(), testJSON43mb, logger, versionCounter.incrementAndGet());
501+
savedClusterState = setupClusterStateListener(internalCluster().getMasterName(), versionCounter.incrementAndGet());
502+
writeJSONFile(internalCluster().getMasterName(), testJSON43mb, logger, versionCounter.get());
506503

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

517514
{
518-
var savedClusterState = setupClusterStateListener(masterNode);
515+
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
519516
// Create the settings.json as a symlink to simulate k8 setup
520517
// settings.json -> ..data/settings.json
521518
// ..data -> ..TIMESTAMP_TEMP_FOLDER_1
522519
var fileDir = Files.createDirectories(baseDir.resolve("..TIMESTAMP_TEMP_FOLDER_1"));
523-
writeJSONFile(masterNode, testJSON, logger, versionCounter.incrementAndGet(), fileDir.resolve("settings.json"));
520+
writeJSONFile(masterNode, testJSON, logger, versionCounter.get(), fileDir.resolve("settings.json"));
524521
var dataDir = Files.createSymbolicLink(baseDir.resolve("..data"), fileDir.getFileName());
525522
Files.createSymbolicLink(baseDir.resolve("settings.json"), dataDir.getFileName().resolve("settings.json"));
526523
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "50mb");
527524
}
528525
{
529-
var savedClusterState = setupClusterStateListener(masterNode);
526+
var savedClusterState = setupClusterStateListener(masterNode, versionCounter.incrementAndGet());
530527
// Update ..data symlink to ..data -> ..TIMESTAMP_TEMP_FOLDER_2 to simulate kubernetes secret update
531528
var fileDir = Files.createDirectories(baseDir.resolve("..TIMESTAMP_TEMP_FOLDER_2"));
532-
writeJSONFile(masterNode, testJSON43mb, logger, versionCounter.incrementAndGet(), fileDir.resolve("settings.json"));
529+
writeJSONFile(masterNode, testJSON43mb, logger, versionCounter.get(), fileDir.resolve("settings.json"));
533530
Files.deleteIfExists(baseDir.resolve("..data"));
534531
Files.createSymbolicLink(baseDir.resolve("..data"), fileDir.getFileName());
535532
assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2(), "43mb");

0 commit comments

Comments
 (0)