resource_control: support set resource limit for background tasks #56020

glorv · 2024-09-11T09:28:27Z

What problem does this PR solve?

Issue Number: close #56019

Problem Summary:

What changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

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 · 2024-09-11T09:28:45Z

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-sigs/prow repository.

…und-limit

codecov · 2024-09-11T09:47:55Z

Codecov Report

Attention: Patch coverage is 78.37838% with 8 lines in your changes missing coverage. Please review.

Project coverage is 76.4109%. Comparing base (19caf52) to head (0b1bf3d).
Report is 13 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #56020        +/-   ##
================================================
+ Coverage   73.3985%   76.4109%   +3.0123%     
================================================
  Files          1601       1658        +57     
  Lines        444356     460679     +16323     
================================================
+ Hits         326151     352009     +25858     
+ Misses        98250      87254     -10996     
- Partials      19955      21416      +1461

Flag	Coverage Δ
integration	`52.5945% <0.0000%> (?)`
unit	`72.9569% <78.3783%> (+0.4842%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.9478% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`65.2755% <ø> (+19.3859%)`	⬆️

glorv · 2024-09-13T03:29:44Z

/retest

tiprow · 2024-09-13T03:30:06Z

@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-sigs/prow repository.

…und-limit

glorv · 2024-09-18T03:35:45Z

@nolouch @Connor1996 @HuSharp PTAL, thanks

glorv · 2024-09-18T03:37:48Z

/retest

tiprow · 2024-09-18T03:38:08Z

@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-sigs/prow repository.

glorv · 2024-09-18T04:33:51Z

/retest

tiprow · 2024-09-18T04:34:15Z

@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-sigs/prow repository.

JmPotato · 2024-09-18T09:22:15Z

pkg/domain/infosync/info.go

@@ -623,6 +623,7 @@ func ModifyResourceGroup(ctx context.Context, group *rmpb.ResourceGroup) error {
 	if err != nil {
 		return err
 	}
+	logutil.BgLogger().Info("ModifyResourceGroup", zap.Stringer("group", group))


Is this an expected log print?

sorry, redundant log for debug, I'll remove it.

glorv · 2024-09-19T07:16:24Z

@nolouch @JmPotato PTAL again

…und-limit

glorv · 2024-09-20T05:54:53Z

@nolouch PTAL again, thanks~

nolouch

rest lgtm

nolouch · 2024-09-23T06:02:34Z

pkg/parser/parser_test.go

 		{`create resource group x ru_per_sec=1000 QUERY_LIMIT (EXEC_ELAPSED "10s" ACTION COOLDOWN WATCH EXACT DURATION='10m')  background (task_types 'br,lightning')`, true, "CREATE RESOURCE GROUP `x` RU_PER_SEC = 1000, QUERY_LIMIT = (EXEC_ELAPSED = '10s' ACTION = COOLDOWN WATCH = EXACT DURATION = '10m'), BACKGROUND = (TASK_TYPES = 'br,lightning')"},
 		{`create resource group x ru_per_sec=1000 QUERY_LIMIT (EXEC_ELAPSED "10s" ACTION COOLDOWN WATCH PLAN DURATION='10m')  background (task_types 'br,lightning')`, true, "CREATE RESOURCE GROUP `x` RU_PER_SEC = 1000, QUERY_LIMIT = (EXEC_ELAPSED = '10s' ACTION = COOLDOWN WATCH = PLAN DURATION = '10m'), BACKGROUND = (TASK_TYPES = 'br,lightning')"},
+		{`create resource group x ru_per_sec=1000 QUERY_LIMIT (EXEC_ELAPSED "10s" ACTION COOLDOWN WATCH PLAN DURATION='10m')  background (task_types 'br,lightning', utilization_limit 10)`, true, "CREATE RESOURCE GROUP `x` RU_PER_SEC = 1000, QUERY_LIMIT = (EXEC_ELAPSED = '10s' ACTION = COOLDOWN WATCH = PLAN DURATION = '10m'), BACKGROUND = (TASK_TYPES = 'br,lightning', UTILIZATION_LIMIT = 10)"},


will it report unsupported operation. Currently, only the default resource group support change background setting under the parser layer?

Its format is valid so it can be parsed, unsupported operation is report at ddl module.

ti-chi-bot · 2024-09-23T06:03:18Z

[LGTM Timeline notifier]

Timeline:

2024-09-19 08:10:28.925220725 +0000 UTC m=+1121498.665644664: ☑️ agreed by JmPotato.
2024-09-23 06:03:17.834486367 +0000 UTC m=+1459467.574910307: ☑️ agreed by nolouch.

glorv · 2024-09-23T06:19:46Z

@tangenta PTAL, thanks

glorv · 2024-09-24T02:55:30Z

@yudongusa PTAL

glorv · 2024-09-24T06:47:05Z

/retest

tiprow · 2024-09-24T06:47:26Z

@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-sigs/prow repository.

…und-limit

glorv · 2024-09-24T08:57:55Z

/retest

tiprow · 2024-09-24T08:58:16Z

@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-sigs/prow repository.

yudongusa

LGTM

ti-chi-bot · 2024-09-25T01:23:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JmPotato, 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:

~~OWNERS~~ [tangenta]
~~pkg/ddl/OWNERS~~ [tangenta]
~~pkg/meta/OWNERS~~ [tangenta]
~~pkg/parser/OWNERS~~ [tangenta,yudongusa]

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

glorv · 2024-09-25T02:25:10Z

/retest

tiprow · 2024-09-25T02:25:32Z

@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-sigs/prow repository.

support set background limit

11c5ce9

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 Sep 11, 2024

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

442e523

…und-limit

fix

de69b92

glorv added 3 commits September 13, 2024 12:07

add missing token

3dd5a66

fix test

49115f2

fix

d69fca1

glorv requested review from nolouch, Connor1996 and HuSharp September 13, 2024 06:48

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

69cca94

…und-limit

JmPotato reviewed Sep 18, 2024

View reviewed changes

glorv added 3 commits September 18, 2024 17:33

remove useless log

74d9dfd

fix format background cfg and add a test

212f424

fix test

340a063

JmPotato approved these changes Sep 19, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Sep 19, 2024

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

8da3e25

…und-limit

nolouch reviewed Sep 23, 2024

View reviewed changes

nolouch approved these changes Sep 23, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 23, 2024

glorv requested a review from tangenta September 23, 2024 06:19

tangenta approved these changes Sep 24, 2024

View reviewed changes

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

0b1bf3d

…und-limit

yudongusa approved these changes Sep 25, 2024

View reviewed changes

ti-chi-bot bot added the approved label Sep 25, 2024

ti-chi-bot bot merged commit 04608f4 into pingcap:master Sep 25, 2024
23 checks passed

glorv deleted the background-limit branch September 25, 2024 04:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource_control: support set resource limit for background tasks #56020

resource_control: support set resource limit for background tasks #56020

glorv commented Sep 11, 2024

tiprow bot commented Sep 11, 2024

codecov bot commented Sep 11, 2024 •

edited

Loading

glorv commented Sep 13, 2024

tiprow bot commented Sep 13, 2024

glorv commented Sep 18, 2024

glorv commented Sep 18, 2024

tiprow bot commented Sep 18, 2024

glorv commented Sep 18, 2024

tiprow bot commented Sep 18, 2024

JmPotato Sep 18, 2024

glorv Sep 18, 2024

glorv commented Sep 19, 2024

glorv commented Sep 20, 2024

nolouch left a comment

nolouch Sep 23, 2024

glorv Sep 23, 2024

ti-chi-bot bot commented Sep 23, 2024

glorv commented Sep 23, 2024

glorv commented Sep 24, 2024

glorv commented Sep 24, 2024

tiprow bot commented Sep 24, 2024

glorv commented Sep 24, 2024

tiprow bot commented Sep 24, 2024

yudongusa left a comment

ti-chi-bot bot commented Sep 25, 2024

glorv commented Sep 25, 2024

tiprow bot commented Sep 25, 2024

resource_control: support set resource limit for background tasks #56020

resource_control: support set resource limit for background tasks #56020

Conversation

glorv commented Sep 11, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Sep 11, 2024

codecov bot commented Sep 11, 2024 • edited Loading

Codecov Report

glorv commented Sep 13, 2024

tiprow bot commented Sep 13, 2024

glorv commented Sep 18, 2024

glorv commented Sep 18, 2024

tiprow bot commented Sep 18, 2024

glorv commented Sep 18, 2024

tiprow bot commented Sep 18, 2024

JmPotato Sep 18, 2024

Choose a reason for hiding this comment

glorv Sep 18, 2024

Choose a reason for hiding this comment

glorv commented Sep 19, 2024

glorv commented Sep 20, 2024

nolouch left a comment

Choose a reason for hiding this comment

nolouch Sep 23, 2024

Choose a reason for hiding this comment

glorv Sep 23, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 23, 2024

[LGTM Timeline notifier]

glorv commented Sep 23, 2024

glorv commented Sep 24, 2024

glorv commented Sep 24, 2024

tiprow bot commented Sep 24, 2024

glorv commented Sep 24, 2024

tiprow bot commented Sep 24, 2024

yudongusa left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 25, 2024

glorv commented Sep 25, 2024

tiprow bot commented Sep 25, 2024

codecov bot commented Sep 11, 2024 •

edited

Loading