Revert "Make Environment.dataFiles singular (#72327)" (#79028)

This reverts commit b1eab79.

This revert was conflict free.

relates #78525
relates #71205

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

@@ -108,7 +108,9 @@ public void testEnvironmentPaths() throws Exception {
         assertExactPermissions(new FilePermission(environment.pluginsFile().toString(), "read,readlink"), permissions);
 
         // data paths: r/w
-        assertExactPermissions(new FilePermission(environment.dataFile().toString(), "read,readlink,write,delete"), permissions);
+        for (Path dataPath : environment.dataFiles()) {
+            assertExactPermissions(new FilePermission(dataPath.toString(), "read,readlink,write,delete"), permissions);
+        }
         assertExactPermissions(new FilePermission(environment.sharedDataFile().toString(), "read,readlink,write,delete"), permissions);
         // logs: r/w
         assertExactPermissions(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"), permissions);

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

@@ -567,8 +567,7 @@ public void testResolvePath() throws Exception {
         for (String nodeName : nodeNames) {
             final Path indexPath = indexPathByNodeName.get(nodeName);
             final OptionSet options = parser.parse("--dir", indexPath.toAbsolutePath().toString());
-            command.findAndProcessShardPath(options, environmentByNodeName.get(nodeName),
-                new Path[] { environmentByNodeName.get(nodeName).dataFile() },
+            command.findAndProcessShardPath(options, environmentByNodeName.get(nodeName), environmentByNodeName.get(nodeName).dataFiles(),
                 state, shardPath -> assertThat(shardPath.resolveIndex(), equalTo(indexPath)));
         }
     }

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

@@ -159,7 +159,9 @@ static Permissions createPermissions(Environment environment) throws IOException
 
     private static Permissions createRecursiveDataPathPermission(Environment environment) throws IOException {
         Permissions policy = new Permissions();
-        addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), environment.dataFile(), "read,readlink,write,delete", true);
+        for (Path path : environment.dataFiles()) {
+            addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete", true);
+        }
         return policy;
     }
 
@@ -201,17 +203,22 @@ static void addFilePermissions(Permissions policy, Environment environment) thro
             addDirectoryPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(),
                 "read,readlink,write,delete", false);
         }
