HDDS-8778. Support recursive volume delete using Ozone sh command. #4842

ashishkumar50 · 2023-06-07T11:46:03Z

What changes were proposed in this pull request?

Currently recursive volume delete is supported only using fs command. But fs can be used only for FSO and Legacy bucket. Since volume can contain mix bucket types and if volume contains some OBS bucket in it then fs rm command fails in between. Currently there is no way for user to delete volume in easy way.
In this PR, As sh can be used for all bucket types, using ozone sh command user can now perform volume delete recursively.

Description about new the command:
Create Volume: vol1 and few buckets/keys inside it.

Volume delete fails as volume is not empty and we are trying to delete without recursive.

ozone sh volume delete o3://localhost:9862/vol1
VOLUME_NOT_EMPTY

Use -r for recursive volume delete. Interactive confirmation before delete, if input is other than "yes" then make operation cancelled.

ozone sh volume delete -r -id localhost:9862 o3://localhost:9862/vol1
This command will delete volume recursively.
There is no recovery option after using this command, and no trash for FSO buckets.
Delay is expected running this command.
Enter 'yes' to proceed': no
Operation cancelled.

Interactive confirmation before recursive delete, Enter "yes" then volume should be deleted

ozone sh volume delete -r -id localhost:9862 o3://localhost:9862/vol1/
This command will delete volume recursively.
There is no recovery option after using this command, and no trash for FSO buckets.
Delay is expected running this command.
Enter 'yes' to proceed': yes
Volume vol1 is deleted

Override interactive confirmation by giving "--yes" while running command itself

ozone sh volume delete -r --yes -id localhost:9862 o3://localhost:9862/vol1
Volume vol1 is deleted

Apart from these options we can also provide number of threads(-t "no of threads") for deletion.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8778

How was this patch tested?

Added new integration test and verified.

ashishkumar50 · 2023-06-07T11:47:38Z

@sadanand48 @sumitagrawl , Please help to review.

sadanand48

Thanks @ashishkumar50 for the patch, left few comments.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java

errose28 · 2023-06-09T20:29:21Z

@ashishkumar50 Could you include what the CLI would look like in the PR description? What subcommand is used, what flags are supported, is there a confirmation prompt, can it be overridden, etc. These UX aspects are probably what people would have the most comments on.

ashishkumar50 · 2023-06-12T12:14:45Z

@ashishkumar50 Could you include what the CLI would look like in the PR description? What subcommand is used, what flags are supported, is there a confirmation prompt, can it be overridden, etc. These UX aspects are probably what people would have the most comments on.

@errose28, Update PR description with all options.

errose28 · 2023-06-13T17:58:35Z

Why is there a -skipTrash option? It seems the same as -r and o3 has no concept of trash.

ashishkumar50 · 2023-06-14T05:43:52Z

Why is there a -skipTrash option? It seems the same as -r and o3 has no concept of trash.

We do support trash using o3 for FSO bucket.
Currently using fs also when we do recursive volume delete it expects -skipTrash option or else it will not allow to delete.
So just to make user aware that once deleted it cannot be recovered by providing this option even for o3 recursive delete.

krishnaasawa1 · 2023-06-14T09:13:09Z

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java

