-
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
pkg/parser: support SWITCH_GROUP syntax for runaway watch #54804
Conversation
Skipping CI for Draft Pull Request. |
Skipping CI for Draft Pull Request. |
ce699f9
to
41d76c3
Compare
41d76c3
to
4ac322e
Compare
cf16f00
to
fc94505
Compare
5925721
to
8a7ba0a
Compare
/retest |
8a7ba0a
to
19d07b9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54804 +/- ##
=================================================
- Coverage 72.8344% 56.7529% -16.0816%
=================================================
Files 1598 1748 +150
Lines 444284 627973 +183689
=================================================
+ Hits 323592 356393 +32801
- Misses 100778 246949 +146171
- Partials 19914 24631 +4717
Flags with carried forward coverage won't be shown. Click here to find out more.
|
19d07b9
to
028d340
Compare
028d340
to
ce75fa0
Compare
319c7b1
to
dca85a4
Compare
pkg/domain/resourcegroup/runaway.go
Outdated
@@ -480,38 +513,47 @@ func newRunawayChecker( | |||
} | |||
|
|||
// BeforeExecutor checks whether query is in watch list before executing and after compiling. | |||
func (r *RunawayChecker) BeforeExecutor() error { | |||
func (r *RunawayChecker) BeforeExecutor() (string, error) { |
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.
How about BeforeExecutor(stmt Ctx)
?
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.
It can be, but introducing *stmtctx.StatementContext
in RunawayChecker
will have the circular dependency problem. Though we could use an interface like resourceGroupNameSetter
to resolve this, it's also simple to just directly return the switch resource group here.
pkg/domain/resourcegroup/runaway.go
Outdated
@@ -127,6 +148,7 @@ func (r *QuarantineRecord) GenInsertionStmt() (string, []any) { | |||
params = append(params, r.WatchText) | |||
params = append(params, r.Source) | |||
params = append(params, r.Action) | |||
params = append(params, r.GetSwitchGroupName()) |
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.
Does action already contain the switch group name here?
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.
No, since r.Action
of type rmpb.RunawayAction
only contains the action type information, extending it in kvproto
could cause backward compatibility issues. Therefore, I chose to add a new column to store the switch group name instead.
dca85a4
to
f80cfb6
Compare
/retest |
3439414
to
1ccc6b1
Compare
/approve for ddl/parser pkg |
1aca797
to
0fb089e
Compare
/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
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: D3Hunter, glorv, GMHDBJD, HuSharp, lcwangchao, nolouch, yudongusa, YuJuncen 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 |
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
Signed-off-by: JmPotato <[email protected]>
0fb089e
to
bd7b88f
Compare
@JmPotato: 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. |
/test unit-test |
@JmPotato: The specified target(s) for
Use 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. |
/test unit-test |
@JmPotato: The specified target(s) for
Use 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. |
What problem does this PR solve?
Issue Number: ref #54434. Should merge after pingcap/kvproto#1254.
What changed and how does it work?
Add
SWITCH_GROUP(<name>)
syntax for runaway watch.Check List
Tests
By
QUERY WATCH ADD
:Release note
Please refer to Release Notes Language Style Guide to write a quality release note.