From e5a3d2b53348eecd16065c86661a2fb553277909 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Tue, 23 Jan 2024 18:07:48 +0530 Subject: [PATCH 1/5] Fetch all the locks for a shard to avoid multiple calls Signed-off-by: Sachin Kale --- .../store/RemoteSegmentStoreDirectory.java | 24 ++++++++----------- .../lockmanager/RemoteStoreLockManager.java | 3 +++ .../RemoteStoreMetadataLockManager.java | 4 ++++ 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 9c1e902606cab..63a135839cfb5 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -749,20 +749,16 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException lastNMetadataFilesToKeep, sortedMetadataFileList.size() ); - List metadataFilesToBeDeleted = metadataFilesEligibleToDelete.stream().filter(metadataFile -> { - try { - return !isLockAcquired(metadataFile); - } catch (IOException e) { - logger.error( - "skipping metadata file (" - + metadataFile - + ") deletion for this run," - + " as checking lock for metadata is failing with error: " - + e - ); - return false; - } - }).collect(Collectors.toList()); + Set allLockFiles; + try { + allLockFiles = new HashSet<>(mdLockManager.listAllLocks(MetadataFilenameUtils.METADATA_PREFIX)); + } catch (Exception e) { + logger.error("Exception while fetching segment metadata lock files, skipping deleteStaleSegments", e); + return; + } + List metadataFilesToBeDeleted = metadataFilesEligibleToDelete.stream() + .filter(metadataFile -> allLockFiles.contains(metadataFile) == false) + .collect(Collectors.toList()); sortedMetadataFileList.removeAll(metadataFilesToBeDeleted); logger.debug( diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java index 4fa23dfe9dc3a..f318036ef63e6 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java @@ -11,6 +11,7 @@ import org.opensearch.common.annotation.PublicApi; import java.io.IOException; +import java.util.Collection; /** * An Interface that defines Remote Store Lock Manager. @@ -43,6 +44,8 @@ public interface RemoteStoreLockManager { */ Boolean isAcquired(LockInfo lockInfo) throws IOException; + public Collection listAllLocks(String prefix) throws IOException; + /** * Acquires lock on the file mentioned in originalLockInfo for acquirer mentioned in clonedLockInfo. * There can occur a race condition where the original file is deleted before we can use it to acquire lock for the new acquirer. Until we have a diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java index 5ebd00f59ef49..c53145448a78e 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java @@ -101,6 +101,10 @@ public Boolean isAcquired(LockInfo lockInfo) throws IOException { return !lockFiles.isEmpty(); } + public Collection listAllLocks(String prefix) throws IOException { + return lockDirectory.listFilesByPrefix(prefix); + } + /** * Acquires lock on the file mentioned in originalLockInfo for acquirer mentioned in clonedLockInfo. * Snapshot layer enforces thread safety by having checks in place to ensure that the source snapshot is not being deleted before proceeding From 68bd6344038d1f1cb02eb3de68b918e9d1cf25d8 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Wed, 24 Jan 2024 11:01:03 +0530 Subject: [PATCH 2/5] Fix lock file comparison issue Signed-off-by: Sachin Kale --- .../index/store/RemoteSegmentStoreDirectory.java | 2 +- .../store/lockmanager/RemoteStoreLockManager.java | 2 -- .../lockmanager/RemoteStoreMetadataLockManager.java | 11 +++++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 63a135839cfb5..7e0bd7cfe0008 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -751,7 +751,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException ); Set allLockFiles; try { - allLockFiles = new HashSet<>(mdLockManager.listAllLocks(MetadataFilenameUtils.METADATA_PREFIX)); + allLockFiles = new HashSet<>(((RemoteStoreMetadataLockManager) mdLockManager).fetchLocks(MetadataFilenameUtils.METADATA_PREFIX)); } catch (Exception e) { logger.error("Exception while fetching segment metadata lock files, skipping deleteStaleSegments", e); return; diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java index f318036ef63e6..375ae50112e2d 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java @@ -44,8 +44,6 @@ public interface RemoteStoreLockManager { */ Boolean isAcquired(LockInfo lockInfo) throws IOException; - public Collection listAllLocks(String prefix) throws IOException; - /** * Acquires lock on the file mentioned in originalLockInfo for acquirer mentioned in clonedLockInfo. * There can occur a race condition where the original file is deleted before we can use it to acquire lock for the new acquirer. Until we have a diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java index c53145448a78e..b7398d347e839 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java @@ -88,6 +88,13 @@ public String fetchLock(String filenamePrefix, String acquirerId) throws IOExcep return lockFilesForAcquirer.get(0); } + public Collection fetchLocks(String filenamePrefix) throws IOException { + Collection lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix); + return lockFiles.stream() + .map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock) + .collect(Collectors.toList()); + } + /** * Checks whether a given file have any lock on it or not. * @param lockInfo File Lock Info instance for which we need to check if lock is acquired. @@ -101,10 +108,6 @@ public Boolean isAcquired(LockInfo lockInfo) throws IOException { return !lockFiles.isEmpty(); } - public Collection listAllLocks(String prefix) throws IOException { - return lockDirectory.listFilesByPrefix(prefix); - } - /** * Acquires lock on the file mentioned in originalLockInfo for acquirer mentioned in clonedLockInfo. * Snapshot layer enforces thread safety by having checks in place to ensure that the source snapshot is not being deleted before proceeding From 9837f28268560450b3408dde65500e305fec43ed Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Wed, 24 Jan 2024 13:36:52 +0530 Subject: [PATCH 3/5] Add unit tests Signed-off-by: Sachin Kale --- .../store/RemoteSegmentStoreDirectory.java | 4 ++- .../lockmanager/RemoteStoreLockManager.java | 1 - .../RemoteStoreMetadataLockManager.java | 4 +-- .../RemoteSegmentStoreDirectoryTests.java | 34 +++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 7e0bd7cfe0008..760cb29284e9c 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -751,7 +751,9 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException ); Set allLockFiles; try { - allLockFiles = new HashSet<>(((RemoteStoreMetadataLockManager) mdLockManager).fetchLocks(MetadataFilenameUtils.METADATA_PREFIX)); + allLockFiles = new HashSet<>( + ((RemoteStoreMetadataLockManager) mdLockManager).fetchLocks(MetadataFilenameUtils.METADATA_PREFIX) + ); } catch (Exception e) { logger.error("Exception while fetching segment metadata lock files, skipping deleteStaleSegments", e); return; diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java index 375ae50112e2d..4fa23dfe9dc3a 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManager.java @@ -11,7 +11,6 @@ import org.opensearch.common.annotation.PublicApi; import java.io.IOException; -import java.util.Collection; /** * An Interface that defines Remote Store Lock Manager. diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java index b7398d347e839..d20f54844d8a7 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java @@ -90,9 +90,7 @@ public String fetchLock(String filenamePrefix, String acquirerId) throws IOExcep public Collection fetchLocks(String filenamePrefix) throws IOException { Collection lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix); - return lockFiles.stream() - .map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock) - .collect(Collectors.toList()); + return lockFiles.stream().map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock).collect(Collectors.toList()); } /** diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index 2c6c4afed69fd..7039dfbfc701e 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -988,6 +988,40 @@ public void testDeleteStaleCommitsActualDelete() throws Exception { verify(remoteMetadataDirectory).deleteFile(metadataFilename3); } + public void testDeleteStaleCommitsActualDeleteWithLocks() throws Exception { + Map> metadataFilenameContentMapping = populateMetadata(); + remoteSegmentStoreDirectory.init(); + + // Locking one of the metadata files to ensure that it is not getting deleted. + when(mdLockManager.fetchLocks(any())).thenReturn(List.of(metadataFilename2)); + + // popluateMetadata() adds stub to return 3 metadata files + // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted + remoteSegmentStoreDirectory.deleteStaleSegmentsAsync(1); + + for (String metadata : metadataFilenameContentMapping.get(metadataFilename3).values()) { + String uploadedFilename = metadata.split(RemoteSegmentStoreDirectory.UploadedSegmentMetadata.SEPARATOR)[1]; + verify(remoteDataDirectory).deleteFile(uploadedFilename); + } + assertBusy(() -> assertThat(remoteSegmentStoreDirectory.canDeleteStaleCommits.get(), is(true))); + verify(remoteMetadataDirectory).deleteFile(metadataFilename3); + verify(remoteMetadataDirectory, times(0)).deleteFile(metadataFilename2); + } + + public void testDeleteStaleCommitsNoDeletesDueToLocks() throws Exception { + remoteSegmentStoreDirectory.init(); + + // Locking all the old metadata files to ensure that none of the segment files are getting deleted. + when(mdLockManager.fetchLocks(any())).thenReturn(List.of(metadataFilename2, metadataFilename3)); + + // popluateMetadata() adds stub to return 3 metadata files + // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted + remoteSegmentStoreDirectory.deleteStaleSegmentsAsync(1); + + assertBusy(() -> assertThat(remoteSegmentStoreDirectory.canDeleteStaleCommits.get(), is(true))); + verify(remoteMetadataDirectory, times(0)).deleteFile(any()); + } + public void testDeleteStaleCommitsDeleteDedup() throws Exception { Map> metadataFilenameContentMapping = new HashMap<>(populateMetadata()); metadataFilenameContentMapping.put(metadataFilename4, metadataFilenameContentMapping.get(metadataFilename3)); From 26fc4cd83bc5308adbe25bff907281221f832e7d Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Wed, 24 Jan 2024 15:48:40 +0530 Subject: [PATCH 4/5] Add more unit tests Signed-off-by: Sachin Kale --- .../store/RemoteSegmentStoreDirectory.java | 4 +--- .../RemoteStoreMetadataLockManager.java | 5 ++-- .../RemoteSegmentStoreDirectoryTests.java | 17 ++++++++++++-- .../RemoteStoreMetadataLockManagerTests.java | 23 +++++++++++++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index 760cb29284e9c..a10deedf022f5 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -751,9 +751,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException ); Set allLockFiles; try { - allLockFiles = new HashSet<>( - ((RemoteStoreMetadataLockManager) mdLockManager).fetchLocks(MetadataFilenameUtils.METADATA_PREFIX) - ); + allLockFiles = ((RemoteStoreMetadataLockManager) mdLockManager).fetchLocks(MetadataFilenameUtils.METADATA_PREFIX); } catch (Exception e) { logger.error("Exception while fetching segment metadata lock files, skipping deleteStaleSegments", e); return; diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java index d20f54844d8a7..3bef47f48b157 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; /** @@ -88,9 +89,9 @@ public String fetchLock(String filenamePrefix, String acquirerId) throws IOExcep return lockFilesForAcquirer.get(0); } - public Collection fetchLocks(String filenamePrefix) throws IOException { + public Set fetchLocks(String filenamePrefix) throws IOException { Collection lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix); - return lockFiles.stream().map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock).collect(Collectors.toList()); + return lockFiles.stream().map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock).collect(Collectors.toSet()); } /** diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index 7039dfbfc701e..bd868678bc984 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -993,7 +993,7 @@ public void testDeleteStaleCommitsActualDeleteWithLocks() throws Exception { remoteSegmentStoreDirectory.init(); // Locking one of the metadata files to ensure that it is not getting deleted. - when(mdLockManager.fetchLocks(any())).thenReturn(List.of(metadataFilename2)); + when(mdLockManager.fetchLocks(any())).thenReturn(Set.of(metadataFilename2)); // popluateMetadata() adds stub to return 3 metadata files // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted @@ -1012,7 +1012,7 @@ public void testDeleteStaleCommitsNoDeletesDueToLocks() throws Exception { remoteSegmentStoreDirectory.init(); // Locking all the old metadata files to ensure that none of the segment files are getting deleted. - when(mdLockManager.fetchLocks(any())).thenReturn(List.of(metadataFilename2, metadataFilename3)); + when(mdLockManager.fetchLocks(any())).thenReturn(Set.of(metadataFilename2, metadataFilename3)); // popluateMetadata() adds stub to return 3 metadata files // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted @@ -1022,6 +1022,19 @@ public void testDeleteStaleCommitsNoDeletesDueToLocks() throws Exception { verify(remoteMetadataDirectory, times(0)).deleteFile(any()); } + public void testDeleteStaleCommitsExceptionWhileFetchingLocks() throws Exception { + remoteSegmentStoreDirectory.init(); + + // Locking one of the metadata files to ensure that it is not getting deleted. + when(mdLockManager.fetchLocks(any())).thenThrow(new RuntimeException("Rate limit exceeded")); + + // popluateMetadata() adds stub to return 3 metadata files + // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted + remoteSegmentStoreDirectory.deleteStaleSegmentsAsync(1); + + verify(remoteMetadataDirectory, times(0)).deleteFile(any()); + } + public void testDeleteStaleCommitsDeleteDedup() throws Exception { Map> metadataFilenameContentMapping = new HashMap<>(populateMetadata()); metadataFilenameContentMapping.put(metadataFilename4, metadataFilenameContentMapping.get(metadataFilename3)); diff --git a/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java b/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java index b4eac2c4548d5..4903241bee413 100644 --- a/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java +++ b/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; +import java.util.Set; import junit.framework.TestCase; @@ -96,4 +97,26 @@ public void testIsAcquiredExceptionCase() { // metadata file is not passed durin FileLockInfo testLockInfo = FileLockInfo.getLockInfoBuilder().withAcquirerId(testAcquirerId).build(); assertThrows(IllegalArgumentException.class, () -> remoteStoreMetadataLockManager.isAcquired(testLockInfo)); } + + public void testFetchLocksEmpty() throws IOException { + when(lockDirectory.listFilesByPrefix("metadata")).thenReturn(Set.of()); + assertEquals(0, remoteStoreMetadataLockManager.fetchLocks("metadata").size()); + } + + public void testFetchLocksNonEmpty() throws IOException { + String metadata1 = "metadata_1_2_3"; + String metadata2 = "metadata_4_5_6"; + when(lockDirectory.listFilesByPrefix("metadata")).thenReturn( + Set.of( + FileLockInfo.LockFileUtils.generateLockName(metadata1, "snapshot1"), + FileLockInfo.LockFileUtils.generateLockName(metadata2, "snapshot2") + ) + ); + assertEquals(Set.of(metadata1, metadata2), remoteStoreMetadataLockManager.fetchLocks("metadata")); + } + + public void testFetchLocksException() throws IOException { + when(lockDirectory.listFilesByPrefix("metadata")).thenThrow(new IOException("Something went wrong")); + assertThrows(IOException.class, () -> remoteStoreMetadataLockManager.fetchLocks("metadata")); + } } From 2381cb719cf36a2b9c876bac6d1559514884f802 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Thu, 25 Jan 2024 13:01:46 +0530 Subject: [PATCH 5/5] Address PR comments Signed-off-by: Sachin Kale --- .../opensearch/index/store/RemoteSegmentStoreDirectory.java | 4 ++-- .../store/lockmanager/RemoteStoreMetadataLockManager.java | 4 ++-- .../index/store/RemoteSegmentStoreDirectoryTests.java | 6 +++--- .../lockmanager/RemoteStoreMetadataLockManagerTests.java | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java index a10deedf022f5..dab99fd25b192 100644 --- a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java +++ b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java @@ -164,7 +164,7 @@ public RemoteSegmentMetadata init() throws IOException { */ public RemoteSegmentMetadata initializeToSpecificCommit(long primaryTerm, long commitGeneration, String acquirerId) throws IOException { String metadataFilePrefix = MetadataFilenameUtils.getMetadataFilePrefixForCommit(primaryTerm, commitGeneration); - String metadataFile = ((RemoteStoreMetadataLockManager) mdLockManager).fetchLock(metadataFilePrefix, acquirerId); + String metadataFile = ((RemoteStoreMetadataLockManager) mdLockManager).fetchLockedMetadataFile(metadataFilePrefix, acquirerId); RemoteSegmentMetadata remoteSegmentMetadata = readMetadataFile(metadataFile); if (remoteSegmentMetadata != null) { this.segmentsUploadedToRemoteStore = new ConcurrentHashMap<>(remoteSegmentMetadata.getMetadata()); @@ -751,7 +751,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException ); Set allLockFiles; try { - allLockFiles = ((RemoteStoreMetadataLockManager) mdLockManager).fetchLocks(MetadataFilenameUtils.METADATA_PREFIX); + allLockFiles = ((RemoteStoreMetadataLockManager) mdLockManager).fetchLockedMetadataFiles(MetadataFilenameUtils.METADATA_PREFIX); } catch (Exception e) { logger.error("Exception while fetching segment metadata lock files, skipping deleteStaleSegments", e); return; diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java index 3bef47f48b157..9c29e03c225e4 100644 --- a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java +++ b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManager.java @@ -76,7 +76,7 @@ public void release(LockInfo lockInfo) throws IOException { } } - public String fetchLock(String filenamePrefix, String acquirerId) throws IOException { + public String fetchLockedMetadataFile(String filenamePrefix, String acquirerId) throws IOException { Collection lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix); List lockFilesForAcquirer = lockFiles.stream() .filter(lockFile -> acquirerId.equals(FileLockInfo.LockFileUtils.getAcquirerIdFromLock(lockFile))) @@ -89,7 +89,7 @@ public String fetchLock(String filenamePrefix, String acquirerId) throws IOExcep return lockFilesForAcquirer.get(0); } - public Set fetchLocks(String filenamePrefix) throws IOException { + public Set fetchLockedMetadataFiles(String filenamePrefix) throws IOException { Collection lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix); return lockFiles.stream().map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock).collect(Collectors.toSet()); } diff --git a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java index bd868678bc984..7944ee681f5fc 100644 --- a/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/RemoteSegmentStoreDirectoryTests.java @@ -993,7 +993,7 @@ public void testDeleteStaleCommitsActualDeleteWithLocks() throws Exception { remoteSegmentStoreDirectory.init(); // Locking one of the metadata files to ensure that it is not getting deleted. - when(mdLockManager.fetchLocks(any())).thenReturn(Set.of(metadataFilename2)); + when(mdLockManager.fetchLockedMetadataFiles(any())).thenReturn(Set.of(metadataFilename2)); // popluateMetadata() adds stub to return 3 metadata files // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted @@ -1012,7 +1012,7 @@ public void testDeleteStaleCommitsNoDeletesDueToLocks() throws Exception { remoteSegmentStoreDirectory.init(); // Locking all the old metadata files to ensure that none of the segment files are getting deleted. - when(mdLockManager.fetchLocks(any())).thenReturn(Set.of(metadataFilename2, metadataFilename3)); + when(mdLockManager.fetchLockedMetadataFiles(any())).thenReturn(Set.of(metadataFilename2, metadataFilename3)); // popluateMetadata() adds stub to return 3 metadata files // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted @@ -1026,7 +1026,7 @@ public void testDeleteStaleCommitsExceptionWhileFetchingLocks() throws Exception remoteSegmentStoreDirectory.init(); // Locking one of the metadata files to ensure that it is not getting deleted. - when(mdLockManager.fetchLocks(any())).thenThrow(new RuntimeException("Rate limit exceeded")); + when(mdLockManager.fetchLockedMetadataFiles(any())).thenThrow(new RuntimeException("Rate limit exceeded")); // popluateMetadata() adds stub to return 3 metadata files // We are passing lastNMetadataFilesToKeep=2 here so that oldest 1 metadata file will be deleted diff --git a/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java b/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java index 4903241bee413..299100b65a43e 100644 --- a/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java +++ b/server/src/test/java/org/opensearch/index/store/lockmanager/RemoteStoreMetadataLockManagerTests.java @@ -100,7 +100,7 @@ public void testIsAcquiredExceptionCase() { // metadata file is not passed durin public void testFetchLocksEmpty() throws IOException { when(lockDirectory.listFilesByPrefix("metadata")).thenReturn(Set.of()); - assertEquals(0, remoteStoreMetadataLockManager.fetchLocks("metadata").size()); + assertEquals(0, remoteStoreMetadataLockManager.fetchLockedMetadataFiles("metadata").size()); } public void testFetchLocksNonEmpty() throws IOException { @@ -112,11 +112,11 @@ public void testFetchLocksNonEmpty() throws IOException { FileLockInfo.LockFileUtils.generateLockName(metadata2, "snapshot2") ) ); - assertEquals(Set.of(metadata1, metadata2), remoteStoreMetadataLockManager.fetchLocks("metadata")); + assertEquals(Set.of(metadata1, metadata2), remoteStoreMetadataLockManager.fetchLockedMetadataFiles("metadata")); } public void testFetchLocksException() throws IOException { when(lockDirectory.listFilesByPrefix("metadata")).thenThrow(new IOException("Something went wrong")); - assertThrows(IOException.class, () -> remoteStoreMetadataLockManager.fetchLocks("metadata")); + assertThrows(IOException.class, () -> remoteStoreMetadataLockManager.fetchLockedMetadataFiles("metadata")); } }