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

executor: make SLOW_QUERY support query slow log at any time #14840

Merged
merged 18 commits into from
Feb 20, 2020

Conversation

crazycs520
Copy link
Contributor

What problem does this PR solve?

This PR is step 2 of #14748.

  • Make table SLOW_QUERY support query slow-log at any time.

What is changed and how it works?

  • Since both slow-log files have the same name prefix, then we can know which file is slow-log.

  • But we can't always parse the all slow-log files, It will too slow. Since TiDB already has the memtable_predicate_extracctor, We can get the query time range, and then locate slow-log files to parse based on the time range.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

Related changes

Release note

  • Make SLOW_QUERY support query slow log at any time.

Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
@crazycs520 crazycs520 requested a review from a team as a code owner February 19, 2020 04:19
@ghost ghost requested review from eurekaka and francis0407 and removed request for a team February 19, 2020 04:19
Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
@francis0407 francis0407 removed their request for review February 19, 2020 08:13
planner/core/memtable_predicate_extractor.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

Already rename max_num to maxLineNum.

why not just seek cursor to a big value and readLine from up to the end?

Because the big value was hard to define. The size plan and query is unpredictable.
But the max line is certain, currently, 1 slow log won't more than 128 line.

Signed-off-by: crazycs <crazycs520@gmail.com>
executor/slow_query.go Outdated Show resolved Hide resolved
Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
Signed-off-by: crazycs <crazycs520@gmail.com>
@crazycs520 crazycs520 added this to the v4.0.0-beta.1 milestone Feb 20, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 20, 2020

Dear contributor, the first Performance challenge program has been closed. But you can still contribute to this issue and we appreciate your contribution.

Copy link
Contributor

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

executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
executor/slow_query.go Outdated Show resolved Hide resolved
crazycs520 and others added 4 commits February 20, 2020 16:44
Signed-off-by: crazycs <crazycs520@gmail.com>
Co-Authored-By: Arenatlx <ailinsilence4@gmail.com>
Co-Authored-By: Arenatlx <ailinsilence4@gmail.com>
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM

@AilinKid AilinKid added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 20, 2020
Copy link
Contributor

@reafans reafans left a comment

Choose a reason for hiding this comment

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

lgtm

@reafans reafans added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 20, 2020
@crazycs520
Copy link
Contributor Author

/run-all-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants