[ 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)