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

*: Add tidbCPU/tikvCPU into system tables #55455

Merged
merged 56 commits into from
Sep 5, 2024

Conversation

yibin87
Copy link
Contributor

@yibin87 yibin87 commented Aug 16, 2024

What problem does this PR solve?

Issue Number: close #55542

Problem Summary:
For on-going slow queries, users naturally want to know some basic execution information to determine possible performance bottlenecks. Although "explain for connection" provide executor-level information, it may be too detailed for some users. Relatively speaking, the process list table provides simpler and more user-friendly information. In many scenarios, Tidb/tikv CPU information can help users quickly determine whether they have encountered CPU resource bottlenecks and further locate whether it is a tidb-side or tikv-side problem.
In addition, these memory system tables can be recorded periodically in the future, providing more information for diagnosing historical problems.

What changed and how does it work?

Several details about the changes:

  1. tidbCPU is collected using topsql mechanism, so it works only when "tidb_enable_top_sql" is enabled.
  2. The PR added extra columns in information_schema.processlists, while not changing the outputs of show processlists command, since the later command shows only brief info.
  3. Use connID + sqlID as the global unique label for each command, sqlID is increased by 1 each time. sqlID will turned to 0 when it exceeds the uint64 MAX, thus there is little possibility that later command with smaller sqlID. Since the possibility is very very small, and affects only the observability not functionality, just ignore.

After the PR, the processlist table schema will add two new columns:
img_v3_02ee_06652f1d-6b3d-4b77-8cc5-c1f95ddfb5bg

The slow_query table schema changes:
img_v3_02ee_6b820351-4331-4d8b-8267-7f8ec8d1af2g

The statements_summary table schema changes:
img_v3_02ee_4561577c-be7e-41e7-aeba-bc424975ce4g

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

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

None

@ti-chi-bot ti-chi-bot bot added 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. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 16, 2024
Copy link

ti-chi-bot bot commented Aug 16, 2024

PR needs rebase.

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.

@ti-chi-bot ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. sig/planner SIG: Planner labels Aug 16, 2024
Copy link

tiprow bot commented Aug 16, 2024

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

@yibin87 yibin87 changed the title [WIPI]executor,util,server: Add tidbCPU/tikvCPU for show processlist command *: Add tidbCPU/tikvCPU for show processlist command Aug 21, 2024
@yibin87
Copy link
Contributor Author

yibin87 commented Aug 21, 2024

/cc @wshwsh12 @crazycs520 @mornyx

@yibin87
Copy link
Contributor Author

yibin87 commented Aug 21, 2024

/retest

Copy link

tiprow bot commented Aug 21, 2024

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

@hawkingrei hawkingrei removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 21, 2024
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 6 lines in your changes missing coverage. Please review.

Project coverage is 57.3975%. Comparing base (487ef06) to head (dc3a491).
Report is 6 commits behind head on master.

Additional details and impacted files
@@                Coverage Diff                @@
##             master     #55455         +/-   ##
=================================================
- Coverage   72.8514%   57.3975%   -15.4539%     
=================================================
  Files          1598       1745        +147     
  Lines        444454     621258     +176804     
=================================================
+ Hits         323791     356587      +32796     
- Misses       100734     240120     +139386     
- Partials      19929      24551       +4622     
Flag Coverage Δ
integration 39.6520% <42.6751%> (?)
tiprow_ft ?
unit 72.0888% <92.2619%> (+0.1181%) ⬆️

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

Components Coverage Δ
dumpling 52.9567% <ø> (ø)
parser ∅ <ø> (∅)
br 63.1774% <ø> (+17.4041%) ⬆️

@yibin87
Copy link
Contributor Author

yibin87 commented Aug 21, 2024

/test unit-test

Copy link

tiprow bot commented Aug 21, 2024

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

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.

@yibin87
Copy link
Contributor Author

yibin87 commented Aug 21, 2024

/test unit-test

Copy link

tiprow bot commented Aug 21, 2024

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

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.

@yibin87
Copy link
Contributor Author

yibin87 commented Aug 21, 2024

/test check-dev2

Copy link

tiprow bot commented Aug 21, 2024

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

In response to this:

/test check-dev2

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 ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 21, 2024
@yibin87
Copy link
Contributor Author

yibin87 commented Aug 26, 2024

/test unit-test

@yibin87
Copy link
Contributor Author

yibin87 commented Sep 5, 2024

LGTM, this PR currently misses a lot of fast-sql's cpu-time, I hope your future data-persistence work can fix this problem.

BTW, the cpu_time in the statements_summary system table can be accurate, after the profile is complete, let the profile collector update the data of the statements_summary.

File a new issut to track the statements_summary improvements: #55869

Copy link
Contributor

@windtalker windtalker left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

@yudongusa yudongusa left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

ti-chi-bot bot commented Sep 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crazycs520, D3Hunter, songrijie, windtalker, wshwsh12, 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:

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

@ti-chi-bot ti-chi-bot bot added the approved label Sep 5, 2024
@yibin87
Copy link
Contributor Author

yibin87 commented Sep 5, 2024

/test pull-lightning-integration-test

Copy link

tiprow bot commented Sep 5, 2024

@yibin87: 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 pull-lightning-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.

@yibin87
Copy link
Contributor Author

yibin87 commented Sep 5, 2024

/retest

@ti-chi-bot ti-chi-bot bot merged commit fef43c5 into pingcap:master Sep 5, 2024
25 checks passed
breezewish added a commit to breezewish/tidb that referenced this pull request Sep 8, 2024
* origin/master: (33 commits)
  build(deps): bump github.com/prometheus/common from 0.55.0 to 0.57.0 (pingcap#55762)
  build(deps): bump golang.org/x/sys from 0.24.0 to 0.25.0 (pingcap#55894)
  planner: fix incorrect estRows with global index and json column (pingcap#55842)
  ddl, stat: switch to new struct for add/truncate/drop partition (pingcap#55893)
  planner: hide instance plan cache eviction log if no plan is evicted (pingcap#55918)
  expression: support tidb encode key function (pingcap#51678)
  planner: fix incorrect maintenance of `handleColHelper` for recursive CTE (pingcap#55732)
  executor: some code refine of hash join v2 (pingcap#55887)
  infoschema, meta: fix wrong auto id after `rename table` (pingcap#55847)
  ddl/ingest: set `minCommitTS` when detect remote duplicate keys (pingcap#55588)
  planner: move index advisor into the kernel (pingcap#55874)
  ddl, stats: switch to new struct for create/truncate table (pingcap#55811)
  executor: avoid new small objects in probe stage of hash join v2 (pingcap#55855)
  *: Add tidbCPU/tikvCPU into system tables (pingcap#55455)
  ddl: use static contexts in `NewReorgCopContext` (pingcap#55823)
  executor: fix index out of range bug in hash join v2 (pingcap#55864)
  executor: record index usage for the clustered primary keys (pingcap#55602)
  domain: load all non-public tables into infoschema (pingcap#55142)
  test: fix unstable TestShowViewPriv (pingcap#55868)
  ttl: add `varbinary` case for `TestSplitTTLScanRangesWithBytes` (pingcap#55863)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm ok-to-test Indicates a PR is ready to be tested. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include CPU usage (TiDB, TiKV) in system tables
8 participants