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

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

Merged
merged 15 commits into from
Sep 21, 2020

Conversation

ti-srebot
Copy link
Contributor

@ti-srebot ti-srebot commented Sep 16, 2020

cherry-pick #19355 to release-4.0


What problem does this PR solve?

Issue Number: close #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:
Changed the EtcdAddrs() to get correct pd members info
Changed the GetPDServerInfo function, add rpc GetMember() in pd and TiDB use grpc to get the pd members info.
How it Works:
Use grpc to get the pd members url, then use pd-api /pd/api/v1/config/cluster-version and /pd/api/v1/config/status to get member version githash info

Related changes

  • PR to update pingcap/tidb/infoschema/tables.go and pingcap/tidb/util/pdapi/const.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 2 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

  • Bug fix:CLUSTER_INFO system table may not work after PD is scaled-in or out

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

jyz0309 and others added 5 commits September 16, 2020 15:20
Signed-off-by: jyz0309 <45495947@qq.com>
Signed-off-by: jyz0309 <45495947@qq.com>
Signed-off-by: jyz0309 <45495947@qq.com>
@jyz0309
Copy link
Contributor

jyz0309 commented Sep 16, 2020

/run-all-tests

1 similar comment
@jyz0309
Copy link
Contributor

jyz0309 commented Sep 16, 2020

/run-all-tests

@Yisaer
Copy link
Contributor

Yisaer commented Sep 17, 2020

/label DNM

@ti-srebot
Copy link
Contributor Author

These labels are not found DNM.

@Yisaer
Copy link
Contributor

Yisaer commented Sep 17, 2020

/label status/DNM

@Yisaer
Copy link
Contributor

Yisaer commented Sep 17, 2020

plz DNM this request as we found that the newly added pdclient interface GetMemberInfo's name needs to be revised. We will fix it before v4.0.7 released.

@HunDunDM
Copy link
Contributor

We also need to change the hash of the PD.

@breezewish
Copy link
Member

/run-all-tests

Signed-off-by: jyz0309 <45495947@qq.com>
Signed-off-by: jyz0309 <45495947@qq.com>
Signed-off-by: jyz0309 <45495947@qq.com>
Signed-off-by: jyz0309 <45495947@qq.com>
Signed-off-by: jyz0309 <45495947@qq.com>
@breezewish
Copy link
Member

/run-all-tests

@jyz0309
Copy link
Contributor

jyz0309 commented Sep 21, 2020

@breeswish @crazycs520 PTAL

@jyz0309
Copy link
Contributor

jyz0309 commented Sep 21, 2020

@wjhuang2016 PTAL

@ti-srebot
Copy link
Contributor Author

@breeswish,Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments.See the corresponding SIG page for more information. Related SIGs: ddl(slack),execution(slack).

@breezewish breezewish modified the milestones: v4.0.8, v4.0.7 Sep 21, 2020
@breezewish breezewish added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Sep 21, 2020
@breezewish
Copy link
Member

/run-all-tests

Copy link
Contributor

@crazycs520 crazycs520 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 the status/LGT1 Indicates that a PR has LGTM 1. label Sep 21, 2020
@crazycs520
Copy link
Contributor

/run-all-tests

@jyz0309
Copy link
Contributor

jyz0309 commented Sep 21, 2020

/run-unit-test

@breezewish
Copy link
Member

/run-sqllogic-test-2

@jyz0309
Copy link
Contributor

jyz0309 commented Sep 21, 2020

@breeswish PTAL, maybe can merge?

@breezewish
Copy link
Member

@lzmhhh123 Please help merge this PR, thanks!

@bb7133
Copy link
Member

bb7133 commented Sep 21, 2020

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 21, 2020
@bb7133
Copy link
Member

bb7133 commented Sep 21, 2020

Thanks for your contribution! @jyz0309

@bb7133 bb7133 merged commit 77fc45e into pingcap:release-4.0 Sep 21, 2020
@breezewish breezewish deleted the release-4.0-c679f847fc5a branch September 21, 2020 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenge-program component/infoschema contribution This PR is from a community contributor. priority/release-blocker This issue blocks a release. Please solve it ASAP. sig/execution SIG execution sig/sql-infra SIG: SQL Infra status/LGT2 Indicates that a PR has LGTM 2. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants