Skip to content

Commit 6aa0735

Browse files
rjernstrjernst
authored
Fail when using multiple data paths (#72184)
This commit converts the deprecation messages for multiple data paths into errors. It effectively removes support for multiple data paths. relates #71205
1 parent 50d0ebb commit 6aa0735

File tree

10 files changed

+22
-179
lines changed

10 files changed

+22
-179
lines changed

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

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

10-
import org.elasticsearch.action.admin.indices.stats.ShardStats;
1110
import org.elasticsearch.action.support.WriteRequest;
1211
import org.elasticsearch.cluster.ClusterInfoService;
1312
import org.elasticsearch.cluster.ClusterInfoServiceUtils;
@@ -25,8 +24,6 @@
2524
import org.elasticsearch.plugins.Plugin;
2625
import org.elasticsearch.test.ESIntegTestCase;
2726

28-
import java.nio.file.Path;
29-
import java.util.Arrays;
3027
import java.util.Collection;
3128
import java.util.Collections;
3229
import java.util.HashMap;
@@ -47,8 +44,6 @@
4744
import static org.hamcrest.Matchers.equalTo;
4845
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
4946
import static org.hamcrest.Matchers.lessThanOrEqualTo;
50-
import static org.hamcrest.Matchers.not;
51-
import static org.hamcrest.Matchers.startsWith;
5247

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

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-
400318
private Map<String, Integer> getShardCountByNodeId() {
401319
final Map<String, Integer> shardCountByNodeId = new HashMap<>();
402320
final ClusterState clusterState = client().admin().cluster().prepareState().get().getState();

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

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

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

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

4140
import static org.elasticsearch.env.NodeEnvironment.INDICES_FOLDER;
4241
import static org.elasticsearch.gateway.MetadataStateFormat.STATE_DIR_NAME;
@@ -241,7 +240,8 @@ public void testListenersInvokedWhenIndexHasLeftOverShard() throws Exception {
241240
.build()
242241
);
243242

244-
final Index[] leftovers = new Index[between(1, 3)];
243+
// TODO: decide if multiple leftovers should/can be tested without MDP
244+
final Index[] leftovers = new Index[1];
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,22 +275,19 @@ 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 List<Path> dataPaths = new ArrayList<>();
278+
final Path dataPath = createTempDir();
279+
final Path shardPath = dataPath.resolve(INDICES_FOLDER).resolve(index.getUUID()).resolve("0");
280+
Files.createDirectories(shardPath);
279281
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);
284282
final Path leftoverPath = dataDirWithLeftOverShards.resolve(INDICES_FOLDER).resolve(leftovers[i].getUUID()).resolve("0");
285283
Files.move(leftoverPath.resolve(STATE_DIR_NAME), shardPath.resolve(STATE_DIR_NAME));
286284
Files.move(leftoverPath.resolve(INDEX_FOLDER_NAME), shardPath.resolve(INDEX_FOLDER_NAME));
287285
}
288286

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

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,12 @@ public Environment(final Settings settings, final Path configPath) {
143143

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

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

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

309309
if (initialEnvironment.dataFiles().length > 1) {
310-
// NOTE: we use initialEnvironment here, but assertEquivalent below ensures the data paths do not change
311-
deprecationLogger.deprecate(DeprecationCategory.SETTINGS, "multiple-data-paths",
312-
"Configuring multiple [path.data] paths is deprecated. Use RAID or other system level features for utilizing " +
313-
"multiple disks. This feature will be removed in 8.0.");
314-
}
315-
if (Environment.dataPathUsesList(tmpSettings)) {
316-
// already checked for multiple values above, so if this is a list it is a single valued list
317-
deprecationLogger.deprecate(DeprecationCategory.SETTINGS, "multiple-data-paths-list",
318-
"Configuring [path.data] with a list is deprecated. Instead specify as a string value.");
310+
throw new IllegalArgumentException("Multiple [path.data] values found. Specify a single data path.");
311+
} else if (Environment.dataPathUsesList(tmpSettings)) {
312+
throw new IllegalArgumentException("[path.data] is a list. Specify as a string value.");
319313
}
320314

321315
if (logger.isDebugEnabled()) {

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,7 +1836,7 @@ protected TestCluster buildTestCluster(Scope scope, long seed) throws IOExceptio
18361836
return new InternalTestCluster(seed, createTempDir(), supportsDedicatedMasters, getAutoManageMasterNodes(),
18371837
minNumDataNodes, maxNumDataNodes,
18381838
InternalTestCluster.clusterName(scope.name(), seed) + "-cluster", nodeConfigurationSource, getNumClientNodes(),
1839-
nodePrefix, mockPlugins, getClientWrapper(), forbidPrivateIndexSettings(), forceSingleDataPath());
1839+
nodePrefix, mockPlugins, getClientWrapper(), forbidPrivateIndexSettings());
18401840
}
18411841

