[
https://issues.apache.org/jira/browse/HADOOP-13010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15242373#comment-15242373
]
Kai Zheng commented on HADOOP-13010:
------------------------------------
Continue to address the original comments.
bq. AbstractRawErasureCoder – why do we need this base class? Its function
seems to be just storing configuration values.
Having the common base class would allow encoder and decoder to share common
properties, not just configurations, but also schema info and some options. We
can also say that encoder and decoder are also coders, which allows to write
some common behaviors to deal with coders, not encoder or decoder specific. I
understand it should also work by composition, but right now I don't see very
much benefits to switch this from one style to the other, or troubles if we
don't.
bq. AbstractRawErasureEncoder /AbstractRawErasureDecoder – why are these
classes separate from RawErasureEncoder / RawErasureDecoder? ... Base classes
are also easier to extend in the future than interfaces because you can add new
methods without breaking backwards compatibility
It sounds better not to have the interfaces since the benefit is obvious. So in
summary how about having these classes (no interface) now: still
AbstractRawErasureCoder, RawErasureEncoder/Decoder (no Abstract prefix now,
with the original interface combined), and all kinds of concrete inherent
encoders/decoders. All client codes will declare RawErasureEncoder/Decoder type
when creating instances.
bq. DummyRawDecoder – NoOpRawDecoder would be a better name than "Dummy". Is
this intended to be used just in unit tests, or is it something the end-user
should be able to configure?
Hmm, I'm not sure but I think "Dummy" is more simple and flexible to interpret.
It's for tests but may be configured and used in a benchmark test when
comparing. [~zhz] and [~lirui] please help clarify if I missed something.
> 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
>
>
> 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)
