Skip to content

🎯 Fix Race Condition: Watch Misses Deletion Event Near Compaction Boundary #5

Description

@morriganreza973

📝 Description

A race condition exists between the MVCC compaction process and new watch registrations. When a client requests a watch starting at a specific revision $R$ close to the compaction boundary, and a compaction operation up to revision $C$ ($C \ge R$) occurs concurrently, the watch can be successfully established without returning ErrCompacted.

However, because the compaction has already purged the history up to $C$, the deletion event (or any update event) occurring at or before $R$ is missed. The watch continues to receive subsequent events, but the missed deletion event leads to silent data inconsistency, causing downstream clients maintaining local caches to retain stale, deleted objects indefinitely.

🎯 Acceptance Criteria

  • A watch request for a revision $R$ that has been compacted must consistently fail with ErrCompacted.
  • Under concurrent compaction and watch creation, there must be no window where a watch is successfully established but misses events prior to or at the compaction revision.
  • The fix must not introduce performance regressions or deadlocks in the MVCC hot path (specifically during high-throughput write and watch registration phases).
  • The solution must be verified by a deterministic integration/unit test simulating the race condition.

🛠️ Technical Specifications & Context

The issue resides within the MVCC storage engine of etcd, specifically around the synchronization of watch registration and compaction.

Key Components:

  • server/storage/mvcc/watchable_store.go: Manages the watchableStore which coordinates active watchers (synced and unsynced groups) and handles compaction events.
  • server/storage/mvcc/watcher.go: Defines the watcher and watchStream behavior.
  • server/storage/mvcc/kvstore_compaction.go: Implements the compaction logic that purges historical revisions from the backend.

Root Cause Analysis:

When a watch is created via watchableStore.watch(), it determines whether the watcher is synced (up-to-date with the current revision) or unsynced (needs to catch up from a past revision $R$).
If $R$ is less than the compact revision, it should return ErrCompacted. However, if the compaction transaction updates the compact revision after the watch checks the boundary but before the watch is added to the progress tracking system (or vice-versa without proper lock acquisition), the watch may bypass the compaction check but fail to read the compacted key-value events from the underlying boltdb backend.

Suggested Implementation Path:

  1. Review the locking mechanism in watchableStore.watch() and watchableStore.compact(). Ensure that the read of the current compact revision and the registration of the unsynced watcher are atomic relative to the compaction transaction commit.
  2. Ensure that if a watcher is moved to the unsynced list, its starting revision is validated against the compacted revision under the store.mu lock.
  3. If a compaction occurs while a watcher is in the process of catching up (in the unsynced group), ensure the watcher is properly cancelled with ErrCompacted if its pending revision falls behind the new compaction physical boundary.

🧪 Verification & Testing

To verify the fix, implement a concurrent stress test in server/storage/mvcc/watchable_store_test.go:

  1. Setup: Create a watchableStore and populate it with keys, then delete one of the keys at revision $R$.
  2. Execution:
    • Start a goroutine that continuously triggers compaction up to revision $R$.
    • Start a concurrent goroutine that attempts to create a watch starting at revision $R$.
  3. Assertion:
    • Verify that the watch either:
      • Successfully starts and receives the deletion event.
      • Fails immediately with ErrCompacted (or sends an filter/control event indicating compaction).
    • Assert that under no circumstances does the watch successfully establish (returning a valid watch ID with no error) while failing to deliver the deletion event.

Opire Bounty


This repo is using Opire - what does it mean? 👇
💵 Everyone can add rewards for this issue commenting /reward 100 (replace 100 with the amount).
🕵️‍♂️ If someone starts working on this issue to earn the rewards, they can comment /try to let everyone know!
🙌 And when they open the PR, they can comment /claim #5 either in the PR description or in a PR's comment.

🪙 Also, everyone can tip any user commenting /tip 20 @morriganreza973 (replace 20 with the amount, and @morriganreza973 with the user to tip).

📖 If you want to learn more, check out our documentation.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions