Skip to content

Commit f49db2d

Browse files
authored
Use latest setting value when initializing setting watch (#134091) (#134315)
When new cluster settings are applied, consumers watchings are notified. The consumer should see the latest value at the time of initializing a watch. However, only the node settings were ever used in finding the value to initialize the consumer. This commit fixes the consumer to use the value from latest applied if it exists. closes #133701
1 parent 9e757d5 commit f49db2d

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

docs/changelog/134091.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 134091
2+
summary: Use latest setting value when initializing setting watch
3+
area: Infra/Settings
4+
type: bug
5+
issues:
6+
- 133701

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,11 @@ public synchronized <T> void initializeAndWatch(Setting<T> setting, Consumer<T>
433433
assert setting.getProperties().contains(Setting.Property.Dynamic)
434434
|| setting.getProperties().contains(Setting.Property.OperatorDynamic) : "Can only watch dynamic settings";
435435
assert setting.getProperties().contains(Setting.Property.NodeScope) : "Can only watch node settings";
436-
consumer.accept(setting.get(settings));
436+
437+
// this mimics the combined settings of last applied and node settings, without building a new settings object
438+
Settings settingsWithValue = setting.exists(lastSettingsApplied) ? lastSettingsApplied : settings;
439+
440+
consumer.accept(setting.get(settingsWithValue));
437441
addSettingsUpdateConsumer(setting, consumer);
438442
}
439443

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.action.admin.cluster.settings;
11+
12+
import org.elasticsearch.common.settings.ClusterSettings;
13+
import org.elasticsearch.common.settings.Setting;
14+
import org.elasticsearch.common.settings.Settings;
15+
import org.elasticsearch.test.ESTestCase;
16+
17+
import java.util.Set;
18+
19+
import static org.hamcrest.Matchers.equalTo;
20+
21+
public class ClusterSettingsTests extends ESTestCase {
22+
23+
public void testWatchAfterApply() {
24+
Setting<String> clusterSetting = Setting.simpleString("cluster.setting", Setting.Property.NodeScope, Setting.Property.Dynamic);
25+
Settings nodeSettings = Settings.builder().put("cluster.setting", "initial_value").build();
26+
27+
ClusterSettings clusterSettings = new ClusterSettings(nodeSettings, Set.of(clusterSetting));
28+
Settings newSettings = Settings.builder().put("cluster.setting", "updated_value").build();
29+
clusterSettings.applySettings(newSettings);
30+
31+
// the value should be current when initializing the consumer
32+
clusterSettings.initializeAndWatch(clusterSetting, value -> { assertThat(value, equalTo("updated_value")); });
33+
}
34+
}

0 commit comments

Comments
 (0)