Skip to content

Commit 8c37255

Browse files
rjernstrjernst
authored
Revert "Fail when using multiple data paths (#72184)" (#79116)
This reverts commit 6aa0735. This revert was conflict free. relates #78525 relates #71205
1 parent 2a37c0b commit 8c37255

File tree

12 files changed

+189
-30
lines changed

12 files changed

+189
-30
lines changed

‎server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/allocation/decider/MockDiskUsagesIT.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
package org.elasticsearch.cluster.routing.allocation.decider;
99

10+
import org.elasticsearch.action.admin.indices.stats.ShardStats;
1011
import org.elasticsearch.action.support.WriteRequest;
1112
import org.elasticsearch.cluster.ClusterInfoService;
1213
import org.elasticsearch.cluster.ClusterInfoServiceUtils;
@@ -24,6 +25,8 @@
2425
import org.elasticsearch.plugins.Plugin;
2526
import org.elasticsearch.test.ESIntegTestCase;
2627

28+
import java.nio.file.Path;
29+
import java.util.Arrays;
2730
import java.util.Collection;
2831
import java.util.Collections;
2932
import java.util.HashMap;
@@ -44,6 +47,8 @@
4447
import static org.hamcrest.Matchers.equalTo;
4548
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
4649
import static org.hamcrest.Matchers.lessThanOrEqualTo;
50+
import static org.hamcrest.Matchers.not;
51+
import static org.hamcrest.Matchers.startsWith;
4752

4853
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0)
4954
public class MockDiskUsagesIT extends ESIntegTestCase {
@@ -315,6 +320,83 @@ public void testDoesNotExceedLowWatermarkWhenRebalancing() throws Exception {
315320
assertThat("node2 has 1 shard", getShardCountByNodeId().get(nodeIds.get(2)), equalTo(1));
316321
}
317322

323+
public void testMovesShardsOffSpecificDataPathAboveWatermark() throws Exception {
324+
325+
// start one node with two data paths
326+
final Path pathOverWatermark = createTempDir();
327+
final Settings.Builder twoPathSettings = Settings.builder();
328+
if (randomBoolean()) {
329+
twoPathSettings.putList(Environment.PATH_DATA_SETTING.getKey(), createTempDir().toString(), pathOverWatermark.toString());
330+
} else {
331+
twoPathSettings.putList(Environment.PATH_DATA_SETTING.getKey(), pathOverWatermark.toString(), createTempDir().toString());
332+
}
333+
internalCluster().startNode(twoPathSettings);
334+
final String nodeWithTwoPaths = client().admin().cluster().prepareNodesInfo().get().getNodes().get(0).getNode().getId();
335+
336+
// other two nodes have one data path each
337+
internalCluster().startNode(Settings.builder().put(Environment.PATH_DATA_SETTING.getKey(), createTempDir()));
338+
internalCluster().startNode(Settings.builder().put(Environment.PATH_DATA_SETTING.getKey(), createTempDir()));
339+
340+
final MockInternalClusterInfoService clusterInfoService = getMockInternalClusterInfoService();
341+
342+
// prevent any effects from in-flight recoveries, since we are only simulating a 100-byte disk
343+
clusterInfoService.setShardSizeFunctionAndRefresh(shardRouting -> 0L);
344+
345+
// start with all paths below the watermark
346+
clusterInfoService.setDiskUsageFunctionAndRefresh((discoveryNode, fsInfoPath) -> setDiskUsage(fsInfoPath, 100, between(10, 100)));
347+
348+
assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder()
349+
.put(CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "90%")
350+
.put(CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "90%")
351+
.put(CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "100%")));
352+
353+
final List<String> nodeIds = StreamSupport.stream(client().admin().cluster().prepareState().get().getState()
354+
.getRoutingNodes().spliterator(), false).map(RoutingNode::nodeId).collect(Collectors.toList());
355+
356+
assertAcked(prepareCreate("test").setSettings(Settings.builder().put("number_of_shards", 6).put("number_of_replicas", 0)));
357+
358+
ensureGreen("test");
359+
360+
{
361+
final Map<String, Integer> shardCountByNodeId = getShardCountByNodeId();
362+
assertThat("node0 has 2 shards", shardCountByNodeId.get(nodeIds.get(0)), equalTo(2));
363+
assertThat("node1 has 2 shards", shardCountByNodeId.get(nodeIds.get(1)), equalTo(2));
364+
assertThat("node2 has 2 shards", shardCountByNodeId.get(nodeIds.get(2)), equalTo(2));
365+
}
366+
367+
final long shardsOnGoodPath = Arrays.stream(client().admin().indices().prepareStats("test").get().getShards())
368+
.filter(shardStats -> shardStats.getShardRouting().currentNodeId().equals(nodeWithTwoPaths)
369+
&& shardStats.getDataPath().startsWith(pathOverWatermark.toString()) == false).count();
370+
logger.info("--> shards on good path: [{}]", shardsOnGoodPath);
371+
372+
// disable rebalancing, or else we might move shards back onto the over-full path since we're not faking that
373+
assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder()
374+
.put(CLUSTER_ROUTING_REBALANCE_ENABLE_SETTING.getKey(), EnableAllocationDecider.Rebalance.NONE)));
375+
376+
// one of the paths on node0 suddenly exceeds the high watermark
377+
clusterInfoService.setDiskUsageFunctionAndRefresh((discoveryNode, fsInfoPath) -> setDiskUsage(fsInfoPath, 100L,
378+
fsInfoPath.getPath().startsWith(pathOverWatermark.toString()) ? between(0, 9) : between(10, 100)));
379+
380+
logger.info("--> waiting for shards to relocate off path [{}]", pathOverWatermark);
381+
382+
assertBusy(() -> {
383+
for (final ShardStats shardStats : client().admin().indices().prepareStats("test").get().getShards()) {
384+
assertThat(shardStats.getDataPath(), not(startsWith(pathOverWatermark.toString())));
385+
}
386+
});
387+
388+
ensureGreen("test");
389+
390+
for (final ShardStats shardStats : client().admin().indices().prepareStats("test").get().getShards()) {
391+
assertThat(shardStats.getDataPath(), not(startsWith(pathOverWatermark.toString())));
392+
}
393+
394+
assertThat("should not have moved any shards off of the path that wasn't too full",
395+
Arrays.stream(client().admin().indices().prepareStats("test").get().getShards())
396+
.filter(shardStats -> shardStats.getShardRouting().currentNodeId().equals(nodeWithTwoPaths)
397+
&& shardStats.getDataPath().startsWith(pathOverWatermark.toString()) == false).count(), equalTo(shardsOnGoodPath));
398+
}
399+
318400
private Map<String, Integer> getShardCountByNodeId() {
319401
final Map<String, Integer> shardCountByNodeId = new HashMap<>();
320402
final ClusterState clusterState = client().admin().cluster().prepareState().get().getState();

‎server/src/internalClusterTest/java/org/elasticsearch/env/NodeEnvironmentIT.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.nio.file.Files;
2727
import java.nio.file.Path;
2828
import java.nio.file.StandardCopyOption;
29+
import java.util.ArrayList;
2930
import java.util.List;
3031
import java.util.Set;
3132
import java.util.stream.Collectors;
@@ -205,4 +206,38 @@ public void testUpgradeDataFolder() throws IOException, InterruptedException {
205206
ensureYellow("test");
206207
assertHitCount(client().prepareSearch().setQuery(matchAllQuery()).get(), 1L);
207208
}
209+
210+
public void testFailsToStartOnDataPathsFromMultipleNodes() throws IOException {
211+
final List<String> nodes = internalCluster().startNodes(2);
212+
ensureStableCluster(2);
213+
214+
final List<String> node0DataPaths = Environment.PATH_DATA_SETTING.get(internalCluster().dataPathSettings(nodes.get(0)));
215+
final List<String> node1DataPaths = Environment.PATH_DATA_SETTING.get(internalCluster().dataPathSettings(nodes.get(1)));
216+
217+
final List<String> allDataPaths = new ArrayList<>(node0DataPaths);
218+
allDataPaths.addAll(node1DataPaths);
219+
220+
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodes.get(1)));
221+
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodes.get(0)));
222+
223+
IllegalStateException illegalStateException = expectThrows(IllegalStateException.class,
224+
() -> PersistedClusterStateService.nodeMetadata(allDataPaths.stream().map(PathUtils::get).toArray(Path[]::new)));
225+
226+
assertThat(illegalStateException.getMessage(), containsString("unexpected node ID in metadata"));
227+
228+
illegalStateException = expectThrows(IllegalStateException.class,
229+
() -> internalCluster().startNode(Settings.builder().putList(Environment.PATH_DATA_SETTING.getKey(), allDataPaths)));
230+
231+
assertThat(illegalStateException.getMessage(), containsString("unexpected node ID in metadata"));
232+
233+
final List<String> node0DataPathsPlusOne = new ArrayList<>(node0DataPaths);
234+
node0DataPathsPlusOne.add(createTempDir().toString());
235+
internalCluster().startNode(Settings.builder().putList(Environment.PATH_DATA_SETTING.getKey(), node0DataPathsPlusOne));
236+
237+
final List<String> node1DataPathsPlusOne = new ArrayList<>(node1DataPaths);
238+
node1DataPathsPlusOne.add(createTempDir().toString());
239+
internalCluster().startNode(Settings.builder().putList(Environment.PATH_DATA_SETTING.getKey(), node1DataPathsPlusOne));
240+
241+
ensureStableCluster(2);
242+
}
208243
}

