Skip to content

Commit

Permalink
Fix tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Sachin Kale <sachinpkale@gmail.com>
  • Loading branch information
@sachinpkale
sachinpkale committed Sep 20, 2024
1 parent 36eba58 commit fae9f37
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 25 deletions.
10 changes: 5 additions & 5 deletions ...rTest/java/org/opensearch/remotestore/RemoteStorePinnedTimestampsGarbageCollectionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void testLiveIndexNoPinnedTimestamps() throws Exception {

assertBusy(() -> {
List<Path> metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList());
assertEquals(1, metadataFiles.size());
assertEquals(2, metadataFiles.size());

verifyTranslogDataFileCount(metadataFiles, translogDataPath);
});
Expand Down Expand Up @@ -222,7 +222,7 @@ public void testLiveIndexNoPinnedTimestampsWithExtraGenSetting() throws Exceptio

assertBusy(() -> {
List<Path> metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList());
assertEquals(4, metadataFiles.size());
assertEquals(5, metadataFiles.size());

verifyTranslogDataFileCount(metadataFiles, translogDataPath);
});
Expand Down Expand Up @@ -282,7 +282,7 @@ public void testLiveIndexWithPinnedTimestamps() throws Exception {

assertBusy(() -> {
List<Path> metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList());
assertEquals(2, metadataFiles.size());
assertEquals(3, metadataFiles.size());

verifyTranslogDataFileCount(metadataFiles, translogDataPath);
});
Expand Down Expand Up @@ -337,7 +337,7 @@ public void testIndexDeletionNoPinnedTimestamps() throws Exception {

assertBusy(() -> {
List<Path> metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList());
assertEquals(1, metadataFiles.size());
assertEquals(2, metadataFiles.size());

verifyTranslogDataFileCount(metadataFiles, translogDataPath);
});
Expand Down Expand Up @@ -405,7 +405,7 @@ public void testIndexDeletionWithPinnedTimestamps() throws Exception {

assertBusy(() -> {
List<Path> metadataFiles = Files.list(translogMetadataPath).collect(Collectors.toList());
assertEquals(2, metadataFiles.size());
assertEquals(3, metadataFiles.size());

verifyTranslogDataFileCount(metadataFiles, translogDataPath);
}, 30, TimeUnit.SECONDS);
Expand Down
22 changes: 14 additions & 8 deletions server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void onResponse(List<BlobMetadata> blobMetadata) {
Set<Long> generationsToBeDeleted = getGenerationsToBeDeleted(
metadataFilesNotToBeDeleted,
metadataFilesToBeDeleted,
indexDeleted ? Long.MAX_VALUE : getMinGenerationToKeep()
indexDeleted ? Long.MAX_VALUE : getMinGenerationToKeepInRemote()
);

logger.debug(() -> "generationsToBeDeleted = " + generationsToBeDeleted);
Expand Down Expand Up @@ -246,15 +246,15 @@ public void onFailure(Exception e) {
translogTransferManager.listTranslogMetadataFilesAsync(listMetadataFilesListener);
}

private long getMinGenerationToKeep() {
private long getMinGenerationToKeepInRemote() {
return minRemoteGenReferenced - indexSettings().getRemoteTranslogExtraKeep();
}

// Visible for testing
protected Set<Long> getGenerationsToBeDeleted(
List<String> metadataFilesNotToBeDeleted,
List<String> metadataFilesToBeDeleted,
long minGenerationToKeep
long minGenerationToKeepInRemote
) throws IOException {
Set<Long> generationsFromMetadataFilesToBeDeleted = new HashSet<>();
for (String mdFile : metadataFilesToBeDeleted) {
Expand All @@ -271,22 +271,28 @@ protected Set<Long> getGenerationsToBeDeleted(
// Check if the generation is not referred by metadata file matching pinned timestamps
// The check with minGenerationToKeep is redundant but kept as to make sure we don't delete generations
// that are not persisted in remote segment store yet.
if (generation < minGenerationToKeep && isGenerationPinned(generation, pinnedGenerations) == false) {
if (generation < minGenerationToKeepInRemote && isGenerationPinned(generation, pinnedGenerations) == false) {
generationsToBeDeleted.add(generation);
}
}
return generationsToBeDeleted;
}

protected List<String> getMetadataFilesToBeDeleted(List<String> metadataFiles, boolean indexDeleted) {
return getMetadataFilesToBeDeleted(metadataFiles, metadataFilePinnedTimestampMap, getMinGenerationToKeep(), indexDeleted, logger);
return getMetadataFilesToBeDeleted(
metadataFiles,
metadataFilePinnedTimestampMap,
getMinGenerationToKeepInRemote(),
indexDeleted,
logger
);
}

// Visible for testing
protected static List<String> getMetadataFilesToBeDeleted(
List<String> metadataFiles,
Map<Long, String> metadataFilePinnedTimestampMap,
long minGenerationToKeep,
long minGenerationToKeepInRemote,
boolean indexDeleted,
Logger logger
) {
Expand Down Expand Up @@ -327,15 +333,15 @@ protected static List<String> getMetadataFilesToBeDeleted(
// Filter out metadata files based on minGenerationToKeep
List<String> metadataFilesContainingMinGenerationToKeep = metadataFilesToBeDeleted.stream().filter(md -> {
long maxGeneration = TranslogTransferMetadata.getMaxGenerationFromFileName(md);
return maxGeneration == -1 || maxGeneration > minGenerationToKeep;
return maxGeneration == -1 || maxGeneration >= minGenerationToKeepInRemote;
}).collect(Collectors.toList());
metadataFilesToBeDeleted.removeAll(metadataFilesContainingMinGenerationToKeep);

logger.trace(
"metadataFilesContainingMinGenerationToKeep.size = {}, metadataFilesToBeDeleted based on minGenerationToKeep filtering = {}, minGenerationToKeep = {}",
metadataFilesContainingMinGenerationToKeep.size(),
metadataFilesToBeDeleted.size(),
minGenerationToKeep
minGenerationToKeepInRemote
);
}

Expand Down
41 changes: 29 additions & 12 deletions server/src/test/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslogTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,43 +335,60 @@ public void testSimpleOperationsUpload() throws Exception {

addToTranslogAndListAndUpload(translog, ops, new Translog.Index("2", 2, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("3", 3, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("4", 4, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("5", 5, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("6", 6, primaryTerm.get(), new byte[] { 1 }));

assertBusy(() -> {
assertEquals(
16,
10,
blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
);
});

assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));

RemoteStoreSettings.setPinnedTimestampsLookbackInterval(TimeValue.ZERO);
assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
// Fetch pinned timestamps so that it won't be stale
updatePinnedTimstampTask.run();
translog.setMinSeqNoToKeep(3);
translog.trimUnreferencedReaders();

addToTranslogAndListAndUpload(translog, ops, new Translog.Index("4", 4, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("5", 5, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("6", 6, primaryTerm.get(), new byte[] { 1 }));

assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
// Fetch pinned timestamps so that it won't be stale
updatePinnedTimstampTask.run();
translog.setMinSeqNoToKeep(6);
translog.trimUnreferencedReaders();
assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));

assertEquals(1, translog.readers.size());
assertBusy(() -> {
assertEquals(2, translog.allUploaded().size());
assertEquals(4, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
assertEquals(
16,
blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
);
}, 30, TimeUnit.SECONDS);

addToTranslogAndListAndUpload(translog, ops, new Translog.Index("7", 7, primaryTerm.get(), new byte[] { 1 }));
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("8", 8, primaryTerm.get(), new byte[] { 1 }));
assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));

assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
// Fetch pinned timestamps so that it won't be stale
updatePinnedTimstampTask.run();
translog.trimUnreferencedReaders();
assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));

assertEquals(3, translog.readers.size());
assertBusy(() -> {
assertEquals(2, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
assertEquals(6, translog.allUploaded().size());
assertEquals(3, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size());
assertEquals(
6,
12,
blobStoreTransferService.listAll(getTranslogDirectory().add(DATA_DIR).add(String.valueOf(primaryTerm.get()))).size()
);
}, 60, TimeUnit.SECONDS);
}, 30, TimeUnit.SECONDS);
}

@Override
Expand Down Expand Up @@ -573,7 +590,7 @@ public void testDrainSync() throws Exception {
assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
assertEquals(1, translog.readers.size());
assertBusy(() -> assertEquals(2, translog.allUploaded().size()));
assertBusy(() -> assertEquals(1, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size()));
assertBusy(() -> assertEquals(2, blobStoreTransferService.listAll(getTranslogDirectory().add(METADATA_DIR)).size()));
}

@Override
Expand Down Expand Up @@ -816,7 +833,7 @@ public void testGetMetadataFilesToBeDeletedExclusionBasedOnGenerationOnly() thro
// MaxGen 12
"metadata__9223372036438563903__9223372036854775795__" + md2Timestamp + "__31__9223372036854775803__1",
// MaxGen 10
"metadata__9223372036438563903__9223372036854775797__" + md3Timestamp + "__31__9223372036854775701__1"
"metadata__9223372036438563903__9223372036854775798__" + md3Timestamp + "__31__9223372036854775701__1"
);

List<String> metadataFilesToBeDeleted = RemoteFsTimestampAwareTranslog.getMetadataFilesToBeDeleted(
Expand Down

0 comments on commit fae9f37

Please sign in to comment.