Removed duplicated 'nodeId' in cache path #7059 (#7065)

* Removed duplicated 'nodeId' in cache path #7059 Signed-off-by: Andrew Ross <[email protected]> * Update tests and restore logic for new path Signed-off-by: Andrew Ross <[email protected]> * Fix spotless errors Signed-off-by: Andrew Ross <[email protected]> * Fix additional tests Signed-off-by: Andrew Ross <[email protected]> * Fix integration test Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Jayesh Suthar <[email protected]> Co-authored-by: Andrew Ross <[email protected]>
opensearch-project · Apr 11, 2023 · ed6141f · ed6141f
1 parent 578c6a2
commit ed6141f
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 34 deletions.
diff --git a/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java b/server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java
@@ -36,7 +36,6 @@
 import org.opensearch.repositories.fs.FsRepository;
 
 import java.io.IOException;
-import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
@@ -595,12 +594,8 @@ private void assertCacheDirectoryReplicaAndIndexCount(int numCacheFolderCount, i
         for (Path fileCachePath : searchNodeFileCachePaths) {
             assertTrue(Files.exists(fileCachePath));
             assertTrue(Files.isDirectory(fileCachePath));
-            try (DirectoryStream<Path> cachePathStream = Files.newDirectoryStream(fileCachePath)) {
-                Path nodeLockIdPath = cachePathStream.iterator().next();
-                assertTrue(Files.isDirectory(nodeLockIdPath));
-                try (Stream<Path> dataPathStream = Files.list(nodeLockIdPath)) {
-                    assertEquals(numIndexCount, dataPathStream.count());
-                }
+            try (Stream<Path> dataPathStream = Files.list(fileCachePath)) {
+                assertEquals(numIndexCount, dataPathStream.count());
             }
         }
         // Verifies if all the shards (primary and replica) have been deleted

diff --git a/server/src/main/java/org/opensearch/env/NodeEnvironment.java b/server/src/main/java/org/opensearch/env/NodeEnvironment.java
@@ -1247,22 +1247,15 @@ private static boolean isIndexMetadataPath(Path path) {
      * The returned paths will point to the shard data folder.
      */
     public static List<Path> collectFileCacheDataPath(NodePath fileCacheNodePath) throws IOException {
+        // Structure is: <file cache path>/<index uuid>/<shard id>/...
         List<Path> indexSubPaths = new ArrayList<>();
         Path fileCachePath = fileCacheNodePath.fileCachePath;
         if (Files.isDirectory(fileCachePath)) {
-            try (DirectoryStream<Path> nodeStream = Files.newDirectoryStream(fileCachePath)) {
-                for (Path nodePath : nodeStream) {
-                    if (Files.isDirectory(nodePath)) {
-                        try (DirectoryStream<Path> indexStream = Files.newDirectoryStream(nodePath)) {
-                            for (Path indexPath : indexStream) {
-                                if (Files.isDirectory(indexPath)) {
-                                    try (Stream<Path> shardStream = Files.list(indexPath)) {
-                                        shardStream.filter(NodeEnvironment::isShardPath)
-                                            .map(Path::toAbsolutePath)
-                                            .forEach(indexSubPaths::add);
-                                    }
-                                }
-                            }
+            try (DirectoryStream<Path> indexStream = Files.newDirectoryStream(fileCachePath)) {
+                for (Path indexPath : indexStream) {
+                    if (Files.isDirectory(indexPath)) {
+                        try (Stream<Path> shardStream = Files.list(indexPath)) {
+                            shardStream.filter(NodeEnvironment::isShardPath).map(Path::toAbsolutePath).forEach(indexSubPaths::add);
                         }
                     }
                 }
@@ -1307,9 +1300,7 @@ private static Path resolveIndexCustomLocation(String customDataPath, String ind
      * @param shardId shard to resolve the path to
      */
     public Path resolveFileCacheLocation(final Path fileCachePath, final ShardId shardId) {
-        return fileCachePath.resolve(Integer.toString(nodeLockId))
-            .resolve(shardId.getIndex().getUUID())
-            .resolve(Integer.toString(shardId.id()));
+        return fileCachePath.resolve(shardId.getIndex().getUUID()).resolve(Integer.toString(shardId.id()));
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheCleaner.java b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheCleaner.java
@@ -91,9 +91,7 @@ public void afterIndexRemoved(
     ) {
         if (isRemoteSnapshot(indexSettings.getSettings())
             && reason == IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.DELETED) {
-            final Path indexCachePath = nodeEnvironment.fileCacheNodePath().fileCachePath.resolve(
-                Integer.toString(nodeEnvironment.getNodeLockId())
-            ).resolve(index.getUUID());
+            final Path indexCachePath = nodeEnvironment.fileCacheNodePath().fileCachePath.resolve(index.getUUID());
             if (Files.exists(indexCachePath)) {
                 try {
                     IOUtils.rm(indexCachePath);

diff --git a/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java b/server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java
@@ -391,7 +391,7 @@ private void createIndexDataFiles(Settings settings, int shardCount, boolean wri
 
             if (cacheMode) {
                 Path cachePath = env.fileCacheNodePath().fileCachePath;
-                cachePath = cachePath.resolve(String.valueOf(env.getNodeLockId())).resolve(INDEX.getUUID());
+                cachePath = cachePath.resolve(INDEX.getUUID());
                 for (int i = 0; i < shardCount; ++i) {
                     Files.createDirectories(cachePath.resolve(Integer.toString(shardDataDirNumber)));
                     shardDataDirNumber += randomIntBetween(1, 10);

diff --git a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java
@@ -92,8 +92,7 @@ public void testShardRemoved() {
     }
 
     public void testIndexRemoved() {
-        final Path indexCachePath = env.fileCacheNodePath().fileCachePath.resolve(Integer.toString(env.getNodeLockId()))
-            .resolve(SHARD_0.getIndex().getUUID());
+        final Path indexCachePath = env.fileCacheNodePath().fileCachePath.resolve(SHARD_0.getIndex().getUUID());
         assertTrue(Files.exists(indexCachePath));
 
         cleaner.beforeIndexShardDeleted(SHARD_0, SETTINGS);

diff --git a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java
@@ -54,9 +54,8 @@ private Path createPath(String middle) {
     }
 
     @SuppressForbidden(reason = "creating a test file for cache")
-    private void createFile(String nodeId, String indexName, String shardId, String fileName) throws IOException {
+    private void createFile(String indexName, String shardId, String fileName) throws IOException {
         Path folderPath = path.resolve(NodeEnvironment.CACHE_FOLDER)
-            .resolve(nodeId)
             .resolve(indexName)
             .resolve(shardId)
             .resolve(RemoteSnapshotDirectoryFactory.LOCAL_STORE_LOCATION);
@@ -266,13 +265,12 @@ public void testStats() {
     }
 
     public void testCacheRestore() throws IOException {
-        String nodeId = "0";
         String indexName = "test-index";
         String shardId = "0";
-        createFile(nodeId, indexName, shardId, "test.0");
+        createFile(indexName, shardId, "test.0");
         FileCache fileCache = createFileCache(GIGA_BYTES);
         assertEquals(0, fileCache.usage().usage());
-        Path fileCachePath = path.resolve(NodeEnvironment.CACHE_FOLDER).resolve(nodeId).resolve(indexName).resolve(shardId);
+        Path fileCachePath = path.resolve(NodeEnvironment.CACHE_FOLDER).resolve(indexName).resolve(shardId);
         fileCache.restoreFromDirectory(List.of(fileCachePath));
         assertTrue(fileCache.usage().usage() > 0);
     }