[ https://issues.apache.org/jira/browse/HDFS-7748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14369817#comment-14369817 ]
Chris Nauroth commented on HDFS-7748: ------------------------------------- Hi Anu. This patch looks mostly good to me. Here are a few comments. # {{BlockReceiver.PacketResponder#run}}: There is a line that calculates a {{myStatus}} local variable. This had been introduced in HDFS-7270. I think it's unused now and can be removed. # datatransfer.proto: There is typo in the comment: "configration" instead of "configuration". Please indent by 4 spaces. (Sorry to nitpick.) # hdfs-site.xml: Please remove this file from the next revision of the patch. (I know you had been doing some local testing with the ECN configuration turned on.) I think the original intent of HDFS-7270 was to try to match the semantics of RFC-3168 as closely as possible. Therefore, the {{ECN}} enum needed 4 possible values, which were named {{DISABLED}}, {{SUPPORTED}}, {{SUPPORTED2}} and {{CONGESTED}}. Shall we keep the same values here in the new protobuf enum? I'm not very familiar with this RFC, so I'll defer to [~wheat9] on what is the best definition of these values. {{TestTracing}} fails for me on current trunk even without this patch. All other test failures appear to be unrelated. > Separate ECN flags from the Status in the DataTransferPipelineAck > ----------------------------------------------------------------- > > Key: HDFS-7748 > URL: https://issues.apache.org/jira/browse/HDFS-7748 > Project: Hadoop HDFS > Issue Type: Bug > Reporter: Haohui Mai > Assignee: Anu Engineer > Priority: Blocker > Attachments: hdfs-7748.001.patch > > > Prior to the discussions on HDFS-7270, the old clients might fail to talk to > the newer server when ECN is turned on. This jira proposes to separate the > ECN flags in a separate protobuf field to make the ack compatible on both > versions. -- This message was sent by Atlassian JIRA (v6.3.4#6332)