Skip to content

Comments

[Draft] Range-free Compaction#21311

Open
silentred wants to merge 1 commit intoetcd-io:mainfrom
silentred:new-compaction
Open

[Draft] Range-free Compaction#21311
silentred wants to merge 1 commit intoetcd-io:mainfrom
silentred:new-compaction

Conversation

@silentred
Copy link
Contributor

@silentred silentred commented Feb 14, 2026

Try to resolve #21284
This PR introduces a new compaction implementation that parallelly does calculating kvhash and deleting obsolete revision to reduce the time required for etcdserver compaction.

What is changed:

  • add a new FeatureGate CompactionRangeFree.
  • change interface of Index. The second parameter map[Revision]struct{} to return means the set of revisions to delete.
  • change compact method of keyIndex
  • add a new scheduleCompactionRangeFree method for *store

To address the suggestions in the review #21284 (comment)

In particular, we must ensure that deleted K/V pairs are still included when calculating the hash; otherwise, it may generate false alarms.

Before starting to delete revisions, a new ReadTx() is obtained. The underlying bbolt read tx should be able to see all the revisions at that moment, including the revisions that will be deleted in compaction.

what happens if the hash calculation triggered by compaction has not finished before the next compaction starts? These edge cases need to be carefully handled.

For the moment, I choose to wait for hash calculation done in the compaction logic, by doing hash := <-kvHashCh before returning. So there should not be overlap of different hash calculation.
There certainly is room for optimization. We could make compaction not wait hash calculation finished, and just return a <-chan KeyValueHash and let other component wait for the result, for example, HashStorage.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: silentred
Once this PR has been reviewed and has the lgtm label, please assign jmhbnz for approval. For more information see the Code Review Process.

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

Details 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

@k8s-ci-robot
Copy link

Hi @silentred. Thanks for your PR.

I'm waiting for a etcd-io 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.

Details

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.

…ating kvhash and deleting obsolete revision to

reduce the time required for etcdserver compaction

Signed-off-by: shenmu.wy <shenmu.wy@antfin.com>
@serathius
Copy link
Member

#21284 (comment)

  1. Test
  2. Microbenchmarks
    3 ...

@silentred
Copy link
Contributor Author

#21284 (comment)

  1. Test
  2. Microbenchmarks
    3 ...

Got it. This PR is for an overview of changes and complexity, for the community to decide whether this feature should be proceeded with. If yes, then we could start adding tests and benchmarks, as you mentioned.

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

Development

Successfully merging this pull request may close these issues.

Proposal: Range-free Compaction

3 participants