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

inforschema, executor, util/kvcache, util/statement_summary : Add STATEMENTS_SUMMARY_EVICTED into information_schema #24513

Merged
merged 69 commits into from
May 28, 2021

Conversation

ClSlaid
Copy link
Contributor

@ClSlaid ClSlaid commented May 10, 2021

What problem does this PR solve?

Issue Number: close #23920

Problem Summary:

What is changed and how it works?

What's Changed:

  • inforschema/tables.go
  • executor/inforschema_reader.go
  • executor/builder.go
  • util/kvcache/simple_lru.go
  • Add util/stmtsummary/evicted.go

How it Works:

  1. Get evicted digests by editing statementSummaryMap's onEvict function
  2. Adding evicted digest elements to corresponding record in ssMap's other list.
  3. register table to information schema.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Tests will be added as soon as possible.

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

ClSlaid and others added 18 commits April 23, 2021 18:17
Enhance LRU cache, enable users access evicted key-value pair on Put()

Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
Release Note: enhanced LRU Cache
some type fixes to cheat CI.
Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
typo fixes.
Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
enhance test on lru's Put() method.

Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
Add a function property in kvcache;
Add a method to kvcache;
Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
Used Gofmt
Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
An initial commit, still cannot work properly.

Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
Catch up with PingCAP

Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
now we can see evicted count
debug logging expressions not deleted...

Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
STATEMENTS_SUMMARY_EVICTED now able to use

Test will be added in tomorrow.
Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
@ClSlaid ClSlaid requested a review from a team as a code owner May 10, 2021 05:28
@ClSlaid ClSlaid requested review from XuHuaiyu and removed request for a team May 10, 2021 05:28
@ti-chi-bot ti-chi-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 10, 2021
@ClSlaid
Copy link
Contributor Author

ClSlaid commented May 10, 2021

/cc @crazycs520

@github-actions github-actions bot added sig/execution SIG execution sig/sql-infra SIG: SQL Infra labels May 10, 2021
Purge logging code used for debugging.
Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
add test to evicted count, fix bugs in ssMap.other.

Signed-off-by: ClSlaid <cailue@bupt.edu.cn>
@@ -2011,6 +2013,14 @@ func (e *memtableRetriever) setDataForClientErrorsSummary(ctx sessionctx.Context
return nil
}

func (e *memtableRetriever) setDataForStmtSummaryEvicted(ctx sessionctx.Context, tableName string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Look like this function is useless? just use e.rows = stmtsummary.StmtSummaryByDigestMap.ToEvictedCountDatum() in line#153?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I wasn't aware of it.

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 28, 2021
@ClSlaid
Copy link
Contributor Author

ClSlaid commented May 28, 2021

/run-check_dev_2

@crazycs520
Copy link
Contributor

/merge

1 similar comment
@crazycs520
Copy link
Contributor

/merge

@ClSlaid
Copy link
Contributor Author

ClSlaid commented May 28, 2021

/run-mybatis-test

@ti-chi-bot ti-chi-bot merged commit 0367c54 into pingcap:master May 28, 2021
@breezewish
Copy link
Member

Should we cherry pick this into v4.0.x?

@ti-srebot
Copy link
Contributor

cherry pick to release-5.0 in PR #25295

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jun 9, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #25296

@ti-srebot
Copy link
Contributor

cherry pick to release-5.1 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-5.0 needs-cherry-pick-release-5.1 sig/execution SIG execution sig/sql-infra SIG: SQL Infra size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. 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.

Record evicted information of statements_summary
7 participants