|
Yes I understand that the blockMap is there right now because of the subdirs and the volumes, but
it would be better to have file names computable. This should probably belong to a different issue though. This patch optimizes FSDataset.getBlockFile() and FSDataset.getLength() so that they perform the data-node
blockMap lookup only once. The patch is pretty straightforward. I don't think we should benchmark this. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12377962/getBlockFile.patch  against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included -1. The patch doesn't appear to include any new or modified tests. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs -1. The patch appears to introduce 1 new Findbugs warnings. core tests -1. The patch failed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1976/testReport/ This message is automatically generated. This fixes findBugs warnings.
I could not reproduce test timeout in TestDFSStorageStateRecovery. This test has a lot of test cases. My suspicion is that if Hudson runs slow it could run out of time on this. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12378094/getBlockFile1.patch against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included -1. The patch doesn't appear to include any new or modified tests. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests -1. The patch failed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1987/testReport/ This message is automatically generated. +1 This patch looks good. It removes a duplicate block map look up.
I verified both test failures. They are not related to the patch imo.
In the first case TestDFSStorageStateRecovery took too long. It finished only 47 cases out of 71 in 13 minutes when the junit framework terminated it. In the second case the cluster fell into a infinite loop trying to replicate a block. Filed I just committed this.
Integrated in Hadoop-trunk #434 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/434/
) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Closed
Major
Inefficient FSDataset.getBlockFile()
there is no need to hold Block to File mapping in memory.
It has the full path like "dir/subdir2/subdir63/ ....". Also existence of block to File mapping indicates that file is still valid.
currently getBlockFile() is expected to throw IOException when there is no mapping. yes, removing double look up would be good.