‎server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Map;
3737
import java.util.Set;
3838
import java.util.concurrent.TimeUnit;
39+
import java.util.stream.Collectors;
3940

4041
import static org.elasticsearch.env.NodeEnvironment.INDICES_FOLDER;
4142
import static org.elasticsearch.gateway.MetadataStateFormat.STATE_DIR_NAME;
@@ -240,8 +241,7 @@ public void testListenersInvokedWhenIndexHasLeftOverShard() throws Exception {
240241
.build()
241242
);
242243

243-
// TODO: decide if multiple leftovers should/can be tested without MDP
244-
final Index[] leftovers = new Index[1];
244+
final Index[] leftovers = new Index[between(1, 3)];
245245
logger.debug("--> creating [{}] leftover indices on data node [{}]", leftovers.length, dataNode);
246246
for (int i = 0; i < leftovers.length; i++) {
247247
final String indexName = "index-" + i;
@@ -275,19 +275,22 @@ public void testListenersInvokedWhenIndexHasLeftOverShard() throws Exception {
275275
final Index index = internalCluster().clusterService(masterNode).state().metadata().index(indexName).getIndex();
276276
logger.debug("--> index [{}] created", index);
277277

278-
final Path dataPath = createTempDir();
279-
final Path shardPath = dataPath.resolve(INDICES_FOLDER).resolve(index.getUUID()).resolve("0");
280-
Files.createDirectories(shardPath);
278+
final List<Path> dataPaths = new ArrayList<>();
281279
for (int i = 0; i < leftovers.length; i++) {
280+
final Path dataPath = createTempDir();
281+
dataPaths.add(dataPath);
282+
final Path shardPath = dataPath.resolve(INDICES_FOLDER).resolve(index.getUUID()).resolve("0");
283+
Files.createDirectories(shardPath);
282284
final Path leftoverPath = dataDirWithLeftOverShards.resolve(INDICES_FOLDER).resolve(leftovers[i].getUUID()).resolve("0");
283285
Files.move(leftoverPath.resolve(STATE_DIR_NAME), shardPath.resolve(STATE_DIR_NAME));
284286
Files.move(leftoverPath.resolve(INDEX_FOLDER_NAME), shardPath.resolve(INDEX_FOLDER_NAME));
285287
}
286288

287-
logger.debug("--> starting another data node with data path [{}]", dataPath);
289+
logger.debug("--> starting another data node with data paths [{}]", dataPaths);
288290
dataNode = internalCluster().startDataOnlyNode(
289291
Settings.builder()
290-
.put(Environment.PATH_DATA_SETTING.getKey(), dataPath.toAbsolutePath().toString())
292+
.putList(Environment.PATH_DATA_SETTING.getKey(),
293+
dataPaths.stream().map(p -> p.toAbsolutePath().toString()).collect(Collectors.toList()))
291294
.putNull(Environment.PATH_SHARED_DATA_SETTING.getKey())
292295
.build());
293296
ensureStableCluster(1 + 1, masterNode);

‎server/src/main/java/org/elasticsearch/env/Environment.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,7 @@ public Environment(final Settings settings, final Path configPath) {
144144

145145
final Settings.Builder finalSettings = Settings.builder().put(settings);
146146
if (PATH_DATA_SETTING.exists(settings)) {
147-
if (dataFiles.length == 1) {
148-
finalSettings.put(PATH_DATA_SETTING.getKey(), dataFiles[0].toString());
149-
} else {
150-
finalSettings.putList(PATH_DATA_SETTING.getKey(),
151-
Arrays.stream(dataFiles).map(Path::toString).collect(Collectors.toList()));
152-
}
147+
finalSettings.putList(PATH_DATA_SETTING.getKey(), Arrays.stream(dataFiles).map(Path::toString).collect(Collectors.toList()));
153148
}
154149
finalSettings.put(PATH_HOME_SETTING.getKey(), homeFile);
155150
finalSettings.put(PATH_LOGS_SETTING.getKey(), logsFile.toString());

‎server/src/main/java/org/elasticsearch/node/Node.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,9 +328,15 @@ protected Node(final Environment initialEnvironment,
328328
}
329329

330330
if (initialEnvironment.dataFiles().length > 1) {
331-
throw new IllegalArgumentException("Multiple [path.data] values found. Specify a single data path.");
332-
} else if (Environment.dataPathUsesList(tmpSettings)) {
333-
throw new IllegalArgumentException("[path.data] is a list. Specify as a string value.");
331+
// NOTE: we use initialEnvironment here, but assertEquivalent below ensures the data paths do not change
332+
deprecationLogger.critical(DeprecationCategory.SETTINGS, "multiple-data-paths",
333+
"Configuring multiple [path.data] paths is deprecated. Use RAID or other system level features for utilizing " +
334+
"multiple disks. This feature will be removed in a future release.");
335+
}
336+
if (Environment.dataPathUsesList(tmpSettings)) {
337+
// already checked for multiple values above, so if this is a list it is a single valued list
338+
deprecationLogger.critical(DeprecationCategory.SETTINGS, "multiple-data-paths-list",
339+
"Configuring [path.data] with a list is deprecated. Instead specify as a string value.");
334340
}
335341

336342
if (logger.isDebugEnabled()) {

‎server/src/test/java/org/elasticsearch/index/shard/ShardPathTests.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@
2929
import java.util.List;
3030
import java.util.concurrent.TimeUnit;
3131

32-
import static org.hamcrest.Matchers.contains;
3332
import static org.hamcrest.Matchers.containsString;
33+
import static org.hamcrest.Matchers.hasItem;
3434
import static org.hamcrest.Matchers.is;
3535

3636
public class ShardPathTests extends ESTestCase {
@@ -237,9 +237,8 @@ public void testDeleteLeftoverShardDirs() throws IOException {
237237
}
238238
ShardPath.deleteLeftoverShardDirectory(logger, env, lock, idxSettings, shardPaths -> {
239239
List<Path> envPathList = Arrays.asList(envPaths);
240-
assertEquals(envPaths.length, shardPaths.length);
241240
for (Path path : shardPaths) {
242-
assertThat(envPathList, contains(path));
241+
assertThat(envPathList, hasItem(path));
243242
}
244243
});
245244
for (Path path : envPaths) {

‎test/framework/src/main/java/org/elasticsearch/test/AbstractMultiClustersTestCase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public final void startClusters() throws Exception {
111111
public List<String> filteredWarnings() {
112112
return Stream.concat(super.filteredWarnings().stream(),
113113
List.of("Configuring multiple [path.data] paths is deprecated. Use RAID or other system level features for utilizing " +
114-
"multiple disks. This feature will be removed in 8.0.").stream()).collect(Collectors.toList());
114+
"multiple disks. This feature will be removed in a future release.").stream()).collect(Collectors.toList());
115115
}
116116

117117
@After

‎test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1840,7 +1840,7 @@ protected TestCluster buildTestCluster(Scope scope, long seed) throws IOExceptio
18401840
return new InternalTestCluster(seed, createTempDir(), supportsDedicatedMasters, getAutoManageMasterNodes(),
18411841
minNumDataNodes, maxNumDataNodes,
18421842
InternalTestCluster.clusterName(scope.name(), seed) + "-cluster", nodeConfigurationSource, getNumClientNodes(),
1843-
nodePrefix, mockPlugins, getClientWrapper(), forbidPrivateIndexSettings());
1843+
nodePrefix, mockPlugins, getClientWrapper(), forbidPrivateIndexSettings(), forceSingleDataPath());
18441844
}
18451845

18461846
private NodeConfigurationSource getNodeConfigSource() {
@@ -2149,6 +2149,13 @@ protected boolean forbidPrivateIndexSettings() {
21492149
return true;
21502150
}
21512151

2152+
/**
2153+
* Override to return true in tests that cannot handle multiple data paths.
2154+
*/
2155+
protected boolean forceSingleDataPath() {
2156+
return false;
2157+
}
2158+
21522159
/**
21532160
* Returns an instance of {@link RestClient} pointing to the current test cluster.
21542161
* Creates a new client if the method is invoked for the first time in the context of the current test scope.

‎test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.lucene.util.LuceneTestCase;
3535
import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
3636
import org.apache.lucene.util.TestRuleMarkFailure;
37+
import org.apache.lucene.util.TestUtil;
3738
import org.apache.lucene.util.TimeUnits;
3839
import org.elasticsearch.Version;
3940
import org.elasticsearch.bootstrap.BootstrapForTesting;
@@ -429,12 +430,15 @@ public void ensureNoWarnings() {
429430
}
430431

431432
protected List<String> filteredWarnings() {
433+
List<String> filtered = new ArrayList<>();
434+
filtered.add("Configuring multiple [path.data] paths is deprecated. Use RAID or other system level features for utilizing" +
435+
" multiple disks. This feature will be removed in a future release.");
436+
filtered.add("Configuring [path.data] with a list is deprecated. Instead specify as a string value");
437+
filtered.add("setting [path.shared_data] is deprecated and will be removed in a future release");
432438
if (JvmInfo.jvmInfo().getBundledJdk() == false) {
433-
return List.of("setting [path.shared_data] is deprecated and will be removed in a future release",
434-
"no-jdk distributions that do not bundle a JDK are deprecated and will be removed in a future release");
435-
} else {
436-
return List.of("setting [path.shared_data] is deprecated and will be removed in a future release");
439+
filtered.add("no-jdk distributions that do not bundle a JDK are deprecated and will be removed in a future release");
437440
}
441+
return filtered;
438442
}
439443

440444
/**
@@ -1050,7 +1054,12 @@ public Path getDataPath(String relativePath) {
10501054

10511055
/** Returns a random number of temporary paths. */
10521056
public String[] tmpPaths() {
1053-
return new String[] { createTempDir().toAbsolutePath().toString() };
1057+
final int numPaths = TestUtil.nextInt(random(), 1, 3);
1058+
final String[] absPaths = new String[numPaths];
1059+
for (int i = 0; i < numPaths; i++) {
1060+
absPaths[i] = createTempDir().toAbsolutePath().toString();
1061+
}
1062+
return absPaths;
10541063
}
10551064

10561065
public NodeEnvironment newNodeEnvironment() throws IOException {
@@ -1061,7 +1070,7 @@ public Settings buildEnvSettings(Settings settings) {
10611070
return Settings.builder()
10621071
.put(settings)
10631072
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
1064-
.put(Environment.PATH_DATA_SETTING.getKey(), createTempDir().toAbsolutePath()).build();
1073+
.putList(Environment.PATH_DATA_SETTING.getKey(), tmpPaths()).build();
10651074
}
10661075

10671076
public NodeEnvironment newNodeEnvironment(Settings settings) throws IOException {

0 commit comments

Comments
 (0)