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

HA for Milvus older versions #67

Closed

Conversation

Rachit-Chaudhary11
Copy link
Contributor

@Rachit-Chaudhary11 Rachit-Chaudhary11 commented Feb 27, 2024

What this PR does / why we need it:

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>
@Rachit-Chaudhary11
Copy link
Contributor Author

/assign @LoveEachDay
@haorenfsa kindly validate.

@haorenfsa
Copy link
Collaborator

Hi @Rachit-Chaudhary11, looks like the previous PR #56 cloud cover this.

@Rachit-Chaudhary11
Copy link
Contributor Author

Rachit-Chaudhary11 commented Feb 27, 2024

Hi @haorenfsa I see that the above PR is created for QueryNode, DataNode, IndexNode and Proxy. While this PR is for the different Milvus Coordinators.
Even after merging the above PR the coordinators will still not have the rolling update, which will be handled by this PR

@Archalbc
Copy link
Contributor

Hi @haorenfsa I see that the above PR is created for QueryNode, DataNode, IndexNode and Proxy. While this PR is for the different Milvus Coordinators. Even after merging the above PR the coordinators will still not have the rolling update, which will be handled by this PR

Hi !
Don't you think you should use the same syntax as PR #56 to add strategy using "with"/"toYaml" ? This will allow users to completely have the freedom to customize this part according to their need.

Another point, using .Values.DeploymentUpdate means you force users to use the same strategy for all coords. Since nobody provide great best practices for either coords or nodes, I think we should let users customize each coords at this moment (this is just my opinion ;) )

Rachit Chaudhary - r0c0axe added 2 commits February 27, 2024 19:56
Signed-off-by: Rachit Chaudhary - r0c0axe <Rachit.Chaudhary@walmart.com>
@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Rachit-Chaudhary11
To complete the pull request process, please ask for approval from loveeachday after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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

@mergify mergify bot added ci-passed and removed ci-passed labels Feb 27, 2024
@Rachit-Chaudhary11
Copy link
Contributor Author

Rachit-Chaudhary11 commented Feb 27, 2024

@haorenfsa I am closing this PR as I have created another PR - #68 68

@Rachit-Chaudhary11 Rachit-Chaudhary11 changed the title [milvus-4.1.18] Enabling High Availability for Milvus HA for Milvus older versions Mar 6, 2024
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.

None yet

5 participants