[
https://issues.apache.org/jira/browse/HDFS-8058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14625453#comment-14625453
]
Jing Zhao commented on HDFS-8058:
---------------------------------
Thanks for updating the patch, Zhe. The new patch looks good to me. Some minor
comments:
# Minor: In FSImageFormatPBINode#save, maybe it is better to always set
"b.setIsStriped(n.isStriped)" so that we do not depend on the protobuf bool
default value here.
{code}
if (n.isStriped()) {
b.setIsStriped(true);
{code}
# In the following code, the variable e can be removed. The Preconditions check
can be moved out of the for loop. Also it's better to use {{blocks}} directly
for the INodeFile's construction, instead of calling {{addBlock}}. So here what
we can do is to prepare the block array before the INodeFile's construction.
{code}
BlockInfo[] blocks = new BlockInfo[bp.size()];
for (int i = 0, e = bp.size(); i < e; ++i) {
BlockProto b = bp.get(i);
if (isStriped) {
Preconditions.checkState(f.hasStripingCellSize());
blocks[i] = new BlockInfoStriped(PBHelper.convert(b), schema,
(int)f.getStripingCellSize());
} else {
blocks[i] = new BlockInfoContiguous(PBHelper.convert(b),
replication);
}
file.addBlock(blocks[i]);
}
{code}
# {{HeaderFormat#getIsStriped}} can be renamed to {{HeaderFormat#isStriped}}.
# It will be better to have "isStriped" in INodeFile's constructor. In this way
{{setStriped}} can be removed.
> Erasure coding: use BlockInfo[] for both striped and contiguous blocks in
> INodeFile
> -----------------------------------------------------------------------------------
>
> Key: HDFS-8058
> URL: https://issues.apache.org/jira/browse/HDFS-8058
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Affects Versions: HDFS-7285
> Reporter: Yi Liu
> Assignee: Zhe Zhang
> Attachments: HDFS-8058-HDFS-7285.003.patch,
> HDFS-8058-HDFS-7285.004.patch, HDFS-8058-HDFS-7285.005.patch,
> HDFS-8058.001.patch, HDFS-8058.002.patch
>
>
> This JIRA is to use {{BlockInfo[] blocks}} for both striped and contiguous
> blocks in INodeFile.
> Currently {{FileWithStripedBlocksFeature}} keeps separate list for striped
> blocks, and the methods there duplicate with those in INodeFile, and current
> code need to judge {{isStriped}} then do different things. Also if file is
> striped, the {{blocks}} in INodeFile occupy a reference memory space.
> These are not necessary, and we can use the same {{blocks}} to make code more
> clear.
> I keep {{FileWithStripedBlocksFeature}} as empty for follow use: I will file
> a new JIRA to move {{dataBlockNum}} and {{parityBlockNum}} from
> *BlockInfoStriped* to INodeFile, since ideally they are the same for all
> striped blocks in a file, and store them in block will waste NN memory.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
