devmadhuu commented on code in PR #10321:
URL: https://github.com/apache/ozone/pull/10321#discussion_r3311184404


##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestNSSummaryTask.java:
##########
@@ -216,5 +239,24 @@ public void testProcessBucket() throws IOException {
         assertEquals(0, fileSizeDist[i]);
       }
     }
+
+    @Test
+    public void testProcessObsBucket() {
+      // bucket 3 (OBS) had file3 from reprocess; the batch added file4.
+      assertEquals(2, nsSummaryForBucket3.getNumOfFiles());
+      assertEquals(KEY_THREE_SIZE + KEY_FOUR_SIZE,
+          nsSummaryForBucket3.getSizeOfFiles());
+    }
+
+    @Test
+    public void testProcessTaskResult() {
+      // Sub-task seek positions must be reported for all three layouts so the
+      // dispatcher can resume each sub-task independently on retry.
+      assertNotNull(processResult);
+      assertTrue(processResult.isTaskSuccess());
+      
assertNotNull(processResult.getSubTaskSeekPositions().get(NSSummaryTask.BucketType.FSO.name()));
+      
assertNotNull(processResult.getSubTaskSeekPositions().get(NSSummaryTask.BucketType.LEGACY.name()));
+      
assertNotNull(processResult.getSubTaskSeekPositions().get(NSSummaryTask.BucketType.OBS.name()));
+    }

Review Comment:
   Can we add test for parallel dispatch correctness under mixed FSO/Legacy/OBS 
batches beyond the happy path (the new OBS test helps).



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -43,6 +45,14 @@ public class NSSummaryTaskDbEventHandler {
   private ReconNamespaceSummaryManager reconNamespaceSummaryManager;
   private ReconOMMetadataManager reconOMMetadataManager;
 
+  // Bucket layout never changes for an existing bucket, so cache OmBucketInfo
+  // lookups across process() calls. Each delta loop hits at most a few 
buckets;
+  // without this cache, every event pays a RocksDB point read in the Legacy 
and
+  // OBS sub-tasks.
+  //
+  // Single-thread access only (one dispatcher thread per task). HashMap is 
fine.
+  private final Map<String, OmBucketInfo> bucketInfoCache = new HashMap<>();

Review Comment:
   This cache assumes that bucket layout/object ID are immutable, which holds 
for an existing bucket. It does not hold if a bucket is deleted and recreated 
under the same volume/bucket name (new object ID, same DB key). After the first 
lookup, `lookupBucketCached()` would return stale `OmBucketInfo` indefinitely.



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