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
HDFS-16737. Fix number of threads in FsDatasetAsyncDiskService#addExecutorForVolume #4784
base: trunk
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
@ayushtkn Sir, can you help me review this patch? |
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.
Apart from the config name stuff things looks good to me.
cc. @ferhui / @sodonnel / @tomscut in case you guys have any comments around this since you folks were involved in HDFS-16386
public static final String DFS_DATANODE_FSDATASETASYNCDISK_MAX_THREADS_PER_VOLUME_KEY = | ||
"dfs.datanode.fsdatasetasyncdisk.max.threads.per.volume"; | ||
public static final int DFS_DATANODE_FSDATASETASYNCDISK_MAX_THREADS_PER_VOLUME_DEFAULT = 4; | ||
public static final String DFS_DATANODE_FSDATASETASYNCDISK_THREADS_PER_VOLUME_KEY = | ||
"dfs.datanode.fsdatasetasyncdisk.threads.per.volume"; | ||
public static final int DFS_DATANODE_FSDATASETASYNCDISK_THREADS_PER_VOLUME_DEFAULT = 1; |
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.
you can't change the config name if that is there in a released version.
@ayushtkn Thanks for your review. About HDFS-16386, the increased memory maybe caused by the stacked async deletion takes, increasing or decreasing the maximum number of threads may cannot fix it. Can increase the corePoolSize of the threadPoolExecutor maybe can reduce the memory. I have updated the patch, please help me review it again. Thanks |
💔 -1 overall
This message was automatically generated. |
@ayushtkn @jianghuazhu Sir, I added one UT to test the active count and core pool size of the executer. Can you help me review it again. @ferhui @haiyang1987 Masters, can you help me review this PR? |
💔 -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.
The work looks good to me, increase the corePoolSize of the threadPoolExecutor to ease stacked blocks pending deletion problem
The failed UT @Hexiaoqiao Sir, can you help me review this patch too? |
Description of PR
The number of threads in FsDatasetAsyncDiskService#addExecutorForVolume is elastic right now, make it fixed.
Presently the corePoolSize is set to 1 and maximumPoolSize is set to maxNumThreadsPerVolume, but since the size of Queue is Integer.MAX, the queue doesn't tend to get full and threads are always confined to 1 irrespective of maxNumThreadsPerVolume.