hemantk-12 commented on code in PR #5317:
URL: https://github.com/apache/ozone/pull/5317#discussion_r1337990116
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java:
##########
@@ -150,10 +151,15 @@ public RDBStore(File dbFile, ManagedDBOptions dbOptions,
// Set CF handle in differ to be used in DB listener
rocksDBCheckpointDiffer.setSnapshotInfoTableCFHandle(
ssInfoTableCF.getHandle());
- // Finish the initialization of compaction DAG tracker by setting the
- // sequence number as current compaction log filename.
- rocksDBCheckpointDiffer.setCurrentCompactionLog(
- db.getLatestSequenceNumber());
+ // Set CF handle in differ to be store compaction log entry.
+ ColumnFamily compactionLogTableTableCF =
Review Comment:
Fixed.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -116,16 +122,6 @@ public class RocksDBCheckpointDiffer implements
AutoCloseable,
private final String compactionLogDir;
- /**
- * Compaction log path for DB compaction history persistence.
- * This is the source of truth for in-memory SST DAG reconstruction upon
- * OM restarts.
- * <p>
- * Initialized to the latest sequence number on OM startup. The log also
rolls
- * over (gets appended to a new file) whenever an Ozone snapshot is taken.
- */
- private volatile String currentCompactionLogPath = null;
Review Comment:
We can't because we need to read exiting compaction log files for backward
compatibility.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotBackgroundServices.java:
##########
@@ -404,36 +403,48 @@ public void testCompactionLogBackgroundService()
cluster.getOzoneManager(leaderOM.getOMNodeId());
Assertions.assertEquals(leaderOM, newFollowerOM);
- // Prepare baseline data for compaction logs
- String currentCompactionLogPath = newLeaderOM
- .getMetadataManager()
- .getStore()
- .getRocksDBCheckpointDiffer()
- .getCurrentCompactionLogPath();
- Assertions.assertNotNull(currentCompactionLogPath);
- int lastIndex = currentCompactionLogPath.lastIndexOf(OM_KEY_PREFIX);
- String compactionLogsPath = currentCompactionLogPath
- .substring(0, lastIndex);
- File compactionLogsDir = new File(compactionLogsPath);
- Assertions.assertNotNull(compactionLogsDir);
- File[] files = compactionLogsDir.listFiles();
- Assertions.assertNotNull(files);
- int numberOfLogFiles = files.length;
- long contentLength;
- Path currentCompactionLog = Paths.get(currentCompactionLogPath);
- try (BufferedReader bufferedReader =
- Files.newBufferedReader(currentCompactionLog)) {
- contentLength = bufferedReader.lines()
- .mapToLong(String::length)
- .reduce(0L, Long::sum);
- }
+ List<CompactionLogEntry> compactionLogEntriesOnPreviousLeader =
+ getCompactionLogEntries(leaderOM);
+
+ List<CompactionLogEntry> compactionLogEntriesOnNewLeader =
+ getCompactionLogEntries(newLeaderOM);
+ Assertions.assertEquals(compactionLogEntriesOnPreviousLeader,
+ compactionLogEntriesOnNewLeader);
+
+ Assertions.assertEquals(leaderOM.getMetadataManager().getStore()
Review Comment:
Yes, that's correct understanding.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -605,19 +530,40 @@ public void onCompactionCompleted(RocksDB db,
waitForTarballCreation();
- // Write input and output file names to compaction log
- appendToCurrentCompactionLog(content);
+
+ // Add the compaction log entry to Compaction log table.
+ addToCompactionLogTable(compactionLogEntry);
// Populate the DAG
// TODO: [SNAPSHOT] Once SnapshotChainManager is put into use,
// set snapshotID to snapshotChainManager.getLatestGlobalSnapshot()
- populateCompactionDAG(inputFiles, outputFiles, null,
+ populateCompactionDAG(compactionLogEntry.getInputFileInfoList(),
+ compactionLogEntry.getOutputFileInfoList(), null,
db.getLatestSequenceNumber());
}
}
};
}
+ @VisibleForTesting
+ void addToCompactionLogTable(CompactionLogEntry compactionLogEntry) {
+ // Key in the transactionId-currentTime
Review Comment:
It should not be because `addToCompactionLogTable` is called in
[synchronized
block](https://github.com/apache/ozone/blob/5f2c271524b60fd59266f92d72a763b8a97b7259/hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java#L520)
during RocksDB compaction.
--
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]
