Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddl: limit the count of getting ddlhistory jobs #55590

Merged
merged 10 commits into from
Aug 28, 2024

Conversation

joccau
Copy link
Member

@joccau joccau commented Aug 22, 2024

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?

  1. Limit the count of jobs when getting the ddl history jobs.
  2. The parameter 'limit' should be large than 1 and less than or equal to 1024.
  3. 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

  1. create 5k table in database with the 'sysbench' tool.
  2. 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)
  1. 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.

  1. 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
  1. 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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

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.

@ti-chi-bot 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
@joccau joccau force-pushed the fix-getddlhistory branch from 9791a33 to 9388b9e Compare August 22, 2024 08:29
Copy link

codecov bot commented Aug 22, 2024

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%) ⬆️

@joccau joccau changed the title [ddl] limit the count of getting ddlhistory jobs ddl:limit the count of getting ddlhistory jobs Aug 22, 2024
@joccau joccau changed the title ddl:limit the count of getting ddlhistory jobs ddl: limit the count of getting ddlhistory jobs Aug 22, 2024
Signed-off-by: joccau <[email protected]>
@joccau joccau force-pushed the fix-getddlhistory branch from 5fc2b0b to 9767d81 Compare August 22, 2024 09:10
@joccau joccau requested review from lance6716 and D3Hunter August 22, 2024 09:49
Copy link
Contributor

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Maybe we should notify other team about this API change

@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Aug 23, 2024
@joccau
Copy link
Member Author

joccau commented Aug 26, 2024

/test fast_test_tiprow

Copy link

ti-chi-bot bot commented Aug 26, 2024

@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

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
Copy link
Member Author

joccau commented Aug 26, 2024

/test unit-test

Copy link

tiprow bot commented Aug 26, 2024

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

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.

Copy link
Contributor

@D3Hunter D3Hunter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest lgtm

Signed-off-by: joccau <[email protected]>
@ti-chi-bot 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
@joccau
Copy link
Member Author

joccau commented Aug 28, 2024

/test fast_test_tiprow

@joccau
Copy link
Member Author

joccau commented Aug 28, 2024

/test check-dev

Copy link

tiprow bot commented Aug 28, 2024

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

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
Copy link
Contributor

/retest

@ti-chi-bot ti-chi-bot bot merged commit f2856e3 into pingcap:master Aug 28, 2024
24 checks passed
@joccau joccau added affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. and removed affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. labels Sep 19, 2024
@ti-chi-bot
Copy link
Member

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

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

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

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

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

@ti-chi-bot
Copy link
Member

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

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. ok-to-test Indicates a PR is ready to be tested. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DDL: limit the number of jobs.
6 participants