raft-engine: reduce the threshold of compression and add related metrics on it. #16906

LykxSassinator · 2024-04-23T09:50:16Z

What is changed and how it works?

Issue Number: Ref #16907 ref tikv/raft-engine#358

What's Changed:

This pr contains:
- Reduce the default compression threshold in RaftEngine to `4kb` as default if users do not manually set it.
- Add relevant metrics on the compression ratio of RaftEngine.write.

The relevant metrics will be shown just like the following shows:

And as for performance, taking tpcc workloads, using v7.5.x (disable async-io and compression-threshold = 8kb as default) as the baseline, the CPU.utilization keeps nearly same in this pr.

And using smaller compression threshold with batch-compression-threshold = 4kb, async-io, keeps the same, and the costs of IO Bandwidth are smaller than v8.1.0-nightly ((batch-compression-threshold = 8kb, async-io). (PS: smaller is better in "Disk IO Throughputs")

QPS Diff	Disk IO Throughputs

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

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

Release note

For the version of TIKV >= v8.1:
- Reduce the default compression threshold in RaftEngine to `4kb` as default if users do not manually set it.
- Add relevant metrics on the compression ratio of RaftEngine.write.

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot · 2024-04-23T09:50:19Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SpadeA-Tang
glorv

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot · 2024-04-23T09:50:19Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

LykxSassinator · 2024-04-23T11:28:09Z

/test all

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator · 2024-04-23T11:54:40Z

/retest-required

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

glorv

rest LGTM

src/config/mod.rs

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

glorv · 2024-04-26T05:23:43Z

src/config/mod.rs

+            // better performance and reduce the IO overhead.
+            // Meanwhile, the batch_compression_threshold cannot be modified dynamically if
+            // the threads count of async-io are changed manually.
+            self.mut_config().batch_compression_threshold = RaftEngineReadableSize(std::cmp::max(


So does this means explicitly set config will also be changed? E.g, if user set the batch-compression-threshold to 16kb, it will be optimized to 8kb, is this expected behavior?
I still think we better make the logic straight and only do adjust if it is the default config.

Yep.

IMO, it's expected. As more async-io threads are set, smaller the batch of raft logs will be, the batch-compression-threshold should be adaptive to the size of async-io threads.

We should add a log if the config value is changed after optimization as it may surprise users.

Why not only set it when it isn't specified explicitly in config. 4kb may not be better for all cases.

Then, how about introducing a flag raft_engine.customed, which will be true if the user manually set raft-engine.batch-compression-ratio with a specific value ?

And it can be combined with the current implementation that the batch-compression-threshold will be adaptive to the size of async-ios if the user does not set it on purpose.

As for 4kb, it's used to keep coincident with the minimal page size by default.

ti-chi-bot · 2024-05-07T03:30:42Z

PR needs rebase.

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.

src/config/mod.rs

Connor1996 · 2024-05-07T03:43:46Z

src/config/mod.rs

+            // better performance and reduce the IO overhead.
+            // Meanwhile, the batch_compression_threshold cannot be modified dynamically if
+            // the threads count of async-io are changed manually.
+            if !self.customized && raft_store.store_io_pool_size > 0 {


you can make batch_compression_threshold as Option<..>, then you can know whether it's sepecified

Not appropriate.

make batch_compression_threshold as Option<..>, then you can know whether it's sepecified

If so, we cannot know whether the user forcely disable compression feature in RaftEngine, as the None is used as the default value for batch_compression_threshold. Moreover, RaftEngine also needs a default value with Some(8kb) and it is not compatible to this proposal.

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

src/config/mod.rs

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

glorv

LGTM

LykxSassinator · 2024-05-08T07:52:00Z

/merge

ti-chi-bot · 2024-05-08T07:52:02Z

@LykxSassinator: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2024-05-08T07:52:04Z

@LykxSassinator: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

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 ti-community-infra/tichi repository.

LykxSassinator · 2024-05-08T09:21:01Z

/merge

ti-chi-bot · 2024-05-08T09:21:03Z

@LykxSassinator: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2024-05-08T09:21:06Z

This pull request has been accepted and is ready to merge.

Commit hash: fd36279

ti-chi-bot · 2024-05-09T02:34:47Z

In response to a cherrypick label: new pull request created to branch release-8.1: #16967.

ref tikv#16907 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…ics on it. (#16906) (#16967) ref #16907 This pr contains: - Reduce the default compression threshold in RaftEngine to `4kb` as default if users do not manually set it. - Add relevant metrics on the compression ratio of RaftEngine.write. Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: lucasliang <nkcs_lykx@hotmail.com> Co-authored-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator added 3 commits April 19, 2024 17:25

raft-engine: add an extra metric monitoring the compression ratio.

5c85337

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Add the panel in grafana to show the compression ratio.

71fb1d9

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Fix errors in tests.

66372fb

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress do-not-merge/release-note-label-needed labels Apr 23, 2024

Merge branch 'master' into add_comp_ratio_metrics

622a06b

ti-chi-bot bot added size/XXL release-note and removed do-not-merge/release-note-label-needed do-not-merge/needs-linked-issue labels Apr 23, 2024

Fix format errs.

d4f0bb6

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Polish codes.

70726db

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator marked this pull request as ready for review April 25, 2024 03:28

ti-chi-bot bot removed the do-not-merge/work-in-progress label Apr 25, 2024

LykxSassinator requested review from overvenus, glorv and tonyxuqqi April 25, 2024 03:28

LykxSassinator added 3 commits April 25, 2024 20:39

Merge branch 'master' into add_comp_ratio_metrics

cba6371

Merge branch 'master' into add_comp_ratio_metrics

6cb250e

Polish codes.

99045df

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

glorv reviewed Apr 26, 2024

View reviewed changes

src/config/mod.rs Outdated Show resolved Hide resolved

Add more uts.

6e333a0

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator requested a review from glorv April 26, 2024 04:16

glorv reviewed Apr 26, 2024

View reviewed changes

Merge branch 'master' into add_comp_ratio_metrics

9d7f534

ti-chi-bot bot added the needs-rebase label May 7, 2024

Connor1996 reviewed May 7, 2024

View reviewed changes

LykxSassinator added 2 commits May 7, 2024 16:49

Merge branch 'master' into add_comp_ratio_metrics

1c80440

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

Address comments.

fade2bf

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

LykxSassinator requested a review from Connor1996 May 8, 2024 01:28

Merge branch 'master' into add_comp_ratio_metrics

0f07c0c

Connor1996 reviewed May 8, 2024

View reviewed changes

src/config/mod.rs Outdated Show resolved Hide resolved

Address comments.

fd36279

Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>

glorv approved these changes May 8, 2024

View reviewed changes

ti-chi-bot bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels May 8, 2024

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label May 8, 2024

Merge branch 'master' into add_comp_ratio_metrics

c1f442d

ti-chi-bot added the needs-cherry-pick-release-8.1 label May 8, 2024

Merge branch 'master' into add_comp_ratio_metrics

927581e

LykxSassinator removed the needs-rebase label May 9, 2024

ti-chi-bot bot merged commit 6f5c642 into tikv:master May 9, 2024
7 checks passed

ti-chi-bot bot added this to the Pool milestone May 9, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request May 9, 2024

This is an automated cherry-pick of tikv#16906

11b604f

ref tikv#16907 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request May 9, 2024

raft-engine: reduce the threshold of compression and add related metrics on it. (#16906) #16967

Merged

9 tasks

LykxSassinator mentioned this pull request Jun 19, 2024

raftstore: optimize the batching mechanism when enabling async-ios. #17029

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

raft-engine: reduce the threshold of compression and add related metrics on it. #16906

raft-engine: reduce the threshold of compression and add related metrics on it. #16906

LykxSassinator commented Apr 23, 2024 •

edited

Loading

ti-chi-bot bot commented Apr 23, 2024 •

edited

Loading

ti-chi-bot bot commented Apr 23, 2024

LykxSassinator commented Apr 23, 2024

LykxSassinator commented Apr 23, 2024

glorv left a comment

glorv Apr 26, 2024

LykxSassinator Apr 26, 2024

overvenus May 6, 2024

Connor1996 May 6, 2024

LykxSassinator May 6, 2024 •

edited

Loading

ti-chi-bot bot commented May 7, 2024

Connor1996 May 7, 2024

LykxSassinator May 7, 2024 •

edited

Loading

glorv left a comment

LykxSassinator commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

LykxSassinator commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

ti-chi-bot commented May 9, 2024

raft-engine: reduce the threshold of compression and add related metrics on it. #16906

raft-engine: reduce the threshold of compression and add related metrics on it. #16906

Conversation

LykxSassinator commented Apr 23, 2024 • edited Loading

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot bot commented Apr 23, 2024 • edited Loading

ti-chi-bot bot commented Apr 23, 2024

LykxSassinator commented Apr 23, 2024

LykxSassinator commented Apr 23, 2024

glorv left a comment

Choose a reason for hiding this comment

glorv Apr 26, 2024

Choose a reason for hiding this comment

LykxSassinator Apr 26, 2024

Choose a reason for hiding this comment

overvenus May 6, 2024

Choose a reason for hiding this comment

Connor1996 May 6, 2024

Choose a reason for hiding this comment

LykxSassinator May 6, 2024 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot bot commented May 7, 2024

Connor1996 May 7, 2024

Choose a reason for hiding this comment

LykxSassinator May 7, 2024 • edited Loading

Choose a reason for hiding this comment

glorv left a comment

Choose a reason for hiding this comment

LykxSassinator commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

LykxSassinator commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

ti-chi-bot bot commented May 8, 2024

ti-chi-bot commented May 9, 2024

LykxSassinator commented Apr 23, 2024 •

edited

Loading

ti-chi-bot bot commented Apr 23, 2024 •

edited

Loading

LykxSassinator May 6, 2024 •

edited

Loading

LykxSassinator May 7, 2024 •

edited

Loading