arunsarin85 commented on code in PR #11324:
URL: https://github.com/apache/ozone/pull/11324#discussion_r4120086861


##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java:
##########
@@ -1006,35 +997,36 @@ private static List<ColumnFamilyDescriptor> 
getColumnFamilyDescriptors() {
   void diffAllSnapshots(RocksDBCheckpointDiffer differ)
       throws IOException {
     final DifferSnapshotInfo src = snapshots.get(snapshots.size() - 1);
+    Set<String> allTables = allTablesForDiff();
+    int validatedSnapshotPairs = 0;
     boolean sawNonEmptyDiff = false;
-    for (DifferSnapshotInfo snap : snapshots) {
-      // Returns a list of SST files to be fed into RocksCheckpointDiffer Dag.
-      List<String> tablesToTrack = new 
ArrayList<>(COLUMN_FAMILIES_TO_TRACK_IN_DAG);
-      // Add some invalid index.
-      tablesToTrack.add("compactionLogTable");
 
-      // Baseline diff when tracking every table. A subset's diff must equal
-      // this baseline filtered to the subset's column families (files with no
-      // column family are always kept). This relationship is deterministic and
-      // stable across RocksDB versions, unlike hard-coded SST file names.
-      Set<String> allTables = new HashSet<>(tablesToTrack);
-      List<SstFileInfo> baseline = differ.getSSTDiffList(
+    for (DifferSnapshotInfo snap : snapshots) {
+      Optional<List<SstFileInfo>> fullDagDiffOpt = differ.getSSTDiffList(
           new DifferSnapshotVersion(src, 0, allTables),
           new DifferSnapshotVersion(snap, 0, allTables),
-          null, allTables, true).orElse(Collections.emptyList());
-      sawNonEmptyDiff = sawNonEmptyDiff || !baseline.isEmpty();
-
-      // Independent structural oracle, not derived from getSSTDiffList's own
-      // output: a snapshot diffed against itself must have no differing SST
-      // files. Together with the sawNonEmptyDiff guard below, this bounds a
-      // systematically broken diff in both directions (returning nothing, or
-      // returning files even for identical snapshots).
+          null, allTables, true);
+      if (!fullDagDiffOpt.isPresent()) {
+        LOG.info("Skipping DAG diff to '{}' because compaction DAG could not 
reach all "
+            + "destination SST files", snap.getDbPath(0));
+        continue;
+      }
+      validatedSnapshotPairs++;
+      List<SstFileInfo> fullDagDiff = fullDagDiffOpt.get();
+      sawNonEmptyDiff = sawNonEmptyDiff || !fullDagDiff.isEmpty();
+
+      // Independent structural oracle: a snapshot diffed against itself must 
have no
+      // differing SST files. Together with sawNonEmptyDiff below, this bounds 
a broken
+      // diff in both directions (returning nothing, or returning files for 
identical snapshots).
       if (snap == src) {
-        assertThat(baseline)
+        assertThat(fullDagDiff)
             .as("diff of a snapshot against itself must be empty")
             .isEmpty();
       }
 
+      List<String> tablesToTrack = new 
ArrayList<>(COLUMN_FAMILIES_TO_TRACK_IN_DAG);
+      tablesToTrack.add("compactionLogTable");

Review Comment:
   Updated to new ArrayList<>(allTables) so subset enumeration stays aligned 
with allTablesForDiff().



-- 
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]

Reply via email to