Skip to content

Commit 2ee4675

Browse files
sachinpkaledk2k
sachinpkale
authored and
dk2k
committed
Add more unit tests for RemoteStoreUtils and RemoteFsTimestampAwareTranslog (opensearch-project#16151)
Signed-off-by: Sachin Kale <sachinpkale@gmail.com>
1 parent c85ebbc commit 2ee4675

4 files changed

Lines changed: 353 additions & 9 deletions

File tree

‎server/src/main/java/org/opensearch/index/remote/RemoteStoreUtils.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ private static Set<String> getPinnedTimestampLockedFiles(
477477
for (Long pinnedTimestamp : pinnedTimestampSet) {
478478
String cachedFile = metadataFilePinnedTimestampMap.get(pinnedTimestamp);
479479
if (cachedFile != null) {
480+
assert metadataFiles.contains(cachedFile) : "Metadata files should contain [" + cachedFile + "]";
480481
implicitLockedFiles.add(cachedFile);
481482
} else {
482483
newPinnedTimestamps.add(pinnedTimestamp);

‎server/src/main/java/org/opensearch/index/translog/RemoteFsTimestampAwareTranslog.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ public void onResponse(List<BlobMetadata> blobMetadata) {
233233

234234
// Update cache to keep only those metadata files that are not getting deleted
235235
oldFormatMetadataFileGenerationMap.keySet().retainAll(metadataFilesNotToBeDeleted);
236+
oldFormatMetadataFilePrimaryTermMap.keySet().retainAll(metadataFilesNotToBeDeleted);
236237
// Delete stale primary terms
237238
deleteStaleRemotePrimaryTerms(metadataFilesNotToBeDeleted);
238239
} else {
@@ -408,9 +409,9 @@ protected Tuple<Long, Long> getMinMaxTranslogGenerationFromMetadataFile(
408409
}
409410
}
410411

411-
private void deleteStaleRemotePrimaryTerms(List<String> metadataFiles) {
412+
private void deleteStaleRemotePrimaryTerms(List<String> metadataFilesNotToBeDeleted) {
412413
deleteStaleRemotePrimaryTerms(
413-
metadataFiles,
414+
metadataFilesNotToBeDeleted,
414415
translogTransferManager,
415416
oldFormatMetadataFilePrimaryTermMap,
416417
minPrimaryTermInRemote,
@@ -425,7 +426,7 @@ private void deleteStaleRemotePrimaryTerms(List<String> metadataFiles) {
425426
* This will also delete all stale translog metadata files from remote except the latest basis the metadata file comparator.
426427
*/
427428
protected static void deleteStaleRemotePrimaryTerms(
428-
List<String> metadataFiles,
429+
List<String> metadataFilesNotToBeDeleted,
429430
TranslogTransferManager translogTransferManager,
430431
Map<String, Tuple<Long, Long>> oldFormatMetadataFilePrimaryTermMap,
431432
AtomicLong minPrimaryTermInRemoteAtomicLong,
@@ -434,15 +435,15 @@ protected static void deleteStaleRemotePrimaryTerms(
434435
// The deletion of older translog files in remote store is on best-effort basis, there is a possibility that there
435436
// are older files that are no longer needed and should be cleaned up. In here, we delete all files that are part
436437
// of older primary term.
437-
if (metadataFiles.isEmpty()) {
438+
if (metadataFilesNotToBeDeleted.isEmpty()) {
438439
logger.trace("No metadata is uploaded yet, returning from deleteStaleRemotePrimaryTerms");
439440
return;
440441
}
441-
Optional<Long> minPrimaryTermFromMetadataFiles = metadataFiles.stream().map(file -> {
442+
Optional<Long> minPrimaryTermFromMetadataFiles = metadataFilesNotToBeDeleted.stream().map(file -> {
442443
try {
443444
return getMinMaxPrimaryTermFromMetadataFile(file, translogTransferManager, oldFormatMetadataFilePrimaryTermMap).v1();
444445
} catch (IOException e) {
445-
return Long.MAX_VALUE;
446+
return Long.MIN_VALUE;
446447
}
447448
}).min(Long::compareTo);
448449
// First we delete all stale primary terms folders from remote store
@@ -459,7 +460,7 @@ protected static void deleteStaleRemotePrimaryTerms(
459460
}
460461
}
461462

462-
private static Long getMinPrimaryTermInRemote(
463+
protected static Long getMinPrimaryTermInRemote(
463464
AtomicLong minPrimaryTermInRemote,
464465
TranslogTransferManager translogTransferManager,
465466
Logger logger

‎server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,4 +1083,101 @@ public void testIsPinnedTimestampStateStaleFeatureEnabled() {
10831083
setupRemotePinnedTimestampFeature(true);
10841084
assertTrue(RemoteStoreUtils.isPinnedTimestampStateStale());
10851085
}
1086+
1087+
public void testGetPinnedTimestampLockedFilesWithCache() {
1088+
setupRemotePinnedTimestampFeature(true);
1089+
1090+
Map<Long, String> metadataFilePinnedTimestampCache = new HashMap<>();
1091+
1092+
// Pinned timestamps 800, 900, 1000, 2000
1093+
// Metadata with timestamp 990, 995, 1000, 1001
1094+
// Metadata timestamp 1000 <= Pinned Timestamp 1000
1095+
// Metadata timestamp 1001 <= Pinned Timestamp 2000
1096+
Tuple<Map<Long, String>, Set<String>> metadataAndLocks = testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1097+
List.of(990L, 995L, 1000L, 1001L),
1098+
Set.of(800L, 900L, 1000L, 2000L),
1099+
metadataFilePinnedTimestampCache
1100+
);
1101+
Map<Long, String> metadataFiles = metadataAndLocks.v1();
1102+
Set<String> implicitLockedFiles = metadataAndLocks.v2();
1103+
1104+
assertEquals(2, implicitLockedFiles.size());
1105+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1000L)));
1106+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1001L)));
1107+
// Now we cache all the matches except the last one.
1108+
assertEquals(1, metadataFilePinnedTimestampCache.size());
1109+
assertEquals(metadataFiles.get(1000L), metadataFilePinnedTimestampCache.get(1000L));
1110+
1111+
metadataAndLocks = testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1112+
List.of(990L, 995L, 1000L, 1001L, 2000L, 2200L),
1113+
Set.of(800L, 900L, 1000L, 2000L, 3000L),
1114+
metadataFilePinnedTimestampCache
1115+
);
1116+
metadataFiles = metadataAndLocks.v1();
1117+
implicitLockedFiles = metadataAndLocks.v2();
1118+
assertEquals(3, implicitLockedFiles.size());
1119+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(1000L)));
1120+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(2000L)));
1121+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(2200L)));
1122+
assertEquals(2, metadataFilePinnedTimestampCache.size());
1123+
assertEquals(metadataFiles.get(1000L), metadataFilePinnedTimestampCache.get(1000L));
1124+
assertEquals(metadataFiles.get(2000L), metadataFilePinnedTimestampCache.get(2000L));
1125+
1126+
metadataAndLocks = testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1127+
List.of(990L, 995L, 1000L, 1001L, 2000L, 2200L, 2500L),
1128+
Set.of(2000L, 3000L),
1129+
metadataFilePinnedTimestampCache
1130+
);
1131+
metadataFiles = metadataAndLocks.v1();
1132+
implicitLockedFiles = metadataAndLocks.v2();
1133+
assertEquals(2, implicitLockedFiles.size());
1134+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(2000L)));
1135+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(2500L)));
1136+
assertEquals(1, metadataFilePinnedTimestampCache.size());
1137+
assertEquals(metadataFiles.get(2000L), metadataFilePinnedTimestampCache.get(2000L));
1138+
1139+
metadataAndLocks = testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1140+
List.of(2000L, 2200L, 2500L, 3001L, 4200L, 4600L, 5010L),
1141+
Set.of(3000L, 4000L, 5000L, 6000L),
1142+
metadataFilePinnedTimestampCache
1143+
);
1144+
metadataFiles = metadataAndLocks.v1();
1145+
implicitLockedFiles = metadataAndLocks.v2();
1146+
assertEquals(4, implicitLockedFiles.size());
1147+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(2500L)));
1148+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(3001L)));
1149+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(4600L)));
1150+
assertTrue(implicitLockedFiles.contains(metadataFiles.get(5010L)));
1151+
assertEquals(3, metadataFilePinnedTimestampCache.size());
1152+
assertEquals(metadataFiles.get(2500L), metadataFilePinnedTimestampCache.get(3000L));
1153+
assertEquals(metadataFiles.get(3001L), metadataFilePinnedTimestampCache.get(4000L));
1154+
assertEquals(metadataFiles.get(4600L), metadataFilePinnedTimestampCache.get(5000L));
1155+
1156+
metadataAndLocks = testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1157+
List.of(),
1158+
Set.of(3000L, 4000L, 5000L, 6000L),
1159+
metadataFilePinnedTimestampCache
1160+
);
1161+
implicitLockedFiles = metadataAndLocks.v2();
1162+
assertEquals(0, implicitLockedFiles.size());
1163+
assertEquals(3, metadataFilePinnedTimestampCache.size());
1164+
1165+
assertThrows(
1166+
AssertionError.class,
1167+
() -> testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1168+
List.of(2000L, 2200L, 3001L, 4200L, 4600L, 5010L),
1169+
Set.of(3000L, 4000L, 5000L, 6000L),
1170+
metadataFilePinnedTimestampCache
1171+
)
1172+
);
1173+
1174+
metadataAndLocks = testGetPinnedTimestampLockedFilesWithPinnedTimestamps(
1175+
List.of(2000L, 2200L, 2500L, 3001L, 4200L, 4600L, 5010L),
1176+
Set.of(),
1177+
metadataFilePinnedTimestampCache
1178+
);
1179+
implicitLockedFiles = metadataAndLocks.v2();
1180+
assertEquals(0, implicitLockedFiles.size());
1181+
assertEquals(0, metadataFilePinnedTimestampCache.size());
1182+
}
10861183
}

0 commit comments

Comments
 (0)