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

infoschema, util/stmtsummary: enhance statements_summary. #25031

Merged
merged 123 commits into from
Jun 11, 2021

Conversation

ClSlaid
Copy link
Contributor

@ClSlaid ClSlaid commented Jun 1, 2021

What problem does this PR solve?

Issue Number: close #24083

Problem Summary:

What is changed and how it works?

What's Changed:

  • changed all files in util/stmtsummary
  • record all evicted information by sum

before:

select * from `INFORMATION_SCHEMA`.`STATEMENTS_SUMMARY`;

output:

BEGIN_TIME ... DIGEST SCHEMA_NAME ...
2019-01-01 00:30 ... digest_2 schema_name_2 ...
2019-01-01 00:30 ... digest_1 schema_name_1 ...
2019-01-01 00:00 ... digest_0 schema_name_0 ...

statement_summary table has a fixed size, some statement digests may get kicked by other digests, leading to lose of information.

This pull request will append a special row to the statement_summary, which would sum up those statement digests got evicted from statement_summary, so that all digests' information will be recorded.

Following MySQL's behavior, the row's DIGEST and SCHEMA_NAME match no record in the table, their value is <nil>.

BEGIN_TIME ... DIGEST SCHEMA_NAME ...
2019-01-01 00:30 ... digest_2 schema_name_2 ...
2019-01-01 00:30 ... digest_1 schema_name_1 ...
2019-01-01 00:00 ... digest_0 schema_name_0 ...
2019-01-01 00:30 ... <nil> <nil> ...

Special rows corresponding to each interval will also be append to statements_summary_history table.

How it Works:

  • adjust stmtSummaryByDigestEvictedElement in evicted.go, added a class member to record details
  • add and Mutex to stmtSummaryByDigestEvicted to prevent data racing.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Under construction

Side effects

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

Release note

  • record all evicted statements information by sum.

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

Signed-off-by: ClSlaid <[email protected]>
Release Note: enhanced LRU Cache
some type fixes to cheat CI.
Signed-off-by: ClSlaid <[email protected]>
typo fixes.
Signed-off-by: ClSlaid <[email protected]>
enhance test on lru's Put() method.

Signed-off-by: ClSlaid <[email protected]>
Add a function property in kvcache;
Add a method to kvcache;
Signed-off-by: ClSlaid <[email protected]>
Used Gofmt
Signed-off-by: ClSlaid <[email protected]>
An initial commit, still cannot work properly.

Signed-off-by: ClSlaid <[email protected]>
Catch up with PingCAP

Signed-off-by: ClSlaid <[email protected]>
now we can see evicted count
debug logging expressions not deleted...

Signed-off-by: ClSlaid <[email protected]>
STATEMENTS_SUMMARY_EVICTED now able to use

Test will be added in tomorrow.
Signed-off-by: ClSlaid <[email protected]>
Purge logging code used for debugging.
Signed-off-by: ClSlaid <[email protected]>
add test to evicted count, fix bugs in ssMap.other.

Signed-off-by: ClSlaid <[email protected]>
Add test to stmtSummaryByDigestMap.ToEvictedCountDatum and stmtSummaryByDigestEvictedElement.ToEvictedCountDatum.
Signed-off-by: ClSlaid <[email protected]>
make disable fail-point
Signed-off-by: ClSlaid <[email protected]>
� Conflicts:
�	executor/infoschema_reader.go
�	infoschema/tables.go
Signed-off-by: ClSlaid <[email protected]>
focus on evicted count and purged unrelated contents.
Signed-off-by: ClSlaid <[email protected]>
Signed-off-by: ClSlaid <[email protected]>
improve AddEvicted() from O(mn) to O(n)
typo fixs
Signed-off-by: ClSlaid <[email protected]>
@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 9, 2021

/run-all-tests

Signed-off-by: ClSlaid <[email protected]>
Signed-off-by: ClSlaid <[email protected]>
@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 10, 2021

/run-all-tests

@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 10, 2021

/run-integration-br-test

@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 10, 2021

/run-integration-br-test
/run-check_dev_2

@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 10, 2021

/run-integration-br-test

rows := make([][]types.Datum, 0, len(values)*historySize)
for _, value := range values {
records := value.(*stmtSummaryByDigest).toHistoryDatum(historySize, user, isSuper)
rows = append(rows, records...)
}

otherDatum := other.toHistoryDatum(historySize)
Copy link
Contributor

Choose a reason for hiding this comment

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

So you always output other even if there's no other? Does MySQL do the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NO, you find a bug here...

Copy link
Contributor Author

@ClSlaid ClSlaid Jun 10, 2021

Choose a reason for hiding this comment

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

After examination, such a situation will not happen.
There's always an other that exists, you may refer to the only situation that other contains no record.
You can take a look in stmtSummaryByDigestEvicted.toHistoryDatum().
If there is no record, no history record will be collected from the linked list inside other, so that otherDatum will be an empty list, nothing will be appended to the table.

@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 10, 2021

/run-all-tests

@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 10, 2021

/run-all-tests

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • crazycs520
  • djshow832

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 11, 2021
@djshow832
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: ce03218

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

ClSlaid commented Jun 11, 2021

/run-integration-common-test
/run-check_dev_2

@ti-chi-bot
Copy link
Member

@ClSlaid: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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 ti-community-infra/tichi repository.

@ClSlaid
Copy link
Contributor Author

ClSlaid commented Jun 11, 2021

/run-integration-common-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 all evicted statements information by sum
5 participants