[ https://issues.apache.org/jira/browse/HDFS-11149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706770#comment-15706770 ]
Xiaoyu Yao commented on HDFS-11149: ----------------------------------- Thanks [~arpitagarwal] for working on this and [~anu] for the review. The latest patch looks good to me. I just have a few questions: DatasetVolumeChecker.java 1. Line 70: The check context is a Boolean which is not used. Can we have a wrapper class to make it easier for future extension? {code} private AsyncChecker<Boolean, VolumeCheckResult> delegateChecker; {code} 2. NIT: line 394 "StorageLocationChecker" should be "DatasetVolumeChecker". {code} LOG.warn("StorageLocationChecker interrupted during shutdown."); {code} 3. NIT: should we verify minDiskCheckGapMs > 0 like other time duration keys {code} 124 minDiskCheckGapMs = conf.getTimeDuration( 125 DFSConfigKeys.DFS_DATANODE_DISK_CHECK_MIN_GAP_KEY, 126 DFSConfigKeys.DFS_DATANODE_DISK_CHECK_MIN_GAP_DEFAULT, 127 TimeUnit.MILLISECONDS); {code} 4. NIT: need a space between {} and ms. {code} LOG.warn("checkAllVolumes timed out after {}ms" + 186 maxAllowedTimeForCheckMs); {code} FsVolumeImpl.java: 5. line 917 Do we need to change the return value from FsDatsetSpi to FsDatasetImpl? {code} public FsDatasetImpl getDataset() {code} 6. Do we plan to keep or remove the DataNode#checkDiskError() after this change? 7. Regarding [~anu]'s concern about disk removal, I think testCheckingClosedVolume has partial coverage on it. > Support for parallel checking of FsVolumes > ------------------------------------------ > > Key: HDFS-11149 > URL: https://issues.apache.org/jira/browse/HDFS-11149 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: datanode > Reporter: Arpit Agarwal > Assignee: Arpit Agarwal > Attachments: HDFS-11149.01.patch, HDFS-11149.02.patch, > HDFS-11149.03.patch, HDFS-11149.04.patch, HDFS-11149.05.patch, > HDFS-11149.06.patch, HDFS-11149.07.patch, HDFS-11149.08.patch, > HDFS-11149.09.patch, HDFS-11149.10.patch, HDFS-11149.11.patch > > > Similar to HDFS-11119, the DataNode can parallelize checking FsVolumes. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org