Re: [PR] HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. [ozone]

["swamirishi (via GitHub)" <gi...@apache.org>]

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


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -2082,79 +2082,98 @@ public List<OmKeyInfo> getPendingDeletionSubDirs(long volumeId, long bucketId,
     try (TableIterator<String,
         ? extends Table.KeyValue<String, OmDirectoryInfo>>
         iterator = dirTable.iterator()) {
-      return gatherSubDirsWithIterator(parentInfo, numEntries,
-          seekDirInDB, countEntries, iterator);
+      return gatherSubDirsWithIterator(parentInfo,
+          seekDirInDB, countEntries, iterator, remainingBufLimit);
     }
 
   }
 
-  private List<OmKeyInfo> gatherSubDirsWithIterator(OmKeyInfo parentInfo,
-      long numEntries, String seekDirInDB,
+  private DeleteKeysResult gatherSubDirsWithIterator(OmKeyInfo parentInfo,
+       String seekDirInDB,
       long countEntries,
       TableIterator<String,
-          ? extends Table.KeyValue<String, OmDirectoryInfo>> iterator)
+          ? extends Table.KeyValue<String, OmDirectoryInfo>> iterator, long remainingBufLimit)
       throws IOException {
     List<OmKeyInfo> directories = new ArrayList<>();
     iterator.seek(seekDirInDB);
+    long consumedSize = 0;
+    boolean processedSubDirs = false;
 
-    while (iterator.hasNext() && numEntries - countEntries > 0) {
+    while (iterator.hasNext() && remainingBufLimit > 0) {
       Table.KeyValue<String, OmDirectoryInfo> entry = iterator.next();
       OmDirectoryInfo dirInfo = entry.getValue();
+      long objectSerializedSize = entry.getRawValue().length;
       if (!OMFileRequest.isImmediateChild(dirInfo.getParentObjectID(),
           parentInfo.getObjectID())) {
+        processedSubDirs = true;
         break;
       }
       if (!metadataManager.getDirectoryTable().isExist(entry.getKey())) {
         continue;
       }
+      if (remainingBufLimit - objectSerializedSize < 0) {
+        break;
+      }
       String dirName = OMFileRequest.getAbsolutePath(parentInfo.getKeyName(),
           dirInfo.getName());
       OmKeyInfo omKeyInfo = OMFileRequest.getOmKeyInfo(
           parentInfo.getVolumeName(), parentInfo.getBucketName(), dirInfo,
           dirName);
       directories.add(omKeyInfo);
       countEntries++;
+      remainingBufLimit -= objectSerializedSize;
+      consumedSize += objectSerializedSize;
     }
 
-    return directories;
+    processedSubDirs = processedSubDirs || (!iterator.hasNext());

Review Comment:
   @sadanand48 @sumitagrawl This is wrong if the last entry was skipped in line 2115 then iterator.hasNext() would return false. But the last entry was never deleted. Because of this directory would end up getting deleting without moving the last subFile/subDirectory entry from fileTable/DirectoryTable to deletedTable/deletedDirectoryTable



-- 
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-unsubscribe@ozone.apache.org

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


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