Make NodeEnvironment.indexPaths singular (elastic#72438)

This commit renames the indexPaths method to be singular and return a
single Path instead of an array. This is done in isolation from other
NodeEnvironment methods to make it reviewable.

relates elastic#71205

server/src/internalClusterTest/java/org/elasticsearch/gateway/MetadataNodesIT.java

@@ -24,7 +24,6 @@
 import org.elasticsearch.test.InternalTestCluster.RestartCallback;
 
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.util.List;
 import java.util.Map;
 
@@ -181,12 +180,7 @@ protected void assertIndexInMetaState(final String nodeName, final String indexN
 
     private boolean indexDirectoryExists(String nodeName, Index index) {
         NodeEnvironment nodeEnv = ((InternalTestCluster) cluster()).getInstance(NodeEnvironment.class, nodeName);
-        for (Path path : nodeEnv.indexPaths(index)) {
-            if (Files.exists(path)) {
-                return true;
-            }
-        }
-        return false;
+        return Files.exists(nodeEnv.indexPath(index));
     }
 
     private ImmutableOpenMap<String, IndexMetadata> getIndicesMetadataOnNode(String nodeName) {

server/src/internalClusterTest/java/org/elasticsearch/indices/store/IndicesStoreIntegrationIT.java

@@ -452,9 +452,7 @@ public void onFailure(String source, Exception e) {
 
     private Path indexDirectory(String server, Index index) {
         NodeEnvironment env = internalCluster().getInstance(NodeEnvironment.class, server);
-        final Path[] paths = env.indexPaths(index);
-        assert paths.length == 1;
-        return paths[0];
+        return env.indexPath(index);
     }
 
     private Path shardDirectory(String server, Index index, int shard) {

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

@@ -688,10 +688,10 @@ public void deleteIndexDirectorySafe(
      * @param indexSettings settings for the index being deleted
      */
     public void deleteIndexDirectoryUnderLock(Index index, IndexSettings indexSettings, Consumer<Path[]> listener) throws IOException {
-        final Path[] indexPaths = indexPaths(index);
-        logger.trace("deleting index {} directory, paths({}): [{}]", index, indexPaths.length, indexPaths);
-        listener.accept(indexPaths);
-        IOUtils.rm(indexPaths);
+        final Path indexPath = indexPath(index);
+        logger.trace("deleting index {} directory: [{}]", index, indexPath);
+        listener.accept(new Path[] { indexPath });
+        IOUtils.rm(indexPath);
         if (indexSettings.hasCustomDataPath()) {
             Path customLocation = resolveIndexCustomLocation(indexSettings.customDataPath(), index.getUUID());
             logger.trace("deleting custom index {} directory [{}]", index, customLocation);
@@ -935,13 +935,9 @@ public NodePath[] nodePaths() {
     /**
      * Returns all index paths.
      */
-    public Path[] indexPaths(Index index) {
+    public Path indexPath(Index index) {
         assertEnvIsLocked();
-        Path[] indexPaths = new Path[nodePaths.length];
-        for (int i = 0; i < nodePaths.length; i++) {
-            indexPaths[i] = nodePaths[i].resolve(index);
-        }
-        return indexPaths;
+        return nodePaths[0].resolve(index);
     }
 
 

server/src/main/java/org/elasticsearch/gateway/MetaStateService.java

@@ -142,7 +142,7 @@ private Tuple<Manifest, Metadata> loadFullStateBWC() throws IOException {
      */
     @Nullable
     public IndexMetadata loadIndexState(Index index) throws IOException {
-        return INDEX_METADATA_FORMAT.loadLatestState(logger, namedXContentRegistry, nodeEnv.indexPaths(index));
+        return INDEX_METADATA_FORMAT.loadLatestState(logger, namedXContentRegistry, nodeEnv.indexPath(index));
     }
 
     /**
@@ -205,7 +205,7 @@ public long writeIndex(String reason, IndexMetadata indexMetadata) throws WriteS
         logger.trace("[{}] writing state, reason [{}]", index, reason);
         try {
             long generation = INDEX_METADATA_FORMAT.write(indexMetadata,
-                    nodeEnv.indexPaths(indexMetadata.getIndex()));
+                    nodeEnv.indexPath(indexMetadata.getIndex()));
             logger.trace("[{}] state written", index);
             return generation;
         } catch (WriteStateException ex) {
@@ -246,7 +246,7 @@ void cleanupGlobalState(long currentGeneration) {
      * @param currentGeneration current state generation to keep in the index directory.
      */
     public void cleanupIndex(Index index, long currentGeneration) {
-        INDEX_METADATA_FORMAT.cleanupOldFiles(currentGeneration, nodeEnv.indexPaths(index));
+        INDEX_METADATA_FORMAT.cleanupOldFiles(currentGeneration, nodeEnv.indexPath(index));
     }
 
     /**

server/src/main/java/org/elasticsearch/gateway/MetadataStateFormat.java

@@ -309,7 +309,7 @@ protected Directory newDirectory(Path dir) throws IOException {
      * @param currentGeneration state generation to keep.
      * @param locations         state paths.
      */
-    public void cleanupOldFiles(final long currentGeneration, Path[] locations) {
+    public void cleanupOldFiles(final long currentGeneration, Path... locations) {
         final String fileNameToKeep = getStateFileName(currentGeneration);
         for (Path location : locations) {
             logger.trace("cleanupOldFiles: cleaning up {}", location);

server/src/main/java/org/elasticsearch/index/IndexService.java

@@ -346,7 +346,7 @@ public synchronized void writeDanglingIndicesInfo() {
             return;
         }
         try {
-            IndexMetadata.FORMAT.writeAndCleanup(getMetadata(), nodeEnv.indexPaths(index()));
+            IndexMetadata.FORMAT.writeAndCleanup(getMetadata(), nodeEnv.indexPath(index()));
         } catch (WriteStateException e) {
             logger.warn(() -> new ParameterizedMessage("failed to write dangling indices state for index {}", index()), e);
         }
@@ -358,7 +358,7 @@ public synchronized void deleteDanglingIndicesInfo() {
             return;
         }
         try {
-            MetadataStateFormat.deleteMetaState(nodeEnv.indexPaths(index()));
+            MetadataStateFormat.deleteMetaState(nodeEnv.indexPath(index()));
         } catch (IOException e) {
             logger.warn(() -> new ParameterizedMessage("failed to delete dangling indices state for index {}", index()), e);
         }

server/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -935,7 +935,7 @@ private void deleteIndexStoreIfDeletionAllowed(final String reason, final Index
             }
             // this is a pure protection to make sure this index doesn't get re-imported as a dangling index.
             // we should in the future rather write a tombstone rather than wiping the metadata.
-            MetadataStateFormat.deleteMetaState(nodeEnv.indexPaths(index));
+            MetadataStateFormat.deleteMetaState(nodeEnv.indexPath(index));
         }
     }
 
@@ -1028,7 +1028,7 @@ public IndexMetadata verifyIndexIsDeleted(final Index index, final ClusterState
         if (clusterState.metadata().index(index) != null) {
             throw new IllegalStateException("Cannot delete index [" + index + "], it is still part of the cluster state.");
         }
-        if (nodeEnv.hasNodeFile() && FileSystemUtils.exists(nodeEnv.indexPaths(index))) {
+        if (nodeEnv.hasNodeFile() && Files.exists(nodeEnv.indexPath(index))) {
             final IndexMetadata metadata;
             try {
                 metadata = metaStateService.loadIndexState(index);

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

@@ -102,10 +102,9 @@ public void testShardLock() throws Exception {
         } catch (ShardLockObtainFailedException ex) {
             // expected
         }
-        for (Path path : env.indexPaths(index)) {
-            Files.createDirectories(path.resolve("0"));
-            Files.createDirectories(path.resolve("1"));
-        }
+        Path path = env.indexPath(index);
+        Files.createDirectories(path.resolve("0"));
+        Files.createDirectories(path.resolve("1"));
         try {
             env.lockAllForIndex(index, idxSettings, "3", randomIntBetween(0, 10));
             fail("shard 0 is locked");
@@ -135,10 +134,9 @@ public void testAvailableIndexFolders() throws Exception {
         Set<String> actualPaths = new HashSet<>();
         for (int i = 0; i < numIndices; i++) {
             Index index = new Index("foo" + i, "fooUUID" + i);
-            for (Path path : env.indexPaths(index)) {
-                Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
-                actualPaths.add(path.getFileName().toString());
-            }
+            Path path = env.indexPath(index);
+            Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
+            actualPaths.add(path.getFileName().toString());
         }
 
         assertThat(actualPaths, equalTo(env.availableIndexFolders()));
@@ -153,12 +151,11 @@ public void testAvailableIndexFoldersWithExclusions() throws Exception {
         Set<String> actualPaths = new HashSet<>();
         for (int i = 0; i < numIndices; i++) {
             Index index = new Index("foo" + i, "fooUUID" + i);
-            for (Path path : env.indexPaths(index)) {
-                Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
-                actualPaths.add(path.getFileName().toString());
-            }
+            Path path = env.indexPath(index);
+            Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
+            actualPaths.add(path.getFileName().toString());
             if (randomBoolean()) {
-                excludedPaths.add(env.indexPaths(index)[0].getFileName().toString());
+                excludedPaths.add(env.indexPath(index).getFileName().toString());
             }
         }
 
@@ -173,17 +170,15 @@ public void testResolveIndexFolders() throws Exception {
         Map<String, List<Path>> actualIndexDataPaths = new HashMap<>();
         for (int i = 0; i < numIndices; i++) {
             Index index = new Index("foo" + i, "fooUUID" + i);
-            Path[] indexPaths = env.indexPaths(index);
-            for (Path path : indexPaths) {
-                Files.createDirectories(path);
-                String fileName = path.getFileName().toString();
-                List<Path> paths = actualIndexDataPaths.get(fileName);
-                if (paths == null) {
-                    paths = new ArrayList<>();
-                }
-                paths.add(path);
-                actualIndexDataPaths.put(fileName, paths);
+            Path path = env.indexPath(index);
+            Files.createDirectories(path);
+            String fileName = path.getFileName().toString();
+            List<Path> paths = actualIndexDataPaths.get(fileName);
+            if (paths == null) {
+                paths = new ArrayList<>();
             }
+            paths.add(path);
+            actualIndexDataPaths.put(fileName, paths);
         }
         for (Map.Entry<String, List<Path>> actualIndexDataPathEntry : actualIndexDataPaths.entrySet()) {
             List<Path> actual = actualIndexDataPathEntry.getValue();
@@ -200,20 +195,18 @@ public void testDeleteSafe() throws Exception {
         final ShardLock fooLock = env.shardLock(new ShardId(index, 0), "1");
         assertEquals(new ShardId(index, 0), fooLock.getShardId());
 
-        for (Path path : env.indexPaths(index)) {
-            Files.createDirectories(path.resolve("0"));
-            Files.createDirectories(path.resolve("1"));
-        }
+        Path path = env.indexPath(index);
+        Files.createDirectories(path.resolve("0"));
+        Files.createDirectories(path.resolve("1"));
 
         expectThrows(ShardLockObtainFailedException.class,
             () -> env.deleteShardDirectorySafe(new ShardId(index, 0), idxSettings, shardPaths -> {
                 assert false : "should not be called " + shardPaths;
             }));
 
-        for (Path path : env.indexPaths(index)) {
-            assertTrue(Files.exists(path.resolve("0")));
-            assertTrue(Files.exists(path.resolve("1")));
-        }
+        path = env.indexPath(index);
+        assertTrue(Files.exists(path.resolve("0")));
+        assertTrue(Files.exists(path.resolve("1")));
 
         {
             SetOnce<Path[]> listener = new SetOnce<>();
@@ -224,10 +217,9 @@ public void testDeleteSafe() throws Exception {
             }
         }
 
-        for (Path path : env.indexPaths(index)) {
-            assertTrue(Files.exists(path.resolve("0")));
-            assertFalse(Files.exists(path.resolve("1")));
-        }
+        path = env.indexPath(index);
+        assertTrue(Files.exists(path.resolve("0")));
+        assertFalse(Files.exists(path.resolve("1")));
 
         expectThrows(ShardLockObtainFailedException.class,
             () -> env.deleteIndexDirectorySafe(index, randomIntBetween(0, 10), idxSettings, indexPaths -> {
@@ -236,9 +228,8 @@ public void testDeleteSafe() throws Exception {
 
         fooLock.close();
 
-        for (Path path : env.indexPaths(index)) {
-            assertTrue(Files.exists(path));
-        }
+        path = env.indexPath(index);
+        assertTrue(Files.exists(path));
 
         final AtomicReference<Throwable> threadException = new AtomicReference<>();
         final CountDownLatch latch = new CountDownLatch(1);
@@ -274,12 +265,11 @@ protected void doRun() throws Exception {
 
         final SetOnce<Path[]> listener = new SetOnce<>();
         env.deleteIndexDirectorySafe(index, 5000, idxSettings, listener::set);
-        assertArrayEquals(env.indexPaths(index), listener.get());
+        assertThat(listener.get()[0], equalTo(env.indexPath(index)));
         assertNull(threadException.get());
 
-        for (Path path : env.indexPaths(index)) {
-            assertFalse(Files.exists(path));
-        }
+        path = env.indexPath(index);
+        assertFalse(Files.exists(path));
         latch.await();
         assertTrue("LockedShards: " + env.lockedShards(), env.lockedShards().isEmpty());
         env.close();
@@ -361,7 +351,7 @@ public void testCustomDataPaths() throws Exception {
             equalTo(dataPath.resolve("indices/" + index.getUUID() + "/0")));
 
         assertThat("index paths uses the regular template",
-            env.indexPaths(index)[0], equalTo(dataPath.resolve("indices/" + index.getUUID())));
+            env.indexPath(index), equalTo(dataPath.resolve("indices/" + index.getUUID())));
 
         assertThat(env.availableShardPaths(sid), equalTo(env.availableShardPaths(sid)));
         assertThat(env.resolveCustomLocation("/tmp/foo", sid).toAbsolutePath(),
@@ -372,7 +362,7 @@ public void testCustomDataPaths() throws Exception {
             equalTo(dataPath.resolve("indices/" + index.getUUID() + "/0")));
 
         assertThat("index paths uses the regular template",
-            env.indexPaths(index)[0], equalTo(dataPath.resolve("indices/" + index.getUUID())));
+            env.indexPath(index), equalTo(dataPath.resolve("indices/" + index.getUUID())));
 
         env.close();
     }
@@ -415,10 +405,8 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException {
 
         Path indexPath;
         try (NodeEnvironment env = newNodeEnvironment(settings)) {
-            for (Path path : env.indexPaths(index)) {
-                Files.createDirectories(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
-            }
-            indexPath = env.indexPaths(index)[0];
+            indexPath = env.indexPath(index);
+            Files.createDirectories(indexPath.resolve(MetadataStateFormat.STATE_DIR_NAME));
         }
 
         verifyFailsOnMetadata(noDataNoMasterSettings, indexPath);
@@ -430,9 +418,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException {
 
         // test that we can create data=false env with only meta information. Also create shard data for following asserts
         try (NodeEnvironment env = newNodeEnvironment(noDataSettings)) {
-            for (Path path : env.indexPaths(index)) {
-                Files.createDirectories(path.resolve(shardDataDirName));
-            }
+            Files.createDirectories(env.indexPath(index).resolve(shardDataDirName));
         }
 
         verifyFailsOnShardData(noDataSettings, indexPath, shardDataDirName);
@@ -448,9 +434,7 @@ public void testEnsureNoShardDataOrIndexMetadata() throws IOException {
 
         // test that we can create data=true, master=true env. Also remove state dir to leave only shard data for following asserts
         try (NodeEnvironment env = newNodeEnvironment(settings)) {
-            for (Path path : env.indexPaths(index)) {
-                Files.delete(path.resolve(MetadataStateFormat.STATE_DIR_NAME));
-            }
+            Files.delete(env.indexPath(index).resolve(MetadataStateFormat.STATE_DIR_NAME));
         }
 
         // assert that we fail on shard data even without the metadata dir.

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

@@ -227,11 +227,10 @@ private void createIndexDataFiles(Settings settings, int shardCount, boolean wri
                         .build());
                 }
             }
-            for (Path path : env.indexPaths(INDEX)) {
-                for (int i = 0; i < shardCount; ++i) {
-                    Files.createDirectories(path.resolve(Integer.toString(shardDataDirNumber)));
-                    shardDataDirNumber += randomIntBetween(1,10);
-                }
+            Path path = env.indexPath(INDEX);
+            for (int i = 0; i < shardCount; ++i) {
+                Files.createDirectories(path.resolve(Integer.toString(shardDataDirNumber)));
+                shardDataDirNumber += randomIntBetween(1,10);
             }
         }
     }

server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java

@@ -21,7 +21,6 @@
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.UUIDs;
-import org.elasticsearch.common.io.FileSystemUtils;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
@@ -57,6 +56,7 @@
 import org.elasticsearch.test.hamcrest.RegexMatcher;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -336,7 +336,7 @@ public void testVerifyIfIndexContentDeleted() throws Exception {
                                                           .metadata(Metadata.builder(csWithIndex.metadata()).remove(index.getName()))
                                                           .build();
         indicesService.verifyIndexIsDeleted(index, withoutIndex);
-        assertFalse("index files should be deleted", FileSystemUtils.exists(nodeEnv.indexPaths(index)));
+        assertFalse("index files should be deleted", Files.exists(nodeEnv.indexPath(index)));
     }
 
     public void testDanglingIndicesWithAliasConflict() throws Exception {