Skip to content

[Bugfix][Store] Retry HA stale-handle cleanup oplog persist on transient backpressure - #27

Closed
Connor-Matthew wants to merge 0 commit into
mainfrom
store/retry-ha-cleanup-oplog
Closed

[Bugfix][Store] Retry HA stale-handle cleanup oplog persist on transient backpressure#27
Connor-Matthew wants to merge 0 commit into
mainfrom
store/retry-ha-cleanup-oplog

Conversation

@Connor-Matthew

@Connor-Matthew Connor-Matthew commented Aug 14, 2026

Copy link
Copy Markdown

Description

In HA mode (enable_ha_ && enable_oplog_), ClearInvalidHandles persists
stale-handle cleanup through the oplog writer before removing metadata.
Today a single transient failure makes the persist call give up
immediately:

  • TASK_PENDING_LIMIT_EXCEEDED — the batch writer's slots are momentarily
    full while it seals the current batch; capacity frees up within
    milliseconds.
  • UNAVAILABLE_IN_CURRENT_STATUS — the writer is briefly rejecting writes
    while its write_batch retries against the KV backend.

When that happens, the stale handles / last-replica metadata survive until
the next cleanup sweep, so invalid client handles linger longer than
necessary and (for the last-replica path) removed objects stay visible in
the metadata map despite the client being gone.

This PR wraps both oplog persist calls in ClearInvalidHandles with a
bounded exponential-backoff retry (RetryOplogPersist):

  • TASK_PENDING_LIMIT_EXCEEDED: retry up to 10 attempts, backoff
    min(2^attempt, 16ms) — the writer frees slots imminently, waiting pays
    off.
  • UNAVAILABLE_IN_CURRENT_STATUS: retry up to 5 attempts with the same
    backoff — recovery depends on the KV backend, so bail out earlier
    instead of spinning on a persistent outage.
  • Any other error (INVALID_PARAMS etc.): no retry, returned immediately.

Control flow is unchanged: if the persist still fails after retries,
the key is skipped exactly as before and cleanup is retried on the next
sweep. No fallback semantics are added or removed.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

# Not run locally: the submitter's host is macOS, and this project can
# only be built inside a Linux container. Intended command:
ctest --test-dir build -R master_service

Test results:

  • Unit tests pass — not verified locally; relying on this PR's CI run
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

The change is a pure retry wrapper around existing calls: the success path
is byte-identical to before, and the failure path after exhausting retries
returns the same error the caller already handles (skip key, retry next
sweep). Formatting verified with clang-format 20.1.8 per
.clang-format (clean, no violations on touched lines).

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
    (clang-format 20.1.8, no violations on touched lines)
  • I have run pre-commit on the files changed in this PR and all hooks
    pass — toolchain unavailable on the submitter's macOS host; relying
    on CI
  • I have updated the documentation (if applicable) — not applicable,
    no behavior contract change
  • I have added tests to prove my changes are effective — retry timing
    depends on the oplog writer's internal backpressure state; open to
    adding a unit test if reviewers suggest a good injection point
  • For changes >500 LOC: I have filed an RFC issue — not applicable
    (+59/-4 LOC)

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

Ported and adapted with AI assistance (Proma Agent): locating the upstream
call sites, adapting the retry helper to current main (the original
commits target a fork whose ClearInvalidHandles control flow has
diverged), and formatting checks. The human submitter has reviewed every
changed line and can defend the change end-to-end.

@Connor-Matthew Connor-Matthew changed the title [Bugfix][Store] Retry HA stale-handle cleanup oplog persist on transient backpressure(上游提 PR 前内部评审) [Bugfix][Store] Retry HA stale-handle cleanup oplog persist on transient backpressure Aug 14, 2026
@Connor-Matthew
Connor-Matthew changed the base branch from main to upstream-pr/store-retry-ha-cleanup-oplog August 14, 2026 10:19
@Connor-Matthew
Connor-Matthew force-pushed the store/retry-ha-cleanup-oplog branch from 2c3c6d5 to e58e966 Compare August 14, 2026 10:30
@Connor-Matthew
Connor-Matthew changed the base branch from upstream-pr/store-retry-ha-cleanup-oplog to main August 14, 2026 10:30
@Connor-Matthew
Connor-Matthew force-pushed the store/retry-ha-cleanup-oplog branch from e58e966 to 9e554a5 Compare August 14, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant