ddl: dynamically adjusting the max write speed of reorganization job #57611

fzzf678 · 2024-11-21T16:12:20Z

What problem does this PR solve?

Issue Number: ref #57526 #57229

Problem Summary: See #57229

What changed and how does it work?

Supply later

Check List

Tests

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

Use sysbench prepare table with 50000000 rows, create index and tune the max_write_speed and check the TiKV IO panel import. To see the result clearly, set the initial @@global.tidb_ddl_reorg_max_write_speed = '5MiB'

sysbench --db-driver=mysql --mysql-db=test --report-interval=10 --mysql-user=root  --mysql-password=''  --time=0 --mysql-port=4000 --mysql-host=127.0.0.1 --tables=1 --table-size=50000000 --threads=1 oltp_read_write prepare

Add index in session 1.

alter table sbtest1 drop index(k);

Alter the max_write_speed in session 2.

admin alter ddl jobs 115 max_write_speed = '20MiB';  -- 5MiB, 20MiB, 10MiB, 0

And check the grafana panel, the config can works

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-21T16:12:45Z

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

fzzf678 · 2024-11-21T16:19:03Z

/ok-to-test

fzzf678 · 2024-11-21T16:23:14Z

/test all

codecov · 2024-11-21T16:44:43Z

Codecov Report

Attention: Patch coverage is 78.00000% with 33 lines in your changes missing coverage. Please review.

Project coverage is 74.7128%. Comparing base (06c6e40) to head (48f1831).
Report is 1 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #57611        +/-   ##
================================================
+ Coverage   72.8668%   74.7128%   +1.8459%     
================================================
  Files          1677       1722        +45     
  Lines        463842     472018      +8176     
================================================
+ Hits         337987     352658     +14671     
+ Misses       104975      97183      -7792     
- Partials      20880      22177      +1297

Flag	Coverage Δ
integration	`49.2567% <33.3333%> (?)`
unit	`72.2088% <77.3333%> (-0.0642%)`	⬇️

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

Components	Coverage Δ
dumpling	`52.7673% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`61.0633% <ø> (+15.5490%)`	⬆️

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

CbcWestwolf · 2024-11-22T06:29:32Z

pkg/meta/model/reorg.go

@@ -75,8 +75,9 @@ type DDLReorgMeta struct {
 	// These two variables are used to control the concurrency and batch size of the reorganization process.
 	// They can be adjusted dynamically through `admin alter ddl jobs` command.
 	// Note: Don't get or set these two variables directly, use the functions instead.


Please make these 3 var private

OK, I will try make these vars private in next PR

CbcWestwolf

Rest LGTM

pkg/meta/model/reorg.go

ti-chi-bot · 2024-11-22T06:33:41Z

[LGTM Timeline notifier]

Timeline:

2024-11-22 06:03:08.187779729 +0000 UTC m=+184375.807434246: ☑️ agreed by winoros.
2024-11-22 06:33:40.782162927 +0000 UTC m=+186208.401817441: ☑️ agreed by CbcWestwolf.

Co-authored-by: CbcWestwolf <1004626265@qq.com>

fzzf678 · 2024-11-22T07:37:33Z

/cc @lance6716, please help take a look

…g_bwlimit

pkg/ddl/db_test.go

lance6716

rest lgtm

/hold

free to unhold

pkg/planner/core/planbuilder.go

Co-authored-by: lance6716 <lance6716@gmail.com>

…g_bwlimit

ti-chi-bot · 2024-11-25T10:07:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CbcWestwolf, D3Hunter, lance6716, winoros

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~~ [CbcWestwolf,D3Hunter,lance6716,winoros]
~~pkg/ddl/OWNERS~~ [D3Hunter,lance6716]
~~pkg/lightning/OWNERS~~ [D3Hunter,lance6716]
~~pkg/meta/OWNERS~~ [D3Hunter,lance6716]
~~pkg/planner/OWNERS~~ [winoros]

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-25T10:08:35Z

/unhold

fzzf678 · 2024-11-25T10:46:30Z

/retest

fzzf678 · 2024-11-25T11:56:34Z

/retest

fzzf678 · 2024-11-25T12:49:54Z

/retest

fzzf678 · 2024-11-25T13:41:56Z

/retest

ti-chi-bot · 2024-11-25T14:14:19Z

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

…ingcap#57611) ref pingcap#57526

…57611) (#57690) ref #57526

fzzf678 added 5 commits November 19, 2024 19:47

build execute max_write_speed

745c0bb

Merge remote-tracking branch 'upstream/master' into polling_bwlimit

2bdcee7

use get and set interface handle the max write speed in reorg meta

f286233

make the limiter can be adjusted

cbbd6ab

adjust max write speed

829c67f

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 21, 2024

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Nov 21, 2024

Update BUILD.bazel

07869e3

fzzf678 added 3 commits November 22, 2024 09:29

fix nogo

b7eed57

fix absurd

3e3955f

show ddl jobs print max write speed

fb8422b

winoros approved these changes Nov 22, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Nov 22, 2024

rename

cc98597

CbcWestwolf reviewed Nov 22, 2024

View reviewed changes

CbcWestwolf approved these changes Nov 22, 2024

View reviewed changes

pkg/meta/model/reorg.go Outdated Show resolved Hide resolved

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

Update pkg/meta/model/reorg.go

bb381f9

Co-authored-by: CbcWestwolf <1004626265@qq.com>

fzzf678 requested a review from lance6716 November 22, 2024 07:40

Merge branch 'polling_bwlimit' of github.com:fzzf678/tidb into pollin…

eb01652

…g_bwlimit

use named return value

f7cce41

fzzf678 commented Nov 25, 2024

View reviewed changes

pkg/ddl/db_test.go Outdated Show resolved Hide resolved

fzzf678 added 5 commits November 25, 2024 16:23

add test and fix comment

d71a826

Update planbuilder_test.go

5ba4278

check thread and batch_size eval type

a9cda92

Update BUILD.bazel

00d0027

correct maxTokens formula

a1cc392

pingcap deleted a comment from ti-chi-bot bot Nov 25, 2024

pingcap deleted a comment from tiprow bot Nov 25, 2024

lance6716 approved these changes Nov 25, 2024

View reviewed changes

pkg/planner/core/planbuilder.go Outdated Show resolved Hide resolved

ti-chi-bot bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved labels Nov 25, 2024

lance6716 requested a review from D3Hunter November 25, 2024 09:46

fzzf678 and others added 3 commits November 25, 2024 17:50

remove handle reorg nil case when decode

becf32e

Update pkg/planner/core/planbuilder.go

edcd1e9

Co-authored-by: lance6716 <lance6716@gmail.com>

Merge branch 'polling_bwlimit' of github.com:fzzf678/tidb into pollin…

417432c

…g_bwlimit

D3Hunter approved these changes Nov 25, 2024

View reviewed changes

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 25, 2024

Merge remote-tracking branch 'upstream/master' into polling_bwlimit

48f1831

ti-chi-bot bot merged commit ec8b81b into pingcap:master Nov 25, 2024
27 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 25, 2024

ddl: dynamically adjusting the max write speed of reorganization job (p…

1525d6f

…ingcap#57611) ref pingcap#57526

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

ddl: dynamically adjusting the max write speed of reorganization job (#57611) #57690

Merged

13 tasks

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

ddl: dynamically adjusting the max write speed of reorganization job (#…

aca349f

…57611) (#57690) ref #57526

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: dynamically adjusting the max write speed of reorganization job #57611

ddl: dynamically adjusting the max write speed of reorganization job #57611

ddl: dynamically adjusting the max write speed of reorganization job #57611

ddl: dynamically adjusting the max write speed of reorganization job #57611

Conversation

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[LGTM Timeline notifier]

Choose a reason for hiding this comment