HDFS-16735. Reduce the number of HeartbeatManager loops. #4780
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
|
LGTM. Should we add |
|
💔 -1 overall
This message was automatically generated. |
Hexiaoqiao
reviewed
Aug 23, 2022
There was a problem hiding this comment.
@zhangshuyan0 Great catch here. It is very useful for large cluster which is time cost to traverse all node many loops. Thanks.
| @@ -557,7 +557,8 @@ public class DFSConfigKeys extends CommonConfigurationKeys { | |||
| // This value uses the times of heartbeat interval to define the minimum value for stale interval. | |||
| public static final String DFS_NAMENODE_STALE_DATANODE_MINIMUM_INTERVAL_KEY = "dfs.namenode.stale.datanode.minimum.interval"; | |||
| public static final int DFS_NAMENODE_STALE_DATANODE_MINIMUM_INTERVAL_DEFAULT = 3; // i.e. min_interval is 3 * heartbeat_interval = 9s | |||
|
|
|||
| public static final String DFS_NAMENODE_REMOVE_BAD_BATCH_NUM = "dfs.namenode.remove.bad.batch.num"; | |||
There was a problem hiding this comment.
DFS_NAMENODE_REMOVE_DEAD_DATANODE_BATCHNUM_KEY = "dfs.namenode.remove.dead.datanode.batchnum"; this will be more readable?
| @@ -71,6 +71,7 @@ class HeartbeatManager implements DatanodeStatistics { | |||
| /** Heartbeat monitor thread. */ | |||
| private final Daemon heartbeatThread = new Daemon(new Monitor()); | |||
| private final StopWatch heartbeatStopWatch = new StopWatch(); | |||
| private final int removeBatchNum; | |||
There was a problem hiding this comment.
just suggest to define one more readable variable name, such as numOfDeadDatanodesRemove?
There was a problem hiding this comment.
OK, I'll change the names.
goiri
reviewed
Aug 23, 2022
| @@ -96,6 +97,9 @@ class HeartbeatManager implements DatanodeStatistics { | |||
| enableLogStaleNodes = conf.getBoolean( | |||
| DFSConfigKeys.DFS_NAMENODE_ENABLE_LOG_STALE_DATANODE_KEY, | |||
| DFSConfigKeys.DFS_NAMENODE_ENABLE_LOG_STALE_DATANODE_DEFAULT); | |||
| this.removeBatchNum = | |||
There was a problem hiding this comment.
this.removeBatchNum = conf.getInt(
DFSConfigKeys.DFS_NAMENODE_REMOVE_BAD_BATCH_NUM,
DFSConfigKeys.DFS_NAMENODE_REMOVE_BAD_BATCH_NUM_DEFAULT);
There was a problem hiding this comment.
Thanks, I've fixed these code style issues.
...adoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java
Show resolved
Hide resolved
| @@ -492,12 +498,12 @@ void heartbeatCheck() { | |||
| // log nodes detected as stale since last heartBeat | |||
| dumpStaleNodes(staleNodes); | |||
|
|
|||
| allAlive = dead == null && failedStorage == null; | |||
| allAlive = deadDatanodes.size() == 0 && failedStorages.size() == 0; | |||
Thanks @slfan1989 , I have added the relevant configuration to the xml file. This patch is mainly to improve the processing performance of HeartbeatManager in large-scale clusters, and has no impact on existing functions, so I did not add a ut. |
|
💔 -1 overall
This message was automatically generated. |
|
@zhangshuyan0 fix checkstyle |
|
💔 -1 overall
This message was automatically generated. |
goiri
approved these changes
Aug 27, 2022
|
Committed to trunk. Thanks @zhangshuyan0 for your contributions! Thanks @goiri , @slfan1989 for your reviews! |
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…. Contributed by Shuyan Zhang. Signed-off-by: Inigo Goiri <inigoiri@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
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.
Description of PR
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?