Skip to content

HDFS-15607. Create trash dir when allowing snapshottable dir #2352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
smengcl merged 8 commits into from
Oct 5, 2020
Merged

HDFS-15607. Create trash dir when allowing snapshottable dir #2352

smengcl merged 8 commits into from
Oct 5, 2020

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented Sep 30, 2020 •
edited
Loading

https://issues.apache.org/jira/browse/HDFS-15607

Changes

  • Snapshottable directory trash root will be automatically provisioned (with permission 777 and sticky bit) when allowing snapshot via DFSAdmin command, if dfs.namenode.snapshot.trashroot.enabled is set to true on the NameNode.
    • Note that calling DistributedFileSystem#allowSnapshot directly will NOT provision trash root at all. This behavior is aligned with DistributedFileSystem#createEncryptionZone.
$ hdfs dfsadmin -allowSnapshot /ssdir3/
Allowing snapshot on /ssdir3/ succeeded
$ hdfs dfs -ls /ssdir3
Found 1 items
drwxrwxrwt   - smeng supergroup          0 2020-09-30 00:00 /ssdir3/.Trash
  • Added new dfsadmin command -provisionSnapshotTrash. Admins can use this new command to provision trash roots when the cluster is upgraded from older HDFS which doesn't have snapshottable directory trash roots feature:
$ hdfs dfsadmin -provisionSnapshotTrash
Usage: hdfs dfsadmin [-provisionSnapshotTrash <snapshotDir>]
$ hdfs dfsadmin -provisionSnapshotTrash /ssdir2/
Provision of snapshot trash in /ssdir2/ succeeded
$ hdfs dfs -ls /ssdir2/
Found 1 items
drwxrwxrwt   - smeng supergroup          0 2020-09-30 00:00 /ssdir2/.Trash
  • Added test in TestDistributedFileSystem and TestDFSAdmin.

@smengcl smengcl added the bug label Sep 30, 2020
@smengcl smengcl requested a review from bshashikant September 30, 2020 07:36
@smengcl smengcl self-assigned this Sep 30, 2020
bshashikant
bshashikant reviewed Sep 30, 2020
View reviewed changes
Copy link
Contributor

@bshashikant bshashikant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add provisionTrash command for WebHdfs as well?

szetszwo
szetszwo reviewed Sep 30, 2020
View reviewed changes
Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for working on this! Some minor comments on the change.

@smengcl
Copy link
Contributor Author

smengcl commented Sep 30, 2020 •
edited
Loading

Do we need to add provisionTrash command for WebHdfs as well?

Good point. I think so, yes.

Update: As I was attempting to add PROVISIONSNAPSHOTTRASH to WebHDFS, I realized NamenodeWebHdfsMethods is already server code. But the whole provision trash logic are on the client side (same as provision EZ trash). This would also imply that WebHDFS ALLOWSNAPSHOT won't trigger provision snapshot trash at the moment.

Note WebHDFS doesn't support encryption zone commands (create, list, etc.).

I have opened another jira HDFS-15612 for discussion on WebHDFS support for provision snapshot trash. Let's rule WebHDFS out in this jira for now. @bshashikant

smengcl added 8 commits October 2, 2020 12:15
@smengcl
Implement provisionSnapshottableDirTrash;
3bb23ef 
add new dfsadmin command;
add test in TestDistributedFileSystem.

Change-Id: I09cf74a13168af6f448b85a60808ee4d3068041f
@smengcl
Add test TestDFSAdmin#testAllowDisallowSnapshot.
aded6c6 
Change-Id: If7ead8db1e2f515777d2ddc9ca5af914ddc58d2a
@smengcl
provisionSnapshottableDirTrash now returns Path.
4458fc4 
Change-Id: I6f662871c877aecbf1734c21d64b454df97b2a58
@smengcl
Address checkstyle/whitespace.
0e66cb5 
Change-Id: I1016a08ecd00cefcb8225cf07d21a5047cda9c2c
@smengcl
Rename provisionSnapshottableDirTrash to provisionSnapshotTrash; Add …
f8f0c1a 
…warning if snapshot trash root is disabled when provisioning trash.

Change-Id: I26f40b6e8296bcd3c3b26c2187d4d942dbf99887
@smengcl
Improve warning message.
f36217b 
Change-Id: I31e2236a6e96ef8bd2e94e09a57f4ef74c5996e7
@smengcl
Fix existing test; checkstyle.
5d71f9e 
Change-Id: Icf8eee168afa3becb122cb5a7d82da89dae20f0e
@smengcl
Fix TestViewFileSystemOverloadSchemeWithDFSAdmin#testAllowAndDisalllo…
9878c4e 
…wSnapShot failure.

Change-Id: I6dc54a8d0dec4fa295faa15475ad4fb5d83dd294
@smengcl
Copy link
Contributor Author

smengcl commented Oct 5, 2020

Thanks @bshashikant and @szetszwo for reviewing this. Will commit in a min.

@smengcl smengcl merged commit 43b0c0b into apache:trunk Oct 5, 2020
@smengcl smengcl mentioned this pull request Nov 9, 2020
Merged
jojochuang pushed a commit to jojochuang/hadoop that referenced this pull request May 23, 2023
@smengcl
HDFS-15607. Create trash dir when allowing snapshottable dir (apache#…
2228ee0 
…2352)

(cherry picked from commit 43b0c0b)
Conflicts: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
	hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
	hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java
Change-Id: Iec14da9b75205f71f834646181b0a49f48618952
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szetszwo szetszwo szetszwo left review comments

@bshashikant bshashikant bshashikant approved these changes

@mukul1987 mukul1987 Awaiting requested review from mukul1987

Assignees

@smengcl smengcl

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smengcl @szetszwo @bshashikant