HDFS-16624. Fix flaky unit test TestDFSAdmin#testAllDatanodesReconfig #4412
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nodesReconfig ERROR.
Contributor
Author
|
@virajjasani @tomscut please help to review the code. |
Contributor
|
Thanks for the PR @slfan1989 |
Contributor
Author
Hi @virajjasani , |
Contributor
|
That's surprising indeed. One of the recent PRs, I see this test passing: |
Contributor
Author
But you can look at the screenshot of my local debugger(in jira HDFS-16624), the index is indeed inaccurate. |
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
I already know the cause of the problem, because the log is out of order due to multi-threading. I compared the logs of several execution failures, as follows, after comparison, it is found that the log order cannot be It is the disorder caused by the following code. Therefore, using a fixed-order assertion judgment will occasionally succeed and occasionally fail. improve proposals: @virajjasani @tomscut Is it ok for me to do this? Or can you give me some advice? |
virajjasani
reviewed
Jun 7, 2022
| if (!status.stopped()) { | ||
| out.println(" and is still running."); | ||
| return 0; | ||
| synchronized (this) { |
Contributor
There was a problem hiding this comment.
@slfan1989 This is good for concurrency control but we should avoid this for performance issues. Rather, we can update test.
Contributor
Author
There was a problem hiding this comment.
thanks for your suggestion, I will modify the junit test.
Comment on lines
1208
to
1210
| assertEquals("SUCCESS: Changed property dfs.datanode.peer.stats.enabled", outs.get(1)); | ||
| assertEquals("\tFrom: \"false\"", outs.get(2)); | ||
| assertEquals("\tTo: \"true\"", outs.get(3)); |
Contributor
There was a problem hiding this comment.
Given that concurrency is at play here, we can do something like this:
assertTrue("SUCCESS: Changed property dfs.datanode.peer.stats.enabled".equals(outs.get(2))
|| "SUCCESS: Changed property dfs.datanode.peer.stats.enabled".equals(outs.get(1)));
Contributor
Author
There was a problem hiding this comment.
I will modify the code.
Contributor
|
Good find btw @slfan1989 |
added 2 commits
June 7, 2022 16:59
This reverts commit 689f74b.
Contributor
Author
|
@virajjasani @tomscut please help me review the code again. |
Contributor
Author
The code you contributed is very useful, I have learned a lot from you, thank you again! |
tomscut
approved these changes
Jun 8, 2022
Contributor
tomscut
left a comment
tomscut
left a comment
There was a problem hiding this comment.
The change looks good to me. @virajjasani Please also take a look. Thanks.
Contributor
|
Thank you for the nice fix @slfan1989 and for the review @tomscut !! |
|
🎊 +1 overall
This message was automatically generated. |
tomscut
changed the title
Contributor
|
Thanks @slfan1989 for your contribution! Thanks @virajjasani for your review. |
tomscut
pushed a commit
to tomscut/hadoop
that referenced
this pull request
Jun 9, 2022
…apache#4412) Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Tao Li <[email protected]>
tomscut
pushed a commit
that referenced
this pull request
Jun 9, 2022
…#4412) Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Tao Li <[email protected]>
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…apache#4412) Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Tao Li <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA:HDFS-16624. Fix org.apache.hadoop.hdfs.tools.TestDFSAdmin#testAllDatanodesReconfig ERROR.
(#4406) found an error message during Junit unit testing, as follows:
After code debugging, it was found that there was an error in the selection outs.get(2) of the assertion(1208), index should be equal to 1.
Please see jira for details.
Introduce code in #4264