-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-16524. Add configuration to control blocks deletion asynchronous… #4139
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.
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
base: trunk
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to user feedback, maybe we need this configuration.
cc @Hexiaoqiao
while (iter.hasNext()) { | ||
writeLock(); | ||
try { | ||
for (int i = 0; i < blockDeletionIncrement && iter.hasNext(); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed blockDeletionIncrement in HDFS-16554
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About "blockDeletionIncrement", in my opinion, why we removed it in HDFS-16554 is that we decided to use delete async to replace the previous method, so the "blockDeletionIncrement" won't be used any more. But when we add a switch to control blocks deletion async, we need to save the previous block deletion method, so we still need it.
@@ -481,6 +481,10 @@ public class DFSConfigKeys extends CommonConfigurationKeys { | |||
"dfs.namenode.block.deletion.increment"; | |||
public static final int DFS_NAMENODE_BLOCK_DELETION_INCREMENT_DEFAULT = 1000; | |||
|
|||
/** Block deletion asynchronous. */ | |||
public static final String DFS_NAMENODE_BLOCK_DELETION_ASYNC_KEY = "dfs.namenode.block.deletion.async"; | |||
public static final boolean DFS_NAMENODE_BLOCK_DELETION_ASYNC_DEFAULT = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it default to true?
💔 -1 overall
This message was automatically generated. |
* a list of blocks that need to be removed from blocksMap | ||
*/ | ||
void removeBlocks(List<BlockInfo> toDeleteList) { | ||
if (this.blockDeletionAsync) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable blockDeletionAsync doesn't seem to be declared.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@@ -3403,6 +3407,33 @@ FSPermissionChecker getPermissionChecker() | |||
return dir.getPermissionChecker(); | |||
} | |||
|
|||
/** | |||
* If blockDeletionAsync enables, blocks will be deleted asynchronously. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use dfs.namenode.block.deletion.async not blockDeletionAsync
writeLock(); | ||
try { | ||
for (int i = 0; i < blockDeletionIncrement && iter.hasNext(); i++) { | ||
blockManager.removeBlock(iter.next()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compared with the previous one, the following code is missing:
toRemoveBlocks.clear();
… or synchronous
Description of PR
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?