This is an automated email from the ASF dual-hosted git repository.
jojochuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 47d93f181cc HDDS-15209. Stabilize
TestRocksDBCheckpointDiffer#testDifferWithDB for RocksDB layout variance
(#11324)
47d93f181cc is described below
commit 47d93f181cc1745c39d5df1b17c4a7ebb7f65d2c
Author: Arun Sarin <[email protected]>
AuthorDate: Mon Sep 28 22:29:53 2026 +0530
HDDS-15209. Stabilize TestRocksDBCheckpointDiffer#testDifferWithDB for
RocksDB layout variance (#11324)
---
.../rocksdiff/TestRocksDBCheckpointDiffer.java | 107 +++++++++++++--------
1 file changed, 65 insertions(+), 42 deletions(-)
diff --git
a/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java
b/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java
index fff123043e9..506fa9ea6aa 100644
---
a/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java
+++
b/hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java
@@ -20,6 +20,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Arrays.asList;
import static java.util.concurrent.TimeUnit.MINUTES;
+import static org.apache.commons.io.FilenameUtils.getBaseName;
import static org.apache.hadoop.hdds.StringUtils.bytes2String;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_SNAPSHOT_COMPACTION_DAG_MAX_TIME_ALLOWED;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_SNAPSHOT_COMPACTION_DAG_MAX_TIME_ALLOWED_DEFAULT;
@@ -115,7 +116,6 @@
import
org.apache.ozone.rocksdiff.RocksDBCheckpointDiffer.DifferSnapshotVersion;
import org.apache.ozone.rocksdiff.RocksDBCheckpointDiffer.NodeComparator;
import org.apache.ozone.test.GenericTestUtils;
-import org.apache.ozone.test.tag.Flaky;
import org.apache.ratis.util.UncheckedAutoCloseable;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
@@ -956,12 +956,13 @@ public void testGetSSTDiffListWithoutDB(String
description,
* Does actual DB write, flush, compaction.
*/
@Test
- @Flaky("HDDS-15209")
void testDifferWithDB() throws Exception {
writeKeysAndCheckpointing();
readRocksDBInstance(ACTIVE_DB_DIR_NAME, activeRocksDB, null,
rocksDBCheckpointDiffer);
+ GenericTestUtils.waitFor(() ->
rocksDBCheckpointDiffer.getInflightCompactions().isEmpty(), 1000, 10000);
+
if (LOG.isDebugEnabled()) {
printAllSnapshots();
}
@@ -971,24 +972,12 @@ void testDifferWithDB() throws Exception {
rocksDBCheckpointDiffer.getForwardCompactionDAG());
diffAllSnapshots(rocksDBCheckpointDiffer);
+ assertCompactionSstBackups(rocksDBCheckpointDiffer);
- // Confirm correct links created
- try (Stream<Path> sstPathStream = Files.list(sstBackUpDir.toPath())) {
- List<String> actualLinks = sstPathStream.map(Path::getFileName)
- .map(Object::toString).sorted().collect(Collectors.toList());
- assertThat(actualLinks).hasSize(7);
- assertThat(actualLinks).allMatch(link -> link.matches("\\d{6}\\.sst"));
- for (String linkName : actualLinks) {
- assertTrue(Files.size(sstBackUpDir.toPath().resolve(linkName)) > 0,
- "SST link should not be empty: " + linkName);
- }
- }
rocksDBCheckpointDiffer.getForwardCompactionDAG().nodes().stream().forEach(compactionNode
-> {
Assertions.assertNotNull(compactionNode.getStartKey());
Assertions.assertNotNull(compactionNode.getEndKey());
});
- GenericTestUtils.waitFor(() ->
rocksDBCheckpointDiffer.getInflightCompactions().isEmpty(), 1000,
- 10000);
if (LOG.isDebugEnabled()) {
rocksDBCheckpointDiffer.dumpCompactionNodeTable();
}
@@ -1006,35 +995,28 @@ private static List<ColumnFamilyDescriptor>
getColumnFamilyDescriptors() {
void diffAllSnapshots(RocksDBCheckpointDiffer differ)
throws IOException {
final DifferSnapshotInfo src = snapshots.get(snapshots.size() - 1);
+ Set<String> allTables = allTablesForDiff();
boolean sawNonEmptyDiff = false;
+
+ // Each test snapshot only stores SST metadata for checkpoint version 0
(see createCheckpoint).
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(
+ List<SstFileInfo> fullDagDiff = 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).orElseThrow();
+ 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<>(allTables);
+
Set<String> tableToLookUp = new HashSet<>();
for (int i = 0; i < Math.pow(2, tablesToTrack.size()); i++) {
tableToLookUp.clear();
@@ -1046,15 +1028,12 @@ void diffAllSnapshots(RocksDBCheckpointDiffer differ)
}
DifferSnapshotVersion srcSnapVersion = new DifferSnapshotVersion(src,
0, tableToLookUp);
DifferSnapshotVersion destSnapVersion = new
DifferSnapshotVersion(snap, 0, tableToLookUp);
- List<SstFileInfo> sstDiffList = differ.getSSTDiffList(srcSnapVersion,
destSnapVersion, null,
- tableToLookUp, true).orElse(Collections.emptyList());
+ List<SstFileInfo> sstDiffList = differ.getSSTDiffList(
+ srcSnapVersion, destSnapVersion, null, tableToLookUp,
true).orElseThrow();
LOG.info("SST diff list from '{}' to '{}': {} tables: {}",
src.getDbPath(0), snap.getDbPath(0), sstDiffList, tableToLookUp);
- // Expected files: baseline entries whose column family is untracked
- // (null) or included in this subset. getSSTDiffList returns the values
- // of a HashMap, so its ordering is not guaranteed; compare as sets.
- List<String> expectedFiles = baseline.stream()
+ List<String> expectedFiles = fullDagDiff.stream()
.filter(sstFileInfo -> sstFileInfo.getColumnFamily() == null
|| tableToLookUp.contains(sstFileInfo.getColumnFamily()))
.map(SstFileInfo::getFileName)
@@ -1065,12 +1044,55 @@ void diffAllSnapshots(RocksDBCheckpointDiffer differ)
assertThat(actualFiles).containsExactlyInAnyOrderElementsOf(expectedFiles);
}
}
- // Guard against getSSTDiffList silently returning nothing for every input.
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 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 = differ.getSSTDiffList(
+ new DifferSnapshotVersion(lastSnapshot, 0, tablesToLookup),
+ new DifferSnapshotVersion(firstSnapshot, 0, tablesToLookup),
+ null, tablesToLookup, true).orElseThrow();
+ // Version 0 is the only RocksDB checkpoint layer we record for each
snapshot in this test.
+ Set<String> lastSnapshotFileNames = lastSnapshot.getSstFiles(0,
tablesToLookup).stream()
+ .map(SstFileInfo::getFileName)
+ .collect(Collectors.toSet());
+ Set<String> diffNotInLastSnapshot = diffSinceFirst.stream()
+ .map(SstFileInfo::getFileName)
+ .filter(name -> !lastSnapshotFileNames.contains(name))
+ .collect(Collectors.toSet());
+ ConcurrentMap<String, CompactionNode> compactionNodes =
differ.getCompactionNodeMap();
+ Set<String> backupBaseNames;
+ try (Stream<Path> sstPathStream = Files.list(sstBackUpDir.toPath())) {
+ List<Path> backupPaths = sstPathStream.collect(Collectors.toList());
+ backupBaseNames = backupPaths.stream()
+ .map(path -> getBaseName(path.getFileName().toString()))
+ .collect(Collectors.toSet());
+ assertThat(backupBaseNames).hasSizeGreaterThanOrEqualTo(7);
+ assertThat(backupPaths).allMatch(path ->
path.getFileName().toString().matches("\\d+\\.sst"));
+ for (Path path : backupPaths) {
+ assertTrue(Files.size(path) > 0, "SST link should not be empty: " +
path);
+ }
+ }
+ assertThat(backupBaseNames)
+ .as("SSTs compacted away since the first snapshot should be backed up")
+ .containsAll(diffNotInLastSnapshot);
+ for (String backupName : backupBaseNames) {
+ assertTrue(compactionNodes.containsKey(backupName),
+ () -> "Backup " + backupName + " should match a tracked compaction
SST");
+ }
+ }
+
/**
* Helper function that creates an RDB checkpoint (= Ozone snapshot).
*/
@@ -1094,6 +1116,7 @@ private void createCheckpoint(ManagedRocksDB rocksDB)
throws RocksDBException {
List<ColumnFamilyHandle> colHandle = new ArrayList<>();
try (ManagedRocksDB rdb = ManagedRocksDB.openReadOnly(cpPath,
getColumnFamilyDescriptors(), colHandle)) {
TreeMap<Integer, List<SstFileInfo>> versionSstFilesMap = new TreeMap<>();
+ // OM snapshots can track multiple linked DB checkpoints; this test
builds just one (version 0).
versionSstFilesMap.put(0,
rdb.getLiveMetadataForSSTFiles().values().stream().map(SstFileInfo::new)
.collect(Collectors.toList()));
final DifferSnapshotInfo currentSnapshot = new
DifferSnapshotInfo((version) -> Paths.get(cpPath),
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
