sumitagrawl commented on code in PR #4626:
URL: https://github.com/apache/ozone/pull/4626#discussion_r1222995639


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OrphanMetaDataManagementTask.java:
##########
@@ -148,19 +158,53 @@ public Pair<String, Boolean> process(OMUpdateEventBatch 
events) {
 
   private void handleBucketDeleteEvent(OmBucketInfo updatedBucketInfo) {
     long objectID = updatedBucketInfo.getObjectID();
-    removeOrphanAndNSSummaryParentEntry(objectID);
+    try {
+      NSSummary nsSummary = 
reconNamespaceSummaryManager.getNSSummary(objectID);
+      if (null != nsSummary) {
+        if (nsSummary.getChildDir().size() == 0 &&
+            nsSummary.getNumOfFiles() == 0) {
+          removeOrphanMetaData(objectID);

Review Comment:
   the handling should be similar to handleDeleteDirEvent, where if bucket is 
removed and not empty, then need add those in orphan map.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OrphanMetaDataManagementTask.java:
##########
@@ -148,19 +158,53 @@ public Pair<String, Boolean> process(OMUpdateEventBatch 
events) {
 
   private void handleBucketDeleteEvent(OmBucketInfo updatedBucketInfo) {
     long objectID = updatedBucketInfo.getObjectID();
-    removeOrphanAndNSSummaryParentEntry(objectID);
+    try {
+      NSSummary nsSummary = 
reconNamespaceSummaryManager.getNSSummary(objectID);
+      if (null != nsSummary) {
+        if (nsSummary.getChildDir().size() == 0 &&
+            nsSummary.getNumOfFiles() == 0) {
+          removeOrphanMetaData(objectID);
+        }
+      }
+    } catch (IOException e) {
+      // Logging as Info as we don't want to log as error when any dir not
+      // found in orphan candidate metadata set. This is done to avoid 2
+      // rocks DB operations - check if present and then delete operation.
+      LOG.info("ObjectId {} may not be found in orphan metadata table.",
+          objectID);
+    }
   }
 
-  private void removeOrphanAndNSSummaryParentEntry(long objectID) {
+  private void removeOrphanMetaData(long objectID) {
     try {
       OrphanKeyMetaData orphanKeyMetaData =
           orphanKeysMetaDataTable.get(objectID);
       if (null != orphanKeyMetaData) {
         orphanKeysMetaDataTable.delete(objectID);
       }
+    } catch (IOException e) {
+      // Logging as Info as we don't want to log as error when any dir not
+      // found in orphan candidate metadata set. This is done to avoid 2
+      // rocks DB operations - check if present and then delete operation.
+      LOG.info("ObjectId {} may not be found in orphan metadata table.",

Review Comment:
   recheck if delete of key which does not exist throw IOException or not, I 
think should throw exception or failure handling.



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OrphanMetaDataManagementTask.java:
##########
@@ -171,9 +215,40 @@ private void removeOrphanAndNSSummaryParentEntry(long 
objectID) {
     }
   }
 
-  private void handlePutDeleteDirEvent(OmKeyInfo updatedKeyInfo) {
+  private void addToOrphanMetaData(OmKeyInfo updatedKeyInfo,
+                                   NSSummary nsSummary) {
     long objectID = updatedKeyInfo.getObjectID();
-    removeOrphanAndNSSummaryParentEntry(objectID);
+    long parentObjectID = updatedKeyInfo.getParentObjectID();
+    String delDirName = updatedKeyInfo.getKeyName();
+    try {
+      OrphanKeyMetaData orphanKeyMetaData =
+          reconNamespaceSummaryManager.getOrphanKeyMetaData(parentObjectID);
+      if (null != orphanKeyMetaData) {
+        Set<Long> objectIds = orphanKeyMetaData.getObjectIds();
+        objectIds.remove(objectID);
+      } else {
+        final String[] keys = delDirName.split(OM_KEY_PREFIX);

Review Comment:
   2 action continuous (not as if/else, 
   - remove itself from its parent as orphan
   - add itself as orphan if have files/objects as children



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OrphanMetaDataManagementTask.java:
##########
@@ -171,9 +215,40 @@ private void removeOrphanAndNSSummaryParentEntry(long 
objectID) {
     }
   }
 
-  private void handlePutDeleteDirEvent(OmKeyInfo updatedKeyInfo) {
+  private void addToOrphanMetaData(OmKeyInfo updatedKeyInfo,
+                                   NSSummary nsSummary) {
     long objectID = updatedKeyInfo.getObjectID();
-    removeOrphanAndNSSummaryParentEntry(objectID);
+    long parentObjectID = updatedKeyInfo.getParentObjectID();
+    String delDirName = updatedKeyInfo.getKeyName();
+    try {
+      OrphanKeyMetaData orphanKeyMetaData =
+          reconNamespaceSummaryManager.getOrphanKeyMetaData(parentObjectID);
+      if (null != orphanKeyMetaData) {
+        Set<Long> objectIds = orphanKeyMetaData.getObjectIds();
+        objectIds.remove(objectID);
+      } else {
+        final String[] keys = delDirName.split(OM_KEY_PREFIX);

Review Comment:
   remove itself as parent to be done outside nssummary check at caller



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OrphanMetaDataManagementTask.java:
##########
@@ -171,9 +215,40 @@ private void removeOrphanAndNSSummaryParentEntry(long 
objectID) {
     }
   }
 
-  private void handlePutDeleteDirEvent(OmKeyInfo updatedKeyInfo) {
+  private void addToOrphanMetaData(OmKeyInfo updatedKeyInfo,
+                                   NSSummary nsSummary) {
     long objectID = updatedKeyInfo.getObjectID();
-    removeOrphanAndNSSummaryParentEntry(objectID);
+    long parentObjectID = updatedKeyInfo.getParentObjectID();
+    String delDirName = updatedKeyInfo.getKeyName();
+    try {
+      OrphanKeyMetaData orphanKeyMetaData =
+          reconNamespaceSummaryManager.getOrphanKeyMetaData(parentObjectID);
+      if (null != orphanKeyMetaData) {
+        Set<Long> objectIds = orphanKeyMetaData.getObjectIds();
+        objectIds.remove(objectID);

Review Comment:
   DB update is not done for objectId removed from orpahanMap in if case



##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OrphanMetaDataManagementTask.java:
##########
@@ -187,4 +262,36 @@ public Pair<String, Boolean> reprocess(OMMetadataManager 
omMetadataManager) {
     return new ImmutablePair<>(getTaskName(), true);
   }
 
+  private List<OmKeyInfo> getPendingDeletionSubFiles(
+      long volumeId, long bucketId, OmKeyInfo parentInfo)
+      throws IOException {
+    List<OmKeyInfo> files = new ArrayList<>();
+    String seekFileInDB =
+        reconOMMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentInfo.getObjectID(), "");
+
+    Table fileTable = reconOMMetadataManager.getFileTable();
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+             iterator = fileTable.iterator()) {
+
+      iterator.seek(seekFileInDB);
+
+      while (iterator.hasNext()) {
+        Table.KeyValue<String, OmKeyInfo> entry = iterator.next();
+        OmKeyInfo fileInfo = entry.getValue();
+        if (!OMFileRequest.isImmediateChild(fileInfo.getParentObjectID(),
+            parentInfo.getObjectID())) {
+          break;
+        }
+        fileInfo.setFileName(fileInfo.getKeyName());
+        String fullKeyPath = OMFileRequest.getAbsolutePath(
+            parentInfo.getKeyName(), fileInfo.getKeyName());
+        fileInfo.setKeyName(fullKeyPath);
+
+        files.add(fileInfo);

Review Comment:
   we can just capture objectId only as required, this to reduce memory 
required at end of iteration.



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