Skip to content
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

HDDS-7058. EC: ReplicationManager - Implement Ratis Container healthCheck #3634

Closed
wants to merge 1 commit into from

Conversation

JacksonYao287
Copy link
Contributor

What changes were proposed in this pull request?

Implement Ratis Container healthCheck, the first step for refactoring legacy RM.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-7058

How was this patch tested?

unit test

Copy link
Contributor

@umamaheswararao umamaheswararao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JacksonYao287 for the patch. Good work towards, Ratis RM migration to new RM.

I would let @sodonnel also take a look at this.


boolean sufficientlyReplicated = replicaCount.isSufficientlyReplicated();
boolean isPolicySatisfied = placementStatus.isPolicySatisfied();
if (!sufficientlyReplicated || !isPolicySatisfied) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably misreplicated state should be given low priority compared to Underplicated state. So, is it make sense to have misreplicated status once under/overreplicated status checks done?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if HDDS-6892 implemented to provide Mis replicated status, we need to revisit here to use that. So, I would suggest to file a followup.

@sodonnel
Copy link
Contributor

sodonnel commented Sep 9, 2022

We are refactoring how the container checks are performed in RM in #3743, so once that goes in this one will need refactored to fit into the new way of working.

@sodonnel
Copy link
Contributor

sodonnel commented Oct 3, 2022

@JacksonYao287 I was wondering if you are able to come back to this PR in the near future? We have made good progress with the new RM in recent weeks and it would be good to get it in. It does need a bit of a refactor as we change things around a little in RM, but the core logic will be the same.

@sodonnel
Copy link
Contributor

sodonnel commented Oct 6, 2022

I submitted a new PR for this one in #3802 using the changes here as a starting point and then refactored to fit into the new framework

@JacksonYao287
Copy link
Contributor Author

@sodonnel I will be back soon, thanks for working on this. please feel free to take this jira and i will close this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants