ddl: use a global owner manager instance for DDL, to avoid owner change #57179

D3Hunter · 2024-11-06T11:40:25Z

What problem does this PR solve?

Issue Number: ref #57185

Problem Summary:

What changed and how does it work?

use a global owner manager instance for DDL, to avoid owner change during bootstrap, we want it stable. or change back to older version node after upgrade, we want the owner stick on the newer version node.
fix br owner manager not closed
fix br/bindinfo owner manager report 'the client connection is closing' as the campaign loop exit after client close

Check List

Tests

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

bootstrap with 3 TiDB of this PR, success, and the owner on first Domain start is still the owner after second start.

upgrade from a 3 TiDB cluster of version v8.1.1, success, and after upgrade, the newer version node is still the owner

No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

tiprow · 2024-11-06T11:40:51Z

Hi @D3Hunter. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-06T11:43:20Z

pkg/domain/domain.go

@@ -1311,62 +1313,6 @@ func NewDomain(store kv.Storage, schemaLease time.Duration, statsLease time.Dura

 const serverIDForStandalone = 1 // serverID for standalone deployment.

-// NewEtcdCli creates a new clientv3.Client from store if the store support it.


moved to store pkg to avoid import cycle

codecov · 2024-11-06T11:54:51Z

Codecov Report

Attention: Patch coverage is 49.46809% with 95 lines in your changes missing coverage. Please review.

Project coverage is 74.8554%. Comparing base (120b36b) to head (02a2c4a).
Report is 13 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #57179        +/-   ##
================================================
+ Coverage   72.9583%   74.8554%   +1.8970%     
================================================
  Files          1659       1708        +49     
  Lines        458526     468463      +9937     
================================================
+ Hits         334533     350670     +16137     
+ Misses       103462      95927      -7535     
- Partials      20531      21866      +1335

Flag	Coverage Δ
integration	`49.2927% <18.6170%> (?)`
unit	`72.4360% <47.3404%> (+0.1420%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.7673% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`60.7558% <33.3333%> (+15.7139%)`	⬆️

D3Hunter · 2024-11-07T02:24:19Z

/retest

tiprow · 2024-11-07T02:24:42Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-07T02:47:20Z

/retest

tiprow · 2024-11-07T02:47:46Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-07T03:55:04Z

/retest

tiprow · 2024-11-07T03:55:32Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-07T04:27:17Z

/test pull-br-integration-test

tiprow · 2024-11-07T04:27:40Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-br-integration-test

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-07T11:55:26Z

/retest

tiprow · 2024-11-07T11:55:48Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

Benjamin2037

LGTM

ti-chi-bot · 2024-11-07T12:24:06Z

[LGTM Timeline notifier]

Timeline:

2024-11-07 04:46:17.196967138 +0000 UTC m=+1102690.036122684: ☑️ agreed by wjhuang2016.
2024-11-07 12:24:05.036006394 +0000 UTC m=+1130157.875161940: ☑️ agreed by Benjamin2037.

D3Hunter · 2024-11-07T13:12:23Z

/retest

tiprow · 2024-11-07T13:12:53Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-08T02:12:29Z

/retest

tiprow · 2024-11-08T02:12:55Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

D3Hunter · 2024-11-08T06:54:20Z

pkg/ddl/owner_mgr.go

+func (om *ownerManager) Start(ctx context.Context, store kv.Storage) error {
+	// BR might start domain multiple times, we need to avoid it. when BR have refactored
+	// this part, we can remove this.
+	if om.started {


cc @Leavrth , please help remove this part after BR refactor

ti-chi-bot · 2024-11-11T02:50:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, Leavrth, tiancaiamao, wjhuang2016, yudongusa

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Benjamin2037,Leavrth,tiancaiamao,wjhuang2016]
~~br/OWNERS~~ [Leavrth]
~~pkg/autoid_service/OWNERS~~ [tiancaiamao]
~~pkg/ddl/OWNERS~~ [Benjamin2037,wjhuang2016]
~~pkg/disttask/OWNERS~~ [Benjamin2037,wjhuang2016]
~~pkg/domain/OWNERS~~ [wjhuang2016]
~~pkg/owner/OWNERS~~ [Benjamin2037,wjhuang2016]
~~pkg/session/OWNERS~~ [yudongusa]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

D3Hunter · 2024-11-11T03:43:41Z

/retest

tiprow · 2024-11-11T03:44:03Z

@D3Hunter: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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 kubernetes-sigs/prow repository.

ti-chi-bot · 2024-11-11T05:13:35Z

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

…ge (#57179) (#57273) ref #57185

D3Hunter added 4 commits November 6, 2024 09:53

tmp

db9d126

change

0e79fd3

change

e3b0cd5

change

0715c44

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 6, 2024

D3Hunter changed the title ~~ddl: use a global owner manager instance for DDL, to avoid owner change~~ [WIP]ddl: use a global owner manager instance for DDL, to avoid owner change Nov 6, 2024

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2024

D3Hunter commented Nov 6, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Nov 6, 2024

D3Hunter added 5 commits November 6, 2024 22:00

change

b6256b5

change

d41bc3b

Merge remote-tracking branch 'upstream/master' into start-ddl-owner-once

1b0498f

change

65efbbb

change

cd2a9dd

D3Hunter mentioned this pull request Nov 7, 2024

routine leak in TestRejectUnsupportedTables #57191

Closed

D3Hunter changed the title ~~[WIP]ddl: use a global owner manager instance for DDL, to avoid owner change~~ ddl: use a global owner manager instance for DDL, to avoid owner change Nov 7, 2024

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2024

D3Hunter requested a review from lance6716 November 7, 2024 04:21

Benjamin2037 approved these changes Nov 7, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 7, 2024

Merge remote-tracking branch 'upstream/master' into start-ddl-owner-once

dea61c2

yudongusa approved these changes Nov 7, 2024

View reviewed changes

change

a815d88

D3Hunter added 4 commits November 8, 2024 12:08

br change

d3db505

change

dcd6b27

br change

597eae0

for br

02a2c4a

D3Hunter commented Nov 8, 2024

View reviewed changes

tiancaiamao approved these changes Nov 8, 2024

View reviewed changes

ti-chi-bot bot added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Nov 8, 2024

Leavrth approved these changes Nov 11, 2024

View reviewed changes

ti-chi-bot bot added the approved label Nov 11, 2024

ti-chi-bot bot merged commit 56e7093 into pingcap:master Nov 11, 2024
25 of 32 checks passed

ti-chi-bot mentioned this pull request Nov 11, 2024

ddl: use a global owner manager instance for DDL, to avoid owner change (#57179) #57273

Merged

13 tasks

D3Hunter deleted the start-ddl-owner-once branch November 11, 2024 05:23

ti-chi-bot bot pushed a commit that referenced this pull request Nov 11, 2024

ddl: use a global owner manager instance for DDL, to avoid owner chan…

ed9af44

…ge (#57179) (#57273) ref #57185

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: use a global owner manager instance for DDL, to avoid owner change #57179

ddl: use a global owner manager instance for DDL, to avoid owner change #57179

D3Hunter commented Nov 6, 2024 •

edited

Loading

tiprow bot commented Nov 6, 2024

D3Hunter Nov 6, 2024

codecov bot commented Nov 6, 2024 •

edited

Loading

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

Benjamin2037 left a comment

ti-chi-bot bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 8, 2024

tiprow bot commented Nov 8, 2024

D3Hunter Nov 8, 2024

ti-chi-bot bot commented Nov 11, 2024

D3Hunter commented Nov 11, 2024

tiprow bot commented Nov 11, 2024

ti-chi-bot commented Nov 11, 2024

		@@ -1311,62 +1313,6 @@ func NewDomain(store kv.Storage, schemaLease time.Duration, statsLease time.Dura

		const serverIDForStandalone = 1 // serverID for standalone deployment.

		// NewEtcdCli creates a new clientv3.Client from store if the store support it.

ddl: use a global owner manager instance for DDL, to avoid owner change #57179

ddl: use a global owner manager instance for DDL, to avoid owner change #57179

Conversation

D3Hunter commented Nov 6, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Nov 6, 2024

D3Hunter Nov 6, 2024

Choose a reason for hiding this comment

codecov bot commented Nov 6, 2024 • edited Loading

Codecov Report

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

Benjamin2037 left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Nov 7, 2024

[LGTM Timeline notifier]

D3Hunter commented Nov 7, 2024

tiprow bot commented Nov 7, 2024

D3Hunter commented Nov 8, 2024

tiprow bot commented Nov 8, 2024

D3Hunter Nov 8, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Nov 11, 2024

D3Hunter commented Nov 11, 2024

tiprow bot commented Nov 11, 2024

ti-chi-bot commented Nov 11, 2024

D3Hunter commented Nov 6, 2024 •

edited

Loading

codecov bot commented Nov 6, 2024 •

edited

Loading