Skip to content

Commit

Permalink
Cache pinned timestamps without any matching metadata file
Browse files Browse the repository at this point in the history 
Signed-off-by: Sachin Kale <[email protected]>
  • Loading branch information
@sachinpkale
sachinpkale committed Sep 10, 2024
1 parent 7cb2bd0 commit bbe423d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 10 deletions.
14 changes: 11 additions & 3 deletions server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,11 @@ private static Set<String> getPinnedTimestampLockedFiles(
// Add cached entries and collect new timestamps
Set<Long> newPinnedTimestamps = new TreeSet<>(Collections.reverseOrder());
for (Long pinnedTimestamp : pinnedTimestampSet) {
String cachedFile = metadataFilePinnedTimestampMap.get(pinnedTimestamp);
if (cachedFile != null) {
implicitLockedFiles.add(cachedFile);
if (metadataFilePinnedTimestampMap.containsKey(pinnedTimestamp)) {
String cachedFile = metadataFilePinnedTimestampMap.get(pinnedTimestamp);
if (cachedFile != null) {
implicitLockedFiles.add(cachedFile);
}
} else {
newPinnedTimestamps.add(pinnedTimestamp);
}
Expand Down Expand Up @@ -517,6 +519,12 @@ private static Set<String> getPinnedTimestampLockedFiles(
}
prevMdTimestamp = currentMdTimestamp;
}
if (metadataFilePinnedTimestampMap.containsKey(currentPinnedTimestamp) == false) {
metadataFilePinnedTimestampMap.put(currentPinnedTimestamp, null);
}
while (timestampIterator.hasNext()) {
metadataFilePinnedTimestampMap.put(timestampIterator.next(), null);
}

return implicitLockedFiles;
}
Expand Down
26 changes: 19 additions & 7 deletions server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,9 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
Set<String> implicitLockedFiles = metadataAndLocks.v2();

assertEquals(0, implicitLockedFiles.size());
assertEquals(0, metadataFilePinnedTimestampCache.size());
assertEquals(2, metadataFilePinnedTimestampCache.size());
assertNull(metadataFilePinnedTimestampCache.get(800L));
assertNull(metadataFilePinnedTimestampCache.get(900L));

// Pinned timestamps 800, 900, 1000
// Metadata with timestamp 990
Expand All @@ -710,7 +712,9 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
assertEquals(1, implicitLockedFiles.size());
assertTrue(implicitLockedFiles.contains(metadataFiles.get(990L)));
// This is still 0 as we don't cache the latest metadata file as it can change (explained in the next test case)
assertEquals(0, metadataFilePinnedTimestampCache.size());
assertEquals(2, metadataFilePinnedTimestampCache.size());
assertNull(metadataFilePinnedTimestampCache.get(800L));
assertNull(metadataFilePinnedTimestampCache.get(900L));

// Pinned timestamps 800, 900, 1000
// Metadata with timestamp 990, 995
Expand All @@ -727,7 +731,9 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
assertEquals(1, implicitLockedFiles.size());
assertTrue(implicitLockedFiles.contains(metadataFiles.get(995L)));
// This is still 0 as we don't cache the latest metadata file as it can change
assertEquals(0, metadataFilePinnedTimestampCache.size());
assertEquals(2, metadataFilePinnedTimestampCache.size());
assertNull(metadataFilePinnedTimestampCache.get(800L));
assertNull(metadataFilePinnedTimestampCache.get(900L));

// Pinned timestamps 800, 900, 1000
// Metadata with timestamp 990, 995, 1000
Expand All @@ -744,7 +750,9 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
assertEquals(1, implicitLockedFiles.size());
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1000L)));
// This is still 0 as we don't cache the latest metadata file as it can change
assertEquals(0, metadataFilePinnedTimestampCache.size());
assertEquals(2, metadataFilePinnedTimestampCache.size());
assertNull(metadataFilePinnedTimestampCache.get(800L));
assertNull(metadataFilePinnedTimestampCache.get(900L));

// Pinned timestamps 800, 900, 1000, 2000
// Metadata with timestamp 990, 995, 1000, 1001
Expand All @@ -763,8 +771,10 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1000L)));
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1001L)));
// Now we cache all the matches except the last one.
assertEquals(1, metadataFilePinnedTimestampCache.size());
assertEquals(3, metadataFilePinnedTimestampCache.size());
assertEquals(metadataFiles.get(1000L), metadataFilePinnedTimestampCache.get(1000L));
assertNull(metadataFilePinnedTimestampCache.get(800L));
assertNull(metadataFilePinnedTimestampCache.get(900L));

// Pinned timestamps 800, 900, 1000, 2000, 3000, 4000, 5000
// Metadata with timestamp 990, 995, 1000, 1001
Expand All @@ -786,8 +796,10 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1000L)));
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1001L)));
// Now we cache all the matches except the last one.
assertEquals(1, metadataFilePinnedTimestampCache.size());
assertEquals(3, metadataFilePinnedTimestampCache.size());
assertEquals(metadataFiles.get(1000L), metadataFilePinnedTimestampCache.get(1000L));
assertNull(metadataFilePinnedTimestampCache.get(800L));
assertNull(metadataFilePinnedTimestampCache.get(900L));

// Pinned timestamps 800, 900, 1000, 2000, 3000, 4000, 5000
// Metadata with timestamp 990, 995, 1000, 1001, 1900, 2300
Expand All @@ -810,7 +822,7 @@ public void testGetPinnedTimestampLockedFilesWithPinnedTimestamps() {
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1900L)));
assertTrue(implicitLockedFiles.contains(metadataFiles.get(2300L)));
// Now we cache all the matches except the last one.
assertEquals(2, metadataFilePinnedTimestampCache.size());
assertEquals(4, metadataFilePinnedTimestampCache.size());
assertEquals(metadataFiles.get(1000L), metadataFilePinnedTimestampCache.get(1000L));
assertEquals(metadataFiles.get(1900L), metadataFilePinnedTimestampCache.get(2000L));

Expand Down

0 comments on commit bbe423d

Please sign in to comment.