-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDDS-1368. Cleanup old ReplicationManager code from SCM. #711
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
This comment has been minimized.
This comment has been minimized.
|
/retest |
1 similar comment
|
/retest |
mukul1987
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.
The changes generally looks good to me. Some minor comments.
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.
DeleteBlock handler is the only user of CommandWatcher now ?
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.
Yes, no other command uses CommandWatcher anymore.
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.
Should closeContainer happen before destroy pipeline, so that we close container cleanly ?
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.
TLDR; destoryPipelines internally calls close container already.
There are three stages where we close the containers when a datanode goes dead
- When the node is marked as stale, we destroy the pipeline which will try to close the containers cleanly.
(Finalizing and destroying the pipeline after the timeout)
-> When the node is marked as dead
2. destroyPipelines(datanodeDetails) --> call
Here we destroy the pipeline which will also try to close the containers, here we don't try to close the container cleanly. This is a fallback mechanism, if everything was working fine we should not have any pipelines at this point.
(Finalizing and destroying the pipeline without timeout)
- closeContainers(datanodeDetails, publisher) --> call
This is the last resort for closing a container, it is to handle a weird case where there is open container and it is not part of any pipeline. [This can (or) should not happen]
3688911 to
2d4f6b6
Compare
|
💔 -1 overall
This message was automatically generated. |
|
/retest |
Signed-off-by: Akira Ajisaka <[email protected]>
…dfs-default.xml Signed-off-by: Masatake Iwasaki <[email protected]>
… Contributed by Yishuang Lu. (apache#747)
…acOS. Contributed by Siyao Meng. Signed-off-by: Wei-Chiu Chuang <[email protected]>
…ements Signed-off-by: Akira Ajisaka <[email protected]>
… due to typos. Contributed by Wanqiang Ji.
Contributed by Prabhu Joseph
Contributed by Siyao Meng
…e Chen. (apache#661) Signed-off-by: Xiaoyu Yao <[email protected]>
…ontributed by Abhishek Modi.
…Contributed by Eric Yang
…r module. Contributed by Tao Yang.
…correct units when the default value is used. Contributed by starphin.
…AM heartbeat. Contributed by Abhishek Modi.
…unk. Contributed by Ayush Saxena.
… handle StandbyException when fetching HAServiceState. Contributed by Erik Krogen.
…ationTokenSecretManager. Contributed by CR Hota.
Contributed by Yesha Vora
Contributed by Dinesh Chitlangia.
…ontributed by Prabhu Joseph.
|
💔 -1 overall
This message was automatically generated. |
Author: Jagadish <[email protected]> Reviewers: Jagadish<[email protected]> Closes apache#711 from vjagadish1989/website-reorg8
PR #620 brings in new ReplicationManager and PR #662 plugs in the new code, this is for removing the old ReplicationManager and related code.