18421842
private NodeConfigurationSource getNodeConfigSource() {
@@ -2145,13 +2145,6 @@ protected boolean forbidPrivateIndexSettings() {
21452145
return true;
21462146
}
21472147

2148-
/**
2149-
* Override to return true in tests that cannot handle multiple data paths.
2150-
*/
2151-
protected boolean forceSingleDataPath() {
2152-
return false;
2153-
}
2154-
21552148
/**
21562149
* Returns an instance of {@link RestClient} pointing to the current test cluster.
21572150
* 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: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
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;
3837
import org.apache.lucene.util.TimeUnits;
3938
import org.elasticsearch.Version;
4039
import org.elasticsearch.bootstrap.BootstrapForTesting;
@@ -1044,12 +1043,7 @@ public Path getDataPath(String relativePath) {
10441043

10451044
/** Returns a random number of temporary paths. */
10461045
public String[] tmpPaths() {
1047-
final int numPaths = TestUtil.nextInt(random(), 1, 3);
1048-
final String[] absPaths = new String[numPaths];
1049-
for (int i = 0; i < numPaths; i++) {
1050-
absPaths[i] = createTempDir().toAbsolutePath().toString();
1051-
}
1052-
return absPaths;
1046+
return new String[] { createTempDir().toAbsolutePath().toString() };
10531047
}
10541048

10551049
public NodeEnvironment newNodeEnvironment() throws IOException {
@@ -1060,7 +1054,7 @@ public Settings buildEnvSettings(Settings settings) {
10601054
return Settings.builder()
10611055
.put(settings)
10621056
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
1063-
.putList(Environment.PATH_DATA_SETTING.getKey(), tmpPaths()).build();
1057+
.put(Environment.PATH_DATA_SETTING.getKey(), createTempDir().toAbsolutePath()).build();
10641058
}
10651059

10661060
public NodeEnvironment newNodeEnvironment(Settings settings) throws IOException {

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

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@
129129
import java.util.function.Function;
130130
import java.util.function.Predicate;
131131
import java.util.stream.Collectors;
132-
import java.util.stream.IntStream;
133132
import java.util.stream.Stream;
134133

135134
import static org.apache.lucene.util.LuceneTestCase.TEST_NIGHTLY;
@@ -233,8 +232,6 @@ public final class InternalTestCluster extends TestCluster {
233232

234233
private final boolean forbidPrivateIndexSettings;
235234

236-
private final int numDataPaths;
237-
238235
/**
239236
* All nodes started by the cluster will have their name set to nodePrefix followed by a positive number
240237
*/
@@ -272,8 +269,7 @@ public InternalTestCluster(
272269
nodePrefix,
273270
mockPlugins,
274271
clientWrapper,
275-
true,
276-
false);
272+
true);
277273
}
278274

279275
public InternalTestCluster(
@@ -289,8 +285,7 @@ public InternalTestCluster(
289285
final String nodePrefix,
290286
final Collection<Class<? extends Plugin>> mockPlugins,
291287
final Function<Client, Client> clientWrapper,
292-
final boolean forbidPrivateIndexSettings,
293-
final boolean forceSingleDataPath) {
288+
final boolean forbidPrivateIndexSettings) {
294289
super(clusterSeed);
295290
this.autoManageMasterNodes = autoManageMasterNodes;
296291
this.clientWrapper = clientWrapper;
@@ -352,8 +347,6 @@ public InternalTestCluster(
352347
numSharedDedicatedMasterNodes, numSharedDataNodes, numSharedCoordOnlyNodes,
353348
autoManageMasterNodes ? "auto-managed" : "manual");
354349
this.nodeConfigurationSource = nodeConfigurationSource;
355-
// use 1 data path if we are forced to, or 80% of the time that we are not, otherwise use between 2 and 4 data paths
356-
numDataPaths = forceSingleDataPath || random.nextDouble() < 0.8 ? 1 : RandomNumbers.randomIntBetween(random, 2, 4);
357350
Builder builder = Settings.builder();
358351
builder.put(Environment.PATH_HOME_SETTING.getKey(), baseDir);
359352
builder.put(Environment.PATH_REPO_SETTING.getKey(), baseDir.resolve("repos"));
@@ -631,14 +624,7 @@ private Settings getNodeSettings(final int nodeId, final long seed, final Settin
631624
final Settings.Builder updatedSettings = Settings.builder();
632625

633626
updatedSettings.put(Environment.PATH_HOME_SETTING.getKey(), baseDir);
634-
635-
if (numDataPaths > 1) {
636-
updatedSettings.putList(Environment.PATH_DATA_SETTING.getKey(), IntStream.range(0, numDataPaths).mapToObj(i ->
637-
baseDir.resolve(name).resolve("d" + i).toString()).collect(Collectors.toList()));
638-
} else {
639-
updatedSettings.put(Environment.PATH_DATA_SETTING.getKey(), baseDir.resolve(name));
640-
}
641-
627+
updatedSettings.put(Environment.PATH_DATA_SETTING.getKey(), baseDir.resolve(name));
642628
updatedSettings.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), baseDir.resolve(name + "-shared"));
643629

644630
// allow overriding the above

‎x-pack/plugin/autoscaling/src/internalClusterTest/java/org/elasticsearch/xpack/autoscaling/AbstractFrozenAutoscalingIntegTestCase.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@ public abstract class AbstractFrozenAutoscalingIntegTestCase extends AbstractSna
5151
protected final String snapshotName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
5252
protected final String policyName = "frozen";
5353

54-
@Override
55-
protected boolean forceSingleDataPath() {
56-
return true;
57-
}
58-
5954
@Override
6055
protected boolean addMockInternalEngine() {
6156
return false;

‎x-pack/plugin/searchable-snapshots/src/internalClusterTest/java/org/elasticsearch/xpack/searchablesnapshots/BaseFrozenSearchableSnapshotsIntegTestCase.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@
1515
import org.elasticsearch.xpack.searchablesnapshots.cache.shared.FrozenCacheService;
1616

1717
public class BaseFrozenSearchableSnapshotsIntegTestCase extends BaseSearchableSnapshotsIntegTestCase {
18-
@Override
19-
protected boolean forceSingleDataPath() {
20-
return true;
21-
}
2218

2319
@Override
2420
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {

0 commit comments

Comments
 (0)