infosync: refactor server config into dynamic and static sections #58473

you06 · 2024-12-23T08:33:21Z

What problem does this PR solve?

Issue Number: close #58405

Problem Summary:

The labels of TiDB are stored within a static structure, meanwhile TiDB have update-label API. Before this PR, the way TiDB handles the mutations of labels have data-race risk.

What changed and how does it work?

Refactor the ServerInfo into static and dynamic sections and fix the data race by atomic operations.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
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-12-23T08:33:37Z

Hi @you06. 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.

codecov · 2024-12-23T09:03:56Z

Codecov Report

Attention: Patch coverage is 96.20253% with 3 lines in your changes missing coverage. Please review.

Project coverage is 75.0679%. Comparing base (918fcdb) to head (dd0f4e1).
Report is 155 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #58473        +/-   ##
================================================
+ Coverage   73.0774%   75.0679%   +1.9904%     
================================================
  Files          1701       1753        +52     
  Lines        469836     489261     +19425     
================================================
+ Hits         343344     367278     +23934     
+ Misses       105319      99432      -5887     
- Partials      21173      22551      +1378

Flag	Coverage Δ
integration	`48.8982% <20.2531%> (?)`
unit	`72.8059% <96.2025%> (+0.2159%)`	⬆️

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

Components	Coverage Δ
dumpling	`∅ <ø> (∅)`
parser	`∅ <ø> (∅)`
br	`60.8374% <ø> (+12.6785%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

you06 · 2024-12-25T01:49:52Z

/retest

tiprow · 2024-12-25T01:50:14Z

@you06: 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.

lcwangchao · 2024-12-27T02:58:50Z

pkg/domain/infosync/info.go

-	// which will be changed on occasions such as connection to PD is restored after broken.
-	ServerIDGetter func() uint64 `json:"-"`
+	StaticServerInfo
+	DynamicServerInfo


Seems DynamicServerInfo is also static because its value never changes for ServerInfo?

It's updated in setDynamicServerInfo, which replaces InfoSyncer.info with a new ServerInfo.

Do you think it's better to move StaticServerInfo out of atomic.Pointer[ServerInfo]? We can let info become a struct { staticServerInfo, atomic.Pointer[dynamicServerInfo] }

The current implementation provides a simpler interface for user, just call getLocalServerInfo to get the entire ServerInfo object.

lcwangchao · 2024-12-27T02:59:42Z

pkg/domain/infosync/info.go

+			ServerIDGetter: serverIDGetter,
+		},
+		DynamicServerInfo: DynamicServerInfo{
+			Labels: labels,


seems to use maps.Clone is more elegant here?

Signed-off-by: you06 <[email protected]> fix lint & update mem after etcd Signed-off-by: you06 <[email protected]> wait goroutine quit Signed-off-by: you06 <[email protected]> fix update label Signed-off-by: you06 <[email protected]> update func name Signed-off-by: you06 <[email protected]> remove import Signed-off-by: you06 <[email protected]> address comment Signed-off-by: you06 <[email protected]> add comments Signed-off-by: you06 <[email protected]> bazel Signed-off-by: you06 <[email protected]>

Signed-off-by: you06 <[email protected]>

you06 · 2025-03-10T06:44:45Z

/retest

tiprow · 2025-03-10T06:45:07Z

@you06: 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.

you06 · 2025-03-12T05:29:52Z

@lance6716, @time-and-fate
This PR has a refactor of server info, and change the tests in some modules, need your approve so, PTAL

YangKeao

LGTM

ti-chi-bot · 2025-03-12T05:51:21Z

[LGTM Timeline notifier]

Timeline:

2025-03-12 04:57:25.813979157 +0000 UTC m=+331801.419508083: ☑️ agreed by lcwangchao.
2025-03-12 05:51:21.044820058 +0000 UTC m=+335036.650348985: ☑️ agreed by YangKeao.

ti-chi-bot · 2025-03-12T06:54:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hawkingrei, lance6716, lcwangchao, YangKeao

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~~ [YangKeao,hawkingrei,lance6716,lcwangchao]
~~pkg/ddl/OWNERS~~ [lance6716]
~~pkg/disttask/OWNERS~~ [lance6716]
~~pkg/domain/OWNERS~~ [lance6716]
~~pkg/statistics/OWNERS~~ [hawkingrei]

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

you06 · 2025-03-12T07:39:15Z

/retest

tiprow · 2025-03-12T07:39:37Z

@you06: 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.

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2025-03-27T06:36:43Z

In response to a cherrypick label: new pull request created to branch release-8.5: #60292.
But this PR has conflicts, please resolve them!

ti-chi-bot · 2025-03-27T06:36:43Z

In response to a cherrypick label: new pull request created to branch release-7.5: #60291.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2025-03-27T06:36:51Z

In response to a cherrypick label: new pull request created to branch release-8.1: #60293.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]> resolve conflict Signed-off-by: you06 <[email protected]>

Signed-off-by: ti-chi-bot <[email protected]> Revert "This is an automated cherry-pick of pingcap#58473" This reverts commit bf5fda5. resolve conflict Signed-off-by: you06 <[email protected]> fix build Signed-off-by: you06 <[email protected]>

Signed-off-by: ti-chi-bot <[email protected]> Revert "This is an automated cherry-pick of pingcap#58473" This reverts commit bf5fda5. resolve conflict Signed-off-by: you06 <[email protected]> fix build Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]>

Signed-off-by: you06 <[email protected]>

Signed-off-by: you06 <[email protected]> enabel race Signed-off-by: you06 <[email protected]> fix build Signed-off-by: you06 <[email protected]> fix build Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]>

Signed-off-by: you06 <[email protected]> enabel race Signed-off-by: you06 <[email protected]> fix build Signed-off-by: you06 <[email protected]> fix build Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]> fix test Signed-off-by: you06 <[email protected]> debug Signed-off-by: you06 <[email protected]> ensure the test chan is init Signed-off-by: you06 <[email protected]> do not test with race mode Signed-off-by: you06 <[email protected]>

ti-chi-bot · 2025-04-09T07:12:44Z

In response to a cherrypick label: new pull request created to branch release-6.1: #60451.
But this PR has conflicts, please resolve them!

Signed-off-by: ti-chi-bot <[email protected]>

…8473) (#60292) close #58405

…8473) (#60293) close #58405

…ngcap#58473) close pingcap#58405

…8473) (#60291) close #58405

ti-chi-bot bot added do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 23, 2024

you06 requested a review from lcwangchao December 24, 2024 09:11

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label Dec 24, 2024

lcwangchao reviewed Dec 27, 2024

View reviewed changes

you06 force-pushed the fix/update-label-race branch from 47bb12d to c50fa72 Compare March 7, 2025 07:36

fix conflict

75430fb

Signed-off-by: you06 <[email protected]>

lcwangchao approved these changes Mar 12, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 12, 2025

YangKeao approved these changes Mar 12, 2025

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 12, 2025

hawkingrei approved these changes Mar 12, 2025

View reviewed changes

lance6716 approved these changes Mar 12, 2025

View reviewed changes

ti-chi-bot bot added the approved label Mar 12, 2025

ti-chi-bot mentioned this pull request Mar 27, 2025

infosync: refactor server config into dynamic and static sections (#58473) #60290

Closed

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Mar 27, 2025

This is an automated cherry-pick of pingcap#58473

bf5fda5

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Mar 27, 2025

This is an automated cherry-pick of pingcap#58473

98e816b

Signed-off-by: ti-chi-bot <[email protected]>

This was referenced Mar 27, 2025

infosync: refactor server config into dynamic and static sections (#58473) #60291

Merged

infosync: refactor server config into dynamic and static sections (#58473) #60292

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Mar 27, 2025

This is an automated cherry-pick of pingcap#58473

cb2a705

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Mar 27, 2025

infosync: refactor server config into dynamic and static sections (#58473) #60293

Merged

13 tasks

you06 added a commit to ti-chi-bot/tidb that referenced this pull request Mar 27, 2025

This is an automated cherry-pick of pingcap#58473

5795ad2

Signed-off-by: ti-chi-bot <[email protected]> resolve conflict Signed-off-by: you06 <[email protected]>

you06 added a commit to ti-chi-bot/tidb that referenced this pull request Mar 27, 2025

cherry-pick pingcap#58473 into release-7.1

a5d7037

Signed-off-by: you06 <[email protected]>

ti-chi-bot bot added needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. and removed needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. labels Apr 9, 2025

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Apr 9, 2025

This is an automated cherry-pick of pingcap#58473

9181d5e

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Apr 9, 2025

infosync: refactor server config into dynamic and static sections (#58473) #60451

Closed

13 tasks

ti-chi-bot bot removed the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Apr 9, 2025

ti-chi-bot bot pushed a commit that referenced this pull request Apr 16, 2025

infosync: refactor server config into dynamic and static sections (#5…

ca939b3

…8473) (#60292) close #58405

ti-chi-bot bot pushed a commit that referenced this pull request Apr 23, 2025

infosync: refactor server config into dynamic and static sections (#5…

2c55e79

…8473) (#60293) close #58405

zeminzhou pushed a commit to zeminzhou/tidb that referenced this pull request May 6, 2025

infosync: refactor server config into dynamic and static sections (pi…

4cec832

…ngcap#58473) close pingcap#58405

ti-chi-bot bot pushed a commit that referenced this pull request May 28, 2025

infosync: refactor server config into dynamic and static sections (#5…

2424a23

…8473) (#60291) close #58405

infosync: refactor server config into dynamic and static sections #58473

infosync: refactor server config into dynamic and static sections #58473

Uh oh!

Conversation

you06 commented Dec 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Uh oh!

tiprow bot commented Dec 23, 2024

Uh oh!

codecov bot commented Dec 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

you06 commented Dec 25, 2024

Uh oh!

tiprow bot commented Dec 25, 2024

Uh oh!

lcwangchao Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

you06 Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

lance6716 Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

you06 Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

lcwangchao Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

you06 commented Mar 10, 2025

Uh oh!

tiprow bot commented Mar 10, 2025

Uh oh!

you06 commented Mar 12, 2025

Uh oh!

YangKeao left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Mar 12, 2025

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Mar 12, 2025

Uh oh!

you06 commented Mar 12, 2025

Uh oh!

tiprow bot commented Mar 12, 2025

Uh oh!

ti-chi-bot commented Mar 27, 2025

Uh oh!

ti-chi-bot commented Mar 27, 2025

Uh oh!

ti-chi-bot commented Mar 27, 2025

Uh oh!

ti-chi-bot commented Apr 9, 2025

Uh oh!

Uh oh!

you06 commented Dec 23, 2024 •

edited

Loading

codecov bot commented Dec 23, 2024 •

edited

Loading