-        final Path dataPath = environment.dataFile();
-        addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), dataPath, "read,readlink,write,delete", false);
-        /*
-         * We have to do this after adding the path because a side effect of that is that the directory is created; the Path#toRealPath
-         * invocation will fail if the directory does not already exist. We use Path#toRealPath to follow symlinks and handle issues
-         * like unicode normalization or case-insensitivity on some filesystems (e.g., the case-insensitive variant of HFS+ on macOS).
-         */
-        try {
-            dataPath.toRealPath();
-        } catch (final IOException e) {
-            throw new IllegalStateException("unable to access [" + dataPath + "]", e);
+        final Set<Path> dataFilesPaths = new HashSet<>();
+        for (Path path : environment.dataFiles()) {
+            addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete", false);
+            /*
+             * We have to do this after adding the path because a side effect of that is that the directory is created; the Path#toRealPath
+             * invocation will fail if the directory does not already exist. We use Path#toRealPath to follow symlinks and handle issues
+             * like unicode normalization or case-insensitivity on some filesystems (e.g., the case-insensitive variant of HFS+ on macOS).
+             */
+            try {
+                final Path realPath = path.toRealPath();
+                if (dataFilesPaths.add(realPath) == false) {
+                    throw new IllegalStateException("path [" + realPath + "] is duplicated by [" + path + "]");
+                }
+            } catch (final IOException e) {
+                throw new IllegalStateException("unable to access [" + path + "]", e);
+            }
         }
         for (Path path : environment.repoFiles()) {
             addDirectoryPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete", false);

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

@@ -170,8 +170,8 @@ public Settings settings() {
     /**
      * The data location.
      */
-    public Path dataFile() {
-        return dataFile;
+    public Path[] dataFiles() {
+        return new Path[] { dataFile };
     }
 
     /**
@@ -321,7 +321,7 @@ public static long getUsableSpace(Path path) throws IOException {
      * object which may contain different setting)
      */
     public static void assertEquivalent(Environment actual, Environment expected) {
-        assertEquals(actual.dataFile(), expected.dataFile(), "dataFiles");
+        assertEquals(actual.dataFiles(), expected.dataFiles(), "dataFiles");
         assertEquals(actual.repoFiles(), expected.repoFiles(), "repoFiles");
         assertEquals(actual.configFile(), expected.configFile(), "configFile");
         assertEquals(actual.pluginsFile(), expected.pluginsFile(), "pluginsFile");

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

@@ -195,24 +195,27 @@ public NodeLock(final Logger logger,
                         final Environment environment,
                         final CheckedFunction<Path, Boolean, IOException> pathFunction,
                         final Function<Path, Path> subPathMapping) throws IOException {
-            nodePaths = new NodePath[1];
-            locks = new Lock[1];
+            nodePaths = new NodePath[environment.dataFiles().length];
+            locks = new Lock[nodePaths.length];
             try {
-                Path dataDir = environment.dataFile();
-                Path dir = subPathMapping.apply(dataDir);
-                if (pathFunction.apply(dir) == false) {
-                    return;
-                }
-                try (Directory luceneDir = FSDirectory.open(dir, NativeFSLockFactory.INSTANCE)) {
-                    logger.trace("obtaining node lock on {} ...", dir.toAbsolutePath());
-                    locks[0] = luceneDir.obtainLock(NODE_LOCK_FILENAME);
-                    nodePaths[0] = new NodePath(dir);
-                } catch (IOException e) {
-                    logger.trace(() -> new ParameterizedMessage(
-                        "failed to obtain node lock on {}", dir.toAbsolutePath()), e);
-                    // release all the ones that were obtained up until now
-                    throw (e instanceof LockObtainFailedException ? e
-                        : new IOException("failed to obtain lock on " + dir.toAbsolutePath(), e));
+                final Path[] dataPaths = environment.dataFiles();
+                for (int dirIndex = 0; dirIndex < dataPaths.length; dirIndex++) {
+                    Path dataDir = dataPaths[dirIndex];
+                    Path dir = subPathMapping.apply(dataDir);
+                    if (pathFunction.apply(dir) == false) {
+                        continue;
+                    }
+                    try (Directory luceneDir = FSDirectory.open(dir, NativeFSLockFactory.INSTANCE)) {
+                        logger.trace("obtaining node lock on {} ...", dir.toAbsolutePath());
+                        locks[dirIndex] = luceneDir.obtainLock(NODE_LOCK_FILENAME);
+                        nodePaths[dirIndex] = new NodePath(dir);
+                    } catch (IOException e) {
+                        logger.trace(() -> new ParameterizedMessage(
+                            "failed to obtain node lock on {}", dir.toAbsolutePath()), e);
+                        // release all the ones that were obtained up until now
+                        throw (e instanceof LockObtainFailedException ? e
+                            : new IOException("failed to obtain lock on " + dir.toAbsolutePath(), e));
+                    }
                 }
             } catch (IOException e) {
                 close();
@@ -244,7 +247,10 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
 
         try {
             sharedDataPath = environment.sharedDataFile();
-            Files.createDirectories(environment.dataFile());
+
+            for (Path path : environment.dataFiles()) {
+                Files.createDirectories(path);
+            }
 
             final NodeLock nodeLock;
             try {
@@ -253,8 +259,8 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
                 final String message = String.format(
                     Locale.ROOT,
                     "failed to obtain node locks, tried %s;" +
-                        " maybe this location is not writable or multiple nodes were started on the same data path?",
-                    environment.dataFile());
+                        " maybe these locations are not writable or multiple nodes were started on the same data path?",
+                    Arrays.toString(environment.dataFiles()));
                 throw new IllegalStateException(message, e);
             }
 
@@ -302,31 +308,33 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
         boolean upgradeNeeded = false;
 
         // check if we can do an auto-upgrade
-        final Path nodesFolderPath = environment.dataFile().resolve("nodes");
-        if (Files.isDirectory(nodesFolderPath)) {
-            final List<Integer> nodeLockIds = new ArrayList<>();
-
-            try (DirectoryStream<Path> stream = Files.newDirectoryStream(nodesFolderPath)) {
-                for (Path nodeLockIdPath : stream) {
-                    String fileName = nodeLockIdPath.getFileName().toString();
-                    if (Files.isDirectory(nodeLockIdPath) && fileName.chars().allMatch(Character::isDigit)) {
-                        int nodeLockId = Integer.parseInt(fileName);
-                        nodeLockIds.add(nodeLockId);
-                    } else if (FileSystemUtils.isDesktopServicesStore(nodeLockIdPath) == false) {
-                        throw new IllegalStateException("unexpected file/folder encountered during data folder upgrade: " +
-                            nodeLockIdPath);
+        for (Path path : environment.dataFiles()) {
+            final Path nodesFolderPath = path.resolve("nodes");
+            if (Files.isDirectory(nodesFolderPath)) {
+                final List<Integer> nodeLockIds = new ArrayList<>();
+
+                try (DirectoryStream<Path> stream = Files.newDirectoryStream(nodesFolderPath)) {
+                    for (Path nodeLockIdPath : stream) {
+                        String fileName = nodeLockIdPath.getFileName().toString();
+                        if (Files.isDirectory(nodeLockIdPath) && fileName.chars().allMatch(Character::isDigit)) {
+                            int nodeLockId = Integer.parseInt(fileName);
+                            nodeLockIds.add(nodeLockId);
+                        } else if (FileSystemUtils.isDesktopServicesStore(nodeLockIdPath) == false) {
+                            throw new IllegalStateException("unexpected file/folder encountered during data folder upgrade: " +
+                                nodeLockIdPath);
+                        }
                     }
                 }
-            }
 
-            if (nodeLockIds.isEmpty() == false) {
-                upgradeNeeded = true;
+                if (nodeLockIds.isEmpty() == false) {
+                    upgradeNeeded = true;
 
-                if (nodeLockIds.equals(Arrays.asList(0)) == false) {
-                    throw new IllegalStateException("data path " + nodesFolderPath + " cannot be upgraded automatically because it " +
-                        "contains data from nodes with ordinals " + nodeLockIds + ", due to previous use of the now obsolete " +
-                        "[node.max_local_storage_nodes] setting. Please check the breaking changes docs for the current version of " +
-                        "Elasticsearch to find an upgrade path");
+                    if (nodeLockIds.equals(Arrays.asList(0)) == false) {
+                        throw new IllegalStateException("data path " + nodesFolderPath + " cannot be upgraded automatically because it " +
+                            "contains data from nodes with ordinals " + nodeLockIds + ", due to previous use of the now obsolete " +
+                            "[node.max_local_storage_nodes] setting. Please check the breaking changes docs for the current version of " +
+                            "Elasticsearch to find an upgrade path");
+                    }
                 }
             }
         }
@@ -336,7 +344,7 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
             return false;
         }
 
-        logger.info("upgrading legacy data folder: {}", environment.dataFile());
+        logger.info("upgrading legacy data folders: {}", Arrays.toString(environment.dataFiles()));
 
         // acquire locks on legacy path for duration of upgrade (to ensure there is no older ES version running on this path)
         final NodeLock legacyNodeLock;
@@ -346,8 +354,8 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
             final String message = String.format(
                 Locale.ROOT,
                 "failed to obtain legacy node locks, tried %s;" +
-                    " maybe this location is not writable or multiple nodes were started on the same data path?",
-                environment.dataFile());
+                    " maybe these locations are not writable or multiple nodes were started on the same data path?",
+                Arrays.toString(environment.dataFiles()));
             throw new IllegalStateException(message, e);
         }
 
@@ -427,7 +435,7 @@ private static boolean upgradeLegacyNodeFolders(Logger logger, Settings settings
         }
 
         // upgrade successfully completed, remove legacy nodes folders
-        IOUtils.rm(environment.dataFile().resolve("nodes"));
+        IOUtils.rm(Stream.of(environment.dataFiles()).map(path -> path.resolve("nodes")).toArray(Path[]::new));
 
         return true;
     }

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

@@ -333,7 +333,7 @@ protected Node(final Environment initialEnvironment,
 
             if (logger.isDebugEnabled()) {
                 logger.debug("using config [{}], data [{}], logs [{}], plugins [{}]",
-                    initialEnvironment.configFile(), initialEnvironment.dataFile(),
+                    initialEnvironment.configFile(), Arrays.toString(initialEnvironment.dataFiles()),
                     initialEnvironment.logsFile(), initialEnvironment.pluginsFile());
             }
 

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

@@ -62,7 +62,7 @@ public void testPathDataWhenNotSet() {
         final Path pathHome = createTempDir().toAbsolutePath();
         final Settings settings = Settings.builder().put("path.home", pathHome).build();
         final Environment environment = new Environment(settings, null);
-        assertThat(environment.dataFile(), equalTo(pathHome.resolve("data")));
+        assertThat(environment.dataFiles(), equalTo(new Path[]{pathHome.resolve("data")}));
     }
 
     public void testPathDataNotSetInEnvironmentIfNotSet() {

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

@@ -32,6 +32,7 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Arrays;
 import java.util.Set;
 import java.util.stream.Stream;
 
@@ -120,7 +121,7 @@ public void testCleanupAll() throws Exception {
 
         String messageText = NodeRepurposeCommand.noMasterMessage(
             1,
-            shardCount,
+            environment.dataFiles().length*shardCount,
             0);
 
         Matcher<String> outputMatcher = allOf(
@@ -147,7 +148,7 @@ public void testCleanupShardData() throws Exception {
         createIndexDataFiles(dataMasterSettings, shardCount, hasClusterState);
 
         Matcher<String> matcher = allOf(
-            containsString(NodeRepurposeCommand.shardMessage(shardCount, 1)),
+            containsString(NodeRepurposeCommand.shardMessage(environment.dataFiles().length * shardCount, 1)),
             conditionalNot(containsString("testUUID"), verbose == false),
             conditionalNot(containsString("testIndex"), verbose == false || hasClusterState == false),
             conditionalNot(containsString("no name for uuid: testUUID"), verbose == false || hasClusterState)
@@ -244,7 +245,8 @@ private void verifyUnchangedDataFiles(CheckedRunnable<? extends Exception> runna
     }
 
     private long digestPaths() {
-        return digestPath(environment.dataFile());
+        // use a commutative digest to avoid dependency on file system order.
+        return Arrays.stream(environment.dataFiles()).mapToLong(this::digestPath).sum();
     }
 
     private long digestPath(Path path) {

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

@@ -92,7 +92,7 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
     }
 
     public TestFileStore getTestFileStore(String nodeName) {
-        return fileSystemProvider.getTestFileStore(internalCluster().getInstance(Environment.class, nodeName).dataFile());
+        return fileSystemProvider.getTestFileStore(internalCluster().getInstance(Environment.class, nodeName).dataFiles()[0]);
     }
 
     protected static class TestFileStore extends FilterFileStore {