-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDFS-16146. All three replicas are lost due to not adding a new DataN… #3247
Conversation
💔 -1 overall
This message was automatically generated. |
*/ | ||
if (!isAppend && lastAckedSeqno < 0 | ||
&& stage == BlockConstructionStage.PIPELINE_SETUP_CREATE) { | ||
//no data have been written | ||
return; | ||
} else if (stage == BlockConstructionStage.PIPELINE_CLOSE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the stage is in PIPELINE_CLOSE state when when packet is the last in the block and all transferred data is acknowledged.
with this change, the re-replication which was triggered by NameNode block manager periodically, is moved to client side.
it looks fine to me. but HDFS re replication logic is complex. We should validate it with a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your suggestion. I just added a UT to this patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just leave some nits. Others LGTM. +1 from my side after fix the annotation.
@jojochuang Would you like to give another reviews? Will commit shortly if no other objections. Thanks.
@@ -1492,6 +1492,8 @@ public void run() { | |||
if (lastPacketInBlock) { | |||
// Finalize the block and close the block file | |||
finalizeBlock(startTime); | |||
/* for test only, no-op in production system */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just suggest to use //
annotation for single line code.
@@ -68,6 +68,9 @@ public void delaySendingAckToUpstream(final String upstreamAddr) | |||
throws IOException { | |||
} | |||
|
|||
public void delayAckLastPacket() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add Javadoc for this new injector method.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. +1. most of failed unit tests due to java.lang.OutOfMemoryError: unable to create new native thread
. I think it is not related to this changes.
looks good. @Hexiaoqiao please go ahead and merge it. thanks |
Committed to trunk. Thanks @zhangshuyan0 for your works. Thanks @jojochuang for your reviews. |
apache#3247) Contributed by Shuyan Zhang. Reviewed-by: He Xiaoqiao <[email protected]> Reviewed-by: Wei-Chiu Chuang <[email protected]>
… a new DataN… (apache#3247) Contributed by Shuyan Zhang. Reviewed-by: He Xiaoqiao <[email protected]> Reviewed-by: Wei-Chiu Chuang <[email protected]> (cherry picked from commit 10a2526) Conflicts: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNodeFaultInjector.java Change-Id: I47d8042771ed9dc634a866e0269989d253659a4e
apache#3247) Contributed by Shuyan Zhang. Reviewed-by: He Xiaoqiao <[email protected]> Reviewed-by: Wei-Chiu Chuang <[email protected]>
apache#3247) Contributed by Shuyan Zhang. Reviewed-by: He Xiaoqiao <[email protected]> Reviewed-by: Wei-Chiu Chuang <[email protected]>
…ode in time.
NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute