ddl: limit the count of getting ddlhistory jobs by joccau · Pull Request #55590 · pingcap/tidb

joccau · 2024-08-22T07:59:51Z

What problem does this PR solve?

Issue Number: close #55711

Problem Summary:
If user want to get all of DDL-history jobs, the tidb-server has the risk of OOM because of large mount of DDL-history jobs.

What changed and how does it work?

Limit the count of jobs when getting the ddl history jobs.
The parameter 'limit' should be large than 1 and less than or equal to 1024.
If use default 'limit', user can get the 1024 ddl history jobs by default.

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.

Manual test steps

create 5k table in database with the 'sysbench' tool.
query the mount of ddl-history jobs, it contains 6052 ddl history jobs.

mysql> select count(*) from mysql.tidb_ddl_history;
+----------+
| count(*) |
+----------+
|     6052 |
+----------+
1 row in set (0.00 sec)

run below command

curl -X POST http://172.16.6.95:11080/ddl/history

and the it gets the 1024 ddl history jobs by default.

run below command, it gets the fault messages when limit < 0 or limit >= 1025

# curl -X POST http://172.16.6.95:11080/ddl/history -d "limit=0"
ddl history limit must be greater than 0 and less than or equal to 1024

# curl -X POST http://172.16.6.95:11080/ddl/history -d "limit=1025"
ddl history limit must be greater than 0 and less than or equal to 1024

curl -X POST http://172.16.6.95:11080/ddl/history -d "limit=1024" > output2.log

run below command, it gets 1024 ddl history jobs with the parameter 'limit'.

curl -X POST http://172.16.6.95:11080/ddl/history -d "limit=1024" > output2.log
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 5357k    0 5357k  100    10  22.0M     42 --:--:-- --:--:-- --:--:-- 22.0M

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.

Limit the count of jobs when getting the ddl history jobs.

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

codecov · 2024-08-22T08:33:48Z

Codecov Report

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

Project coverage is 75.5382%. Comparing base (290dc46) to head (c5e92ac).
Report is 155 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #55590        +/-   ##
================================================
+ Coverage   72.9295%   75.5382%   +2.6086%     
================================================
  Files          1581       1582         +1     
  Lines        442365     443435      +1070     
================================================
+ Hits         322615     334963     +12348     
+ Misses        99938      87869     -12069     
- Partials      19812      20603       +791

Flag	Coverage Δ
integration	`50.4807% <0.0000%> (?)`
unit	`71.9416% <85.7142%> (-0.0585%)`	⬇️

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

