[
https://issues.apache.org/jira/browse/HDFS-8062?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14519344#comment-14519344
]
Kai Zheng commented on HDFS-8062:
---------------------------------
Thanks Kai for the hard work and the large patch!
1. It doesn't make much sense to have the cast. Maybe just use *int* type in
all the places?
{code}
+ groupSize = (short)ecInfo.getSchema().getNumDataUnits();
{code}
2. Minor, better to use *final* for the following in {{StripedDataStreamer}}:
{code}
+ private ECInfo ecInfo;
+ private ECSchema schema;
{code}
3. I thought {{cellSize}} should be passed similar to the other two, all from a
schema. Please note the system default schema should only be used when creating
an EC zone. Please check other places as well.
{code}
public BlockInfoStriped(Block blk, short dataBlockNum, short parityBlockNum) {
initIndices();
this.dataBlockNum = dataBlockNum;
this.parityBlockNum = parityBlockNum;
+ this.cellSize = ECSchemaManager.getSystemDefaultSchema().getChunkSize();
}
{code}
4. How about adding the following codes below the comment? In all the similar
places.
{code}
+ // TODO: ECSchema can be restored from persisted FSEditLog file (HDFS-7859).
ECSchema schema = ECSchemaManager.getSystemDefaultSchema(); // TODO: should be
from image
// then proceed with the schema ...
{code}
5. I'm wondering about the following change, regarding the *else* part. Maybe
just throw error exception?.
{code}
- return isStriped() ?
- HdfsConstants.NUM_DATA_BLOCKS + HdfsConstants.NUM_PARITY_BLOCKS : max;
+
+ FileWithStripedBlocksFeature sb = getStripedBlocksFeature();
+ if (isStriped() && sb != null) {
+ BlockInfoStriped blockInfoStriped = (BlockInfoStriped)getLastBlock();
+ if (blockInfoStriped != null) {
+ return blockInfoStriped.getTotalBlockNum();
+ } else {
+ // In case that there is no block in block group, it can be regarded
as default schema.
+ return
(short)(ECSchemaManager.getSystemDefaultSchema().getNumDataUnits()
+ + ECSchemaManager.getSystemDefaultSchema().getNumParityUnits());
+ }
+ }
+ return max;
{code}
6. In {{TestDFSStripedInputStream}}, how about having the following? Again,
please save using the default schema, it has no much difference comparing with
the hard-coded constants. Please check other places as well.
{code}
private ECSchema testSchema = ECSchemaManager.getSystemDefaultSchema(); // May
use other schema in future
private static final int dataBlocks = testSchema.getNumDataUnits();
private static final int parityBlocks = testSchema.getNumParityUnits();
{code}
> Remove hard-coded values in favor of EC schema
> ----------------------------------------------
>
> Key: HDFS-8062
> URL: https://issues.apache.org/jira/browse/HDFS-8062
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Kai Zheng
> Assignee: Kai Sasaki
> Attachments: HDFS-8062.1.patch, HDFS-8062.2.patch, HDFS-8062.3.patch,
> HDFS-8062.4.patch, HDFS-8062.5.patch
>
>
> Related issues about EC schema in NameNode side:
> HDFS-7859 is to change fsimage and editlog in NameNode to persist EC schemas;
> HDFS-7866 is to manage EC schemas in NameNode, loading, syncing between
> persisted ones in image and predefined ones in XML.
> This is to revisit all the places in NameNode that uses hard-coded values in
> favor of {{ECSchema}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
