Skip to content

Commit ed6141f

Browse files
govegitoJayesh Sutharandrross
authored
Removed duplicated 'nodeId' in cache path #7059 (#7065)
* Removed duplicated 'nodeId' in cache path #7059 Signed-off-by: Andrew Ross <andrross@amazon.com> * Update tests and restore logic for new path Signed-off-by: Andrew Ross <andrross@amazon.com> * Fix spotless errors Signed-off-by: Andrew Ross <andrross@amazon.com> * Fix additional tests Signed-off-by: Andrew Ross <andrross@amazon.com> * Fix integration test Signed-off-by: Andrew Ross <andrross@amazon.com> --------- Signed-off-by: Andrew Ross <andrross@amazon.com> Co-authored-by: Jayesh Suthar <jayeshsuthar@Jayeshs-MacBook-Pro.local> Co-authored-by: Andrew Ross <andrross@amazon.com>
1 parent 578c6a2 commit ed6141f

File tree

6 files changed

+15
-34
lines changed

6 files changed

+15
-34
lines changed

server/src/internalClusterTest/java/org/opensearch/snapshots/SearchableSnapshotIT.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.opensearch.repositories.fs.FsRepository;
3737

3838
import java.io.IOException;
39-
import java.nio.file.DirectoryStream;
4039
import java.nio.file.Files;
4140
import java.nio.file.Path;
4241
import java.util.List;
@@ -595,12 +594,8 @@ private void assertCacheDirectoryReplicaAndIndexCount(int numCacheFolderCount, i
595594
for (Path fileCachePath : searchNodeFileCachePaths) {
596595
assertTrue(Files.exists(fileCachePath));
597596
assertTrue(Files.isDirectory(fileCachePath));
598-
try (DirectoryStream<Path> cachePathStream = Files.newDirectoryStream(fileCachePath)) {
599-
Path nodeLockIdPath = cachePathStream.iterator().next();
600-
assertTrue(Files.isDirectory(nodeLockIdPath));
601-
try (Stream<Path> dataPathStream = Files.list(nodeLockIdPath)) {
602-
assertEquals(numIndexCount, dataPathStream.count());
603-
}
597+
try (Stream<Path> dataPathStream = Files.list(fileCachePath)) {
598+
assertEquals(numIndexCount, dataPathStream.count());
604599
}
605600
}
606601
// Verifies if all the shards (primary and replica) have been deleted

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

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,22 +1247,15 @@ private static boolean isIndexMetadataPath(Path path) {
12471247
* The returned paths will point to the shard data folder.
12481248
*/
12491249
public static List<Path> collectFileCacheDataPath(NodePath fileCacheNodePath) throws IOException {
1250+
// Structure is: <file cache path>/<index uuid>/<shard id>/...
12501251
List<Path> indexSubPaths = new ArrayList<>();
12511252
Path fileCachePath = fileCacheNodePath.fileCachePath;
12521253
if (Files.isDirectory(fileCachePath)) {
1253-
try (DirectoryStream<Path> nodeStream = Files.newDirectoryStream(fileCachePath)) {
1254-
for (Path nodePath : nodeStream) {
1255-
if (Files.isDirectory(nodePath)) {
1256-
try (DirectoryStream<Path> indexStream = Files.newDirectoryStream(nodePath)) {
1257-
for (Path indexPath : indexStream) {
1258-
if (Files.isDirectory(indexPath)) {
1259-
try (Stream<Path> shardStream = Files.list(indexPath)) {
1260-
shardStream.filter(NodeEnvironment::isShardPath)
1261-
.map(Path::toAbsolutePath)
1262-
.forEach(indexSubPaths::add);
1263-
}
1264-
}
1265-
}
1254+
try (DirectoryStream<Path> indexStream = Files.newDirectoryStream(fileCachePath)) {
1255+
for (Path indexPath : indexStream) {
1256+
if (Files.isDirectory(indexPath)) {
1257+
try (Stream<Path> shardStream = Files.list(indexPath)) {
1258+
shardStream.filter(NodeEnvironment::isShardPath).map(Path::toAbsolutePath).forEach(indexSubPaths::add);
12661259
}
12671260
}
12681261
}
@@ -1307,9 +1300,7 @@ private static Path resolveIndexCustomLocation(String customDataPath, String ind
13071300
* @param shardId shard to resolve the path to
13081301
*/
13091302
public Path resolveFileCacheLocation(final Path fileCachePath, final ShardId shardId) {
1310-
return fileCachePath.resolve(Integer.toString(nodeLockId))
1311-
.resolve(shardId.getIndex().getUUID())
1312-
.resolve(Integer.toString(shardId.id()));
1303+
return fileCachePath.resolve(shardId.getIndex().getUUID()).resolve(Integer.toString(shardId.id()));
13131304
}
13141305

13151306
/**

server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheCleaner.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,7 @@ public void afterIndexRemoved(
9191
) {
9292
if (isRemoteSnapshot(indexSettings.getSettings())
9393
&& reason == IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.DELETED) {
94-
final Path indexCachePath = nodeEnvironment.fileCacheNodePath().fileCachePath.resolve(
95-
Integer.toString(nodeEnvironment.getNodeLockId())
96-
).resolve(index.getUUID());
94+
final Path indexCachePath = nodeEnvironment.fileCacheNodePath().fileCachePath.resolve(index.getUUID());
9795
if (Files.exists(indexCachePath)) {
9896
try {
9997
IOUtils.rm(indexCachePath);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ private void createIndexDataFiles(Settings settings, int shardCount, boolean wri
391391

392392
if (cacheMode) {
393393
Path cachePath = env.fileCacheNodePath().fileCachePath;
394-
cachePath = cachePath.resolve(String.valueOf(env.getNodeLockId())).resolve(INDEX.getUUID());
394+
cachePath = cachePath.resolve(INDEX.getUUID());
395395
for (int i = 0; i < shardCount; ++i) {
396396
Files.createDirectories(cachePath.resolve(Integer.toString(shardDataDirNumber)));
397397
shardDataDirNumber += randomIntBetween(1, 10);

server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ public void testShardRemoved() {
9292
}
9393

9494
public void testIndexRemoved() {
95-
final Path indexCachePath = env.fileCacheNodePath().fileCachePath.resolve(Integer.toString(env.getNodeLockId()))
96-
.resolve(SHARD_0.getIndex().getUUID());
95+
final Path indexCachePath = env.fileCacheNodePath().fileCachePath.resolve(SHARD_0.getIndex().getUUID());
9796
assertTrue(Files.exists(indexCachePath));
9897

9998
cleaner.beforeIndexShardDeleted(SHARD_0, SETTINGS);

server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,8 @@ private Path createPath(String middle) {
5454
}
5555

5656
@SuppressForbidden(reason = "creating a test file for cache")
57-
private void createFile(String nodeId, String indexName, String shardId, String fileName) throws IOException {
57+
private void createFile(String indexName, String shardId, String fileName) throws IOException {
5858
Path folderPath = path.resolve(NodeEnvironment.CACHE_FOLDER)
59-
.resolve(nodeId)
6059
.resolve(indexName)
6160
.resolve(shardId)
6261
.resolve(RemoteSnapshotDirectoryFactory.LOCAL_STORE_LOCATION);
@@ -266,13 +265,12 @@ public void testStats() {
266265
}
267266

268267
public void testCacheRestore() throws IOException {
269-
String nodeId = "0";
270268
String indexName = "test-index";
271269
String shardId = "0";
272-
createFile(nodeId, indexName, shardId, "test.0");
270+
createFile(indexName, shardId, "test.0");
273271
FileCache fileCache = createFileCache(GIGA_BYTES);
274272
assertEquals(0, fileCache.usage().usage());
275-
Path fileCachePath = path.resolve(NodeEnvironment.CACHE_FOLDER).resolve(nodeId).resolve(indexName).resolve(shardId);
273+
Path fileCachePath = path.resolve(NodeEnvironment.CACHE_FOLDER).resolve(indexName).resolve(shardId);
276274
fileCache.restoreFromDirectory(List.of(fileCachePath));
277275
assertTrue(fileCache.usage().usage() > 0);
278276
}

0 commit comments

Comments
 (0)