-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-17858. Fix TestDFSAdmin#testAllDatanodesReconfig fails #8107
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
base: trunk
Are you sure you want to change the base?
Conversation
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java
Outdated
Show resolved
Hide resolved
39e81f4 to
4aba428
Compare
| LambdaTestUtils.await(1000, 50, () -> { | ||
| int result = admin.startReconfiguration("datanode", "livenodes"); | ||
| assertThat(result).isEqualTo(0); | ||
| final List<String> outs = new ArrayList<>(); | ||
| final List<String> errs = new ArrayList<>(); | ||
| awaitReconfigurationFinished("datanode", "livenodes", outs, errs); | ||
| return errs.isEmpty(); | ||
| }); | ||
|
|
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.
in this logic we are doing admin.startReconfiguration("datanode", "livenodes"); like in every iteration, we should just wait for the result and not perform any action, that changes the flow of the 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.
We cannot determine whether the reconfiguration has finished based on the return value of admin.startReconfiguration(), because even if it returns 0, the datanode's reconfig thread may not have finished. We need to actively get the reconfiguration status using getReconfigurationStatus, to wait reconfiguration finished.
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.
I have replaced awaitReconfigurationFinished() with getReconfigurationStatus().
4aba428 to
72de13a
Compare
|
💔 -1 overall
This message was automatically generated. |
72de13a to
a26cc18
Compare
|
💔 -1 overall
This message was automatically generated. |
a26cc18 to
bdd537b
Compare
|
💔 -1 overall
This message was automatically generated. |
bdd537b to
25d5571
Compare
|
💔 -1 overall
This message was automatically generated. |
| } | ||
| return !outs.isEmpty() && outs.get(0).contains("finished"); | ||
| }); | ||
|
|
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.
@ayushtkn The test has passed. Do you agree with this modification? Also, is it possible to replace LambdaTestUtils.await() with awaitReconfigurationFinished(), since their logic is the same?
TestDFSAdmin::testAllDatanodesReconfig() always failed because the last startReconfiguration() need some time to finish.