swamirishi commented on code in PR #8587:
URL: https://github.com/apache/ozone/pull/8587#discussion_r2411612197


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -194,35 +196,47 @@ private Pair<Pair<Integer, Long>, Boolean> 
submitPurgeKeysRequest(
       String deletedKey = result.getObjectKey();
       if (result.isSuccess()) {
         // Add key to PurgeKeys list.
-        if (keysToModify != null && !keysToModify.containsKey(deletedKey)) {
-          // Parse Volume and BucketName
-          purgeKeys.add(deletedKey);
+        if (keysToModify == null || !keysToModify.containsKey(deletedKey)) {
+          completePurgedKeys.add(deletedKey);
           if (LOG.isDebugEnabled()) {
-            LOG.debug("Key {} set to be updated in OM DB, Other versions " +
-                "of the key that are reclaimable are reclaimed.", deletedKey);
+            LOG.debug("Key {} set to be purged from OM DB", deletedKey);
           }
-        } else if (keysToModify == null) {
-          purgeKeys.add(deletedKey);
+        } else {
           if (LOG.isDebugEnabled()) {
-            LOG.debug("Key {} set to be purged from OM DB", deletedKey);
+            LOG.debug("Key {} set to be updated in OM DB, Other versions " +
+                "of the key that are reclaimable are reclaimed.", deletedKey);
           }
         }
-        if (keyBlockReplicatedSize != null) {
-          deletedReplSize += keyBlockReplicatedSize.getOrDefault(deletedKey, 
0L);
+        if (purgedKeys.containsKey(deletedKey)) {
+          deletedReplSize += purgedKeys.get(deletedKey).getPurgedBytes();
         }
         deletedCount++;
       } else {
-        // If the block deletion failed, then the deleted keys should also not 
be modified.
+        // If the block deletion failed, then the deleted keys should also not 
be modified and
+        // any other version of the key should also not be purged.
         failedDeletedKeys.add(deletedKey);
         purgeSuccess = false;
+        if (LOG.isDebugEnabled()) {
+          LOG.error("Failed Block Delete corresponding to Key {} with block 
result : {}.", deletedKey,
+              result.getBlockResultList());
+        } else {
+          LOG.error("Failed Block Delete corresponding to Key {}.", 
deletedKey);
+        }
       }
     }
+    // Filter out the key even if one version of the key purge has failed. 
This is to prevent orphan blocks, and
+    // this needs to be retried.
+    completePurgedKeys = completePurgedKeys.stream()
+        .filter(i -> 
!failedDeletedKeys.contains(i)).collect(Collectors.toSet());
+    // Filter out any keys that have failed and sort the purge keys based on 
volume and bucket.
+    List<PurgedKey> purgedKeyList = purgedKeys.values().stream()
+        .filter(purgedKey -> 
!failedDeletedKeys.contains(purgedKey.getBlockGroup().getGroupID()))
+        
.sorted(Comparator.comparing(PurgedKey::getVolume).thenComparing(PurgedKey::getBucket))
+        .collect(Collectors.toList());

Review Comment:
   yeah there are already pre existing test cases covering this



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/DeletingServiceMetrics.java:
##########
@@ -109,6 +110,11 @@ public final class DeletingServiceMetrics {
   @Metric("Snapshot: No. of not reclaimable keys the last run")
   private MutableGaugeLong snapKeysNotReclaimableLast;
 
+  @Metric("Last Purge Key termIndex on Active Object Store")
+  private MutableGaugeLong lastAOSPurgeTermId;
+  @Metric("Last Purge Key transactionId on Active Object Store")
+  private MutableGaugeLong lastAOSPurgeTransactionId;

Review Comment:
   done



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