session: add no register API for using TiDB as a library#17513
session: add no register API for using TiDB as a library#17513ti-srebot merged 20 commits intopingcap:masterfrom
Conversation
|
|
|
No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md |
Codecov Report
@@ Coverage Diff @@
## master #17513 +/- ##
===========================================
Coverage 79.3272% 79.3272%
===========================================
Files 541 541
Lines 145636 145636
===========================================
Hits 115529 115529
Misses 20812 20812
Partials 9295 9295 |
|
@breeswish PTAL, or... Please add some labels... |
|
When BR is integrated into TiDB, we don't need this PR, right? I think the integration is very close. |
domain/domain.go
Outdated
|
|
||
| // Init initializes a domain. | ||
| func (do *Domain) Init(ddlLease time.Duration, sysFactory func(*Domain) (pools.Resource, error)) error { | ||
| if err := do.InitWithNoRegister(ddlLease, sysFactory); err != nil { |
There was a problem hiding this comment.
This also disables the DDL syncer, will it cause problems?
There was a problem hiding this comment.
Seems InfoSyncer won't do things about DDL... Is there something I ignored...?
There was a problem hiding this comment.
I don't know how BR uses the DDL structure. If BR uses InitWithNoRegister to new a DDL, then the DDL also will campaign DDL owner, but it will make load schema slower then Init with InfoSyncer. Is it OK?
domain/infosync/info.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| if config.GetGlobalConfig().NoRegister { |
There was a problem hiding this comment.
I think we'd better to use config.GetGlobalConfig().NoRegister as a argument to GlobalInfoSyncerInit. It is best to reduce the strong correlation between internal objects and config.GetGlobalConfig().
|
/run-all-tests |
|
That is strange... It fails but I cannot find any useful information about why it fails... Let me retry it. /run-all-tests |
|
@zimulala,Thanks for your review. |
|
@breeswish PTAL~ |
|
/run-all-tests ...Why? 🤔 |
|
/run-common-test
.... |
|
/run-common-test
We died at different place each time 🙃... |
|
/run-unit-test |
|
/merge |
|
/merge |
|
/run-all-tests |
* 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>
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.Getwill 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-registerto TiDB.Then, when this is enabled,
topologySyncerwon't be started, hence this TiDB won't register itself to dashboard.How it Works:
We add a if-guard to where initializing
topologySynceratinfoSyncer.InitandSession.Init.Check List
Tests
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
WIPtag for this:Add test cases.(IMO, this is too hard to be tested automatically.)Don't make(We can setNoRegistered TiDB become DDL owner.run-DDL = false)Release note