HDFS-15614. Initialize snapshot trash root during NameNode startup if enabled #2370
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add UT. Change-Id: If679da69024056697bf633094a359530bffc74d3
smengcl
commented
Oct 8, 2020
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
bshashikant
reviewed
Oct 9, 2020
There was a problem hiding this comment.
Let's say snapshotTrash root is enabled , Trash directories re created and files moved in the Trash directory. Now, if the feature is turned off, the trash will be intact .
a) The trash emptier won't clean up this trash as "getTrashRoots()" won't return this trash location.
b) Now, once the feature is turned off, existing files continue to live the trash inside the snapshot root trash while new set of deleted files from the same location will now move to user trash outside the directory as the config is turned off.
i guess, this is the limitation of this feature.
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Outdated
Show resolved
Hide resolved
...-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
Outdated
Show resolved
Hide resolved
remove redundant getIsSnapshotTrashRootEnabled method. Change-Id: Ic9293e36907ee88ab2bf17855a8927e4e6c4c949
Change-Id: Ibd167312dd6a2db5f90e15fe6b01300274770867
|
💔 -1 overall
This message was automatically generated. |
bshashikant
approved these changes
Oct 13, 2020
|
@smengcl , would you like to retrigger the checks to ensure the snapshot tests work just fine. they seem to fail in last run bcoz of memory issue with the server. |
sure. will do triggered checks with an empty commit: https://ci-hadoop.apache.org/blue/organizations/jenkins/hadoop-multibranch/detail/PR-2370/3/ btw I ran |
Change-Id: Ib761a58fec35f8f967e008268a020db28effbc8f
|
Resolved merge conflict in UT with HADOOP-16878 which is just merged to trunk. |
|
💔 -1 overall
This message was automatically generated. |
|
The last CI run looks good. Will merge in a minute. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 23, 2023
… enabled (apache#2370) (cherry picked from commit a308a1e) Conflicts: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java Change-Id: Ia8f2fb39768a135f4014f64f323ef0a6f740ad06
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/HDFS-15614
Added
checkAndProvisionSnapshotTrashRootsinFSNamesystem. Triggered inNameNodeon startup ifdfs.namenode.snapshot.trashroot.enabledset totrue.UT added in
TestDistributedFileSystem.The logic is ready for review.
I am considering adding another UT for HA case.Now that we put the check instartActiveServices()it shouldn't be a problem anymore. Thanks @bshashikant for the suggestion.