[
https://issues.apache.org/jira/browse/HDFS-1833?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13019846#comment-13019846
]
Daryn Sharp commented on HDFS-1833:
-----------------------------------
I'm still trying to completely wrap my head around the logic, but here's a few
questions.
1) "UNKNOWN" is misspelled in PipelineAck.UNKOWN_SEQNO
2) Should this be checking for type ==
PacketResponderType.HAS_DOWNSTREAM_IN_PIPELINE? Otherwise this will try to
read a packet when there is no pipeline.
{code}
if (type != PacketResponderType.LAST_IN_PIPELINE && !mirrorError) {
// read an ack from downstream datanode
ack.readFields(downstreamIn);
{code}
3) Thread.interrupted() seems to be unnecessarily invoked multiple times. Once
in the IOException handler, and then immediately thereafter. Perhaps the call
should be removed in the exception handler? Or at a minimum flip these
operands to avoid the second call?
{code}
if (Thread.interrupted() || isInterrupted) {
{code}
4) The "continue" appears to be equivalent to performing a "break" because the
loop condition requests running to be true. This isn't a big deal, but would a
break be more obvious?
{code}
LOG.info(myString + ": Thread is interrupted.");
running = false;
continue;
{code}
5) Should this:
{code}
short ackLen = type == PacketResponderType.LAST_IN_PIPELINE? 0 :
ack.getNumOfReplies();
{code}
be:
short ackLen = type == PacketResponderType.HAS_DOWNSTREAM_IN_PIPELINE ?
ack.getNumOfReplies() : 0;
> Refactor BlockReceiver
> ----------------------
>
> Key: HDFS-1833
> URL: https://issues.apache.org/jira/browse/HDFS-1833
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: data-node
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Tsz Wo (Nicholas), SZE
> Priority: Minor
> Fix For: 0.23.0
>
> Attachments: h1833_20110412.patch, h1833_20110413.patch
>
>
> There are repeated codes for creating log/error messages in BlockReceiver.
> Also, some comment in the codes are incorrect, e.g.
> {code}
> private int numTargets; // number of downstream datanodes including myself
> {code}
> but the count indeed excludes the current datanode.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
