load based compaction

[mittalrishabh]

This change implements the load based compaction. Changes

1. Added the code to monitor mvcc on a region
2. Use mvcc reads to calculate compaction score

What is changed and how it works?

Issue Number: Close #19133

What's Changed:

Related changes

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

Check List

Tests

• [X] Unit test
• [X] 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

[ti-chi-bot[bot]]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

[ti-chi-bot[bot]]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign cfzjywxk for approval. For more information see the Code Review Process. Please ensure that each of them provides their approval before proceeding.

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

Needs approval from an approver in each of these files:

• OWNERS
• src/server/gc_worker/OWNERS

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[bot]]

Hi @mittalrishabh. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[mittalrishabh]

/needs-ok-to-test

[mittalrishabh]

/retest

[ti-chi-bot[bot]]

@mittalrishabh: 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.

[mittalrishabh]

/ok-to-test

[ti-chi-bot[bot]]

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

In response to this:

/ok-to-test

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.

[zhangjinpeng87]

@mittalrishabh Does this PR a little bit duplicate with https://github.com/tikv/tikv/pull/18670 in terms of purpose?

[mittalrishabh]

@mittalrishabh Does this PR a little bit duplicate with #18670 in terms of purpose?

This PR takes mvcc reads in read traffic into account while calculating the compaction score. It is developed on top of https://github.com/tikv/tikv/issues/18695

[mittalrishabh]

@v01dstar PTAL

[mittalrishabh]

@v01dstar @zhangjinpeng87 are you ok at high level with these changes ?

[v01dstar]

@v01dstar @zhangjinpeng87 are you ok at high level with these changes ?

I think at high level this PR make sense.

There are 2 implementation details we probably need to polish:

• Read stats stability, as commented above
• Avoid unnecessary write amplification and resource waste. A region may be flagged as high-priority for compaction, but if it contains few GC-eligible entries, we risk compacting it repeatedly with minimal benefit.

[mittalrishabh]

@v01dstar @zhangjinpeng87 are you ok at high level with these changes ?

I think at high level this PR make sense.

There are 2 implementation details we probably need to polish:

* Read stats stability, as commented above

* Avoid unnecessary write amplification and resource waste. A region may be flagged as high-priority for compaction, but if it contains few GC-eligible entries, we risk compacting it repeatedly with minimal benefit.

1. i fixed the read stats stability. can you take a look
2. I added mvcc_read_weight. User can adjust it based on their workload.

[v01dstar]

2. I added mvcc_read_weight. User can adjust it based on their workload.

If we can clearly identify a region that is starving others, we can address it by tuning the corresponding parameters. My bigger concern is when we’re unaware of the issue—or only notice it when it’s already too late.

[mittalrishabh]

2. I added mvcc_read_weight. User can adjust it based on their workload.

If we can clearly identify a region that is starving others, we can address it by tuning the corresponding parameters. My bigger concern is when we’re unaware of the issue—or only notice it when it’s already too late.

I can add min_mvcc_entries_exist check before taking the mvcc score into account. are number of discardable versions in MvccProperties always correct or is it just an estimate ?

[v01dstar]

2. I added mvcc_read_weight. User can adjust it based on their workload.

If we can clearly identify a region that is starving others, we can address it by tuning the corresponding parameters. My bigger concern is when we’re unaware of the issue—or only notice it when it’s already too late.

I can add min_mvcc_entries_exist check before taking the mvcc score into account. are number of discardable versions in MvccProperties always correct or is it just an estimate ?

Discardable versions in MVCC is an estimate. I assume min_mvcc_entries_exist works like amplify the compaction score only if discardable_entries > min_mvcc_entries_exists? Something like min_mvcc_entries_exist = mvcc_scan_threshold * 0.5?

[mittalrishabh]

2. I added mvcc_read_weight. User can adjust it based on their workload.

If we can clearly identify a region that is starving others, we can address it by tuning the corresponding parameters. My bigger concern is when we’re unaware of the issue—or only notice it when it’s already too late.

I can add min_mvcc_entries_exist check before taking the mvcc score into account. are number of discardable versions in MvccProperties always correct or is it just an estimate ?

Discardable versions in MVCC is an estimate. I assume min_mvcc_entries_exist works like amplify the compaction score only if discardable_entries > min_mvcc_entries_exists? Something like min_mvcc_entries_exist = mvcc_scan_threshold * 0.5?

I am recording the mvcc versions only when mvcc entries in a request exceed mvcc_threshold. PTAL.

[mittalrishabh]

/ok-to-test