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

*: collect index usage information from point get and dump them to KV #20126

Merged
merged 34 commits into from
Oct 16, 2020

Conversation

rebelice
Copy link
Contributor

@rebelice rebelice commented Sep 21, 2020

What problem does this PR solve?

Issue Number: #19209

Problem Summary:

Collect index usage information from point get and dump them to KV.
This is a part of #19209 . And I'll collect index usage information from others in the following pr.

What is changed and how it works?

Proposal: Index usage information
What's Changed:

How it Works:

  • Add syncIndexUsageWorker to dump in-memory index usage information to KV
  • Add SessionIndexUsageCollector to collect index usage information
  • Support collecting index usage information from point get.

Related changes

Check List

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM

Release note

  • No release note.

@rebelice rebelice requested review from a team as code owners September 21, 2020 11:56
@rebelice rebelice requested review from SunRunAway and removed request for a team September 21, 2020 11:56
@rebelice
Copy link
Contributor Author

/label componet/statistics

@ti-srebot
Copy link
Contributor

These labels are not found componet/statistics.

@rebelice
Copy link
Contributor Author

/label component/statistics

@rebelice
Copy link
Contributor Author

/label sig/planner

@ti-srebot ti-srebot added the sig/planner SIG: Planner label Sep 21, 2020
@rebelice
Copy link
Contributor Author

/cc @qw4990

@ti-srebot ti-srebot requested a review from qw4990 September 21, 2020 12:09
@zz-jason zz-jason removed the request for review from a team September 21, 2020 12:57
@rebelice rebelice mentioned this pull request Sep 22, 2020
12 tasks
@rebelice rebelice requested a review from qw4990 October 13, 2020 06:15
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 14, 2020
@qw4990
Copy link
Contributor

qw4990 commented Oct 15, 2020

PTAL @eurekaka @winoros

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 16, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 16, 2020
```sql
create table SCHEMA_INDEX_USAGE (
TABLE_SCHEMA varchar(64),
TABLE_NAME varchar(64),
Copy link
Member

Choose a reason for hiding this comment

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

We'd better store table id and index id?
The table name and index name can be changed by ddl stmt.

Copy link
Contributor Author

@rebelice rebelice Oct 16, 2020

Choose a reason for hiding this comment

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

I think name is more user-friendly than id. And there are two solutions to this problem.

  1. One is to handle the DDL of rename to modify the name in the table.
  2. And the other is to create a more user-friendly view based on this table.

I prefer the latter. If you agree, I'll modify it in next PR and write it in the document.

Copy link
Member

Choose a reason for hiding this comment

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

The second one LGTM.

@winoros
Copy link
Member

winoros commented Oct 16, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 16, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@rebelice merge failed.

@winoros
Copy link
Member

winoros commented Oct 16, 2020

/run-unit-test
/run-tics-test

@qw4990 qw4990 merged commit bdb6c49 into pingcap:master Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config component/statistics sig/execution SIG execution sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants