Skip to content

Commit 7b8d036

Browse files
s1monws1monw
authored
Replace group map settings with affix setting (#26819)
We use group settings historically instead of using a prefix setting which is more restrictive and type safe. The majority of the usecases needs to access a key, value map based on the _leave node_ of the setting ie. the setting `index.tag.*` might be used to tag an index with `index.tag.test=42` and `index.tag.staging=12` which then would be turned into a `{"test": 42, "staging": 12}` map. The group settings would always use `Settings#getAsMap` which is loosing type information and uses internal representation of the settings. Using prefix settings allows now to access such a method type-safe and natively.
1 parent 3cb99ae commit 7b8d036

File tree

20 files changed

+237
-79
lines changed

20 files changed

+237
-79
lines changed

‎core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,18 @@ static Setting<Integer> buildNumberOfShardsSetting() {
240240
public static final String INDEX_ROUTING_REQUIRE_GROUP_PREFIX = "index.routing.allocation.require";
241241
public static final String INDEX_ROUTING_INCLUDE_GROUP_PREFIX = "index.routing.allocation.include";
242242
public static final String INDEX_ROUTING_EXCLUDE_GROUP_PREFIX = "index.routing.allocation.exclude";
243-
public static final Setting<Settings> INDEX_ROUTING_REQUIRE_GROUP_SETTING =
244-
Setting.groupSetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.IndexScope);
245-
public static final Setting<Settings> INDEX_ROUTING_INCLUDE_GROUP_SETTING =
246-
Setting.groupSetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.IndexScope);
247-
public static final Setting<Settings> INDEX_ROUTING_EXCLUDE_GROUP_SETTING =
248-
Setting.groupSetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.IndexScope);
249-
public static final Setting<Settings> INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING =
250-
Setting.groupSetting("index.routing.allocation.initial_recovery."); // this is only setable internally not a registered setting!!
243+
public static final Setting.AffixSetting<String> INDEX_ROUTING_REQUIRE_GROUP_SETTING =
244+
Setting.prefixKeySetting(INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".", (key) ->
245+
Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
246+
public static final Setting.AffixSetting<String> INDEX_ROUTING_INCLUDE_GROUP_SETTING =
247+
Setting.prefixKeySetting(INDEX_ROUTING_INCLUDE_GROUP_PREFIX + ".", (key) ->
248+
Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
249+
public static final Setting.AffixSetting<String> INDEX_ROUTING_EXCLUDE_GROUP_SETTING =
250+
Setting.prefixKeySetting(INDEX_ROUTING_EXCLUDE_GROUP_PREFIX + ".", (key) ->
251+
Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.IndexScope));
252+
public static final Setting.AffixSetting<String> INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING =
253+
Setting.prefixKeySetting("index.routing.allocation.initial_recovery.", key -> Setting.simpleString(key));
254+
// this is only setable internally not a registered setting!!
251255

252256
/**
253257
* The number of active shard copies to check for before proceeding with a write operation.
@@ -1012,28 +1016,28 @@ public IndexMetaData build() {
10121016
filledInSyncAllocationIds.put(i, Collections.emptySet());
10131017
}
10141018
}
1015-
final Map<String, String> requireMap = INDEX_ROUTING_REQUIRE_GROUP_SETTING.get(settings).getAsMap();
1019+
final Map<String, String> requireMap = INDEX_ROUTING_REQUIRE_GROUP_SETTING.getAsMap(settings);
10161020
final DiscoveryNodeFilters requireFilters;
10171021
if (requireMap.isEmpty()) {
10181022
requireFilters = null;
10191023
} else {
10201024
requireFilters = DiscoveryNodeFilters.buildFromKeyValue(AND, requireMap);
10211025
}
1022-
Map<String, String> includeMap = INDEX_ROUTING_INCLUDE_GROUP_SETTING.get(settings).getAsMap();
1026+
Map<String, String> includeMap = INDEX_ROUTING_INCLUDE_GROUP_SETTING.getAsMap(settings);
10231027
final DiscoveryNodeFilters includeFilters;
10241028
if (includeMap.isEmpty()) {
10251029
includeFilters = null;
10261030
} else {
10271031
includeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, includeMap);
10281032
}
1029-
Map<String, String> excludeMap = INDEX_ROUTING_EXCLUDE_GROUP_SETTING.get(settings).getAsMap();
1033+
Map<String, String> excludeMap = INDEX_ROUTING_EXCLUDE_GROUP_SETTING.getAsMap(settings);
10301034
final DiscoveryNodeFilters excludeFilters;
10311035
if (excludeMap.isEmpty()) {
10321036
excludeFilters = null;
10331037
} else {
10341038
excludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, excludeMap);
10351039
}
1036-
Map<String, String> initialRecoveryMap = INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING.get(settings).getAsMap();
1040+
Map<String, String> initialRecoveryMap = INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING.getAsMap(settings);
10371041
final DiscoveryNodeFilters initialRecoveryFilters;
10381042
if (initialRecoveryMap.isEmpty()) {
10391043
initialRecoveryFilters = null;

‎core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -161,21 +161,20 @@ public void updateSettings(final UpdateSettingsClusterStateUpdateRequest request
161161
final Settings normalizedSettings = Settings.builder().put(request.settings()).normalizePrefix(IndexMetaData.INDEX_SETTING_PREFIX).build();
162162
Settings.Builder settingsForClosedIndices = Settings.builder();
163163
Settings.Builder settingsForOpenIndices = Settings.builder();
164-
Settings.Builder skipppedSettings = Settings.builder();
164+
final Set<String> skippedSettings = new HashSet<>();
165165

166166
indexScopedSettings.validate(normalizedSettings);
167167
// never allow to change the number of shards
168-
for (Map.Entry<String, String> entry : normalizedSettings.getAsMap().entrySet()) {
169-
Setting setting = indexScopedSettings.get(entry.getKey());
168+
for (String key : normalizedSettings.getKeys()) {
169+
Setting setting = indexScopedSettings.get(key);
170170
assert setting != null; // we already validated the normalized settings
171-
settingsForClosedIndices.put(entry.getKey(), entry.getValue());
171+
settingsForClosedIndices.copy(key, normalizedSettings);
172172
if (setting.isDynamic()) {
173-
settingsForOpenIndices.put(entry.getKey(), entry.getValue());
173+
settingsForOpenIndices.copy(key, normalizedSettings);
174174
} else {
175-
skipppedSettings.put(entry.getKey(), entry.getValue());
175+
skippedSettings.add(key);
176176
}
177177
}
178-
final Settings skippedSettigns = skipppedSettings.build();
179178
final Settings closedSettings = settingsForClosedIndices.build();
180179
final Settings openSettings = settingsForOpenIndices.build();
181180
final boolean preserveExisting = request.isPreserveExisting();
@@ -210,11 +209,9 @@ public ClusterState execute(ClusterState currentState) {
210209
}
211210
}
212211

213-
if (!skippedSettigns.isEmpty() && !openIndices.isEmpty()) {
212+
if (!skippedSettings.isEmpty() && !openIndices.isEmpty()) {
214213
throw new IllegalArgumentException(String.format(Locale.ROOT,
215-
"Can't update non dynamic settings [%s] for open indices %s",
216-
skippedSettigns.getAsMap().keySet(),
217-
openIndices
214+
"Can't update non dynamic settings [%s] for open indices %s", skippedSettings, openIndices
218215
));
219216
}
220217

‎core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,8 @@ public DiscoveryNode(String nodeName, String nodeId, String ephemeralId, String
189189

190190
/** Creates a DiscoveryNode representing the local node. */
191191
public static DiscoveryNode createLocal(Settings settings, TransportAddress publishAddress, String nodeId) {
192-
Map<String, String> attributes = new HashMap<>(Node.NODE_ATTRIBUTES.get(settings).getAsMap());
192+
Map<String, String> attributes = Node.NODE_ATTRIBUTES.getAsMap(settings);
193193
Set<Role> roles = getRolesFromSettings(settings);
194-
195194
return new DiscoveryNode(Node.NODE_NAME_SETTING.get(settings), nodeId, publishAddress, attributes, roles, Version.CURRENT);
196195
}
197196

‎core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeFilters.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
import java.util.HashMap;
3131
import java.util.Map;
32+
import java.util.function.BiConsumer;
3233
import java.util.function.Consumer;
3334

3435
public class DiscoveryNodeFilters {
@@ -43,15 +44,10 @@ public enum OpType {
4344
* "_ip", "_host_ip", and "_publish_ip" and ensuring each of their comma separated values
4445
* that has no wildcards is a valid IP address.
4546
*/
46-
public static final Consumer<Settings> IP_VALIDATOR = (settings) -> {
47-
Map<String, String> settingsMap = settings.getAsMap();
48-
for (Map.Entry<String, String> entry : settingsMap.entrySet()) {
49-
String propertyKey = entry.getKey();
50-
if (entry.getValue() == null) {
51-
continue; // this setting gets reset
52-
}
53-
if ("_ip".equals(propertyKey) || "_host_ip".equals(propertyKey) || "_publish_ip".equals(propertyKey)) {
54-
for (String value : Strings.tokenizeToStringArray(entry.getValue(), ",")) {
47+
public static final BiConsumer<String, String> IP_VALIDATOR = (propertyKey, rawValue) -> {
48+
if (rawValue != null) {
49+
if (propertyKey.endsWith("._ip") || propertyKey.endsWith("._host_ip") || propertyKey.endsWith("_publish_ip")) {
50+
for (String value : Strings.tokenizeToStringArray(rawValue, ",")) {
5551
if (Regex.isSimpleMatchPattern(value) == false && InetAddresses.isInetAddress(value) == false) {
5652
throw new IllegalArgumentException("invalid IP address [" + value + "] for [" + propertyKey + "]");
5753
}

‎core/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.elasticsearch.common.settings.Settings;
3232

3333
import java.util.EnumSet;
34+
import java.util.Map;
3435

3536
import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.IP_VALIDATOR;
3637
import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.AND;
@@ -70,12 +71,15 @@ public class FilterAllocationDecider extends AllocationDecider {
7071
private static final String CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX = "cluster.routing.allocation.require";
7172
private static final String CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX = "cluster.routing.allocation.include";
7273
private static final String CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX = "cluster.routing.allocation.exclude";
73-
public static final Setting<Settings> CLUSTER_ROUTING_REQUIRE_GROUP_SETTING =
74-
Setting.groupSetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.NodeScope);
75-
public static final Setting<Settings> CLUSTER_ROUTING_INCLUDE_GROUP_SETTING =
76-
Setting.groupSetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.NodeScope);
77-
public static final Setting<Settings> CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING =
78-
Setting.groupSetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", IP_VALIDATOR, Property.Dynamic, Property.NodeScope);
74+
public static final Setting.AffixSetting<String> CLUSTER_ROUTING_REQUIRE_GROUP_SETTING =
75+
Setting.prefixKeySetting(CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX + ".", (key) ->
76+
Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
77+
public static final Setting.AffixSetting<String> CLUSTER_ROUTING_INCLUDE_GROUP_SETTING =
78+
Setting.prefixKeySetting(CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX + ".", (key) ->
79+
Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
80+
public static final Setting.AffixSetting<String>CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING =
81+
Setting.prefixKeySetting(CLUSTER_ROUTING_EXCLUDE_GROUP_PREFIX + ".", (key) ->
82+
Setting.simpleString(key, (value, map) -> IP_VALIDATOR.accept(key, value), Property.Dynamic, Property.NodeScope));
7983

8084
/**
8185
* The set of {@link RecoverySource.Type} values for which the
@@ -94,12 +98,12 @@ public class FilterAllocationDecider extends AllocationDecider {
9498

9599
public FilterAllocationDecider(Settings settings, ClusterSettings clusterSettings) {
96100
super(settings);
97-
setClusterRequireFilters(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING.get(settings));
98-
setClusterExcludeFilters(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.get(settings));
99-
setClusterIncludeFilters(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.get(settings));
100-
clusterSettings.addSettingsUpdateConsumer(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING, this::setClusterRequireFilters);
101-
clusterSettings.addSettingsUpdateConsumer(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING, this::setClusterExcludeFilters);
102-
clusterSettings.addSettingsUpdateConsumer(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING, this::setClusterIncludeFilters);
101+
setClusterRequireFilters(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING.getAsMap(settings));
102+
setClusterExcludeFilters(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING.getAsMap(settings));
103+
setClusterIncludeFilters(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING.getAsMap(settings));
104+
clusterSettings.addAffixMapUpdateConsumer(CLUSTER_ROUTING_REQUIRE_GROUP_SETTING, this::setClusterRequireFilters, (a,b)-> {}, true);
105+
clusterSettings.addAffixMapUpdateConsumer(CLUSTER_ROUTING_EXCLUDE_GROUP_SETTING, this::setClusterExcludeFilters, (a,b)-> {}, true);
106+
clusterSettings.addAffixMapUpdateConsumer(CLUSTER_ROUTING_INCLUDE_GROUP_SETTING, this::setClusterIncludeFilters, (a,b)-> {}, true);
103107
}
104108

105109
@Override
@@ -196,13 +200,13 @@ private Decision shouldClusterFilter(RoutingNode node, RoutingAllocation allocat
196200
return null;
197201
}
198202

199-
private void setClusterRequireFilters(Settings settings) {
200-
clusterRequireFilters = DiscoveryNodeFilters.buildFromKeyValue(AND, settings.getAsMap());
203+
private void setClusterRequireFilters(Map<String, String> filters) {
204+
clusterRequireFilters = DiscoveryNodeFilters.buildFromKeyValue(AND, filters);
201205
}
202-
private void setClusterIncludeFilters(Settings settings) {
203-
clusterIncludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, settings.getAsMap());
206+
private void setClusterIncludeFilters(Map<String, String> filters) {
207+
clusterIncludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, filters);
204208
}
205-
private void setClusterExcludeFilters(Settings settings) {
206-
clusterExcludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, settings.getAsMap());
209+
private void setClusterExcludeFilters(Map<String, String> filters) {
210+
clusterExcludeFilters = DiscoveryNodeFilters.buildFromKeyValue(OR, filters);
207211
}
208212
}

‎core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,20 @@ public synchronized <T> void addAffixUpdateConsumer(Setting.AffixSetting<T> sett
207207
addSettingsUpdater(setting.newAffixUpdater(consumer, logger, validator));
208208
}
209209

210+
/**
211+
* Adds a settings consumer for affix settings. Affix settings have a namespace associated to it that needs to be available to the
212+
* consumer in order to be processed correctly. This consumer will get a namespace to value map instead of each individual namespace
213+
* and value as in {@link #addAffixUpdateConsumer(Setting.AffixSetting, BiConsumer, BiConsumer)}
214+
*/
215+
public synchronized <T> void addAffixMapUpdateConsumer(Setting.AffixSetting<T> setting, Consumer<Map<String, T>> consumer,
216+
BiConsumer<String, T> validator, boolean omitDefaults) {
217+
final Setting<?> registeredSetting = this.complexMatchers.get(setting.getKey());
218+
if (setting != registeredSetting) {
219+
throw new IllegalArgumentException("Setting is not registered for key [" + setting.getKey() + "]");
220+
}
221+
addSettingsUpdater(setting.newAffixMapUpdater(consumer, logger, validator, omitDefaults));
222+
}
223+
210224
synchronized void addSettingsUpdater(SettingUpdater<?> updater) {
211225
this.settingUpdaters.add(updater);
212226
}

0 commit comments

Comments
 (0)