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

*: Bug Fix/CLUSTER_INFO system table may not work after PD is scaled-in (#2830) #2980

Merged
merged 4 commits into from
Sep 21, 2020
Merged

*: Bug Fix/CLUSTER_INFO system table may not work after PD is scaled-in (#2830) #2980

merged 4 commits into from
Sep 21, 2020

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Sep 17, 2020 •
edited by HunDunDM
Loading

cherry-pick #2830 to release-4.0

What problem does this PR solve?

Issue Number: close pingcap/tidb#18990

Problem Summary:

CLUSTER_INFO system table may not work after PD is scaled-in

What is changed and how it works?

What's Changed:

Add the GetMemberInfo function to client interface, use grpc GetMembers() to get the pd members info.

How it Works:

use grpc GetMembers() to get the pd members info.

Related changes

  • PR to update pingcap/pd/client/client.go;
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

steps:

  • use tiup playground scale-out --pd 1 to scale out pd
  • use mysql to connect TiDB
  • select * from INFORMATION_SCHEMA.CLUSTER_INFO; to test scale out and scale in

Side effects

  • Performance regression
    • Consumes more CPU

Release note

  • add GetAllMembers function to get pd member info

@jyz0309 @ti-srebot
cherry pick tikv#2830 to release-4.0
c324041 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Sep 17, 2020
Merged
@ti-srebot ti-srebot added component/client Client logic. contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/cherry-pick labels Sep 17, 2020
@ti-srebot ti-srebot added this to the v4.0.7 milestone Sep 17, 2020
@ti-srebot ti-srebot self-assigned this Sep 17, 2020
@codecov
Copy link

codecov bot commented Sep 17, 2020 •
edited
Loading

Codecov Report

Merging #2980 into release-4.0 will increase coverage by 0.07%.
The diff coverage is 66.01%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-4.0    #2980      +/-   ##
===============================================
+ Coverage        77.16%   77.24%   +0.07%     
===============================================
  Files              204      209       +5     
  Lines            21940    22880     +940     
===============================================
+ Hits             16931    17674     +743     
- Misses            3724     3862     +138     
- Partials          1285     1344      +59
Impacted Files Coverage Δ
client/metrics.go 100.00% <ø> (ø)
pkg/apiutil/apiutil.go 67.34% <ø> (ø)
pkg/cache/cache.go 0.00% <ø> (ø)
pkg/cache/fifo.go 100.00% <ø> (ø)
pkg/cache/lru.go 80.35% <ø> (ø)
pkg/cache/two_queue.go 66.66% <ø> (ø)
pkg/codec/codec.go 78.57% <ø> (ø)
pkg/component/manager.go 81.01% <ø> (ø)
pkg/dashboard/adapter/redirector.go 94.23% <ø> (ø)
pkg/dashboard/keyvisual/input/core.go 33.33% <ø> (ø)
... and 211 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88b0561...eb8618f. Read the comment docs.

@crazycs520
Copy link
Contributor

@Yisaer @nolouch PTAL

@jyz0309
update the func name
6989da7 
Signed-off-by: jyz0309 <45495947@qq.com>
nolouch
nolouch approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Sep 17, 2020
Yisaer
Yisaer approved these changes Sep 18, 2020
View reviewed changes
Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 18, 2020
@Yisaer
Copy link
Contributor

Yisaer commented Sep 18, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 18, 2020
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@jyz0309
Copy link
Contributor

jyz0309 commented Sep 20, 2020

/run-all-tests

2 similar comments
@Yisaer
Copy link
Contributor

Yisaer commented Sep 21, 2020

/run-all-tests

@breezewish
Copy link
Member

/run-all-tests

@disksing
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 2967

@Yisaer
Copy link
Contributor

Yisaer commented Sep 21, 2020

/merge

@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 2967
  • 2985
  • 2991
  • 2994

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot merged commit 9ee41c4 into tikv:release-4.0 Sep 21, 2020
This was referenced Sep 25, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nolouch nolouch nolouch approved these changes

@Yisaer Yisaer Yisaer approved these changes

@JmPotato JmPotato Awaiting requested review from JmPotato

@rleungx rleungx Awaiting requested review from rleungx

Assignees

@ti-srebot ti-srebot

Labels
component/client Client logic. contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
v4.0.7
Development

Successfully merging this pull request may close these issues.
7 participants
@ti-srebot @crazycs520 @Yisaer @jyz0309 @breezewish @disksing @nolouch