Skip to content

server: commit compaction deletes after unlocking batch tx#21982

Open
xigang wants to merge 1 commit into
etcd-io:mainfrom
xigang:fix/compaction-delete-commit
Open

server: commit compaction deletes after unlocking batch tx#21982
xigang wants to merge 1 commit into
etcd-io:mainfrom
xigang:fix/compaction-delete-commit

Conversation

@xigang

@xigang xigang commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

This PR reduces lock contention during compaction deletes.

Previously, compaction synchronously committed bbolt while holding the batch-tx lock after deleting old revisions. Any apply operation queued behind it had to wait for the fsync to complete.

The change releases the batch-tx lock before committing, so queued apply operations can proceed without being blocked by compaction’s fsync.

Normal client deletes are unchanged and retain their existing immediate-commit semantics.

@kubernetes-prow

Copy link
Copy Markdown

Hi @xigang. 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.

@kubernetes-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xigang
Once this PR has been reviewed and has the lgtm label, please assign ahrtr 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

@xigang

xigang commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Why deferring the commit is safe for compaction deletes

The commit during Unlock() is required for normal client deletes because deletes are not represented in the read buffer. Without an immediate commit, a subsequent request could read stale data from bbolt and break linearizability.

Compaction deletes are different. They remove historical revisions at or below compactMainRev. These revisions are no longer client-visible: historical reads at or below the compaction revision return ErrCompacted, while latest reads are resolved through the MVCC index.

UnsafeDeleteForCompaction therefore skips only the delete-triggered commit during Unlock(). The deletion still belongs to the current writable transaction and is persisted by the explicit ForceCommit() immediately afterward. Normal client deletes retain their existing commit behavior.

The final batch also calls ForceCommit() after unlocking, so its deletes and the finishedCompact marker are committed together. If the process crashes before that commit, neither is persisted, and compaction resumes from the persisted scheduled/finished markers after restart.

Microbenchmark

This benchmark validates the lock-contention mechanism rather than representing an end-to-end workload. It uses real bbolt fsyncs, high batch limits, and an apply waiter queued for at least 2 ms before compaction unlocks.

Metric Commit during Unlock Commit after Unlock
Queued apply wait 8.52 ms/op 0.194 ms/op
Compaction batch 8.34 ms/op 8.31 ms/op

The total compaction cost is unchanged because the commit is moved, not removed. The improvement is that an already queued apply operation can acquire the batch-tx lock between compaction's Unlock() and ForceCommit(), instead of waiting behind the fsync.

@xigang xigang force-pushed the fix/compaction-delete-commit branch from e8793e3 to 9938360 Compare June 21, 2026 11:32
@kubernetes-prow kubernetes-prow Bot added size/L and removed size/M labels Jun 21, 2026
Signed-off-by: xigang <wangxigang2014@gmail.com>
@xigang xigang force-pushed the fix/compaction-delete-commit branch from 9938360 to 9f62dc8 Compare June 21, 2026 11:39
@xigang xigang changed the title server: avoid forced commit for compaction deletes server: commit compaction deletes after unlocking batch tx Jun 21, 2026
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.

1 participant