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 Sep 3, 2024
1 parent dd26bd0 commit b80e9ba
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 27 deletions.
5 changes: 2 additions & 3 deletions ...r/src/main/java/org/opensearch/index/translog/RemoteBlobStoreInternalTranslogFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public Translog newTranslog(
assert repository instanceof BlobStoreRepository : "repository should be instance of BlobStoreRepository";
BlobStoreRepository blobStoreRepository = ((BlobStoreRepository) repository);
if (RemoteStoreSettings.isPinnedTimestampsEnabled()) {
return new RemoteFsTranslogWithPinnedTimestamps(
return new RemoteFsTimestampAwareTranslog(
config,
translogUUID,
deletionPolicy,
Expand All @@ -81,8 +81,7 @@ public Translog newTranslog(
threadPool,
startedPrimarySupplier,
remoteTranslogTransferTracker,
remoteStoreSettings,
0
remoteStoreSettings
);
} else {
return new RemoteFsTranslog(
Expand Down
19 changes: 9 additions & 10 deletions ...RemoteFsTranslogWithPinnedTimestamps.java → ...nslog/RemoteFsTimestampAwareTranslog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,19 @@
* A Translog implementation which syncs local FS with a remote store
* The current impl uploads translog , ckp and metadata to remote store
* for every sync, post syncing to disk. Post that, a new generation is
* created.
* created. This implementation is also aware of pinned timestamp and makes
* sure data against pinned timestamp is retained.
*
* @opensearch.internal
*/
public class RemoteFsTranslogWithPinnedTimestamps extends RemoteFsTranslog {
public class RemoteFsTimestampAwareTranslog extends RemoteFsTranslog {

private final Logger logger;
private final Map<Long, String> metadataFilePinnedTimestampMap;
// For metadata files, with no min generation in the name, we cache generation data to avoid multiple reads.
private final Map<String, Tuple<Long, Long>> oldFormatMetadataFileGenerationMap;

public RemoteFsTranslogWithPinnedTimestamps(
public RemoteFsTimestampAwareTranslog(
TranslogConfig config,
String translogUUID,
TranslogDeletionPolicy deletionPolicy,
Expand All @@ -67,8 +68,7 @@ public RemoteFsTranslogWithPinnedTimestamps(
ThreadPool threadPool,
BooleanSupplier startedPrimarySupplier,
RemoteTranslogTransferTracker remoteTranslogTransferTracker,
RemoteStoreSettings remoteStoreSettings,
long timestamp
RemoteStoreSettings remoteStoreSettings
) throws IOException {
super(
config,
Expand All @@ -81,8 +81,7 @@ public RemoteFsTranslogWithPinnedTimestamps(
threadPool,
startedPrimarySupplier,
remoteTranslogTransferTracker,
remoteStoreSettings,
timestamp
remoteStoreSettings
);
logger = Loggers.getLogger(getClass(), shardId);
this.metadataFilePinnedTimestampMap = new HashMap<>();
Expand Down Expand Up @@ -179,19 +178,19 @@ public void onResponse(List<BlobMetadata> blobMetadata) {
return;
}

logger.debug("metadataFilesToBeDeleted = {}", metadataFilesToBeDeleted);
logger.debug(() -> "metadataFilesToBeDeleted = " + metadataFilesToBeDeleted);
// For all the files that we are keeping, fetch min and max generations
List<String> metadataFilesNotToBeDeleted = new ArrayList<>(metadataFiles);
metadataFilesNotToBeDeleted.removeAll(metadataFilesToBeDeleted);

logger.debug("metadataFilesNotToBeDeleted = {}", metadataFilesNotToBeDeleted);
logger.debug(() -> "metadataFilesNotToBeDeleted = " + metadataFilesNotToBeDeleted);
Set<Long> generationsToBeDeleted = getGenerationsToBeDeleted(
metadataFilesNotToBeDeleted,
metadataFilesToBeDeleted,
indexDeleted
);

logger.debug("generationsToBeDeleted = {}", generationsToBeDeleted);
logger.debug(() -> "generationsToBeDeleted = " + generationsToBeDeleted);
if (generationsToBeDeleted.isEmpty() == false) {
// Delete stale generations
translogTransferManager.deleteGenerationAsync(
Expand Down
27 changes: 13 additions & 14 deletions ...teFsTranslogWithPinnedTimestampTests.java → .../RemoteFsTimestampAwareTranslogTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
import static org.mockito.Mockito.when;

@LuceneTestCase.SuppressFileSystems("ExtrasFS")
public class RemoteFsTranslogWithPinnedTimestampTests extends RemoteFsTranslogTests {
public class RemoteFsTimestampAwareTranslogTests extends RemoteFsTranslogTests {

Runnable updatePinnedTimstampTask;
BlobContainer blobContainer;
Expand Down Expand Up @@ -132,7 +132,7 @@ protected RemoteFsTranslog createTranslogInstance(
String translogUUID,
TranslogDeletionPolicy deletionPolicy
) throws IOException {
return new RemoteFsTranslogWithPinnedTimestamps(
return new RemoteFsTimestampAwareTranslog(
translogConfig,
translogUUID,
deletionPolicy,
Expand All @@ -143,8 +143,7 @@ protected RemoteFsTranslog createTranslogInstance(
threadPool,
primaryMode::get,
new RemoteTranslogTransferTracker(shardId, 10),
DefaultRemoteStoreSettings.INSTANCE,
0
DefaultRemoteStoreSettings.INSTANCE
);
}

Expand Down Expand Up @@ -242,7 +241,7 @@ public void testIndexDeletionWithNoPinnedTimestampNoRecentMdFiles() throws Excep

assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
updatePinnedTimstampTask.run();
((RemoteFsTranslogWithPinnedTimestamps) translog).trimUnreferencedReaders(true, false);
((RemoteFsTimestampAwareTranslog) translog).trimUnreferencedReaders(true, false);

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

Expand All @@ -265,7 +264,7 @@ public void testIndexDeletionWithNoPinnedTimestampButRecentFiles() throws Except
addToTranslogAndListAndUpload(translog, ops, new Translog.Index("4", 4, primaryTerm.get(), new byte[] { 1 }));

updatePinnedTimstampTask.run();
((RemoteFsTranslogWithPinnedTimestamps) translog).trimUnreferencedReaders(true, false);
((RemoteFsTimestampAwareTranslog) translog).trimUnreferencedReaders(true, false);

assertBusy(() -> assertTrue(translog.isRemoteGenerationDeletionPermitsAvailable()));
assertBusy(() -> {
Expand Down Expand Up @@ -611,7 +610,7 @@ public void testGetGenerationsToBeDeletedEmptyMetadataFilesNotToBeDeleted() thro
// 27 to 42
"metadata__9223372036438563903__9223372036854775765__9223370311919910403__31__9223372036854775780__1"
);
Set<Long> generations = ((RemoteFsTranslogWithPinnedTimestamps) translog).getGenerationsToBeDeleted(
Set<Long> generations = ((RemoteFsTimestampAwareTranslog) translog).getGenerationsToBeDeleted(
metadataFilesNotToBeDeleted,
metadataFilesToBeDeleted,
true
Expand Down Expand Up @@ -646,7 +645,7 @@ public void testGetGenerationsToBeDeleted() throws IOException {
// 27 to 42
"metadata__9223372036438563903__9223372036854775765__9223370311919910403__31__9223372036854775780__1"
);
Set<Long> generations = ((RemoteFsTranslogWithPinnedTimestamps) translog).getGenerationsToBeDeleted(
Set<Long> generations = ((RemoteFsTimestampAwareTranslog) translog).getGenerationsToBeDeleted(
metadataFilesNotToBeDeleted,
metadataFilesToBeDeleted,
true
Expand All @@ -673,7 +672,7 @@ public void testGetMetadataFilesToBeDeletedNoExclusion() {
"metadata__9223372036438563903__9223372036854775701__9223370311919910403__31__9223372036854775701__1"
);

assertEquals(metadataFiles, ((RemoteFsTranslogWithPinnedTimestamps) translog).getMetadataFilesToBeDeleted(metadataFiles));
assertEquals(metadataFiles, ((RemoteFsTimestampAwareTranslog) translog).getMetadataFilesToBeDeleted(metadataFiles));
}

public void testGetMetadataFilesToBeDeletedExclusionBasedOnAgeOnly() {
Expand All @@ -689,7 +688,7 @@ public void testGetMetadataFilesToBeDeletedExclusionBasedOnAgeOnly() {
"metadata__9223372036438563903__9223372036854775701__" + md3Timestamp + "__31__9223372036854775701__1"
);

List<String> metadataFilesToBeDeleted = ((RemoteFsTranslogWithPinnedTimestamps) translog).getMetadataFilesToBeDeleted(
List<String> metadataFilesToBeDeleted = ((RemoteFsTimestampAwareTranslog) translog).getMetadataFilesToBeDeleted(
metadataFiles
);
assertEquals(1, metadataFilesToBeDeleted.size());
Expand All @@ -713,7 +712,7 @@ public void testGetMetadataFilesToBeDeletedExclusionBasedOnPinningOnly() throws
"metadata__9223372036438563903__9223372036854775701__" + md3Timestamp + "__31__9223372036854775701__1"
);

List<String> metadataFilesToBeDeleted = ((RemoteFsTranslogWithPinnedTimestamps) translog).getMetadataFilesToBeDeleted(
List<String> metadataFilesToBeDeleted = ((RemoteFsTimestampAwareTranslog) translog).getMetadataFilesToBeDeleted(
metadataFiles
);
assertEquals(2, metadataFilesToBeDeleted.size());
Expand All @@ -738,7 +737,7 @@ public void testGetMetadataFilesToBeDeletedExclusionBasedOnAgeAndPinning() throw
"metadata__9223372036438563903__9223372036854775701__" + md3Timestamp + "__31__9223372036854775701__1"
);

List<String> metadataFilesToBeDeleted = ((RemoteFsTranslogWithPinnedTimestamps) translog).getMetadataFilesToBeDeleted(
List<String> metadataFilesToBeDeleted = ((RemoteFsTimestampAwareTranslog) translog).getMetadataFilesToBeDeleted(
metadataFiles
);
assertEquals(1, metadataFilesToBeDeleted.size());
Expand All @@ -765,7 +764,7 @@ public void testIsGenerationPinned() {
pinnedGenerations.add(new Tuple<>(142L, 180L));
pinnedGenerations.add(new Tuple<>(4L, 9L));

RemoteFsTranslogWithPinnedTimestamps translog = (RemoteFsTranslogWithPinnedTimestamps) this.translog;
RemoteFsTimestampAwareTranslog translog = (RemoteFsTimestampAwareTranslog) this.translog;

assertFalse(translog.isGenerationPinned(3, pinnedGenerations));
assertFalse(translog.isGenerationPinned(10, pinnedGenerations));
Expand All @@ -784,7 +783,7 @@ public void testIsGenerationPinned() {
public void testGetMinMaxTranslogGenerationFromMetadataFile() throws IOException {
TranslogTransferManager translogTransferManager = mock(TranslogTransferManager.class);

RemoteFsTranslogWithPinnedTimestamps translog = (RemoteFsTranslogWithPinnedTimestamps) this.translog;
RemoteFsTimestampAwareTranslog translog = (RemoteFsTimestampAwareTranslog) this.translog;

// Fetch generations directly from the filename
assertEquals(
Expand Down

0 comments on commit b80e9ba

Please sign in to comment.