[
https://issues.apache.org/jira/browse/HADOOP-11646?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14348510#comment-14348510
]
Vinayakumar B commented on HADOOP-11646:
----------------------------------------
Hi kai,
Here are some of the comments of first look.
1. {code}
+ /**
+ *
+ * @return true if it's missing, otherwise false
+ */
+ public boolean isErased() {
+ return isErased;
+ }{code}
{{isErased}}, Do you mean is block is missing and needs to re-construct?
if Yes, I think better name would be {{isMissed}} itself.
2.
{code}+ @Override
+ public ErasureCodingStep decode(ECBlockGroup blockGroup) {
+ return performDecoding(blockGroup);
+ }
+
+ /**
+ * Perform decoding against a block blockGroup.
+ * @param blockGroup
+ * @return decoding step for caller to do the real work
+ */
+ protected abstract ErasureCodingStep performDecoding(ECBlockGroup
blockGroup);{code}
Why there are two separate methods with same params. Why can't just directly
IMPLs can implement {{decode(..)}} instead of implementing
{{performDecoding(..)}} ?
Same Q in AbstractErasureEncoder also
3.
For copying from one array to another System.arrayCopy(..) would be better.
{code}
+ int idx = 0;
+ for (int i = 0; i < getNumParityUnits(); i++) {
+ inputBlocks[idx ++] = blockGroup.getParityBlocks()[i];
+ }
+ for (int i = 0; i < getNumDataUnits(); i++) {
+ inputBlocks[idx ++] = blockGroup.getDataBlocks()[i];
+ }{code}
could be done as
{code} System.arraycopy(blockGroup.getParityBlocks(), 0, inputBlocks, 0,
getNumParityUnits());
System.arraycopy(blockGroup.getDataBlocks(), 0, inputBlocks,
getNumParityUnits(), getNumDataUnits());{code}
Correct?
4. I think {{protected ECBlock[] getOutputBlocks(ECBlockGroup blockGroup) {}}
could be re-written as below for simplicity.
{code} protected int getNumErasedBlocks(ECBlockGroup blockGroup) {
int num = getNumErasedBlocks(blockGroup.getParityBlocks());
num += getNumErasedBlocks(blockGroup.getDataBlocks());
return num;
}{code}
5. Need to track the index of the erasedBlocks separately. Otherwise will throw
AIOBException
{code}+ ECBlock[] erasedBlocks = new ECBlock[numErased];
+ for (int i = 0; i < inputBlocks.length; i++) {
+ if (inputBlocks[i].isErased()) {
+ erasedBlocks[i] = inputBlocks[i];
+ }
+ }{code}
6. Rename the patch with HADOOP prefix
> Erasure Coder API for encoding and decoding of block group
> ----------------------------------------------------------
>
> Key: HADOOP-11646
> URL: https://issues.apache.org/jira/browse/HADOOP-11646
> Project: Hadoop Common
> Issue Type: Sub-task
> Reporter: Kai Zheng
> Assignee: Kai Zheng
> Fix For: HDFS-7285
>
> Attachments: HDFS-7662-v1.patch, HDFS-7662-v2.patch,
> HDFS-7662-v3.patch
>
>
> This is to define ErasureCoder API for encoding and decoding of BlockGroup.
> Given a BlockGroup, ErasureCoder extracts data chunks from the blocks and
> leverages RawErasureCoder defined in HDFS-7353 to perform concrete encoding
> or decoding.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
