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

resource_group: support patch for altering resource group #44322

Merged
merged 44 commits into from
Jun 13, 2023
Merged

resource_group: support patch for altering resource group #44322

Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
7fd561b
add resource group run away statement
CabinfeverB May 15, 2023
d9d85e6
merge master
CabinfeverB May 15, 2023
ea2546e
merge master
CabinfeverB May 15, 2023
e135c5b
unify int type
CabinfeverB May 16, 2023
378ed64
add test
CabinfeverB May 16, 2023
b620b8d
add test
CabinfeverB May 16, 2023
5aef7a8
extend IS resource_groups
CabinfeverB May 16, 2023
9629c5a
merge master
CabinfeverB May 23, 2023
8841b19
merge master
CabinfeverB May 23, 2023
fc82179
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 23, 2023
36edf90
merge master
CabinfeverB May 23, 2023
834fdfe
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 23, 2023
b45fe72
address comment
CabinfeverB May 23, 2023
97ba6da
address comment
CabinfeverB May 23, 2023
4022180
address comment
CabinfeverB May 23, 2023
a608efa
address comment
CabinfeverB May 23, 2023
ce32af0
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 25, 2023
e437724
merge
CabinfeverB May 25, 2023
4378b08
merge master
CabinfeverB May 26, 2023
1389dcf
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 26, 2023
1afc266
Merge branch 'master' into resource_manager/runaway_ruke
CabinfeverB May 29, 2023
4d62418
address comment
CabinfeverB May 29, 2023
2894b71
address comment
CabinfeverB May 30, 2023
4786bfe
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 30, 2023
62ee255
merge master
CabinfeverB May 30, 2023
91f75c7
address comment
CabinfeverB May 30, 2023
568224a
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 30, 2023
1cb8895
refactor feilds
CabinfeverB May 30, 2023
baf2563
address comment
CabinfeverB May 30, 2023
bce2a1b
update kvproto
CabinfeverB May 30, 2023
a8b5ce6
update kvproto
CabinfeverB May 30, 2023
a40dcce
Merge branch 'resource_manager/runaway_ruke' into resource_manager/ru…
CabinfeverB May 30, 2023
e868458
update kvproto
CabinfeverB May 30, 2023
041aa43
merge master
CabinfeverB May 31, 2023
5475373
merge master
CabinfeverB May 31, 2023
d7cd8ed
support patch for alter resource group
CabinfeverB Jun 1, 2023
44cb2b9
merge master
CabinfeverB Jun 6, 2023
e9ecd11
fix dryrun
CabinfeverB Jun 8, 2023
ca5e460
address comment
CabinfeverB Jun 12, 2023
cf54452
address comment
CabinfeverB Jun 12, 2023
54513ed
address comment
CabinfeverB Jun 12, 2023
cc2b3b1
merge master
CabinfeverB Jun 13, 2023
44e268a
address comment
CabinfeverB Jun 13, 2023
244b986
Merge branch 'master' into resource_manager/alter_patch
CabinfeverB Jun 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions ddl/ddl_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3057,8 +3057,8 @@ func SetDirectPlacementOpt(placementSettings *model.PlacementSettings, placement
return nil
}

