Copilot commented on code in PR #10890:
URL: https://github.com/apache/ozone/pull/10890#discussion_r3669115935
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java:
##########
@@ -1077,12 +1065,93 @@ void diffAllSnapshots(RocksDBCheckpointDiffer differ)
assertThat(actualFiles).containsExactlyInAnyOrderElementsOf(expectedFiles);
}
}
- // Guard against getSSTDiffList silently returning nothing for every input.
+ assertThat(validatedSnapshotPairs)
+ .as("expected compaction DAG diffs for at least one snapshot pair")
+ .isPositive();
assertThat(sawNonEmptyDiff)
.as("expected at least one non-empty SST diff across snapshots")
.isTrue();
}
+ private Set<String> allTablesForDiff() {
+ Set<String> tables = new HashSet<>(COLUMN_FAMILIES_TO_TRACK_IN_DAG);
+ tables.add("compactionLogTable");
+ return tables;
+ }
+
+ private List<SstFileInfo> getTrackedSstFilesFromSnapshot(DifferSnapshotInfo
snap) {
+ return snap.getSstFiles(0, allTablesForDiff());
+ }
+
+ /**
+ * Snapshot-only SST diff (same rules as {@code getSSTDiffList(...,
useCompactionDag=false)}).
+ */
+ private List<SstFileInfo> buildNonDagMetadataDiff(DifferSnapshotInfo srcSnap,
+ DifferSnapshotInfo destSnap, Set<String> tablesToLookup) {
+ Set<SstFileInfo> srcSstFileInfos = new HashSet<>(srcSnap.getSstFiles(0,
tablesToLookup));
+ Set<SstFileInfo> destSstFileInfos = new HashSet<>(destSnap.getSstFiles(0,
tablesToLookup));
+ Map<String, SstFileInfo> differentFiles = new HashMap<>();
+ for (SstFileInfo srcSstFileInfo : srcSstFileInfos) {
+ if (!destSstFileInfos.contains(srcSstFileInfo)) {
+ differentFiles.put(srcSstFileInfo.getFileName(), srcSstFileInfo);
+ }
+ }
+ for (SstFileInfo destSstFileInfo : destSstFileInfos) {
+ if (!srcSstFileInfos.contains(destSstFileInfo)) {
+ differentFiles.put(destSstFileInfo.getFileName(), destSstFileInfo);
+ }
+ }
+ return new ArrayList<>(differentFiles.values());
+ }
+
+ private static List<SstFileInfo> requireSstDiffList(
+ Optional<List<SstFileInfo>> diffList,
+ DifferSnapshotInfo src,
+ DifferSnapshotInfo dest) {
+ if (diffList.isPresent()) {
+ return diffList.get();
+ }
+ throw new AssertionError(String.format(
+ "getSSTDiffList returned empty Optional (DAG could not reach all
destination SSTs) "
+ + "from '%s' to '%s'", src.getDbPath(0), dest.getDbPath(0)));
+ }
+
+ private void assertCompactionSstBackups(RocksDBCheckpointDiffer differ)
throws IOException {
+ Set<String> tablesToLookup = allTablesForDiff();
+ DifferSnapshotInfo firstSnapshot = snapshots.get(0);
+ DifferSnapshotInfo lastSnapshot = snapshots.get(snapshots.size() - 1);
+ List<SstFileInfo> diffSinceFirst = requireSstDiffList(
+ differ.getSSTDiffList(
+ new DifferSnapshotVersion(lastSnapshot, 0, tablesToLookup),
+ new DifferSnapshotVersion(firstSnapshot, 0, tablesToLookup),
+ null, tablesToLookup, true),
+ lastSnapshot, firstSnapshot);
+ Set<String> lastSnapshotFileNames =
getTrackedSstFilesFromSnapshot(lastSnapshot).stream()
+ .map(SstFileInfo::getFileName)
+ .collect(Collectors.toSet());
+ Set<String> backupBaseNames;
+ try (Stream<Path> sstPathStream = Files.list(sstBackUpDir.toPath())) {
+ backupBaseNames = sstPathStream.map(path ->
getBaseName(path.getFileName().toString()))
+ .collect(Collectors.toSet());
+ assertThat(backupBaseNames).isNotEmpty();
+ assertThat(backupBaseNames).allMatch(name -> name.matches("\\d+"));
+ }
Review Comment:
The new backup-dir assertions no longer verify that the entries are actually
SST link files (eg `*.sst`) or that they correspond to tracked SSTs in the
compaction DAG. This weakens the “confirm correct links created” intent and
also contradicts the PR description’s “.sst suffix + membership in
getCompactionNodeMap()” checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