+  private boolean bSkipTrash = false;
+  @CommandLine.Option(
+      names = {"-r"},
+      description = "Delete volume recursively"


Should we add in description similar to Ozonefsdelete description->"Delay is " +
"expected when walking over large directory recursively to count".
Also Ozonefsdelete has some params used for limit hadoop.shell.delete.limit.num.files along with Safely confimration, will that be useful here as well?

Added in the interactive description when user does recursive delete. About limit number we can have improvement in future for this command. In PR description I have added how the interactive description looks like.

errose28 · 2023-06-15T20:19:57Z

If the command always fails if --skipTrash is not provided then that argument is redundant. I think the confirmation message should specify that there is no trash recovery for FSO buckets when this command is used, then -y or user input becomes the manual override required.

Also I don't think the recently added ozone sh key delete --skipTrash option should be present either. I had a discussion with @swamirishi where this flag seemed to imply that OBS buckets had trash if the flag was not used. I think ozone sh key delete should always go to trash on FSO bucket and if --skipTrash is needed, the user should use ofs.

Incorporating these two suggestions together provides a consistent experience across the two commands without redundant arguments implying that trash exists where it does not.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java

ashishkumar50 · 2023-06-19T04:08:14Z

If the command always fails if --skipTrash is not provided then that argument is redundant. I think the confirmation message should specify that there is no trash recovery for FSO buckets when this command is used, then -y or user input becomes the manual override required.

Also I don't think the recently added ozone sh key delete --skipTrash option should be present either. I had a discussion with @swamirishi where this flag seemed to imply that OBS buckets had trash if the flag was not used. I think ozone sh key delete should always go to trash on FSO bucket and if --skipTrash is needed, the user should use ofs.

Incorporating these two suggestions together provides a consistent experience across the two commands without redundant arguments implying that trash exists where it does not.

Removed -skipTrash option, instead showing in interactive description. Updated the change in PR description.

errose28 · 2023-06-20T21:12:38Z

ozone sh volume delete -r=yes -id localhost:9862 o3://localhost:9862/vol2
Volume vol2 is deleted

I think the override should be a separate flag, not an argument to -r. When in doubt, look at existing CLIs for the intuitive thing to do. rm -r requires -f to bypass the prompt. pacman uses --noconfirm, apt requires -y etc. Just a few examples I could think of off the top of my head. Something like --yes/-y seems more in line with standards than -r=yes.

ozone sh volume delete -r -id localhost:9862 o3://localhost:9862/vol1
Enter value for -r (This command will delete volume recursively.
There is no trash recovery for FSO buckets using this command.
Delay is expected running this command.
Enter 'yes' to proceed): no
VOLUME_NOT_EMPTY

This should probably also specify that there is no recovery option for OBS buckets either, although we don't have that anyways. Maybe a message like "There is no recovery option after using this command, and no trash for FSO buckets" would help.

Also, is there a follow up PR to implement recursive bucket delete as well?

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java

ashishkumar50 · 2023-06-21T07:02:07Z

ozone sh volume delete -r=yes -id localhost:9862 o3://localhost:9862/vol2
Volume vol2 is deleted

I think the override should be a separate flag, not an argument to -r. When in doubt, look at existing CLIs for the intuitive thing to do. rm -r requires -f to bypass the prompt. pacman uses --noconfirm, apt requires -y etc. Just a few examples I could think of off the top of my head. Something like --yes/-y seems more in line with standards than -r=yes.

ozone sh volume delete -r -id localhost:9862 o3://localhost:9862/vol1
Enter value for -r (This command will delete volume recursively.
There is no trash recovery for FSO buckets using this command.
Delay is expected running this command.
Enter 'yes' to proceed): no
VOLUME_NOT_EMPTY

This should probably also specify that there is no recovery option for OBS buckets either, although we don't have that anyways. Maybe a message like "There is no recovery option after using this command, and no trash for FSO buckets" would help.

Also, is there a follow up PR to implement recursive bucket delete as well?

I have updated with separate flag to override.
Updated user interactive message.
Yes I will have a follow up PR for recursive volume delete.

errose28 · 2023-06-22T17:12:29Z

CLI options LGTM although I will not have time to review the code changes. Thanks for updating those @ashishkumar50

sadanand48 · 2023-06-23T10:02:41Z

Thanks @ashishkumar50 for addressing the comments. The changes look good to me. We should also document the newly made changes regarding ozone sh deletes to get clarity on items like trash. Opened HDDS-8916 for the same.

ashishkumar50 · 2023-06-23T11:26:53Z

Thanks @ashishkumar50 for addressing the comments. The changes look good to me. We should also document the newly made changes regarding ozone sh deletes to get clarity on items like trash. Opened HDDS-8916 for the same.

Okay @sadanand48, I will document these new changes in HDDS-8916

* master: (79 commits) HDDS-8914. Datanode may fail to start due to duplicate VolumeInfoMetrics (apache#4966) HDDS-8921. Add support for EC in Freon SCM block generator (apache#4982) HDDS-8927. Metadata scanner should not scan unhealthy containers. (apache#4976) HDDS-8929. Avoid list allocation for pipeline search (apache#4980) HDDS-8778. Support recursive volume delete using Ozone sh command. (apache#4842) HDDS-8885. Quota repair count enable quota feature for old bucket/volume. (apache#4941) HDDS-8771. Refactor volume level tmp directory for generic usage. (apache#4838) HDDS-8922. Random EC read pipeline ID causes XceiverClient cache churn (apache#4971) HDDS-8586 Recon. - API for Count of deletePending keys and amount of data mapped to such keys. (apache#4923) HDDS-8908. Intermittent failure in TestBlockDeletion#testBlockDeletion (apache#4958) HDDS-8910. Replace LockManager with striped lock in ContainerStateManager (apache#4962) HDDS-8917. Move protobuf conversion out of the lock in PipelineStateManagerImpl (apache#4965) HDDS-8825. Use apache/hadoop 3.3.5 docker image (apache#4963) HDDS-8906. Avoid stream when getting in-service healthy nodes (apache#4960) HDDS-8907. Store volume count when storage report is updated (apache#4957) HDDS-8905. PipelineManager metrics should not be synchronized (apache#4959) HDDS-8553. Improve scanner integration tests. (apache#4936) HDDS-8854. Avoid unnecessary DatanodeDetails creation for NodeStateManager lookup (apache#4925) HDDS-8315. [Snapshot] Added unit tests for SnapshotDiffManager (apache#4716) HDDS-7968. [Snapshot] Improve KeyDeletingService to reclaim eligible key blocks in snapshot's deletedTable (apache#4935) ...

* master: HDDS-8914. Datanode may fail to start due to duplicate VolumeInfoMetrics (apache#4966) HDDS-8921. Add support for EC in Freon SCM block generator (apache#4982) HDDS-8927. Metadata scanner should not scan unhealthy containers. (apache#4976) HDDS-8929. Avoid list allocation for pipeline search (apache#4980) HDDS-8778. Support recursive volume delete using Ozone sh command. (apache#4842) HDDS-8885. Quota repair count enable quota feature for old bucket/volume. (apache#4941) HDDS-8771. Refactor volume level tmp directory for generic usage. (apache#4838) HDDS-8922. Random EC read pipeline ID causes XceiverClient cache churn (apache#4971)

…pache#4842)

HDDS-8778. Support recursive volume delete using Ozone sh command.

e3c9d07

ashishkr200 added 2 commits June 7, 2023 18:11

HDDS-8778. Fix findbug issue.

25fde63

HDDS-8778. Fix ServiceID issue.

2609247

sadanand48 reviewed Jun 7, 2023

View reviewed changes

ashishkr200 added 3 commits June 7, 2023 21:03

HDDS-8778. Fix Test case.

143f819

HDDS-8778. Fix address issue.

5d9b760

HDDS-8778. Fix review comments.

6c06e1b

krishnaasawa1 reviewed Jun 14, 2023

View reviewed changes

sadanand48 reviewed Jun 16, 2023

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java Show resolved Hide resolved

ashishkumar50 and others added 2 commits June 16, 2023 16:12

Merge branch 'master' into HDDS-8778

550db40

HDDS-8778. Fix review comments.

21098fe

HDDS-8778. Update bucket layout in test case.

800033b

HDDS-8778. Update user interactive option and message.

87a6587

sadanand48 reviewed Jun 21, 2023

View reviewed changes

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java Outdated Show resolved Hide resolved

HDDS-8778. Update thread description.

ec50b58

HDDS-8778. Handling for single serviceID.

41cd458

sadanand48 approved these changes Jun 26, 2023

View reviewed changes

sadanand48 merged commit 01007af into apache:master Jun 26, 2023

vtutrinov pushed a commit to Cyrill/ozone that referenced this pull request Jul 3, 2023

HDDS-8778. Support recursive volume delete using Ozone sh command. (a…

c336ee5

…pache#4842)

vtutrinov pushed a commit to Cyrill/ozone that referenced this pull request Jul 3, 2023

HDDS-8778. Support recursive volume delete using Ozone sh command. (a…

9e48e88

…pache#4842)

HDDS-8778. Support recursive volume delete using Ozone sh command. #4842

HDDS-8778. Support recursive volume delete using Ozone sh command. #4842

Uh oh!

Conversation

ashishkumar50 commented Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

ashishkumar50 commented Jun 7, 2023

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

errose28 commented Jun 9, 2023

Uh oh!

ashishkumar50 commented Jun 12, 2023

Uh oh!

errose28 commented Jun 13, 2023

Uh oh!

ashishkumar50 commented Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krishnaasawa1 Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

ashishkumar50 Jun 19, 2023

Choose a reason for hiding this comment

Uh oh!

errose28 commented Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ashishkumar50 commented Jun 19, 2023

Uh oh!

errose28 commented Jun 20, 2023

Uh oh!

Uh oh!

ashishkumar50 commented Jun 21, 2023

Uh oh!

errose28 commented Jun 22, 2023

Uh oh!

sadanand48 commented Jun 23, 2023

Uh oh!

ashishkumar50 commented Jun 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ashishkumar50 commented Jun 7, 2023 •

edited

Loading

ashishkumar50 commented Jun 14, 2023 •

edited

Loading

errose28 commented Jun 15, 2023 •

edited

Loading