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.

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

resource_manager: add background resource limit #1264

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Sep 4, 2024

No description provided.

@ti-chi-bot ti-chi-bot bot requested a review from shihongzhi September 4, 2024 07:22
@ti-chi-bot ti-chi-bot bot added the size/XL label Sep 4, 2024
@glorv glorv requested review from nolouch, Connor1996 and HuSharp and removed request for shihongzhi September 4, 2024 07:23
@HuSharp
Copy link
Contributor

HuSharp commented Sep 9, 2024

can you paste the TiKV/TiDB related PR?

@glorv
Copy link
Contributor Author

glorv commented Sep 9, 2024

can you paste the TiKV/TiDB related PR?

Not submitted yet.

@glorv glorv requested a review from overvenus September 9, 2024 09:20
@glorv
Copy link
Contributor Author

glorv commented Sep 9, 2024

@overvenus PTAL

// background task types.
repeated string job_types = 1;
// the percentage limit of total resource(cpu/io) that background tasks can use.
uint64 utilization_limit = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Should we use separated limits for IO and CPU?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently not, if both cpu and io are limited, the lower will take effect. BTW, in most cases, IO is not limited because it depends on the total IO quota which is not set by default.

Copy link

ti-chi-bot bot commented Sep 10, 2024

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

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

@glorv
Copy link
Contributor Author

glorv commented Sep 10, 2024

@Connor1996 @overvenus PTAL

@ti-chi-bot ti-chi-bot bot added the lgtm label Sep 10, 2024
Copy link

ti-chi-bot bot commented Sep 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

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

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

ti-chi-bot bot commented Sep 10, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-09-09 03:16:32.768186973 +0000 UTC m=+239862.508611102: ☑️ agreed by nolouch.
  • 2024-09-10 15:41:31.870936412 +0000 UTC m=+370961.611360335: ☑️ agreed by overvenus.

@ti-chi-bot ti-chi-bot bot merged commit b242104 into pingcap:master Sep 10, 2024
5 checks passed
ti-chi-bot bot added a commit to tikv/pd that referenced this pull request Sep 12, 2024
close #8616

update kvproto to include pingcap/kvproto#1264 to support set background task resource limit

Signed-off-by: glorv <glorvs@163.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants