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.

Sign up for GitHub

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

Jump to bottom

*: add a variable to force statement priority of TiDB server #7694

Merged
merged 5 commits into from
Sep 20, 2018
Merged

*: add a variable to force statement priority of TiDB server #7694

merged 5 commits into from
Sep 20, 2018

Conversation

jackysp
Copy link
Member

@jackysp jackysp commented Sep 14, 2018 •
edited
Loading

What problem does this PR solve?

Add a variable to force statement priority of TiDB server. Fix #7524 .

What is changed and how it works?

Force the priority for each statement when tidb_force_priority is set to high/low/delayed priority.

Check List

Tests

  • Unit test
  • Manual test
    Watch the grafana of coprocess thread CPU usage when set tidb_force_priority = "high_priority"/"low_priority"/"delayed"

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Side effects

  • Increased code complexity

Related changes

  • Need to be included in the release note

PTAL @coocood @morgo

@jackysp jackysp added the type/enhancement The issue or PR belongs to an enhancement. label Sep 14, 2018
@morgo
Copy link
Contributor

morgo commented Sep 15, 2018

LGTM

winoros
winoros reviewed Sep 20, 2018
View reviewed changes
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.

This won't influence the priority of TiDB's internal SQL?

@jackysp
Copy link
Member Author

jackysp commented Sep 20, 2018

For analyze statement, it will set its own priority, for others, I have to say yes, @winoros .

@winoros
Copy link
Member

winoros commented Sep 20, 2018

Need we use different variable to control internal ones and external ones?

@jackysp
Copy link
Member Author

jackysp commented Sep 20, 2018

I think we can skip the internal SQLs by its connection ID, when this variable is set.

winoros
winoros previously approved these changes Sep 20, 2018
View reviewed changes
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

coocood
coocood approved these changes Sep 20, 2018
View reviewed changes
Copy link
Member

@coocood coocood left a comment

Choose a reason for hiding this comment

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

LGTM

@shenli shenli added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 20, 2018
@zz-jason zz-jason merged commit 334e925 into pingcap:master Sep 20, 2018
@morgo morgo mentioned this pull request Sep 26, 2018
Closed
@jackysp jackysp deleted the force_priority branch October 8, 2018 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coocood coocood coocood approved these changes

@winoros winoros winoros left review comments

Assignees
No one assigned
Labels
status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jackysp @morgo @winoros @coocood @shenli @zz-jason