server: fix the leader cannot election after pd leader lost while etcd leader intact by nolouch · Pull Request #6447 · tikv/pd

nolouch · 2023-05-11T18:24:07Z

What problem does this PR solve?

Issue Number: Close #6403

What is changed and how does it work?

server: fix the leader cannot election after pd leader lost while etcd leader intact

Check List

Tests

Unit test
Integration test

Release note

None.

ti-chi-bot · 2023-05-11T18:24:09Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JmPotato
rleungx

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

…d leader intact Signed-off-by: nolouch <nolouch@gmail.com>

lhy1024

LGTM, but ci failed

rleungx · 2023-05-12T03:24:33Z

server/server.go

+						zap.String("current-leader-member-id", types.ID(etcdLeader).String()),
+						zap.String("transferee-member-id", types.ID(s.member.ID()).String()),
+					)
+					s.member.MoveEtcdLeader(s.ctx, etcdLeader, s.member.ID())


Should we consider the leader's priority?

In most cases, there are no priority. if there exists priority, it also not affect the priority because the higher priority will do move the leader again. so, I think we can keep simple with this implementation.

JmPotato · 2023-05-12T03:26:35Z

tests/server/member/member_test.go

+	re.NoError(failpoint.Enable("github.com/tikv/pd/server/exitCampaignLeader", fmt.Sprintf("return(\"%d\")", memberID)))
+	re.NoError(failpoint.Enable("github.com/tikv/pd/server/timeoutWaitPDLeader", `return(true)`))
+	leader2 := waitLeaderChange(re, cluster, leader1)
+	t.Log("leader2:", leader2)


Suggested change

t.Log("leader2:", leader2)

JmPotato · 2023-05-12T03:27:11Z

tests/server/member/member_test.go

+	re.NoError(failpoint.Enable("github.com/tikv/pd/server/timeoutWaitPDLeader", `return(true)`))
+	leader2 := waitLeaderChange(re, cluster, leader1)
+	t.Log("leader2:", leader2)
+	re.NotEqual(leader1, leader2)


Do we need to check the duration that leader changing costs won't be too long?

without this fix, this test will be failed after timeout after 30s in waitLeaderChange.

Signed-off-by: nolouch <nolouch@gmail.com>

codecov · 2023-05-12T05:02:37Z

Codecov Report

❗ No coverage uploaded for pull request base (master@511115f). Click here to learn what that means.
Patch coverage: 88.88% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #6447   +/-   ##
=========================================
  Coverage          ?   74.85%           
=========================================
  Files             ?      410           
  Lines             ?    41718           
  Branches          ?        0           
=========================================
  Hits              ?    31227           
  Misses            ?     7782           
  Partials          ?     2709

Flag	Coverage Δ
unittests	`74.85% <88.88%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/tso/allocator_manager.go	`66.61% <ø> (ø)`
pkg/member/participant.go	`51.93% <42.85%> (ø)`
pkg/member/member.go	`70.04% <100.00%> (ø)`
server/server.go	`74.91% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

JmPotato · 2023-05-12T05:14:13Z

/merge

ti-chi-bot · 2023-05-12T05:14:14Z

@JmPotato: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Details

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-05-12T05:14:16Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 9eaa004

nolouch · 2023-05-12T08:00:30Z

/merge

ti-chi-bot · 2023-05-12T08:00:31Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Details

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-05-12T08:00:33Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 14e9f16

rleungx · 2023-05-12T08:37:48Z

server/server.go

 		}

 		leader, checkAgain := s.member.CheckLeader()
+		// add failpoint to test leader check go to stuck.


The file permission has been changed.

rleungx · 2023-05-12T08:38:01Z

/hold

Signed-off-by: nolouch <nolouch@gmail.com>

rleungx · 2023-05-12T08:48:16Z

/hold cancel

rleungx · 2023-05-12T08:48:23Z

/merge

ti-chi-bot · 2023-05-12T08:48:25Z

@rleungx: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Details

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-05-12T08:48:26Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 6d810b4

ti-chi-bot · 2023-05-12T10:32:33Z

@nolouch: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Details

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-05-12T10:47:00Z

In response to a cherrypick label: new pull request created to branch release-6.5: #6460.