// SetDirectResourceGroupUnit tries to set the ResourceGroupSettings.
func SetDirectResourceGroupUnit(resourceGroupSettings *model.ResourceGroupSettings, opt *ast.ResourceGroupOption) error {
// SetDirectResourceGroupSettings tries to set the ResourceGroupSettings.
func SetDirectResourceGroupSettings(resourceGroupSettings *model.ResourceGroupSettings, opt *ast.ResourceGroupOption) error {
switch opt.Tp {
case ast.ResourceRURate:
resourceGroupSettings.RURate = opt.UintValue
Expand All @@ -3081,6 +3081,9 @@ func SetDirectResourceGroupUnit(resourceGroupSettings *model.ResourceGroupSettin
}
resourceGroupSettings.BurstLimit = limit
case ast.ResourceGroupRunaway:
if len(opt.ResourceGroupRunawayOptionList) == 0 {
resourceGroupSettings.Runaway = nil
}
for _, opt := range opt.ResourceGroupRunawayOptionList {
err := SetDirectResourceGroupRunawayOption(resourceGroupSettings, opt.Tp, opt.StrValue, opt.IntValue)
if err != nil {
Expand All @@ -3096,7 +3099,9 @@ func SetDirectResourceGroupUnit(resourceGroupSettings *model.ResourceGroupSettin
// SetDirectResourceGroupRunawayOption tries to set runaway part of the ResourceGroupSettings.
func SetDirectResourceGroupRunawayOption(resourceGroupSettings *model.ResourceGroupSettings, typ ast.RunawayOptionType, stringVal string, intVal int32) error {
if resourceGroupSettings.Runaway == nil {
resourceGroupSettings.Runaway = &model.ResourceGroupRunawaySettings{}
resourceGroupSettings.Runaway = &model.ResourceGroupRunawaySettings{
Action: model.RunawayNoneAction,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here are new modifications and add test. @glorv @nolouch @Connor1996

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems this default value is useless

}
}
settings := resourceGroupSettings.Runaway
switch typ {
Expand Down Expand Up @@ -8113,8 +8118,11 @@ func (d *ddl) DropResourceGroup(ctx sessionctx.Context, stmt *ast.DropResourceGr

func buildResourceGroup(oldGroup *model.ResourceGroupInfo, options []*ast.ResourceGroupOption) (*model.ResourceGroupInfo, error) {
groupInfo := &model.ResourceGroupInfo{Name: oldGroup.Name, ID: oldGroup.ID, ResourceGroupSettings: model.NewResourceGroupSettings()}
if oldGroup.ResourceGroupSettings != nil {
*groupInfo.ResourceGroupSettings = *oldGroup.ResourceGroupSettings
}
for _, opt := range options {
err := SetDirectResourceGroupUnit(groupInfo.ResourceGroupSettings, opt)
err := SetDirectResourceGroupSettings(groupInfo.ResourceGroupSettings, opt)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion ddl/resourcegroup/group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewGroupFromOptions(groupName string, options *model.ResourceGroupSettings)
return nil, ErrInvalidResourceGroupRunawayExecElapsedTime
}
runaway.Rule.ExecElapsedTimeMs = options.Runaway.ExecElapsedTimeMs
if options.Runaway.Action == 0 {
if options.Runaway.Action == model.RunawayNoneAction {
return nil, ErrUnknownResourceGroupRunawayAction
}
runaway.Action = rmpb.RunawayAction(options.Runaway.Action)
Expand Down
38 changes: 20 additions & 18 deletions ddl/resourcegrouptest/resource_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestResourceGroupBasic(t *testing.T) {
// test default resource group.
tk.MustQuery("select * from information_schema.resource_groups where name = 'default'").Check(testkit.Rows("default UNLIMITED MEDIUM YES <nil>"))
tk.MustExec("alter resource group `default` RU_PER_SEC=1000 PRIORITY=LOW")
tk.MustQuery("select * from information_schema.resource_groups where name = 'default'").Check(testkit.Rows("default 1000 LOW NO <nil>"))
tk.MustQuery("select * from information_schema.resource_groups where name = 'default'").Check(testkit.Rows("default 1000 LOW YES <nil>"))
tk.MustContainErrMsg("drop resource group `default`", "can't drop reserved resource group")

tk.MustExec("create resource group x RU_PER_SEC=1000")
Expand Down Expand Up @@ -95,16 +95,16 @@ func TestResourceGroupBasic(t *testing.T) {

tk.MustGetErrCode("create resource group x RU_PER_SEC=1000 ", mysql.ErrResourceGroupExists)

tk.MustExec("alter resource group x RU_PER_SEC=2000 BURSTABLE QUERY_LIMIT=(EXEC_ELAPSED='15s' ACTION COOLDOWN WATCH SIMILAR DURATION '10m')")
tk.MustExec("alter resource group x RU_PER_SEC=2000 BURSTABLE QUERY_LIMIT=(EXEC_ELAPSED='15s' ACTION DRYRUN WATCH SIMILAR DURATION '10m')")
g = testResourceGroupNameFromIS(t, tk.Session(), "x")
re.Equal(uint64(2000), g.RURate)
re.Equal(int64(-1), g.BurstLimit)
re.Equal(uint64(time.Second*15/time.Millisecond), g.Runaway.ExecElapsedTimeMs)
re.Equal(model.RunawayActionCooldown, g.Runaway.Action)
re.Equal(model.RunawayActionDryRun, g.Runaway.Action)
re.Equal(model.WatchSimilar, g.Runaway.WatchType)
re.Equal(uint64(time.Minute*10/time.Millisecond), g.Runaway.WatchDurationMs)

tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 2000 MEDIUM YES EXEC_ELAPSED=15s, ACTION=COOLDOWN, WATCH=SIMILAR[10m0s]"))
tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 2000 MEDIUM YES EXEC_ELAPSED=15s, ACTION=DRYRUN, WATCH=SIMILAR[10m0s]"))

tk.MustExec("drop resource group x")
g = testResourceGroupNameFromIS(t, tk.Session(), "x")
Expand Down Expand Up @@ -167,27 +167,29 @@ func TestResourceGroupBasic(t *testing.T) {
tk.MustExec("create resource group x RU_PER_SEC=1000 PRIORITY=LOW")
tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 1000 LOW NO <nil>"))
tk.MustExec("alter resource group x RU_PER_SEC=2000 BURSTABLE QUERY_LIMIT=(EXEC_ELAPSED='15s' action kill)")
tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 2000 MEDIUM YES EXEC_ELAPSED=15s, ACTION=KILL"))
tk.MustQuery("show create resource group x").Check(testkit.Rows("x CREATE RESOURCE GROUP `x` RU_PER_SEC=2000, PRIORITY=MEDIUM, BURSTABLE, QUERY_LIMIT=(EXEC_ELAPSED=\"15s\" ACTION=KILL)"))
tk.MustExec("CREATE RESOURCE GROUP `x_new` RU_PER_SEC=2000 PRIORITY=MEDIUM BURSTABLE QUERY_LIMIT=(EXEC_ELAPSED=\"15s\" ACTION=KILL)")
tk.MustQuery("select * from information_schema.resource_groups where name = 'x_new'").Check(testkit.Rows("x_new 2000 MEDIUM YES EXEC_ELAPSED=15s, ACTION=KILL"))
tk.MustExec("alter resource group x BURSTABLE RU_PER_SEC=3000")
tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 3000 MEDIUM YES <nil>"))
tk.MustQuery("show create resource group x").Check(testkit.Rows("x CREATE RESOURCE GROUP `x` RU_PER_SEC=3000, PRIORITY=MEDIUM, BURSTABLE"))
tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 2000 LOW YES EXEC_ELAPSED=15s, ACTION=KILL"))
tk.MustQuery("show create resource group x").Check(testkit.Rows("x CREATE RESOURCE GROUP `x` RU_PER_SEC=2000, PRIORITY=LOW, BURSTABLE, QUERY_LIMIT=(EXEC_ELAPSED=\"15s\" ACTION=KILL)"))
tk.MustExec("CREATE RESOURCE GROUP `x_new` RU_PER_SEC=2000 PRIORITY=LOW BURSTABLE=true QUERY_LIMIT=(EXEC_ELAPSED=\"15s\" ACTION=KILL)")
tk.MustQuery("select * from information_schema.resource_groups where name = 'x_new'").Check(testkit.Rows("x_new 2000 LOW YES EXEC_ELAPSED=15s, ACTION=KILL"))
tk.MustExec("alter resource group x BURSTABLE=false RU_PER_SEC=3000")
tk.MustQuery("select * from information_schema.resource_groups where name = 'x'").Check(testkit.Rows("x 3000 LOW NO EXEC_ELAPSED=15s, ACTION=KILL"))
tk.MustQuery("show create resource group x").Check(testkit.Rows("x CREATE RESOURCE GROUP `x` RU_PER_SEC=3000, PRIORITY=LOW, QUERY_LIMIT=(EXEC_ELAPSED=\"15s\" ACTION=KILL)"))

tk.MustExec("create resource group y BURSTABLE RU_PER_SEC=2000 QUERY_LIMIT=(EXEC_ELAPSED='1s' action COOLDOWN WATCH EXACT duration '1h')")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y'").Check(testkit.Rows("y 2000 MEDIUM YES EXEC_ELAPSED=1s, ACTION=COOLDOWN, WATCH=EXACT[1h0m0s]"))
tk.MustQuery("show create resource group y").Check(testkit.Rows("y CREATE RESOURCE GROUP `y` RU_PER_SEC=2000, PRIORITY=MEDIUM, BURSTABLE, QUERY_LIMIT=(EXEC_ELAPSED=\"1s\" ACTION=COOLDOWN WATCH=EXACT DURATION=\"1h0m0s\")"))
tk.MustExec("CREATE RESOURCE GROUP `y_new` RU_PER_SEC=2000 PRIORITY=MEDIUM, BURSTABLE, QUERY_LIMIT=(EXEC_ELAPSED=\"1s\" ACTION=COOLDOWN WATCH=EXACT DURATION=\"1h0m0s\")")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y_new'").Check(testkit.Rows("y_new 2000 MEDIUM YES EXEC_ELAPSED=1s, ACTION=COOLDOWN, WATCH=EXACT[1h0m0s]"))
tk.MustExec("CREATE RESOURCE GROUP `y_new` RU_PER_SEC=2000 PRIORITY=MEDIUM QUERY_LIMIT=(EXEC_ELAPSED=\"1s\" ACTION=COOLDOWN WATCH EXACT DURATION=\"1h0m0s\")")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y_new'").Check(testkit.Rows("y_new 2000 MEDIUM NO EXEC_ELAPSED=1s, ACTION=COOLDOWN, WATCH=EXACT[1h0m0s]"))
tk.MustExec("alter resource group y_new RU_PER_SEC=3000")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y_new'").Check(testkit.Rows("y_new 3000 MEDIUM NO EXEC_ELAPSED=1s, ACTION=COOLDOWN, WATCH=EXACT[1h0m0s]"))

tk.MustExec("alter resource group y RU_PER_SEC=4000 BURSTABLE")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y'").Check(testkit.Rows("y 4000 MEDIUM YES <nil>"))
tk.MustQuery("show create resource group y").Check(testkit.Rows("y CREATE RESOURCE GROUP `y` RU_PER_SEC=4000, PRIORITY=MEDIUM, BURSTABLE"))
tk.MustExec("alter resource group y RU_PER_SEC=4000")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y'").Check(testkit.Rows("y 4000 MEDIUM YES EXEC_ELAPSED=1s, ACTION=COOLDOWN, WATCH=EXACT[1h0m0s]"))
tk.MustQuery("show create resource group y").Check(testkit.Rows("y CREATE RESOURCE GROUP `y` RU_PER_SEC=4000, PRIORITY=MEDIUM, BURSTABLE, QUERY_LIMIT=(EXEC_ELAPSED=\"1s\" ACTION=COOLDOWN WATCH=EXACT DURATION=\"1h0m0s\")"))

tk.MustExec("alter resource group y RU_PER_SEC=4000 PRIORITY=HIGH BURSTABLE")
tk.MustQuery("select * from information_schema.resource_groups where name = 'y'").Check(testkit.Rows("y 4000 HIGH YES <nil>"))
tk.MustQuery("show create resource group y").Check(testkit.Rows("y CREATE RESOURCE GROUP `y` RU_PER_SEC=4000, PRIORITY=HIGH, BURSTABLE"))
tk.MustQuery("select * from information_schema.resource_groups where name = 'y'").Check(testkit.Rows("y 4000 HIGH YES EXEC_ELAPSED=1s, ACTION=COOLDOWN, WATCH=EXACT[1h0m0s]"))
tk.MustQuery("show create resource group y").Check(testkit.Rows("y CREATE RESOURCE GROUP `y` RU_PER_SEC=4000, PRIORITY=HIGH, BURSTABLE, QUERY_LIMIT=(EXEC_ELAPSED=\"1s\" ACTION=COOLDOWN WATCH=EXACT DURATION=\"1h0m0s\")"))

tk.MustQuery("select count(*) from information_schema.resource_groups").Check(testkit.Rows("5"))
tk.MustGetErrCode("create user usr_fail resource group nil_group", mysql.ErrResourceGroupNotExists)
Expand Down
21 changes: 15 additions & 6 deletions parser/ast/ddl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
package ast

import (
"fmt"
"strings"

"github.com/pingcap/errors"
"github.com/pingcap/tidb/parser/auth"
"github.com/pingcap/tidb/parser/format"
Expand Down Expand Up @@ -2160,11 +2163,14 @@ func (n *ResourceGroupOption) Restore(ctx *format.RestoreCtx) error {
ctx.WritePlain("= ")
ctx.WriteString(n.StrValue)
case ResourceBurstableOpiton:
ctx.WriteKeyWord("BURSTABLE")
ctx.WriteKeyWord("BURSTABLE ")
ctx.WritePlain("= ")
ctx.WritePlain(strings.ToUpper(fmt.Sprintf("%v", n.BoolValue)))
case ResourceGroupRunaway:
ctx.WritePlain("QUERY_LIMIT")
ctx.WritePlain(" = ")
Copy link
Member

Choose a reason for hiding this comment

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

let's keep it consistent with others

Suggested change
ctx.WritePlain("QUERY_LIMIT")
ctx.WritePlain(" = ")
ctx.WritePlain("QUERY_LIMIT ")
ctx.WritePlain("= ")

if len(n.ResourceGroupRunawayOptionList) > 0 {
ctx.WriteKeyWord("QUERY_LIMIT")
ctx.WritePlain(" = (")
ctx.WritePlain("(")
for i, option := range n.ResourceGroupRunawayOptionList {
if i > 0 {
ctx.WritePlain(" ")
Expand All @@ -2174,6 +2180,8 @@ func (n *ResourceGroupOption) Restore(ctx *format.RestoreCtx) error {
}
}
ctx.WritePlain(")")
} else {
ctx.WritePlain("NULL")
}
default:
return errors.Errorf("invalid ResourceGroupOption: %d", n.Tp)
Expand Down Expand Up @@ -4542,9 +4550,10 @@ func CheckRunawayAppend(ops []*ResourceGroupRunawayOption, newOp *ResourceGroupR
type AlterResourceGroupStmt struct {
ddlNode

ResourceGroupName model.CIStr
IfExists bool
ResourceGroupOptionList []*ResourceGroupOption
ResourceGroupName model.CIStr
IfExists bool
ResourceGroupOptionList []*ResourceGroupOption
ResourceGroupRunawayOptionList []*ResourceGroupRunawayOption
Copy link
Contributor

Choose a reason for hiding this comment

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

Why need this field? I see there are already ResourceGroupRunawayOptionList in ResourceGroupOptionList

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forget to remove it

}

func (n *AlterResourceGroupStmt) Restore(ctx *format.RestoreCtx) error {
Expand Down
1 change: 1 addition & 0 deletions parser/model/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1866,6 +1866,7 @@ const (
RunawayActionDryRun RunawayActionType = iota
RunawayActionCooldown
RunawayActionKill
RunawayNoneAction
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: better let this default RunawayNoneAction to be the iota

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems this default value is useless

Because we do not support patch in the runaway setting clause, it means that the user must declare the configuration of action, but action is just an int and cannot judge nil like a pointer, so this default value can judge whether the user has declared action.

RunawayNoneAction

Because there is no RunawayNoneAction in kvproto, I have made RunawayNoneAction equal to 3 to avoid semantic conversion between kvproto and tidb

Copy link
Contributor

Choose a reason for hiding this comment

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

Because we do not support patch in the runaway setting clause, it means that the user must declare the configuration of action, but action is just an int and cannot judge nil like a pointer, so this default value can judge whether the user has declared action.

When action = RunawayNoneAction, we can just set to ResourceGroupSettings and vice verse, so there is even no need to add this action type here.

)

// RunawayWatchType is the type of runaway watch.
Expand Down
Loading