Skip to content

Commit 3531172

Browse files
mushao999mushao999
authored
optimize duplicated code block in MetadataUpdateSettingsService (#82048)
Optimize duplicated code block in MetadataUpdateSettingsService This change also introduces a new unit test for the refactored function.
1 parent 55c3033 commit 3531172

File tree

2 files changed

+205
-64
lines changed

2 files changed

+205
-64
lines changed

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java‎

Lines changed: 61 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.HashSet;
3838
import java.util.Locale;
3939
import java.util.Set;
40+
import java.util.function.BiFunction;
4041

4142
import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
4243
import static org.elasticsearch.index.IndexSettings.same;
@@ -159,71 +160,31 @@ public ClusterState execute(ClusterState currentState) {
159160
}
160161
}
161162

162-
if (openIndices.isEmpty() == false) {
163-
for (Index index : openIndices) {
164-
IndexMetadata indexMetadata = metadataBuilder.getSafe(index);
165-
Settings.Builder updates = Settings.builder();
166-
Settings.Builder indexSettings = Settings.builder().put(indexMetadata.getSettings());
167-
if (indexScopedSettings.updateDynamicSettings(openSettings, indexSettings, updates, index.getName())) {
168-
if (preserveExisting) {
169-
indexSettings.put(indexMetadata.getSettings());
170-
}
171-
/*
172-
* The setting index.number_of_replicas is special; we require that this setting has a value
173-
* in the index. When creating the index, we ensure this by explicitly providing a value for
174-
* the setting to the default (one) if there is a not value provided on the source of the
175-
* index creation. A user can update this setting though, including updating it to null,
176-
* indicating that they want to use the default value. In this case, we again have to
177-
* provide an explicit value for the setting to the default (one).
178-
*/
179-
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) {
180-
indexSettings.put(
181-
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
182-
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY)
183-
);
184-
}
185-
Settings finalSettings = indexSettings.build();
186-
indexScopedSettings.validate(
187-
finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false),
188-
true
189-
);
190-
metadataBuilder.put(IndexMetadata.builder(indexMetadata).settings(finalSettings));
191-
}
192-
}
193-
}
163+
updateIndexSettings(
164+
openIndices,
165+
metadataBuilder,
166+
(index, indexSettings) -> indexScopedSettings.updateDynamicSettings(
167+
openSettings,
168+
indexSettings,
169+
Settings.builder(),
170+
index.getName()
171+
),
172+
preserveExisting,
173+
indexScopedSettings
174+
);
194175

195-
if (closeIndices.isEmpty() == false) {
196-
for (Index index : closeIndices) {
197-
IndexMetadata indexMetadata = metadataBuilder.getSafe(index);
198-
Settings.Builder updates = Settings.builder();
199-
Settings.Builder indexSettings = Settings.builder().put(indexMetadata.getSettings());
200-
if (indexScopedSettings.updateSettings(closedSettings, indexSettings, updates, index.getName())) {
201-
if (preserveExisting) {
202-
indexSettings.put(indexMetadata.getSettings());
203-
}
204-
/*
205-
* The setting index.number_of_replicas is special; we require that this setting has a value
206-
* in the index. When creating the index, we ensure this by explicitly providing a value for
207-
* the setting to the default (one) if there is a not value provided on the source of the
208-
* index creation. A user can update this setting though, including updating it to null,
209-
* indicating that they want to use the default value. In this case, we again have to
210-
* provide an explicit value for the setting to the default (one).
211-
*/
212-
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) {
213-
indexSettings.put(
214-
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
215-
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY)
216-
);
217-
}
218-
Settings finalSettings = indexSettings.build();
219-
indexScopedSettings.validate(
220-
finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false),
221-
true
222-
);
223-
metadataBuilder.put(IndexMetadata.builder(indexMetadata).settings(finalSettings));
224-
}
225-
}
226-
}
176+
updateIndexSettings(
177+
closeIndices,
178+
metadataBuilder,
179+
(index, indexSettings) -> indexScopedSettings.updateSettings(
180+
closedSettings,
181+
indexSettings,
182+
Settings.builder(),
183+
index.getName()
184+
),
185+
preserveExisting,
186+
indexScopedSettings
187+
);
227188

