sumitagrawl commented on code in PR #7365:
URL: https://github.com/apache/ozone/pull/7365#discussion_r1888807813
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -217,13 +221,13 @@ public BackgroundTaskResult call() {
LOG.debug("Running DirectoryDeletingService");
}
isRunningOnAOS.set(true);
- long rnCnt = getRunCount().incrementAndGet();
long dirNum = 0L;
long subDirNum = 0L;
long subFileNum = 0L;
long remainNum = pathLimitPerTask;
- int consumedSize = 0;
- List<PurgePathRequest> purgePathRequestList = new ArrayList<>();
+ long remainingBufLimit = ratisByteLimit;
+ List<List<PurgePathRequest>> purgeRequestListBatches = new
ArrayList<>();
Review Comment:
There is a concurrency issue if sending multiple request, specially in
multi-threaded scenario. Its like,
Batch1: 5 sub-dir moved to deleted Table
Batch2: 5 sub-dir is deleted
Sequence:
T1: Batch1 is submitted and sub-dir is moved
T2: can pickup these sub-dir again
T1: send Batch2 where these sub-dir is deleted
T2: again will send 5 sub-dir for deletion
So, if need to meet the limit, we need return call() with reduced limit ...
So limit can be defined as
- Max elements moved/deleted
- Or, remove the limit case and use only ratis size.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -2098,19 +2100,22 @@ private List<OmKeyInfo>
gatherSubDirsWithIterator(OmKeyInfo parentInfo,
dirName);
directories.add(omKeyInfo);
countEntries++;
+ remainingBufLimit -= objectSerializedSize;
Review Comment:
We may not allow remainingBufLimit going below "0" as it can cause RatisSize
overflow in certain scenario.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -2121,6 +2126,7 @@ public List<OmKeyInfo> getPendingDeletionSubFiles(long
volumeId,
while (iterator.hasNext() && numEntries - countEntries > 0) {
Review Comment:
we need check size also here, else size may cross limit.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java:
##########
@@ -388,7 +393,7 @@ protected PurgePathRequest prepareDeleteDirRequest(
// limit. If count reached limit then there can be some more child
// paths to be visited and will keep the parent deleted directory
// for one more pass.
- String purgeDeletedDir = remainNum > 0 ? delDirName : null;
+ String purgeDeletedDir = (remainNum > 0 && remainingBufLimit > 0) ?
delDirName : null;
Review Comment:
The check can be risky, as its possible that dir is `not eligible` for
delete, but counted.
RemainBufLimit = 5, record size = 10, so its not added, but remainBufLimit
will be > 0.
Also, remainNum > 0.
In this case, dir should not be deleted, but above case can cause malicious
deletion.
We need have flag if all counted for that dir in DeleteKeysResult to handle
this. Can not decide from above limit value now.
--
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]
