[
https://issues.apache.org/jira/browse/HADOOP-6949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13006586#comment-13006586
]
Suresh Srinivas commented on HADOOP-6949:
-----------------------------------------
This is an important patch. Thanks for doing it in a backward compatible way
and also in a way where both on disk format and RPC format can co-exist.
Comments:
# ArrayPrimitiveWriable
#* Class javadoc is not very clear - not sure what you mean by boxed type not
supported by ObjectWritable and why that should be mentioned in the javadoc. I
am not sure also how clients can provide external locking; more appropriate
would be to say, pass a copy of the array, if concurrent modifications are
expected.
#* Comments related to declaredComponentType is not clear. Can you please add
description on what componentType and declaredComponentType are and how they
are used, bef adding further description.
#* Is PrimitiveArrayWritable a better name?
#* Add @InterfaceAudience.Public and @InterfaceStability.Stable
#* Throw HadoopIllegalArgumentException instead of NullPointerException and
IllegalArgumentException
#* Optional: The read write methods could be made static methods in
WritableUtils class, so it can be used by others. Also PrimitiveType map,
isCanditdatePrimitive() could also move to helper.
#* Rename isCandidatePrimitive() to isPrimitive()?
#* Is constructor with componentType as argument used any where?
#* Some methods could be private - for example - getDeclaredComponentType(),
isDeclaredComponentType(),
#* Can you use Text instead of UTF8? This avoids deprecation warnings. You
could also use WritableUtils to write and read string. This can be done in many
other places in the patch.
#* I prefer MalformedInputException instead of ProtocolException (see Text.java
for example)
# ObjectWritable.java
#* Can you call set() method while setting instance and declaredClass in
#writeObject() method?
#* Not sure about the comment: // This case must come after the "isArray()" case
#* Not sure why this condition is required in #writeObject() -
instance.getClass().getName().equals(declaredClass.getName())
# TestArrayWritable
#* In the class javadoc can you add link to PrimitiveArrayWritable?
#* Can you make in, out, bigSet, resultSet and expectedSet final?
#* Should we prefer reading and writing objects using readFields and
writeFields - since they are the methods supporting Writable contract?
> Reduces RPC packet size for primitive arrays, especially long[], which is
> used at block reporting
> -------------------------------------------------------------------------------------------------
>
> Key: HADOOP-6949
> URL: https://issues.apache.org/jira/browse/HADOOP-6949
> Project: Hadoop Common
> Issue Type: Improvement
> Components: io
> Affects Versions: 0.22.0
> Reporter: Navis
> Assignee: Matt Foley
> Fix For: 0.23.0
>
> Attachments: ObjectWritable.diff, arrayprim.patch,
> arrayprim_v4.patch, arrayprim_v5.patch, arrayprim_v6.patch
>
> Original Estimate: 10m
> Remaining Estimate: 10m
>
> Current implementation of oah.io.ObjectWritable marshals primitive array
> types as general object array ; array type string + array length + (element
> type string + value)*n
> It would not be needed to specify each element types for primitive arrays.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
