Skip to content

Commit b1eab79

Browse files
rjernstrjernst
authored
Make Environment.dataFiles singular (#72327)
The path.data setting is now a singular string, but the method dataFiles() that gives access to the path is still an array. This commit renames the method and makes the return type a single Path. relates #71205
1 parent 633e132 commit b1eab79

12 files changed

Lines changed: 103 additions & 155 deletions

File tree

‎qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,7 @@ public void testEnvironmentPaths() throws Exception {
108108
assertExactPermissions(new FilePermission(environment.pluginsFile().toString(), "read,readlink"), permissions);
109109

110110
// data paths: r/w
111-
for (Path dataPath : environment.dataFiles()) {
112-
assertExactPermissions(new FilePermission(dataPath.toString(), "read,readlink,write,delete"), permissions);
113-
}
111+
assertExactPermissions(new FilePermission(environment.dataFile().toString(), "read,readlink,write,delete"), permissions);
114112
assertExactPermissions(new FilePermission(environment.sharedDataFile().toString(), "read,readlink,write,delete"), permissions);
115113
// logs: r/w
116114
assertExactPermissions(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"), permissions);

‎server/src/internalClusterTest/java/org/elasticsearch/index/shard/RemoveCorruptedShardDataCommandIT.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,8 @@ public void testResolvePath() throws Exception {
566566
for (String nodeName : nodeNames) {
567567
final Path indexPath = indexPathByNodeName.get(nodeName);
568568
final OptionSet options = parser.parse("--dir", indexPath.toAbsolutePath().toString());
569-
command.findAndProcessShardPath(options, environmentByNodeName.get(nodeName), environmentByNodeName.get(nodeName).dataFiles(),
569+
command.findAndProcessShardPath(options, environmentByNodeName.get(nodeName),
570+
new Path[] { environmentByNodeName.get(nodeName).dataFile() },
570571
state, shardPath -> assertThat(shardPath.resolveIndex(), equalTo(indexPath)));
571572
}
572573
}

‎server/src/main/java/org/elasticsearch/bootstrap/Security.java‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,7 @@ static Permissions createPermissions(Environment environment) throws IOException
158158

159159
private static Permissions createRecursiveDataPathPermission(Environment environment) throws IOException {
160160
Permissions policy = new Permissions();
161-
for (Path path : environment.dataFiles()) {
162-
addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete", true);
163-
}
161+
addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), environment.dataFile(), "read,readlink,write,delete", true);
164162
return policy;
165163
}
166164

@@ -202,22 +200,17 @@ static void addFilePermissions(Permissions policy, Environment environment) thro
202200
addDirectoryPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(),
203201
"read,readlink,write,delete", false);
204202
}
205-
final Set<Path> dataFilesPaths = new HashSet<>();
206-
for (Path path : environment.dataFiles()) {
207-
addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete", false);
208-
/*
209-
* We have to do this after adding the path because a side effect of that is that the directory is created; the Path#toRealPath
210-
* invocation will fail if the directory does not already exist. We use Path#toRealPath to follow symlinks and handle issues
211-
* like unicode normalization or case-insensitivity on some filesystems (e.g., the case-insensitive variant of HFS+ on macOS).
212-
*/
213-
try {
214-
final Path realPath = path.toRealPath();
215-
if (dataFilesPaths.add(realPath) == false) {
216-
throw new IllegalStateException("path [" + realPath + "] is duplicated by [" + path + "]");
217-
}
218-
} catch (final IOException e) {
219-
throw new IllegalStateException("unable to access [" + path + "]", e);
220-
}
203+
final Path dataPath = environment.dataFile();
204+
addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), dataPath, "read,readlink,write,delete", false);
205+
/*
206+
* We have to do this after adding the path because a side effect of that is that the directory is created; the Path#toRealPath
207+
* invocation will fail if the directory does not already exist. We use Path#toRealPath to follow symlinks and handle issues
208+
* like unicode normalization or case-insensitivity on some filesystems (e.g., the case-insensitive variant of HFS+ on macOS).
209+
*/
210+
try {
211+
dataPath.toRealPath();
212+
} catch (final IOException e) {
213+
throw new IllegalStateException("unable to access [" + dataPath + "]", e);
221214
}
222215
for (Path path : environment.repoFiles()) {
223216
addDirectoryPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete", false);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ public Settings settings() {
169169
/**
170170
* The data location.
171171
*/
172-
public Path[] dataFiles() {
173-
return new Path[] { dataFile };
172+
public Path dataFile() {
173+
return dataFile;
174174
}
175175

176176
/**
@@ -320,7 +320,7 @@ public static long getUsableSpace(Path path) throws IOException {
320320
* object which may contain different setting)
321321
*/
322322
public static void assertEquivalent(Environment actual, Environment expected) {
323-
assertEquals(actual.dataFiles(), expected.dataFiles(), "dataFiles");
323+
assertEquals(actual.dataFile(), expected.dataFile(), "dataFiles");
324324
assertEquals(actual.repoFiles(), expected.repoFiles(), "repoFiles");
325325
assertEquals(actual.configFile(), expected.configFile(), "configFile");
326326
assertEquals(actual.pluginsFile(), expected.pluginsFile(), "pluginsFile");

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

Lines changed: 45 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -195,27 +195,24 @@ public NodeLock(final Logger logger,
195195
final Environment environment,
196196
final CheckedFunction<Path, Boolean, IOException> pathFunction,
197197
final Function<Path, Path> subPathMapping) throws IOException {
198-
nodePaths = new NodePath[environment.dataFiles().length];
199-
locks = new Lock[nodePaths.length];
198+
nodePaths = new NodePath[1];
199+
locks = new Lock[1];
200200
try {
201-
final Path[] dataPaths = environment.dataFiles();
202-
for (int dirIndex = 0; dirIndex < dataPaths.length; dirIndex++) {
203-
Path dataDir = dataPaths[dirIndex];
204-
Path dir = subPathMapping.apply(dataDir);
205-
if (pathFunction.apply(dir) == false) {
206-
continue;
207-
}
208-
try (Directory luceneDir = FSDirectory.open(dir, NativeFSLockFactory.INSTANCE)) {
209-
logger.trace("obtaining node lock on {} ...", dir.toAbsolutePath());
210-
locks[dirIndex] = luceneDir.obtainLock(NODE_LOCK_FILENAME);
211-
nodePaths[dirIndex] = new NodePath(dir);
212-
} catch (IOException e) {
213-
logger.trace(() -> new ParameterizedMessage(
214-
"failed to obtain node lock on {}", dir.toAbsolutePath()), e);
215-
// release all the ones that were obtained up until now
216-
throw (e instanceof LockObtainFailedException ? e
217-
: new IOException("failed to obtain lock on " + dir.toAbsolutePath(), e));
218-
}
201+
Path dataDir = environment.dataFile();
202+
Path dir = subPathMapping.apply(dataDir);
203+
if (pathFunction.apply(dir) == false) {
204+
return;
205+
}
206+
try (Directory luceneDir = FSDirectory.open(dir, NativeFSLockFactory.INSTANCE)) {
207+
logger.trace("obtaining node lock on {} ...", dir.toAbsolutePath());
208+
locks[0] = luceneDir.obtainLock(NODE_LOCK_FILENAME);
209+
nodePaths[0] = new NodePath(dir);
210+
} catch (IOException e) {
211+
logger.trace(() -> new ParameterizedMessage(
212+
"failed to obtain node lock on {}", dir.toAbsolutePath()), e);
213+
// release all the ones that were obtained up until now
214+
throw (e instanceof LockObtainFailedException ? e
215+
: new IOException("failed to obtain lock on " + dir.toAbsolutePath(), e));
219216
}
220217
} catch (IOException e) {
221218
close();
@@ -247,10 +244,7 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
247244

248245
try {
249246
sharedDataPath = environment.sharedDataFile();
250-
251-
for (Path path : environment.dataFiles()) {
252-
Files.createDirectories(path);
253-
}
247+
Files.createDirectories(environment.dataFile());
254248

255249
final NodeLock nodeLock;
256250
try {
@@ -259,8 +253,8 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
259253
final String message = String.format(
260254
Locale.ROOT,
261255
"failed to obtain node locks, tried %s;" +
262-
" maybe these locations are not writable or multiple nodes were started on the same data path?",
263-
Arrays.toString(environment.dataFiles()));
256+
" maybe this location is not writable or multiple nodes were started on the same data path?",
257+
environment.dataFile());
264258
throw new IllegalStateException(message, e);
265259
}
266260

@@ -308,33 +302,31 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
308302
boolean upgradeNeeded = false;
309303

310304
// check if we can do an auto-upgrade
311-
for (Path path : environment.dataFiles()) {
312-
final Path nodesFolderPath = path.resolve("nodes");
313-
if (Files.isDirectory(nodesFolderPath)) {
314-
final List<Integer> nodeLockIds = new ArrayList<>();
315-
316-
try (DirectoryStream<Path> stream = Files.newDirectoryStream(nodesFolderPath)) {
317-
for (Path nodeLockIdPath : stream) {
318-
String fileName = nodeLockIdPath.getFileName().toString();
319-
if (Files.isDirectory(nodeLockIdPath) && fileName.chars().allMatch(Character::isDigit)) {
320-
int nodeLockId = Integer.parseInt(fileName);
321-
nodeLockIds.add(nodeLockId);
322-
} else if (FileSystemUtils.isDesktopServicesStore(nodeLockIdPath) == false) {
323-
throw new IllegalStateException("unexpected file/folder encountered during data folder upgrade: " +
324-
nodeLockIdPath);
325-
}
305+
final Path nodesFolderPath = environment.dataFile().resolve("nodes");
306+
if (Files.isDirectory(nodesFolderPath)) {
307+
final List<Integer> nodeLockIds = new ArrayList<>();
308+
309+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(nodesFolderPath)) {
310+
for (Path nodeLockIdPath : stream) {
311+
String fileName = nodeLockIdPath.getFileName().toString();
312+
if (Files.isDirectory(nodeLockIdPath) && fileName.chars().allMatch(Character::isDigit)) {
313+
int nodeLockId = Integer.parseInt(fileName);
314+
nodeLockIds.add(nodeLockId);
315+
} else if (FileSystemUtils.isDesktopServicesStore(nodeLockIdPath) == false) {
316+
throw new IllegalStateException("unexpected file/folder encountered during data folder upgrade: " +
317+
nodeLockIdPath);
326318
}
327319
}
320+
}
328321

329-
if (nodeLockIds.isEmpty() == false) {
330-
upgradeNeeded = true;
322+
if (nodeLockIds.isEmpty() == false) {
323+
upgradeNeeded = true;
331324

332-
if (nodeLockIds.equals(Arrays.asList(0)) == false) {
333-
throw new IllegalStateException("data path " + nodesFolderPath + " cannot be upgraded automatically because it " +
334-
"contains data from nodes with ordinals " + nodeLockIds + ", due to previous use of the now obsolete " +
335-
"[node.max_local_storage_nodes] setting. Please check the breaking changes docs for the current version of " +
336-
"Elasticsearch to find an upgrade path");
337-
}
325+
if (nodeLockIds.equals(Arrays.asList(0)) == false) {
326+
throw new IllegalStateException("data path " + nodesFolderPath + " cannot be upgraded automatically because it " +
327+
"contains data from nodes with ordinals " + nodeLockIds + ", due to previous use of the now obsolete " +
328+
"[node.max_local_storage_nodes] setting. Please check the breaking changes docs for the current version of " +
329+
"Elasticsearch to find an upgrade path");
338330
}
339331
}
340332
}
@@ -344,7 +336,7 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
344336
return false;
345337
}
346338

347-
logger.info("upgrading legacy data folders: {}", Arrays.toString(environment.dataFiles()));
339+
logger.info("upgrading legacy data folder: {}", environment.dataFile());
348340

349341
// acquire locks on legacy path for duration of upgrade (to ensure there is no older ES version running on this path)
350342
final NodeLock legacyNodeLock;
@@ -354,8 +346,8 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
354346
final String message = String.format(
355347
Locale.ROOT,
356348
"failed to obtain legacy node locks, tried %s;" +
357-
" maybe these locations are not writable or multiple nodes were started on the same data path?",
358-
Arrays.toString(environment.dataFiles()));
349+
" maybe this location is not writable or multiple nodes were started on the same data path?",
350+
environment.dataFile());
359351
throw new IllegalStateException(message, e);
360352
}
361353

@@ -429,7 +421,7 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
429421
}
430422

431423
// upgrade successfully completed, remove legacy nodes folders
432-
IOUtils.rm(Stream.of(environment.dataFiles()).map(path -> path.resolve("nodes")).toArray(Path[]::new));
424+
IOUtils.rm(environment.dataFile().resolve("nodes"));
433425

434426
return true;
435427
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ protected Node(final Environment initialEnvironment,
312312

313313
if (logger.isDebugEnabled()) {
314314
logger.debug("using config [{}], data [{}], logs [{}], plugins [{}]",
315-
initialEnvironment.configFile(), Arrays.toString(initialEnvironment.dataFiles()),
315+
initialEnvironment.configFile(), initialEnvironment.dataFile(),
316316
initialEnvironment.logsFile(), initialEnvironment.pluginsFile());
317317
}
318318

‎server/src/test/java/org/elasticsearch/env/EnvironmentTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void testPathDataWhenNotSet() {
6262
final Path pathHome = createTempDir().toAbsolutePath();
6363
final Settings settings = Settings.builder().put("path.home", pathHome).build();
6464
final Environment environment = new Environment(settings, null);
65-
assertThat(environment.dataFiles(), equalTo(new Path[]{pathHome.resolve("data")}));
65+
assertThat(environment.dataFile(), equalTo(pathHome.resolve("data")));
6666
}
6767

6868
public void testPathDataNotSetInEnvironmentIfNotSet() {

‎server/src/test/java/org/elasticsearch/env/NodeRepurposeCommandTests.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import java.io.IOException;
3333
import java.nio.file.Files;
3434
import java.nio.file.Path;
35-
import java.util.Arrays;
3635
import java.util.Set;
3736
import java.util.stream.Stream;
3837

@@ -121,7 +120,7 @@ public void testCleanupAll() throws Exception {
121120

122121
String messageText = NodeRepurposeCommand.noMasterMessage(
123122
1,
124-
environment.dataFiles().length*shardCount,
123+
shardCount,
125124
0);
126125

127126
Matcher<String> outputMatcher = allOf(
@@ -148,7 +147,7 @@ public void testCleanupShardData() throws Exception {
148147
createIndexDataFiles(dataMasterSettings, shardCount, hasClusterState);
149148

150149
Matcher<String> matcher = allOf(
151-
containsString(NodeRepurposeCommand.shardMessage(environment.dataFiles().length * shardCount, 1)),
150+
containsString(NodeRepurposeCommand.shardMessage(shardCount, 1)),
152151
conditionalNot(containsString("testUUID"), verbose == false),
153152
conditionalNot(containsString("testIndex"), verbose == false || hasClusterState == false),
154153
conditionalNot(containsString("no name for uuid: testUUID"), verbose == false || hasClusterState)
@@ -245,8 +244,7 @@ private void verifyUnchangedDataFiles(CheckedRunnable<? extends Exception> runna
245244
}
246245

247246
private long digestPaths() {
248-
// use a commutative digest to avoid dependency on file system order.
249-
return Arrays.stream(environment.dataFiles()).mapToLong(this::digestPath).sum();
247+
return digestPath(environment.dataFile());
250248
}
251249

252250
private long digestPath(Path path) {

‎test/framework/src/main/java/org/elasticsearch/cluster/DiskUsageIntegTestCase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
9292
}
9393

9494
public TestFileStore getTestFileStore(String nodeName) {
95-
return fileSystemProvider.getTestFileStore(internalCluster().getInstance(Environment.class, nodeName).dataFiles()[0]);
95+
return fileSystemProvider.getTestFileStore(internalCluster().getInstance(Environment.class, nodeName).dataFile());
9696
}
9797

9898
protected static class TestFileStore extends FilterFileStore {

0 commit comments

Comments
 (0)