SaketaChalamchala commented on code in PR #10363:
URL: https://github.com/apache/ozone/pull/10363#discussion_r3321094471
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java:
##########
@@ -1009,33 +1017,60 @@ private static List<ColumnFamilyDescriptor>
getColumnFamilyDescriptors() {
.map(ColumnFamilyDescriptor::new).collect(Collectors.toList());
}
+ /**
+ * Resolves column family for an SST id: compaction DAG first, then
snapshots.
+ *
+ * @return (true, cf) when the SST is known for this run (cf may be null);
+ * (false, ignored) when the id does not appear (e.g. different SST
numbering than the hard-coded list).
+ */
+ private Pair<Boolean, String>
resolveColumnFamilyForDiffFile(RocksDBCheckpointDiffer differ, String diffFile,
+ DifferSnapshotInfo srcSnap, DifferSnapshotInfo destSnap) {
+ CompactionNode node = differ.getCompactionNodeMap().get(diffFile);
+ if (node != null) {
+ return Pair.of(true, node.getColumnFamily());
+ }
+ SstFileInfo meta = srcSnap.getSstFile(0, diffFile);
+ if (meta == null) {
+ meta = destSnap.getSstFile(0, diffFile);
+ }
+ if (meta == null) {
+ for (DifferSnapshotInfo s : snapshots) {
+ meta = s.getSstFile(0, diffFile);
+ if (meta != null) {
+ break;
+ }
+ }
+ }
+ if (meta == null) {
+ return Pair.of(false, null);
+ }
+ return Pair.of(true, meta.getColumnFamily());
+ }
+
/**
* Test SST differ.
*/
void diffAllSnapshots(RocksDBCheckpointDiffer differ)
throws IOException {
final DifferSnapshotInfo src = snapshots.get(snapshots.size() - 1);
- // Hard-coded expected output.
- // The results are deterministic. Retrieved from a successful run.
- final List<List<String>> expectedDifferResult = asList(
- asList("000023", "000029", "000026", "000019", "000021", "000031"),
- asList("000023", "000029", "000026", "000021", "000031"),
- asList("000023", "000029", "000026", "000031"),
- asList("000029", "000026", "000031"),
- asList("000029", "000031"),
- Collections.singletonList("000031"),
- Collections.emptyList()
- );
- assertEquals(snapshots.size(), expectedDifferResult.size());
-
- int index = 0;
List<String> expectedDiffFiles = new ArrayList<>();
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");
+ Set<String> fullTableToLookUp = new HashSet<>(tablesToTrack);
+ List<String> baselineDiffFileNames =
+ differ.getSSTDiffList(
+ new DifferSnapshotVersion(src, 0, fullTableToLookUp),
+ new DifferSnapshotVersion(snap, 0, fullTableToLookUp),
+ null, fullTableToLookUp, true)
+ .orElse(Collections.emptyList())
+ .stream()
+ .map(SstFileInfo::getFileName)
+ .collect(Collectors.toList());
Review Comment:
Thanks for the patch @arunsarin85. Constructing the
baselineDiffFileNames(from which expectedDiffFiles are derived) using the same
method `differ.getSSTDiffList()` that the `actualNames` is constructed changes
the intention of the test.
This test is meant to verify the correctness of `differ.getSSTDiffList()` so
the expected diff list needs to be built independently of
`differ.getSSTDiffList()`
Maybe a better approach would be:
Pseudocode:
```
setup:
open active DB with auto-compactions disabled.
cfOpts.setDisableAutoCompactions(true);
write_and_checkpoint():
// First snapshot
Insert keys
snap-1 = create_checkpoint()
snap_1_files = sst_file_list(snap_1) // grouped by key, file,
directorytable column families
// For the rest of the snapshots
Insert keys
// Before taking the snapshot flush all txns, track all uncompacted
SST files, run manual compaction then take a snapshot
activeRocksDB.get().flush()
snap_n_files = sst_file_list(activeRocksDB) // grouped by key, file,
directorytable column families
compactOptions.setBottommostLevelCompaction(kForce)
activeRocksDB.get().compactRange(key/file/directoryTable, null, null,
compactOptions)
snap-n = create_checkpoint()
build_expected_diffs_between_adjacent_snaps():
expected_diff_between_snap-2_snap-1 = snap_2_files - snap_1_files
expected_diff_between_snap-3_snap-2 = snap_3_files - snap_2_files
...
expected_diff_between_snap-n_snap-n-1 = snap_n_files - snap_n-1_files
diffAllSnapshots:
build_expected_diffs_between_adjacent_snaps()
for each snap_n and snap_prev from (snap_1 .. snap_n-1);
actual_diff = differ.getSSTDiffList(snap_n, snap_prev)
expected_diff = expected_diff_between_snap-n_snap-n-1 +
expected_diff_between_snap-n-1_snap-n-2 + .... +
expected_diff_between_snap-prev+1_expected_diff_between_snap-prev
assert expected_diff == actual_diff
```
--
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]
