Skip to content

Add write amp rate limiter#368

Merged
ti-chi-bot[bot] merged 3 commits intotikv:8.10.tikvfrom
v01dstar:write-amp-rate-limiter
Sep 24, 2024
Merged

Add write amp rate limiter#368
ti-chi-bot[bot] merged 3 commits intotikv:8.10.tikvfrom
v01dstar:write-amp-rate-limiter

Conversation

@v01dstar
Copy link
Copy Markdown
Member

@v01dstar v01dstar commented Sep 5, 2024

This PR are following PRs combined:

The WriteAmpBasedRateLimiter automatically adjusts rate limiting by predicting I/O trends using heuristic methods based on the write amplification factor. e.g. a high write throughput with HIGH priority (flush) implies, highly likely, there will be LOW priority write throughput soon, because RocksDB needs to compact the L0 files to lower levels, it is better to provision in advance. Compared to the default rate limiter, it reduces the likelihood of triggering write stalls and helps prevent I/O jitter.

Comments:
* Env::IOPriority has more options now, LOW, MID, HIGH, USER instead of just LOW and HIGH, write amplification rate limiter's tuning logic may need to adjust to this change.

@v01dstar v01dstar changed the base branch from 8.10.fb to 8.10.tikv September 6, 2024 06:11
@v01dstar v01dstar requested a review from Connor1996 September 6, 2024 06:11
@v01dstar
Copy link
Copy Markdown
Member Author

v01dstar commented Sep 6, 2024

/run-all-test

@purelind
Copy link
Copy Markdown

purelind commented Sep 6, 2024

/run-all-tests

3 similar comments
@purelind
Copy link
Copy Markdown

purelind commented Sep 6, 2024

/run-all-tests

@v01dstar
Copy link
Copy Markdown
Member Author

v01dstar commented Sep 6, 2024

/run-all-tests

@v01dstar
Copy link
Copy Markdown
Member Author

v01dstar commented Sep 6, 2024

/run-all-tests

@v01dstar v01dstar force-pushed the write-amp-rate-limiter branch from 8a4ebc9 to afb4fc9 Compare September 14, 2024 04:56
@ti-chi-bot ti-chi-bot Bot added dco-signoff: yes Indicates the PR's author has signed the dco. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 14, 2024
@v01dstar v01dstar marked this pull request as draft September 18, 2024 06:53
@ti-chi-bot ti-chi-bot Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2024
@v01dstar v01dstar force-pushed the write-amp-rate-limiter branch from afb4fc9 to 68c5156 Compare September 20, 2024 05:49
@v01dstar v01dstar marked this pull request as ready for review September 20, 2024 05:49
@ti-chi-bot ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 20, 2024
@v01dstar v01dstar force-pushed the write-amp-rate-limiter branch from 68c5156 to 5e674d8 Compare September 20, 2024 06:23
@v01dstar
Copy link
Copy Markdown
Member Author

@hbisheng

Signed-off-by: v01dstar <yang.zhang@pingcap.com>
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@v01dstar v01dstar force-pushed the write-amp-rate-limiter branch from 5e674d8 to ca2cf28 Compare September 20, 2024 23:45
Copy link
Copy Markdown

@LykxSassinator LykxSassinator left a comment

Choose a reason for hiding this comment

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

overall LGTM

@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Sep 23, 2024

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

Details

In response to this:

overall 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.

Comment thread utilities/rate_limiters/write_amp_based_rate_limiter.cc
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Sep 24, 2024

@SpadeA-Tang: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

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.

@wuhuizuo
Copy link
Copy Markdown

wait for merging of #376.

@wuhuizuo
Copy link
Copy Markdown

/lgtm

refresh

@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Sep 24, 2024

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

Details

In response to this:

/lgtm

refresh

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
Copy link
Copy Markdown

ti-chi-bot Bot commented Sep 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LykxSassinator, SpadeA-Tang, wuhuizuo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [LykxSassinator,SpadeA-Tang]

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
Copy link
Copy Markdown

ti-chi-bot Bot commented Sep 24, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-09-24 13:05:32.608226997 +0000 UTC m=+1571202.348650935: ✖️🔁 reset by ti-chi-bot.

@ti-chi-bot ti-chi-bot Bot removed the lgtm label Sep 24, 2024
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Sep 24, 2024

New changes are detected. LGTM label has been removed.

@ti-chi-bot ti-chi-bot Bot merged commit 192739d into tikv:8.10.tikv Sep 24, 2024
@v01dstar v01dstar deleted the write-amp-rate-limiter branch September 24, 2024 17:17
@v01dstar
Copy link
Copy Markdown
Member Author

Merge #381 with this for future cherry-pick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants