[
https://issues.apache.org/jira/browse/HDFS-6984?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15724789#comment-15724789
]
Andrew Wang commented on HDFS-6984:
-----------------------------------
Thanks for revving this Chris. I don't have a ton of background here, but my
review:
* Can we split the Serializable stuff into a separate change? Changing booleans
into Boolean objects is additional overhead, and combining these two changes
makes it harder to review. Also, I am not a Java serialization expert, but IIUC
maintaining compatibility is difficult, which means maybe people should just
use PB anyway.
* The serialization loses the extra {{isEncrypted}} and
{{FsPermission#getAclBit}} bits since it calls {{toShort}} rather than
{{toExtendedShort}}. Seems like we should save these, though the fact we pack
these bits into FsPermission is an internal implementation detail. Adding new
booleans to FileStatus might break cross-serialization with HdfsFileStatus
though. What is the usecase for cross-serialization?
* Could use some more extensive unit tests that test the default values and
error conditions with the shorts.
* Test nit: please also avoid the wildcard import.
Regarding Steve's comment on bounds checking, PB by default has a 64MB max
message size. We could use that as a reasonable upper-bound on the size of a
FileStatus.
> In Hadoop 3, make FileStatus serialize itself via protobuf
> ----------------------------------------------------------
>
> Key: HDFS-6984
> URL: https://issues.apache.org/jira/browse/HDFS-6984
> Project: Hadoop HDFS
> Issue Type: Improvement
> Affects Versions: 3.0.0-alpha1
> Reporter: Colin P. McCabe
> Assignee: Colin P. McCabe
> Labels: BB2015-05-TBR
> Attachments: HDFS-6984.001.patch, HDFS-6984.002.patch,
> HDFS-6984.003.patch
>
>
> FileStatus was a Writable in Hadoop 2 and earlier. Originally, we used this
> to serialize it and send it over the wire. But in Hadoop 2 and later, we
> have the protobuf {{HdfsFileStatusProto}} which serves to serialize this
> information. The protobuf form is preferable, since it allows us to add new
> fields in a backwards-compatible way. Another issue is that already a lot of
> subclasses of FileStatus don't override the Writable methods of the
> superclass, breaking the interface contract that read(status.write) should be
> equal to the original status.
> In Hadoop 3, we should just make FileStatus serialize itself via protobuf so
> that we don't have to deal with these issues. It's probably too late to do
> this in Hadoop 2, since user code may be relying on the existing FileStatus
> serialization there.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
