You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/11/28 14:42:58 UTC
[GitHub] [ozone] sumitagrawl opened a new pull request, #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
sumitagrawl opened a new pull request, #4008:
URL: https://github.com/apache/ozone/pull/4008
## What changes were proposed in this pull request?
In DirectoryDeletingService, added sub-directory also for deletion including its sub-file and sub-dir. This will be handled automatically by PurgeDiretoriesRequest.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7541
## How was this patch tested?
1. Tested E2E with recursive deletion of directory
--
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
[GitHub] [ozone] sadanand48 commented on a diff in pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #4008:
URL: https://github.com/apache/ozone/pull/4008#discussion_r1037829088
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -231,13 +231,14 @@ public void testDeleteWithLargeSubPathsThanBatchSize() throws Exception {
assertSubPathsCount(dirDeletingService::getMovedFilesCount, 15);
// 15 subDir + 3 parentDir
- assertSubPathsCount(dirDeletingService::getMovedDirsCount, 18);
- assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 19);
+ // handled 5 move dir or file as capacity as recursive
+ assertSubPathsCount(dirDeletingService::getMovedDirsCount, 17);
+ assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 25);
Review Comment:
Total number of subdirectories under appRoot i.e the directory on which delete is issued is 18 (3 immediate child * 5 + 3 + 1 (appRoot itself) makes the total recursive directory count 19. Total directories = 19 but deleted directories count is 25 ?
--
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
[GitHub] [ozone] sadanand48 commented on a diff in pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #4008:
URL: https://github.com/apache/ozone/pull/4008#discussion_r1037927351
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -148,61 +150,52 @@ public BackgroundTaskResult call() throws Exception {
deleteTableIterator = ozoneManager.getMetadataManager().
getDeletedDirTable().iterator()) {
+ List<Pair<String, OmKeyInfo>> allSubDirList
+ = new ArrayList<>((int) remainNum);
long startTime = Time.monotonicNow();
while (remainNum > 0 && deleteTableIterator.hasNext()) {
pendingDeletedDirInfo = deleteTableIterator.next();
- // step-0: Get one pending deleted directory
- if (LOG.isDebugEnabled()) {
- LOG.debug("Pending deleted dir name: {}",
- pendingDeletedDirInfo.getValue().getKeyName());
- }
- final String[] keys = pendingDeletedDirInfo.getKey()
- .split(OM_KEY_PREFIX);
- final long volumeId = Long.parseLong(keys[1]);
- final long bucketId = Long.parseLong(keys[2]);
-
- // step-1: get all sub directories under the deletedDir
- List<OmKeyInfo> subDirs = ozoneManager.getKeyManager()
- .getPendingDeletionSubDirs(volumeId, bucketId,
- pendingDeletedDirInfo.getValue(), remainNum);
- remainNum = remainNum - subDirs.size();
-
- if (LOG.isDebugEnabled()) {
- for (OmKeyInfo dirInfo : subDirs) {
- LOG.debug("Moved sub dir name: {}", dirInfo.getKeyName());
- }
- }
-
- // step-2: get all sub files under the deletedDir
- List<OmKeyInfo> subFiles = ozoneManager.getKeyManager()
- .getPendingDeletionSubFiles(volumeId, bucketId,
- pendingDeletedDirInfo.getValue(), remainNum);
- remainNum = remainNum - subFiles.size();
-
- if (LOG.isDebugEnabled()) {
- for (OmKeyInfo fileInfo : subFiles) {
- LOG.debug("Moved sub file name: {}", fileInfo.getKeyName());
- }
- }
-
- // step-3: Since there is a boundary condition of 'numEntries' in
- // each batch, check whether the sub paths count reached batch size
- // 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 ?
- pendingDeletedDirInfo.getKey() : null;
- PurgePathRequest request = wrapPurgeRequest(volumeId, bucketId,
- purgeDeletedDir, subFiles, subDirs);
+ PurgePathRequest request = prepareDeleteDirRequest(
+ remainNum, pendingDeletedDirInfo.getValue(),
Review Comment:
Got it , Thanks for explaining!
--
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
[GitHub] [ozone] sadanand48 merged pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sadanand48 merged PR #4008:
URL: https://github.com/apache/ozone/pull/4008
--
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
[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on code in PR #4008:
URL: https://github.com/apache/ozone/pull/4008#discussion_r1037907077
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -231,13 +231,14 @@ public void testDeleteWithLargeSubPathsThanBatchSize() throws Exception {
assertSubPathsCount(dirDeletingService::getMovedFilesCount, 15);
// 15 subDir + 3 parentDir
- assertSubPathsCount(dirDeletingService::getMovedDirsCount, 18);
- assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 19);
+ // handled 5 move dir or file as capacity as recursive
+ assertSubPathsCount(dirDeletingService::getMovedDirsCount, 17);
+ assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 25);
Review Comment:
Fixed, after refactoring, I was check for dirDeleted which is passed as null, but proto class is having as empty string, added that check to fix.
--
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
[GitHub] [ozone] sadanand48 commented on a diff in pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #4008:
URL: https://github.com/apache/ozone/pull/4008#discussion_r1037842901
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -148,61 +150,52 @@ public BackgroundTaskResult call() throws Exception {
deleteTableIterator = ozoneManager.getMetadataManager().
getDeletedDirTable().iterator()) {
+ List<Pair<String, OmKeyInfo>> allSubDirList
+ = new ArrayList<>((int) remainNum);
long startTime = Time.monotonicNow();
while (remainNum > 0 && deleteTableIterator.hasNext()) {
pendingDeletedDirInfo = deleteTableIterator.next();
- // step-0: Get one pending deleted directory
- if (LOG.isDebugEnabled()) {
- LOG.debug("Pending deleted dir name: {}",
- pendingDeletedDirInfo.getValue().getKeyName());
- }
- final String[] keys = pendingDeletedDirInfo.getKey()
- .split(OM_KEY_PREFIX);
- final long volumeId = Long.parseLong(keys[1]);
- final long bucketId = Long.parseLong(keys[2]);
-
- // step-1: get all sub directories under the deletedDir
- List<OmKeyInfo> subDirs = ozoneManager.getKeyManager()
- .getPendingDeletionSubDirs(volumeId, bucketId,
- pendingDeletedDirInfo.getValue(), remainNum);
- remainNum = remainNum - subDirs.size();
-
- if (LOG.isDebugEnabled()) {
- for (OmKeyInfo dirInfo : subDirs) {
- LOG.debug("Moved sub dir name: {}", dirInfo.getKeyName());
- }
- }
-
- // step-2: get all sub files under the deletedDir
- List<OmKeyInfo> subFiles = ozoneManager.getKeyManager()
- .getPendingDeletionSubFiles(volumeId, bucketId,
- pendingDeletedDirInfo.getValue(), remainNum);
- remainNum = remainNum - subFiles.size();
-
- if (LOG.isDebugEnabled()) {
- for (OmKeyInfo fileInfo : subFiles) {
- LOG.debug("Moved sub file name: {}", fileInfo.getKeyName());
- }
- }
-
- // step-3: Since there is a boundary condition of 'numEntries' in
- // each batch, check whether the sub paths count reached batch size
- // 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 ?
- pendingDeletedDirInfo.getKey() : null;
- PurgePathRequest request = wrapPurgeRequest(volumeId, bucketId,
- purgeDeletedDir, subFiles, subDirs);
+ PurgePathRequest request = prepareDeleteDirRequest(
+ remainNum, pendingDeletedDirInfo.getValue(),
Review Comment:
The above PurgePathRequest will move subdirs to the DeletedDirectoryTable and on the next iteration these will get picked up and added to count but we are also counting below the same subdirectory in the second newly added while loop again as deleted which can cause some directories to be counted twice.
--
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
[GitHub] [ozone] sadanand48 commented on a diff in pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #4008:
URL: https://github.com/apache/ozone/pull/4008#discussion_r1037829088
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -231,13 +231,14 @@ public void testDeleteWithLargeSubPathsThanBatchSize() throws Exception {
assertSubPathsCount(dirDeletingService::getMovedFilesCount, 15);
// 15 subDir + 3 parentDir
- assertSubPathsCount(dirDeletingService::getMovedDirsCount, 18);
- assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 19);
+ // handled 5 move dir or file as capacity as recursive
+ assertSubPathsCount(dirDeletingService::getMovedDirsCount, 17);
+ assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 25);
Review Comment:
Total number of subdirectories under appRoot i.e the directory on which delete is issued is 18 (3 immediate child * 5 + 3 + 1 (appRoot itself) makes the total recursive directory count 19. Total directories = 19 but count is 25 ?
--
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
[GitHub] [ozone] sumitagrawl commented on pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on PR #4008:
URL: https://github.com/apache/ozone/pull/4008#issuecomment-1331657916
> Thanks for the patch @sumitagrawl.
>
> Maybe I'm missing something, I got a question (maybe it's out of scope for this change). Why doesn't DirectoryDeletingService explore all the deleted files (recursively) and put items to deletedKeyTable by itself instead of taking the Ratis path to call `DirectoriesPurgeRequest`? The current process looks pretty cumbersome to me.
>
> I assume when a directory is deleted, the existence of the deleted directory in deletedDirectoryTable is reflected in all OM replicas, and so DirectoryDeletingService run with the same input per replica.
As understanding, below are reason,
1. For HA, deletion of operation needs to be synchronized to other OM in cluster, so it make use of Ratis for this replication, so ratis path is taken. The rocks db handling is done in ratis for this purpose.
2. The task are done in batches to avoid resource consumed entirely for delete service, as batch can be too big. A configuration for handling limit in every iteration is present, default 100,000. Also Ratis synchronization is done with this limit.
3. The task include other things also, notifying SCM for delete of blocks which can consume time. So all these things are done in DirectoryDeletingService to reduce load in Raits flow which will be blocking other activity if taking time.
@duongkame Please share if above make sense.
--
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
[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on code in PR #4008:
URL: https://github.com/apache/ozone/pull/4008#discussion_r1037910586
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -148,61 +150,52 @@ public BackgroundTaskResult call() throws Exception {
deleteTableIterator = ozoneManager.getMetadataManager().
getDeletedDirTable().iterator()) {
+ List<Pair<String, OmKeyInfo>> allSubDirList
+ = new ArrayList<>((int) remainNum);
long startTime = Time.monotonicNow();
while (remainNum > 0 && deleteTableIterator.hasNext()) {
pendingDeletedDirInfo = deleteTableIterator.next();
- // step-0: Get one pending deleted directory
- if (LOG.isDebugEnabled()) {
- LOG.debug("Pending deleted dir name: {}",
- pendingDeletedDirInfo.getValue().getKeyName());
- }
- final String[] keys = pendingDeletedDirInfo.getKey()
- .split(OM_KEY_PREFIX);
- final long volumeId = Long.parseLong(keys[1]);
- final long bucketId = Long.parseLong(keys[2]);
-
- // step-1: get all sub directories under the deletedDir
- List<OmKeyInfo> subDirs = ozoneManager.getKeyManager()
- .getPendingDeletionSubDirs(volumeId, bucketId,
- pendingDeletedDirInfo.getValue(), remainNum);
- remainNum = remainNum - subDirs.size();
-
- if (LOG.isDebugEnabled()) {
- for (OmKeyInfo dirInfo : subDirs) {
- LOG.debug("Moved sub dir name: {}", dirInfo.getKeyName());
- }
- }
-
- // step-2: get all sub files under the deletedDir
- List<OmKeyInfo> subFiles = ozoneManager.getKeyManager()
- .getPendingDeletionSubFiles(volumeId, bucketId,
- pendingDeletedDirInfo.getValue(), remainNum);
- remainNum = remainNum - subFiles.size();
-
- if (LOG.isDebugEnabled()) {
- for (OmKeyInfo fileInfo : subFiles) {
- LOG.debug("Moved sub file name: {}", fileInfo.getKeyName());
- }
- }
-
- // step-3: Since there is a boundary condition of 'numEntries' in
- // each batch, check whether the sub paths count reached batch size
- // 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 ?
- pendingDeletedDirInfo.getKey() : null;
- PurgePathRequest request = wrapPurgeRequest(volumeId, bucketId,
- purgeDeletedDir, subFiles, subDirs);
+ PurgePathRequest request = prepareDeleteDirRequest(
+ remainNum, pendingDeletedDirInfo.getValue(),
Review Comment:
Its getting removed from deletedTable also, its processing as below,
Old:
Iteration1: Dir1 <-- delete, Dir2,Dir3 <-- moved to deleted table
Iteration2: Dir2 and Dir3 deleted as no further files
New:
Request have 3 entry
Dir1<-- delete, Dir2,Dir3 <-- move to deletedTable
Dir2 <-- delete
Dir3<-- delete
So here same as old, it move to deletedTable and same is next removed. So it will not count twice.
--
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
[GitHub] [ozone] kerneltime commented on pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #4008:
URL: https://github.com/apache/ozone/pull/4008#issuecomment-1329443041
cc @duongkame @swamirishi
--
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
[GitHub] [ozone] duongkame commented on pull request #4008: HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup
Posted by GitBox <gi...@apache.org>.
duongkame commented on PR #4008:
URL: https://github.com/apache/ozone/pull/4008#issuecomment-1331516993
Thanks for the patch @sumitagrawl.
Maybe I'm missing something, I got a question (maybe it's out of scope for this change). Why doesn't DirectoryDeletingService explore all the deleted files (recursively) and put items to deletedKeyTable by itself instead of taking the Ratis path to call `DirectoriesPurgeRequest`? The current process looks pretty cumbersome to me.
I assume when a directory is deleted, the existence of the deleted directory in deletedDirectoryTable is reflected in all OM replicas, and so DirectoryDeletingService run with the same input per replica.
--
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