-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
runaway: Mitigation tidb_runaway_queries flooding #55664
Conversation
Hi @HuSharp. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. 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 kubernetes-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #55664 +/- ##
=================================================
- Coverage 72.8403% 56.8043% -16.0360%
=================================================
Files 1599 1731 +132
Lines 444670 635469 +190799
=================================================
+ Hits 323899 360974 +37075
- Misses 100835 250512 +149677
- Partials 19936 23983 +4047
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can u add some tests?
f7ca2d1
to
acc9e35
Compare
2523bdb
to
d8fbe0d
Compare
/retest-required |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
added map-related and flood-related tests :) |
/retest-required |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
/retest-required |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
/retest-required |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest lgtm
pkg/session/bootstrap_test.go
Outdated
@@ -270,6 +270,13 @@ func revertVersionAndVariables(t *testing.T, se sessiontypes.Session, ver int) { | |||
// for version <= version195, tidb_enable_dist_task should be disabled before upgrade | |||
MustExec(t, se, "update mysql.global_variables set variable_value='off' where variable_name='tidb_enable_dist_task'") | |||
} | |||
if ver < version212 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only for tests? if in real environment, do we have a way or need a way to revert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for testing purposes only. We will not encounter calling unsetStoreBootstrapped
and supporting fallback/revert in the real world. PTAL, thx~ @tangenta
/retest-required |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
/retest-required |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
7c6a2ed
to
6b76e24
Compare
Signed-off-by: husharp <ihusharp@gmail.com>
f2a1f00
to
91e3cbb
Compare
Signed-off-by: husharp <ihusharp@gmail.com>
91e3cbb
to
b36e3e2
Compare
Signed-off-by: husharp <ihusharp@gmail.com>
61ced36
to
1607836
Compare
Signed-off-by: husharp <ihusharp@gmail.com>
/test unit-test |
@HuSharp: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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 kubernetes-sigs/prow repository. |
/ok-to-test |
/retest-required |
Signed-off-by: husharp <ihusharp@gmail.com>
Signed-off-by: husharp <ihusharp@gmail.com>
/retest-required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for session part
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: glorv, JmPotato, lcwangchao, nolouch, tangenta, yudongusa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest-required |
@HuSharp: The following test failed, say
Full PR test history. Your PR dashboard. 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 kubernetes/test-infra repository. I understand the commands that are listed here. |
/retest-required |
@HuSharp: The following test failed, say
Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: ref #54434
Problem Summary: There could be a large number of runaway queries that are identified in a short period.
Currently, the TiDB logs each occurrence which flood the log table. Since it's likely the most queries are identical, we shall introduce a way to compress the logs.
What changed and how does it work?
changed
sql_digest
of this query in this table. If users add manual rules with sql_digest, they can easily identify the hits by filtering sql_digest.tidb_runaway_queries
could be flooded if massive queries are identified as runaway queries in a short period. For example, SQL injections. Since this table is informational, we can afford to lose details.result
And then execute
select sleep(3) from t;
which has same UNIQUE KEY runaway_task(resource_group_name, sql_digest, plan_digest) asselect sleep(3) from t;
And then execute
select sleep(2) from t;
which will be regarded aswatch
type rather thankill
.Check List
Tests
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.