[
https://issues.apache.org/jira/browse/HADOOP-13010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15254521#comment-15254521
]
Kai Zheng commented on HADOOP-13010:
------------------------------------
The change look good in following the discussion, some comments:
1. ErasureCoderOptions may be prepared as part of HH coder's initialization
work? You don't have to pass {{false, false}} as they're the default.
{code}
private RawErasureEncoder checkCreateXorRawEncoder() {
if (xorRawEncoder == null) {
+ ErasureCoderOptions erasureCoderOptions = new ErasureCoderOptions(
+ getNumDataUnits(), getNumParityUnits(), false, false);
xorRawEncoder = CodecUtil.createXORRawEncoder(getConf(),
- getNumDataUnits(), getNumParityUnits());
- xorRawEncoder.setCoderOption(CoderOption.ALLOW_CHANGE_INPUTS, false);
+ erasureCoderOptions);
}
return xorRawEncoder;
}
{code}
2. Missed a point and need to address: {{CoderUtil:convertInputBuffer}}. Ref.
comment from Colin above, it was suggested to be renamed as:
cloneAsDirectByteBuffer.
3. Missed a point and need to address: {{makeValidIndexes}} needs to be renamed
to {{getNullIndexes}} and consistent with others, ref. above relevant
discussions.
4. {{ErasureCoderOptions conf}} better to be: {{ErasureCoderOptions
coderOptions}}, please check all places thru the large change.
Sounds good to do the similar refactoring for the block level coders and do it
separately. Thanks Rui for the major help and work!
> Refactor raw erasure coders
> ---------------------------
>
> Key: HADOOP-13010
> URL: https://issues.apache.org/jira/browse/HADOOP-13010
> Project: Hadoop Common
> Issue Type: Sub-task
> Reporter: Kai Zheng
> Assignee: Kai Zheng
> Fix For: 3.0.0
>
> Attachments: HADOOP-13010-v1.patch, HADOOP-13010-v2.patch,
> HADOOP-13010-v3.patch, HADOOP-13010-v4.patch
>
>
> This will refactor raw erasure coders according to some comments received so
> far.
> * As discussed in HADOOP-11540 and suggested by [~cmccabe], better not to
> rely class inheritance to reuse the codes, instead they can be moved to some
> utility.
> * Suggested by [~jingzhao] somewhere quite some time ago, better to have a
> state holder to keep some checking results for later reuse during an
> encode/decode call.
> This would not get rid of some inheritance levels as doing so isn't clear yet
> for the moment and also incurs big impact. I do wish the end result by this
> refactoring will make all the levels more clear and easier to follow.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
