parser,ddl: add background job syntax and meta management for resource control #45189

glorv · 2023-07-05T09:51:48Z

What problem does this PR solve?

Issue Number: ref #44517

Problem Summary:

What is changed and how it works?

Resource control add new syntax for background tasks management as following:

// Set background tasks
CREATE RESOURCE GROUP `rg1` ru_per_sec = 1000 BACKGROUND=(TASK_NAMES="br,lightning");
// change settings
ALTER RESOURCE GROUP `rg1` BACKGROUND = (TASK_NAMES="br");
// reset background tasks
ALTER RESOURCE GROUP `rg1` BACKGROUND = NULL;

NOTE: in the first phase, we only support changing the default resource group's background config.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

tiprow · 2023-07-05T09:52:16Z

Hi @glorv. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

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/test-infra repository.

CabinfeverB · 2023-07-06T09:10:33Z

parser/parser.y

+	}
+
+DirectResourceGroupBackgroundOption:
+	"TASK_NAMES" EqOpt stringLit


Is the task name an enumeration type? I find PD and TiDB won't check whether name is valid, user can't get feedback if the name is wrong.

Yes, you can check the enum here. @CabinfeverB
https://github.com/tikv/client-go/pull/868/files#diff-6bd1a8e867c588123e135e97a8474639b648d9b6e4ab28c5b29338b388505dfbR41

My main concern is that when setting the backgroup, if the user input is wrong, it will not report an error.

Added a check in ddl api

…und-meta

glorv · 2023-07-10T03:58:07Z

@CabinfeverB @HuSharp @nolouch PTAL again, thanks

nolouch

lgtm

HuSharp · 2023-07-10T09:40:27Z

ddl/ddl_api.go

