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

[milvus-4.1.18] Enabling High Availability for Milvus #68

Merged

Conversation

Rachit-Chaudhary11
Copy link
Contributor

What this PR does / why we need it:

By enabling this feature, Milvus Instances will be resilient to failures, ensuring continuous availability and reliability for users. The implementation includes fault-tolerant mechanisms and redundancy configurations to mitigate downtime and main uninterrupted service in the event of node failures or network issues. Overall, this enhancement enhances the robustness and performance of Milvus, providing a more resilient infrastructure for data storage and retrieval.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [mychartname])
  • PR only contains changes for one chart

Signed-off-by: Rachit Chaudhary - r0c0axe <Rachit.Chaudhary@walmart.com>
@haorenfsa
Copy link
Collaborator

context: #67

@haorenfsa
Copy link
Collaborator

haorenfsa commented Feb 28, 2024

Hi @Rachit-Chaudhary11, a little advice, better continue the work on the same PR, so that we could tract the context of the problem directly.

For example, your previous work is on branch rollingupgrade, then you refine it on the branch deploymentStrategy.
So after you finish your work, you can checkout rollingupgrade again, and git reset deploymentStrategy --hard to make this branch the same as deploymentStrategy. Then you can use git push origin rollingupgrade -f to force update your rollingupgrade branch on github.
In this way, we can now continue the progress on the previous PR opened with branch rollingupgrade.

@haorenfsa
Copy link
Collaborator

/lgtm

@haorenfsa
Copy link
Collaborator

/cc @LoveEachDay please help review

@sre-ci-robot
Copy link
Collaborator

@haorenfsa: GitHub didn't allow me to request PR reviews from the following users: review, please.

Note that only zilliztech members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @LoveEachDay please help review

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Rachit-Chaudhary11
Copy link
Contributor Author

Rachit-Chaudhary11 commented Feb 28, 2024

Hi @Rachit-Chaudhary11, a little advice, better continue the work on the same PR, so that we could tract the context of the problem directly.

@haorenfsa
I have done this to keep the alternative approaches in separate branches as the other approach we are planning to use for an older version. I will still keep this in mind. Thanks.

@LoveEachDay
Copy link
Collaborator

@Rachit-Chaudhary11 Only if we have enabled activeStandby for milvus coordinator components, we can use rollingUpdate strategy, otherwise the pod will crash when rolling update.

Could you also check whether rootCoordinator.avtiveStandby.enabled is true before change the deployment strategy?

Signed-off-by: Rachit Chaudhary - r0c0axe <Rachit.Chaudhary@walmart.com>
@sre-ci-robot sre-ci-robot removed the lgtm label Feb 28, 2024
@mergify mergify bot added ci-passed and removed ci-passed labels Feb 28, 2024
@Rachit-Chaudhary11
Copy link
Contributor Author

Rachit-Chaudhary11 commented Feb 29, 2024

@LoveEachDay Thank you for the suggestion. I have made the necessary changes, kindly validate.
cc: @haorenfsa

Copy link
Contributor Author

@Rachit-Chaudhary11 Rachit-Chaudhary11 left a comment

Choose a reason for hiding this comment

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

This is a needed check to avoid any wrong replicas value to be inserted.

@haorenfsa
Copy link
Collaborator

/lgtm

@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: Rachit-Chaudhary11

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@haorenfsa
Copy link
Collaborator

@Rachit-Chaudhary11 Looks good. Thank you for the contribution!

@sre-ci-robot sre-ci-robot merged commit 79653e2 into zilliztech:master Mar 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants