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


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -121,16 +123,18 @@ List<RepeatedOmKeyInfo> listTrash(String volumeName, 
String bucketName,
       String startKeyName, String keyPrefix, int maxKeys) throws IOException;
 
   /**
-   * Returns a list of pending deletion key info that ups to the given count.
-   * Each entry is a {@link BlockGroup}, which contains the info about the
-   * key name and all its associated block IDs. A pending deletion key is
-   * stored with #deleting# prefix in OM DB.
+   * Returns a Pair. First is a list of pending deletion key info that ups to
+   * the given count.Each entry is a {@link BlockGroup}, which contains
+   * the info about the key name and all its associated block IDs. Second is a
+   * Mapping of Key-Value pair which is updated in the deletedTable.
    *
    * @param count max number of keys to return.
-   * @return a list of {@link BlockGroup} representing keys and blocks.
+   * @return a Pair of list of {@link BlockGroup} representing keys and blocks,
+   * and a hashmap for key-value pair to be updated in the deletedTable.
    * @throws IOException
    */
-  List<BlockGroup> getPendingDeletionKeys(int count) throws IOException;
+  Pair<List<BlockGroup>, HashMap<String, RepeatedOmKeyInfo>>

Review Comment:
   I guess we can move this into another class instead of using a pair & change 
HashMap to Map



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1571,11 +1562,33 @@ public List<BlockGroup> getPendingDeletionKeys(final 
int keyCount,
             }
             currentCount++;
           }
-          keyBlocksList.addAll(blockGroupList);
+
+          List<OmKeyInfo> notReclaimableKeyInfoList =
+              notReclaimableKeyInfo.getOmKeyInfoList();
+
+          // If all the versions are not reclaimable, then do nothing.
+          if (notReclaimableKeyInfoList.size() > 0 &&
+              notReclaimableKeyInfoList.size() !=
+                  infoList.getOmKeyInfoList().size()) {
+            keysToModify.put(kv.getKey(), notReclaimableKeyInfo);
+          }
+
+          if (notReclaimableKeyInfoList.size() !=
+              infoList.getOmKeyInfoList().size()) {
+            keyBlocksList.addAll(blockGroupList);
+          }
         }
       }
     }
-    return keyBlocksList;
+    return Pair.of(keyBlocksList, keysToModify);
+  }
+
+  private boolean versionExistsInPreviousSnapshot(OmKeyInfo omKeyInfo,

Review Comment:
   Parameter names nomenclature could be better.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -610,6 +615,37 @@ public void submitRequest(OMRequest omRequest) {
     }
   }
 
+  public static boolean isBlockLocationInfoSame(OmKeyInfo prevKeyInfo,
+                                                OmKeyInfo deletedKeyInfo) {
+    OmKeyLocationInfoGroup deletedOmKeyLocation =
+        deletedKeyInfo.getLatestVersionLocations();

Review Comment:
   It would be safe to put a check on the size of the locations list.



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