close tikv#6403 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2023-05-12T10:47:38Z

In response to a cherrypick label: new pull request created to branch release-7.1: #6461.

…d leader intact (#6447) (#6461) close #6403, ref #6447 server: fix the leader cannot election after pd leader lost while etcd leader intact Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: nolouch <nolouch@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: nolouch <nolouch@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

…d leader intact (#6447) (#6460) close #6403, ref #6447 server: fix the leader cannot election after pd leader lost while etcd leader intact Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: nolouch <nolouch@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: nolouch <nolouch@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

ti-chi-bot bot added needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. release-note-none Denotes a PR that doesn't merit a release note. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. labels May 11, 2023

ti-chi-bot bot requested review from JmPotato and disksing May 11, 2023 18:24

nolouch requested review from lhy1024 and rleungx and removed request for JmPotato and disksing May 11, 2023 18:24

nolouch force-pushed the miss-leader branch from 10eb99e to 72fe845 Compare May 11, 2023 18:26

server: fix the leader cannot election after pd leader lost while etc…

0acf937

…d leader intact Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the miss-leader branch from 72fe845 to 0acf937 Compare May 12, 2023 01:29

lhy1024 reviewed May 12, 2023

View reviewed changes

rleungx reviewed May 12, 2023

View reviewed changes

JmPotato reviewed May 12, 2023

View reviewed changes

nolouch force-pushed the miss-leader branch from 9acc4db to 89c061d Compare May 12, 2023 04:48

address comments

9eaa004

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the miss-leader branch from 89c061d to 9eaa004 Compare May 12, 2023 04:50

rleungx approved these changes May 12, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 12, 2023

JmPotato approved these changes May 12, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 12, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 12, 2023

Merge remote-tracking branch 'nolouch/miss-leader' into miss-leader

14e9f16

ti-chi-bot bot removed the status/can-merge Indicates a PR has been approved by a committer. label May 12, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 12, 2023

rleungx reviewed May 12, 2023

View reviewed changes

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2023

fix

6d810b4

Signed-off-by: nolouch <nolouch@gmail.com>

ti-chi-bot bot removed the status/can-merge Indicates a PR has been approved by a committer. label May 12, 2023

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 12, 2023

ti-chi-bot bot added 2 commits May 12, 2023 08:48

Merge branch 'master' into miss-leader

7f9b6ba

Merge branch 'master' into miss-leader

4353bf7

ti-chi-bot bot merged commit 3e40564 into tikv:master May 12, 2023

ti-chi-bot mentioned this pull request May 12, 2023

server: fix the leader cannot election after pd leader lost while etcd leader intact (#6447) #6460

Merged

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 12, 2023

This is an automated cherry-pick of tikv#6447

0854d40

close tikv#6403 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 12, 2023

This is an automated cherry-pick of tikv#6447

3478768

close tikv#6403 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request May 12, 2023

server: fix the leader cannot election after pd leader lost while etcd leader intact (#6447) #6461

Merged

nolouch deleted the miss-leader branch August 11, 2023 02:39

Conversation

nolouch commented May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot bot commented May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lhy1024 left a comment

Choose a reason for hiding this comment

Uh oh!

rleungx May 12, 2023

Choose a reason for hiding this comment

Uh oh!

nolouch May 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JmPotato May 12, 2023

Choose a reason for hiding this comment

Uh oh!

JmPotato May 12, 2023

Choose a reason for hiding this comment

Uh oh!

nolouch May 12, 2023

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JmPotato commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

nolouch commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

rleungx May 12, 2023

Choose a reason for hiding this comment

Uh oh!

rleungx commented May 12, 2023

Uh oh!

rleungx commented May 12, 2023

Uh oh!

rleungx commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

ti-chi-bot bot commented May 12, 2023

Uh oh!

ti-chi-bot commented May 12, 2023

Uh oh!

ti-chi-bot commented May 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nolouch commented May 11, 2023 •

edited

Loading

ti-chi-bot bot commented May 11, 2023 •

edited

Loading

nolouch May 12, 2023 •

edited

Loading

codecov bot commented May 12, 2023 •

edited

Loading