+			}
+		}
+	case ast.ResourceGroupBackground:
+		if groupInfo.Name.L != rg.DefaultResourceGroupName {


I noticed this restriction, so maybe need to update pr description?

There is a NOTE in the PR description.

oops, sorrrry! I must be blind!

HuSharp · 2023-07-10T09:55:33Z

parser/ast/ddl.go

+					ctx.WritePlain(", ")
+				}
+				if err := option.Restore(ctx); err != nil {
+					return errors.Annotatef(err, "An error occurred while splicing ResourceGroup Background Option")


Do we need to unify error with ResourceGroupRunaway to expose i?

Suggested change

return errors.Annotatef(err, "An error occurred while splicing ResourceGroup Background Option")

return errors.Annotatef(err, "An error occurred while splicing ResourceGroup Background Option: [%v]", i")

HuSharp · 2023-07-10T09:55:44Z

parser/ast/ddl.go

+					ctx.WritePlain(" ")
+				}
+				if err := option.Restore(ctx); err != nil {
+					return errors.Annotatef(err, "An error occurred while splicing CreateResourceGroupStmt Option: [%v]", i)


Suggested change

return errors.Annotatef(err, "An error occurred while splicing CreateResourceGroupStmt Option: [%v]", i)

return errors.Annotatef(err, "An error occurred while splicing ResourceGroupRunaway Option: [%v]", i)

HuSharp

merge master failed, rest LGTM!

ti-chi-bot · 2023-07-11T06:07:40Z

@HuSharp: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

merge master failed, reset LGTM!

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.

…und-meta

ti-chi-bot · 2023-07-11T07:16:15Z

@glorv: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-br-integration-test	`fe020f3`	link	true	`/test pull-br-integration-test`

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.

glorv · 2023-07-11T08:00:09Z

/retest

tiprow · 2023-07-11T08:00:52Z

@glorv: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

tiprow · 2023-07-11T08:05:14Z

@glorv: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test check-dev2

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.

ti-chi-bot · 2023-07-11T09:01:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, HuSharp, nolouch

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:

~~OWNERS~~ [3pointer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2023-07-11T09:01:11Z

[LGTM Timeline notifier]

Timeline:

2023-07-10 09:17:53.274943992 +0000 UTC m=+323565.045282708: ☑️ agreed by nolouch.
2023-07-11 09:01:10.064715895 +0000 UTC m=+408961.835054607: ☑️ agreed by 3pointer.

hawkingrei · 2023-07-11T09:03:24Z

/hold

tiprow · 2023-07-11T09:06:16Z

@glorv: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
tiprow_fast_test	`7e5236f`	link	true	`/test tiprow_fast_test`

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.

hawkingrei · 2023-07-11T09:09:56Z

/unhold

supprot resource group background parser and meta

6252610

glorv requested review from nolouch, CabinfeverB, HuSharp and BornChanger July 5, 2023 09:51

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 5, 2023

glorv added 2 commits July 5, 2023 18:02

fix lint

105b7e0

fix build

fe020f3

ti-chi-bot bot deleted a comment from ti-chi-bot Jul 5, 2023

CabinfeverB reviewed Jul 6, 2023

View reviewed changes

validate resource group background task names

b81c599

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2023

ti-chi-bot bot deleted a comment from ti-chi-bot Jul 10, 2023

Merge branch 'master' of https://github.com/pingcap/tidb into backgro…

f82601a

…und-meta

ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2023

ti-chi-bot bot deleted a comment from ti-chi-bot Jul 10, 2023

nolouch approved these changes Jul 10, 2023

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jul 10, 2023

HuSharp reviewed Jul 10, 2023

View reviewed changes

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2023

fix

0a6c2fd

HuSharp approved these changes Jul 11, 2023

View reviewed changes

Merge branch 'master' of https://github.com/pingcap/tidb into backgro…

57b05a8

…und-meta

ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2023

fix bazel

7e5236f

3pointer approved these changes Jul 11, 2023

View reviewed changes

ti-chi-bot bot added the lgtm label Jul 11, 2023

ti-chi-bot bot added approved and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 11, 2023

ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2023

remove useless dep

2a0b3ea

ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2023

ti-chi-bot bot merged commit 43763d2 into pingcap:master Jul 11, 2023
3 checks passed

glorv deleted the background-meta branch July 11, 2023 09:59

tangenta mentioned this pull request Jul 12, 2023

parser: change token TASK_NAMES to TASK_TYPES #45331

Merged

12 tasks

glorv mentioned this pull request Jun 26, 2023

Tracking Issue of Background Resource Control #44517

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parser,ddl: add background job syntax and meta management for resource control #45189

parser,ddl: add background job syntax and meta management for resource control #45189

glorv commented Jul 5, 2023 •

edited

Loading

tiprow bot commented Jul 5, 2023

CabinfeverB Jul 6, 2023

HuSharp Jul 6, 2023 •

edited

Loading

CabinfeverB Jul 6, 2023

glorv Jul 10, 2023

glorv commented Jul 10, 2023

nolouch left a comment

HuSharp Jul 10, 2023

glorv Jul 10, 2023

HuSharp Jul 10, 2023

HuSharp Jul 10, 2023

HuSharp Jul 10, 2023

HuSharp left a comment •

edited

Loading

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023 •

edited

Loading

glorv commented Jul 11, 2023

tiprow bot commented Jul 11, 2023

tiprow bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

hawkingrei commented Jul 11, 2023

tiprow bot commented Jul 11, 2023

hawkingrei commented Jul 11, 2023

	return errors.Annotatef(err, "An error occurred while splicing ResourceGroup Background Option")
	return errors.Annotatef(err, "An error occurred while splicing ResourceGroup Background Option: [%v]", i")

parser,ddl: add background job syntax and meta management for resource control #45189

parser,ddl: add background job syntax and meta management for resource control #45189

Conversation

glorv commented Jul 5, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

tiprow bot commented Jul 5, 2023

Choose a reason for hiding this comment

HuSharp Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glorv commented Jul 10, 2023

nolouch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuSharp left a comment • edited Loading

Choose a reason for hiding this comment

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023 • edited Loading

glorv commented Jul 11, 2023

tiprow bot commented Jul 11, 2023

tiprow bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

[LGTM Timeline notifier]

hawkingrei commented Jul 11, 2023

tiprow bot commented Jul 11, 2023

hawkingrei commented Jul 11, 2023

glorv commented Jul 5, 2023 •

edited

Loading

HuSharp Jul 6, 2023 •

edited

Loading

HuSharp left a comment •

edited

Loading

ti-chi-bot bot commented Jul 11, 2023 •

edited

Loading