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


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -199,4 +230,47 @@ protected boolean flushAndCommitNSToDB(Map<Long, 
NSSummary> nsSummaryMap) {
     }
     return true;
   }
+
+  /**
+   * Propagates size and count changes upwards through the parent chain.
+   * This ensures that when files are added/deleted, all ancestor directories
+   * reflect the total changes in their sizeOfFiles and numOfFiles fields.
+   */
+  protected void propagateSizeUpwards(long objectId, long sizeChange, 
+                                       long countChange, Map<Long, NSSummary> 
nsSummaryMap) 
+                                       throws IOException {
+    // Get the current directory's NSSummary
+    NSSummary nsSummary = nsSummaryMap.get(objectId);
+    if (nsSummary == null) {
+      nsSummary = reconNamespaceSummaryManager.getNSSummary(objectId);
+    }
+    if (nsSummary == null) {
+      return; // No more parents to update
+    }
+
+    // Continue propagating to parent
+    long parentId = nsSummary.getParentId();
+    if (parentId != 0) {
+      // Get parent's NSSummary
+      NSSummary parentSummary = nsSummaryMap.get(parentId);
+      if (parentSummary == null) {
+        parentSummary = reconNamespaceSummaryManager.getNSSummary(parentId);
+      }
+      if (parentSummary != null) {
+        // Update parent's totals
+        parentSummary.setSizeOfFiles(parentSummary.getSizeOfFiles() + 
sizeChange);
+        parentSummary.setNumOfFiles(parentSummary.getNumOfFiles() + 
(int)countChange);
+        int[] fileBucket = parentSummary.getFileSizeBucket();
+        int binIndex = ReconUtils.getFileSizeBinIndex(Math.abs(sizeChange));
+        ++fileBucket[binIndex];

Review Comment:
   Here this is always increment irrespective of `PUT` or `DELETE` event. By 
doing `Math.abs(sizeChange)` you correctly find the bin for a deleted file 
(since sizeChange is negative), but you still call ++fileBucket[...] even when 
you’ve removed a file in case of DELETE event. That will silently “add” a file 
to file size histogram when in fact it is  meant to remove one file. Am I 
missing something here ?



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskDbEventHandler.java:
##########
@@ -199,4 +230,47 @@ protected boolean flushAndCommitNSToDB(Map<Long, 
NSSummary> nsSummaryMap) {
     }
     return true;
   }
+
+  /**
+   * Propagates size and count changes upwards through the parent chain.
+   * This ensures that when files are added/deleted, all ancestor directories
+   * reflect the total changes in their sizeOfFiles and numOfFiles fields.
+   */
+  protected void propagateSizeUpwards(long objectId, long sizeChange, 
+                                       long countChange, Map<Long, NSSummary> 
nsSummaryMap) 
+                                       throws IOException {
+    // Get the current directory's NSSummary
+    NSSummary nsSummary = nsSummaryMap.get(objectId);
+    if (nsSummary == null) {
+      nsSummary = reconNamespaceSummaryManager.getNSSummary(objectId);
+    }
+    if (nsSummary == null) {
+      return; // No more parents to update
+    }
+
+    // Continue propagating to parent
+    long parentId = nsSummary.getParentId();
+    if (parentId != 0) {
+      // Get parent's NSSummary
+      NSSummary parentSummary = nsSummaryMap.get(parentId);
+      if (parentSummary == null) {
+        parentSummary = reconNamespaceSummaryManager.getNSSummary(parentId);
+      }
+      if (parentSummary != null) {
+        // Update parent's totals
+        parentSummary.setSizeOfFiles(parentSummary.getSizeOfFiles() + 
sizeChange);
+        parentSummary.setNumOfFiles(parentSummary.getNumOfFiles() + 
(int)countChange);
+        int[] fileBucket = parentSummary.getFileSizeBucket();
+        int binIndex = ReconUtils.getFileSizeBinIndex(Math.abs(sizeChange));

Review Comment:
   And shouldn't this bin identification be for effective size for that parent 
after this size change, instead of just size change ?



-- 
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: issues-unsubscr...@ozone.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org

Reply via email to