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.

Sign up for GitHub

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

Jump to bottom

Register before start to prevent two same coordinators coexist #21641

Merged
merged 1 commit into from
Jan 12, 2023
Merged

Register before start to prevent two same coordinators coexist #21641

merged 1 commit into from
Jan 12, 2023

Conversation

xiaocai2333
Copy link
Contributor

Signed-off-by: cai.zhang cai.zhang@zilliz.com
issue: #21269
/kind improvement

@sre-ci-robot sre-ci-robot added the kind/improvement Changes related to something improve, likes ut and code refactor label Jan 11, 2023
@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Jan 11, 2023
@mergify mergify bot added the dco-passed DCO check passed. label Jan 11, 2023
@xiaocai2333 xiaocai2333 requested review from wayblink, xiaofan-luan, czs007 and jaime0815 and removed request for cydrain and godchen0212 January 11, 2023 06:49
@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@codecov
Copy link

codecov bot commented Jan 11, 2023 •
edited
Loading

Codecov Report

Merging #21641 (04e595d) into 2.2.0 (04e595d) will not change coverage.
The diff coverage is n/a.

❗ Current head 04e595d differs from pull request most recent head 2489346. Consider uploading reports for the commit 2489346 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##            2.2.0   #21641   +/-   ##
=======================================
  Coverage   81.96%   81.96%           
=======================================
  Files         697      697           
  Lines       95992    95992           
=======================================
  Hits        78682    78682           
  Misses      14303    14303           
  Partials     3007     3007

@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 ut workflow job failed, comment rerun ut can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@xiaocai2333
Copy link
Contributor Author

/run-cpu-e2e

@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 ut workflow job failed, comment rerun ut can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

wayblink
wayblink approved these changes Jan 11, 2023
View reviewed changes
Copy link
Collaborator

@wayblink wayblink left a comment

Choose a reason for hiding this comment

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

Generally LGTM

internal/datacoord/server.go
log.Info("DataCoord switch from standby to active, activating")
if err := s.initDataCoord(); err != nil {
log.Warn("DataCoord init failed", zap.Error(err))
// TODO: panic if error occurred?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have another pr about active-standy. activeFunc is updated to func() error then we can throw this error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good job, LGTM

@sre-ci-robot sre-ci-robot added area/compilation size/XL Denotes a PR that changes 500-999 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Jan 11, 2023
@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 ut workflow job failed, comment rerun ut can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2023

@xiaocai2333 E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@xiaocai2333
Register after start to prevent there are tow coordinator at the same…
2489346 
… time

Signed-off-by: cai.zhang <cai.zhang@zilliz.com>
@czs007
Copy link
Contributor

czs007 commented Jan 12, 2023

/approve
/lgtm

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czs007, wayblink, xiaocai2333

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

@sre-ci-robot sre-ci-robot merged commit 03ce5c2 into milvus-io:2.2.0 Jan 12, 2023
@zhuwenxing zhuwenxing mentioned this pull request Jan 12, 2023
Closed
1 task
@xiaocai2333 xiaocai2333 changed the title Register after start to prevent there are tow coordinator at the same time Register before start to prevent two coordinators coexist Jan 13, 2023
@xiaocai2333 xiaocai2333 changed the title Register before start to prevent two coordinators coexist Register before start to prevent two same coordinators coexist Jan 13, 2023
xiaocai2333 added a commit to xiaocai2333/milvus that referenced this pull request Jan 28, 2023
@xiaocai2333
Register after start to prevent there are tow coordinator at the same…
d18fce7 
… time (milvus-io#21641)

Signed-off-by: cai.zhang <cai.zhang@zilliz.com>
sre-ci-robot pushed a commit that referenced this pull request Jan 30, 2023
@xiaocai2333
Register after start to prevent there are tow coordinator at the same…
bcae97b 
… time (#21641) (#21707)

Signed-off-by: cai.zhang <cai.zhang@zilliz.com>
@xiaocai2333 xiaocai2333 deleted the register_before_start branch March 14, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaofan-luan xiaofan-luan xiaofan-luan left review comments

@wayblink wayblink wayblink approved these changes

@czs007 czs007 Awaiting requested review from czs007

@jaime0815 jaime0815 Awaiting requested review from jaime0815

Assignees

@czs007 czs007

Labels
approved area/compilation ci-passed dco-passed DCO check passed. kind/improvement Changes related to something improve, likes ut and code refactor lgtm size/XL Denotes a PR that changes 500-999 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@xiaocai2333 @czs007 @sre-ci-robot @wayblink @xiaofan-luan