Skip to content

session: add no register API for using TiDB as a library#18587

Merged
zimulala merged 3 commits intopingcap:release-4.0from
YuJuncen:17513-4.0
Jul 15, 2020
Merged

session: add no register API for using TiDB as a library#18587
zimulala merged 3 commits intopingcap:release-4.0from
YuJuncen:17513-4.0

Conversation

@YuJuncen
Copy link
Contributor

@YuJuncen YuJuncen commented Jul 15, 2020

cherry-pick #17513


What problem does this PR solve?

Issue Number: close br#299

Problem Summary:

BR uses TiDB as a library, but currently when we call session.GetDomain, domap.Get will automatically register itself to PD. Then a strange TiDB will present at dashboard.

see more at br#299.

What is changed and how it works?

What's Changed:

I added a config option no-register to TiDB.

Then, when this is enabled, topologySyncer won't be started, hence this TiDB won't register itself to dashboard.

How it Works:

We add a if-guard to where initializing topologySyncer at infoSyncer.Init and Session.Init.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • I replace this version of TiDB to BR, and set no-register = true, then BR won't register it self as a TiDB anymore.

More things

Maybe we have more things to do, for now please add WIP tag for this:

  • Add test cases. (IMO, this is too hard to be tested automatically.)
  • Don't make NoRegistered TiDB become DDL owner. (We can set run-DDL = false)

Release note

  • No release note

* domain,session: add NoRegister entry

* session: run fmt

* session: remove old NoRegister API

* config: add no register config

* config: add example to example config file.

* session: remove unused type

* *: rename NoRegister to SkipRegisterToDashboard

Co-authored-by: crazycs <crazycs520@gmail.com>
@sre-bot
Copy link
Contributor

sre-bot commented Jul 15, 2020

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 Jul 15, 2020
@crazycs520 crazycs520 added the sig/sql-infra SIG: SQL Infra label Jul 15, 2020
Copy link
Contributor

@zimulala zimulala 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 removed the status/LGT1 Indicates that a PR has LGTM 1. label Jul 15, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 15, 2020
@zimulala zimulala added the status/can-merge Indicates a PR has been approved by a committer. label Jul 15, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@YuJuncen merge failed.

@YuJuncen
Copy link
Contributor Author

/run-unit-test

@zimulala zimulala merged commit c05ceac into pingcap:release-4.0 Jul 15, 2020
@YuJuncen YuJuncen deleted the 17513-4.0 branch July 15, 2020 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/config sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. 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.

5 participants