228189
if (IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.exists(normalizedSettings)
229190
|| IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.exists(normalizedSettings)) {
@@ -287,6 +248,42 @@ public ClusterState execute(ClusterState currentState) {
287248
);
288249
}
289250

251+
public static void updateIndexSettings(
252+
Set<Index> indices,
253+
Metadata.Builder metadataBuilder,
254+
BiFunction<Index, Settings.Builder, Boolean> settingUpdater,
255+
Boolean preserveExisting,
256+
IndexScopedSettings indexScopedSettings
257+
258+
) {
259+
for (Index index : indices) {
260+
IndexMetadata indexMetadata = metadataBuilder.getSafe(index);
261+
Settings.Builder indexSettings = Settings.builder().put(indexMetadata.getSettings());
262+
if (settingUpdater.apply(index, indexSettings)) {
263+
if (preserveExisting) {
264+
indexSettings.put(indexMetadata.getSettings());
265+
}
266+
/*
267+
* The setting index.number_of_replicas is special; we require that this setting has a value
268+
* in the index. When creating the index, we ensure this by explicitly providing a value for
269+
* the setting to the default (one) if there is a not value provided on the source of the
270+
* index creation. A user can update this setting though, including updating it to null,
271+
* indicating that they want to use the default value. In this case, we again have to
272+
* provide an explicit value for the setting to the default (one).
273+
*/
274+
if (IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettings) == false) {
275+
indexSettings.put(
276+
IndexMetadata.SETTING_NUMBER_OF_REPLICAS,
277+
IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(Settings.EMPTY)
278+
);
279+
}
280+
Settings finalSettings = indexSettings.build();
281+
indexScopedSettings.validate(finalSettings.filter(k -> indexScopedSettings.isPrivateSetting(k) == false), true);
282+
metadataBuilder.put(IndexMetadata.builder(indexMetadata).settings(finalSettings));
283+
}
284+
}
285+
}
286+
290287
/**
291288
* Updates the cluster block only iff the setting exists in the given settings
292289
*/

