Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
Signed-off-by: Sachin Kale <[email protected]>
  • Loading branch information
Sachin Kale committed Jan 25, 2024
1 parent 26fc4cd commit 2381cb7
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Check warning on line 167 in server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java#L167

Added line #L167 was not covered by tests
RemoteSegmentMetadata remoteSegmentMetadata = readMetadataFile(metadataFile);
if (remoteSegmentMetadata != null) {
this.segmentsUploadedToRemoteStore = new ConcurrentHashMap<>(remoteSegmentMetadata.getMetadata());
Expand Down Expand Up @@ -751,7 +751,7 @@ public void deleteStaleSegments(int lastNMetadataFilesToKeep) throws IOException
);
Set<String> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix);
List<String> lockFilesForAcquirer = lockFiles.stream()
.filter(lockFile -> acquirerId.equals(FileLockInfo.LockFileUtils.getAcquirerIdFromLock(lockFile)))
Expand All @@ -89,7 +89,7 @@ public String fetchLock(String filenamePrefix, String acquirerId) throws IOExcep
return lockFilesForAcquirer.get(0);
}

public Set<String> fetchLocks(String filenamePrefix) throws IOException {
public Set<String> fetchLockedMetadataFiles(String filenamePrefix) throws IOException {
Collection<String> lockFiles = lockDirectory.listFilesByPrefix(filenamePrefix);
return lockFiles.stream().map(FileLockInfo.LockFileUtils::getFileToLockNameFromLock).collect(Collectors.toSet());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"));
}
}

0 comments on commit 2381cb7

Please sign in to comment.