[
https://issues.apache.org/jira/browse/HDFS-8873?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14903220#comment-14903220
]
Colin Patrick McCabe commented on HDFS-8873:
--------------------------------------------
Thanks, [~templedf].
{code}
416 if ((throttle > 1000) || (throttle <= 0)) {
417 if (throttle > 1000) {
418 LOG.error(
419
DFSConfigKeys.DFS_DATANODE_DIRECTORYSCAN_THROTTLE_LIMIT_MS_PER_SEC_KEY
420 + " set to value above 1000 ms/sec. Assuming default value
of " +
421
DFSConfigKeys.DFS_DATANODE_DIRECTORYSCAN_THROTTLE_LIMIT_MS_PER_SEC_DEFAULT);
{code}
Can we have a constant here for {{MS_PER_SEC}}? I think I commented on this
earlier
{code}
455 if (throttleLimitMsPerSec < 1000) {
456 logMsg = String.format("Periodic Directory Tree Verification scan"
457 + " starting at %dms with interval %dms and run limit %dms/s",
458 firstScanTime, scanPeriodMsecs, throttleLimitMsPerSec);
{code}
Maybe say "throttle" instead of "run limit"?
{code}
766 // Variable for tracking time spent running and waiting for testing
767 // purposes
768 private Long markMs;
{code}
Does this need to be an object, or can it be a primitive? I don't see any case
where we need it to be null.
{code}
895 while (nowMs % 1000L > throttleLimitMsPerSec) {
896 try {
897 Thread.sleep(1000L - (nowMs % 1000L));
898 } catch (InterruptedException ex) {
899 // Try sleeping again and mark the thread as interrupted
900 Thread.currentThread().interrupt();
901 }
{code}
This logic seems flawed. If we sleep for a whole second, we'll be back in the
case where nowMs % 1000 is what it was before, and make no progress. We should
gracefully handle the case where the sleep is longer than a second by not
re-throttling.
> throttle directoryScanner
> -------------------------
>
> Key: HDFS-8873
> URL: https://issues.apache.org/jira/browse/HDFS-8873
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: datanode
> Affects Versions: 2.7.1
> Reporter: Nathan Roberts
> Assignee: Daniel Templeton
> Attachments: HDFS-8873.001.patch, HDFS-8873.002.patch,
> HDFS-8873.003.patch, HDFS-8873.004.patch, HDFS-8873.005.patch
>
>
> The new 2-level directory layout can make directory scans expensive in terms
> of disk seeks (see HDFS-8791) for details.
> It would be good if the directoryScanner() had a configurable duty cycle that
> would reduce its impact on disk performance (much like the approach in
> HDFS-8617).
> Without such a throttle, disks can go 100% busy for many minutes at a time
> (assuming the common case of all inodes in cache but no directory blocks
> cached, 64K seeks are required for full directory listing which translates to
> 655 seconds)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
