-
Notifications
You must be signed in to change notification settings - Fork 594
HDDS-2499. IsLeader information is lost when update pipeline state. #180
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
Conversation
adoroszlai
left a comment
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.
Thanks @ChenSammi for fixing this bug. The unit test is OK now:
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.023 s - in org.apache.hadoop.hdds.scm.pipeline.TestSCMPipelineManager
I think you might have intended to ask @swagle for review.
|
You are right. I'm asking @swagle for the review. Sorry to ping the wrong person. The odd thinging is I cannot find him in reviewer list. |
swagle
left a comment
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.
Thanks, @ChenSammi for fixing this issue, I missed both of them in HDDS-1868.
| pipelineReportHandler | ||
| .onMessage(pipelineReportFromDatanode, new EventQueue()); | ||
| } | ||
| List<DatanodeDetails> nodes = pipeline.getNodes(); |
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.
There is a similar negative case in Line 199-205. It does not cause failure but we can fix them similarly. The previous approach can also be fixed with slightly modification of TestUtils.getPipelineReportFromDatanode as demoed in the dup PR #195.
xiaoyuyao
left a comment
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, some minor comments inline.
|
Thanks for the update. LGTM, +1. |
|
Thanks @xiaoyuyao and @swagle for the review. |
…state. (apache#180)" This reverts commit 0b5df11.
https://issues.apache.org/jira/browse/HDDS-2499?filter=12346871