smengcl commented on code in PR #3981:
URL: https://github.com/apache/ozone/pull/3981#discussion_r1035335164


##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java:
##########
@@ -90,8 +97,141 @@ public static void init() {
     GenericTestUtils.setLogLevel(TestRocksDBCheckpointDiffer.LOG, Level.INFO);
   }
 
+  /**
+   * Tests core SST diff list logic. Does not involve DB.
+   * Focuses on testing edge cases in internalGetSSTDiffList().
+   */
   @Test
-  void testMain() throws Exception {
+  void testGetSSTDiffListWithoutDB() {
+
+    RocksDBCheckpointDiffer differ = new RocksDBCheckpointDiffer();
+
+    // Construct DAG from compaction log input
+    final String compactionLog1 = ""
+        + "S 1000 df6410c7-151b-4e90-870e-5ef12875acd5\n"  // Snapshot 0
+        + "C 000001,000002:000062\n"
+            // Additional "compaction" to trigger and test early exit condition
+        + "S 3008 ef6410c7-151b-4e90-870e-5ef12875acd5\n"  // Snapshot 1
+        + "C 000068,000062:000069\n"  // Regular compaction
+        + "C 000071,000064,000060,000052:000071,000064,000060,000052\n"
+            // Trivial move
+        + "C 000073,000066:000074\n"
+        + "C 000082,000076,000069:000083\n"
+        + "C 000087,000080,000074:000088\n"
+        + "C 000093,000090,000083:\n"  // Deletion ?
+        + "S 14980 e7ad72f8-52df-4430-93f6-0ee91d4a47fd\n"  // Snapshot 2
+        + "C 000098,000096,000085,000078,000071,000064,000060,000052:000099\n"
+        + "C 000105,000095,000088:000107\n"

Review Comment:
   Hmm. I took the compaction log from `TestOMSnapshotDAG` where I set a 
breakpoint right before OM is restarted (`cluster.restartOzoneManager()`).
   
   It looks like this compaction is indeed not captured in snapshot 3. As I 
checked the active DB, it seems the compaction happens after snapshot 3 is 
taken.
   
   So it seems when the compaction log is being appended (even in 
`onCompactionCompleted`), RocksDB hasn't really completed the flush (and 
updated the meta info). Thus, the immediately following DB checkpoint operation 
wouldn't pick it up.



##########
hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java:
##########
@@ -90,8 +97,141 @@ public static void init() {
     GenericTestUtils.setLogLevel(TestRocksDBCheckpointDiffer.LOG, Level.INFO);
   }
 
+  /**
+   * Tests core SST diff list logic. Does not involve DB.
+   * Focuses on testing edge cases in internalGetSSTDiffList().
+   */
   @Test
-  void testMain() throws Exception {
+  void testGetSSTDiffListWithoutDB() {
+
+    RocksDBCheckpointDiffer differ = new RocksDBCheckpointDiffer();
+
+    // Construct DAG from compaction log input
+    final String compactionLog1 = ""
+        + "S 1000 df6410c7-151b-4e90-870e-5ef12875acd5\n"  // Snapshot 0
+        + "C 000001,000002:000062\n"
+            // Additional "compaction" to trigger and test early exit condition
+        + "S 3008 ef6410c7-151b-4e90-870e-5ef12875acd5\n"  // Snapshot 1
+        + "C 000068,000062:000069\n"  // Regular compaction
+        + "C 000071,000064,000060,000052:000071,000064,000060,000052\n"
+            // Trivial move
+        + "C 000073,000066:000074\n"
+        + "C 000082,000076,000069:000083\n"
+        + "C 000087,000080,000074:000088\n"
+        + "C 000093,000090,000083:\n"  // Deletion ?
+        + "S 14980 e7ad72f8-52df-4430-93f6-0ee91d4a47fd\n"  // Snapshot 2
+        + "C 000098,000096,000085,000078,000071,000064,000060,000052:000099\n"
+        + "C 000105,000095,000088:000107\n"

Review Comment:
   Though this (unexpected ordering) alone shouldn't affect the correctness of 
the differ since we currently read the whole compaction log and reconstruct the 
DAG upon OM restarts, I'm curious what is actually happening.



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