‎server/src/test/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsServiceTests.java‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
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 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
package org.elasticsearch.cluster.metadata;
10+
11+
import org.elasticsearch.Version;
12+
import org.elasticsearch.common.UUIDs;
13+
import org.elasticsearch.common.settings.IndexScopedSettings;
14+
import org.elasticsearch.common.settings.Settings;
15+
import org.elasticsearch.index.Index;
16+
import org.elasticsearch.index.IndexModule;
17+
import org.elasticsearch.index.IndexSettings;
18+
import org.elasticsearch.index.codec.CodecService;
19+
import org.elasticsearch.index.engine.EngineConfig;
20+
import org.elasticsearch.index.translog.Translog;
21+
import org.elasticsearch.test.ESTestCase;
22+
23+
import java.util.Set;
24+
25+
import static org.elasticsearch.index.IndexModule.Type.MMAPFS;
26+
import static org.elasticsearch.index.IndexModule.Type.NIOFS;
27+
28+
public class MetadataUpdateSettingsServiceTests extends ESTestCase {
29+
private final Index index = new Index("test", UUIDs.randomBase64UUID());
30+
private final Settings metaSettings = Settings.builder()
31+
.put(IndexMetadata.SETTING_INDEX_UUID, index.getUUID())
32+
.put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
33+
.put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), Version.CURRENT)
34+
.build();
35+
private final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(
36+
Settings.EMPTY,
37+
IndexScopedSettings.BUILT_IN_INDEX_SETTINGS
38+
);
39+
40+
/**
41+
* test update dynamic settings for open index, also test override existing settings if preserveExisting is false
42+
*/
43+
public void testUpdateOpenIndexSettings() {
44+
// original settings: {"index.number_of_replicas": 5, "index.refresh_interval": "5s"}
45+
Metadata metadata = mockMetadata(
46+
index,
47+
Settings.builder()
48+
.put(metaSettings)
49+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 5)
50+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
51+
.build()
52+
);
53+
Metadata.Builder metadataBuilder = Metadata.builder(metadata);
54+
// settings to apply: {"index.refresh_interval": "2s", "index.translog.durability": "ASYNC"}
55+
Settings settingToApply = Settings.builder()
56+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "2s")
57+
.put(IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING.getKey(), Translog.Durability.ASYNC)
58+
.build();
59+
// update settings
60+
MetadataUpdateSettingsService.updateIndexSettings(
61+
Set.of(index),
62+
metadataBuilder,
63+
(index, indexSettings) -> indexScopedSettings.updateDynamicSettings(
64+
settingToApply,
65+
indexSettings,
66+
Settings.builder(),
67+
index.getName()
68+
),
69+
false,
70+
indexScopedSettings
71+
);
72+
// expected settings: {"index.refresh_interval": "2s", "index.number_of_replicas": 5, "index.translog.durability": "ASYNC"}
73+
assertEquals(
74+
Settings.builder()
75+
.put(metaSettings)
76+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 5)
77+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "2s")
78+
.put(IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING.getKey(), Translog.Durability.ASYNC)
79+
.build(),
80+
metadataBuilder.get(index.getName()).getSettings()
81+
);
82+
}
83+
84+
/**
85+
* test update dynamic and static settings for closed index, also test not override existing settings if preserveExisting is ture
86+
*/
87+
public void testUpdateClosedIndexSettings() {
88+
// original dynamic settings: {"index.number_of_replicas": 5, "index.refresh_interval": "5s"}
89+
// original static settings: {"index.codec": "best_compression", "index.store.type": "niofs"}
90+
Metadata metadata = mockMetadata(
91+
index,
92+
Settings.builder()
93+
.put(metaSettings)
94+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 5)
95+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
96+
.put(EngineConfig.INDEX_CODEC_SETTING.getKey(), CodecService.BEST_COMPRESSION_CODEC)
97+
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), NIOFS.getSettingsKey())
98+
.build()
99+
);
100+
Metadata.Builder metadataBuilder = Metadata.builder(metadata);
101+
// dynamic settings to apply: {"index.refresh_interval": "2s", "index.translog.durability": "ASYNC"}
102+
// static settings to apply : {"index.store.type": "mmapfs", "index.store.preload": ["dvd", "tmp"]}
103+
Settings settingToApply = Settings.builder()
104+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "2s")
105+
.put(IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING.getKey(), Translog.Durability.ASYNC)
106+
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), MMAPFS.getSettingsKey())
107+
.putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "tmp")
108+
.build();
109+
// update settings
110+
MetadataUpdateSettingsService.updateIndexSettings(
111+
Set.of(index),
112+
metadataBuilder,
113+
(index, indexSettings) -> indexScopedSettings.updateSettings(
114+
settingToApply,
115+
indexSettings,
116+
Settings.builder(),
117+
index.getName()
118+
),
119+
true,
120+
indexScopedSettings
121+
);
122+
// expected dynamic settings: {"index.refresh_interval": "2s", "index.number_of_replicas": 5, "index.translog.durability": "async"}
123+
// expected static settings: {"index.codec": "best_compression", "index.store.type": "mmapfs", "index.store.preload": ["dvd",
124+
// "tmp"]}
125+
assertEquals(
126+
Settings.builder()
127+
.put(metaSettings)
128+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 5)
129+
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), "5s")
130+
.put(IndexSettings.INDEX_TRANSLOG_DURABILITY_SETTING.getKey(), Translog.Durability.ASYNC)
131+
.put(EngineConfig.INDEX_CODEC_SETTING.getKey(), CodecService.BEST_COMPRESSION_CODEC)
132+
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), NIOFS.getSettingsKey())
133+
.putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "tmp")
134+
.build(),
135+
metadataBuilder.get(index.getName()).getSettings()
136+
);
137+
}
138+
139+
private Metadata mockMetadata(Index index, Settings indexSettings) {
140+
return Metadata.builder()
141+
.put(IndexMetadata.builder(index.getName()).settings(Settings.builder().put(indexSettings)).build(), true)
142+
.build();
143+
}
144+
}

0 commit comments

Comments
 (0)