Components	Coverage Δ
dumpling	`52.9567% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`62.8444% <ø> (+17.3895%)`	⬆️

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

lance6716

and the it gets the 1024 ddl history jobs by default.

Maybe we should notify other team about this API change

pkg/ddl/ddl_history_test.go

joccau · 2024-08-26T08:59:01Z

/test fast_test_tiprow

ti-chi-bot · 2024-08-26T08:59:04Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test pull-br-integration-test
/test pull-integration-ddl-test
/test pull-lightning-integration-test
/test pull-mysql-client-test
/test unit-test

The following commands are available to trigger optional jobs:

/test canary-notify-when-compatibility-sections-changed
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-integration-nodejs-test
/test pull-sqllogic-test
/test pull-tiflash-test

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test
pingcap/tidb/pull_br_integration_test
pingcap/tidb/pull_integration_ddl_test
pingcap/tidb/pull_mysql_client_test

Details

In response to this:

/test fast_test_tiprow

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/test-infra repository.

joccau · 2024-08-26T08:59:23Z

/test unit-test

tiprow · 2024-08-26T08:59:28Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test unit-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.

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

D3Hunter

rest lgtm

pkg/ddl/ddl_history.go

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

joccau · 2024-08-28T02:47:20Z

/test fast_test_tiprow

joccau · 2024-08-28T11:27:58Z

/test check-dev

tiprow · 2024-08-28T11:28:04Z

@joccau: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

Details

In response to this:

/test check-dev

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.

lance6716 · 2024-08-28T12:52:11Z

/retest

ti-chi-bot · 2024-09-19T02:17:56Z

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

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

ti-chi-bot · 2024-09-19T02:18:01Z

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

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

ti-chi-bot · 2024-09-19T02:18:04Z

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

ti-chi-bot · 2024-09-19T02:18:21Z

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

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

close #55711

…55590) (pingcap#56143) close pingcap#55711

close #55711

ti-chi-bot bot added do-not-merge/invalid-title do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 22, 2024

limit the count of getting ddlhistory jobs

9388b9e

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

joccau force-pushed the fix-getddlhistory branch from 9791a33 to 9388b9e Compare August 22, 2024 08:29

joccau changed the title ~~[ddl] limit the count of getting ddlhistory jobs~~ ddl:limit the count of getting ddlhistory jobs Aug 22, 2024

joccau changed the title ~~ddl:limit the count of getting ddlhistory jobs~~ ddl: limit the count of getting ddlhistory jobs Aug 22, 2024

ti-chi-bot bot removed do-not-merge/invalid-title do-not-merge/needs-tests-checked labels Aug 22, 2024

add unit test

9767d81

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

joccau force-pushed the fix-getddlhistory branch from 5fc2b0b to 9767d81 Compare August 22, 2024 09:10

joccau requested review from D3Hunter and lance6716 August 22, 2024 09:49

lance6716 approved these changes Aug 23, 2024

View reviewed changes

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

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

rename local variables

7eaa70b

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

D3Hunter reviewed Aug 27, 2024

View reviewed changes

pkg/ddl/ddl_history.go Show resolved Hide resolved

maintain test case

61b4dcd

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

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

update document about tidb https

f3b5084

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

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 28, 2024

ti-chi-bot bot merged commit f2856e3 into pingcap:master Aug 28, 2024

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

This is an automated cherry-pick of pingcap#55590

5fdebac

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

ti-chi-bot mentioned this pull request Sep 19, 2024

ddl: limit the count of getting ddlhistory jobs (#55590) #56140

Open

13 tasks

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

This is an automated cherry-pick of pingcap#55590

bb002dd

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

ti-chi-bot mentioned this pull request Sep 19, 2024

ddl: limit the count of getting ddlhistory jobs (#55590) #56141

Merged

13 tasks

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

This is an automated cherry-pick of pingcap#55590

05de9bb

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

ti-chi-bot mentioned this pull request Sep 19, 2024

ddl: limit the count of getting ddlhistory jobs (#55590) #56142

Merged

13 tasks

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

This is an automated cherry-pick of pingcap#55590

a8e68c9

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

ti-chi-bot mentioned this pull request Sep 19, 2024

ddl: limit the count of getting ddlhistory jobs (#55590) #56143

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Sep 24, 2024

ddl: limit the count of getting ddlhistory jobs (#55590) (#56141)

a64e23d

close #55711

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

ddl: limit the count of getting ddlhistory jobs (#55590) (#56143)

10e82c0

close #55711

joccau pushed a commit to ti-chi-bot/tidb that referenced this pull request Feb 11, 2025

cherry-pick: ddl:limit the count of getting ddlhistory jobs (pingcap#…

a8a3c90

…55590) (pingcap#56143) close pingcap#55711

ti-chi-bot bot pushed a commit that referenced this pull request Feb 11, 2025

ddl: limit the count of getting ddlhistory jobs (#55590) (#56142)

7ae654c

close #55711

Conversation

joccau commented Aug 22, 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

Manual test steps

Release note

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

lance6716 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joccau commented Aug 26, 2024

Uh oh!

ti-chi-bot bot commented Aug 26, 2024

Uh oh!

joccau commented Aug 26, 2024

Uh oh!

tiprow bot commented Aug 26, 2024

Uh oh!

D3Hunter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joccau commented Aug 28, 2024

Uh oh!

joccau commented Aug 28, 2024

Uh oh!

tiprow bot commented Aug 28, 2024

Uh oh!

lance6716 commented Aug 28, 2024

Uh oh!

ti-chi-bot commented Sep 19, 2024

Uh oh!

ti-chi-bot commented Sep 19, 2024

Uh oh!

ti-chi-bot commented Sep 19, 2024

Uh oh!

ti-chi-bot commented Sep 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joccau commented Aug 22, 2024 •

edited

Loading

codecov bot commented Aug 22, 2024 •

edited

Loading