[ 
https://issues.apache.org/jira/browse/HDFS-9231?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14971662#comment-14971662
 ] 

Yongjun Zhang commented on HDFS-9231:
-------------------------------------

Hi [~xiaochen],

Thanks for the new rev. Some further comments:

1. 
{quote}
My apologies for the confusion, I added some comments in this method. But 
getting sf for each snapshottable dir is needed, since /d1 and /d2 have 
different snapshotlist.
{quote}
I meant to precalcuate a list of "sf"s, and pass the list to 
FSDirSnapshotOp#getSnapshotFiles so to avoid doing it once for each file. Sorry 
for not having been clear.

2. 
{quote}
Added a link to FSNamesystem#listCorruptFileBlocksWithSnapshot which explains 
that parameter in javadoc.
{quote}
I think it's ok to not have this link since it looks a bit redundant. So 
consider I took back my earlier comment since you have javadoc in 
FSNamesystem#listCorruptFileBlocksWithSnapshot. :-) sorry about that.

3. 
{quote}
Good catch! I updated the code to call getINode which invokes getINodesInPath.
{quote}
Good change to use {{getINode}}.

4. In {{testFsckListCorruptSnapshotFiles() }}
Suggest to introduce two variables, numFiles, numSnapshotFiles, initially 
numFile = 3 and numSnapshotFiles = 0, then when you create /corruptData/file, 
increment numFiles by 1, when you create snapshot, set numSnapshotFiles to 4. 
when deleting the file, decrement numFile by 1. When gets to assertion part,  
the first one check against (numFiles + numSnapshotFiles) (in place of the 
"7"), the second one check against numFiles.

Thanks.


> fsck doesn't explicitly list when Bad Replicas/Blocks are in a snapshot
> -----------------------------------------------------------------------
>
>                 Key: HDFS-9231
>                 URL: https://issues.apache.org/jira/browse/HDFS-9231
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: snapshots
>            Reporter: Xiao Chen
>            Assignee: Xiao Chen
>         Attachments: HDFS-9231.001.patch, HDFS-9231.002.patch, 
> HDFS-9231.003.patch, HDFS-9231.004.patch, HDFS-9231.005.patch
>
>
> Currently for snapshot files, {{fsck -list-corruptfileblocks}} shows corrupt 
> blocks with the original file dir instead of the snapshot dir, and {{fsck 
> -list-corruptfileblocks -includeSnapshots}} behave the same.
> This can be confusing because even when the original file is deleted, fsck 
> will still show that deleted file as corrupted, although what's actually 
> corrupted is the snapshot. 
> As a side note, {{fsck -files -includeSnapshots}} shows the snapshot dirs.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to