[
https://issues.apache.org/jira/browse/HDFS-881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12896205#action_12896205
]
Jakob Homan commented on HDFS-881:
----------------------------------
Patch review:
* Overall looks good, but unfortunately has become unsync'ed with trunk. Todd,
if you can re-base, I'll work on getting this committed.
* It might be good to move the sanity check (line 50-52 in patch) into a method
on the PacketHeader ({code}boolean sanityCheck(int lastSeqNo){code}) class to
make PacketHeader repsonsible for its own sanity check and to simplify
DFSClient. Less important might be a method to do the debug log entry in
PacketHeader rather than DFSClient; either is fine.
* Once this is done, it'll be easier to unit test the PacketHeader class, which
should also be done, if just for the sanityCheck and the writable methods.
Otherwise, we can consider this a re-factor and verify correctness through
existing tests.
* Having two readFields seems reasonable for now.
This patch will lead to more object creation/churn, but that shouldn't be an
issue; I think benchmarks before committing will be unnecessary.
> Refactor DataNode Packet header into DataTransferProtocol
> ---------------------------------------------------------
>
> Key: HDFS-881
> URL: https://issues.apache.org/jira/browse/HDFS-881
> Project: Hadoop HDFS
> Issue Type: Improvement
> Affects Versions: 0.22.0
> Reporter: Todd Lipcon
> Assignee: Todd Lipcon
> Attachments: hdfs-881.txt
>
>
> The Packet Header format is used ad-hoc in various places. This JIRA is to
> refactor it into a class inside DataTransferProtocol (like was done with
> PipelineAck)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
