Skip to content

[Bugfix] Keep Ping responsive when remount admission is busy - #7

Merged
zozyo merged 1 commit into
mainfrom
codex/remount-retry-main-20261014
Oct 14, 2026
Merged

zozyo merged 1 commit into
mainfrom
codex/remount-retry-main-20261014

Conversation

@zozyo

@zozyo zozyo commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Description

When expiry cleanup or a snapshot writer holds the remount barrier, the synchronous ReMountSegment handler waits on an RPC io thread. Other connections assigned to that thread can then stop processing Ping. Use std::try_lock for client/snapshot admission and return the existing UNAVAILABLE_IN_CURRENT_STATUS before changing state; the current heartbeat loop retries after the barrier clears.

This main integration preserves its existing expired-owner cleanup and lifecycle pinning. The pinned maintenance release also backports targeted expiry cleanup to its older deployed base; that implementation is already present on main and is not duplicated here. Upstream #4183 depends on the broader session/lock redesign in #4180; this PR is the smaller admission repair and does not add a heartbeat server.

Module

  • Mooncake Store (mooncake-store)

Type of Change

  • Bug fix

How Has This Been Tested?

The maintenance implementation at c680059fb7b288bc6548fdad94ec1cade9ea6096 passed two real single-io-thread TCP RPC regressions and all 89 HA cases on an isolated CPU builder. Its six new regressions fail on the unmodified deployed base. This integration keeps the five tests corresponding to busy remount admission; main already has the separate expiry repair.

Test commands:

cmake --build build --target remount_rpc_retry_test master_service_ha_test -j4
build/mooncake-store/tests/remount_rpc_retry_test
build/mooncake-store/tests/master_service_ha_test --gtest_filter=MasterServiceHATest.*Remount*:MasterServiceHATest.*SnapshotBarrier*:MasterServiceHATest.LocalFirstAllocationDoesNotReacquireClientLockUnderSnapshotBarrier

Test results:

  • Exact main integration c75b0635f04333218053e8b35e6259f657b9deb6: host TCP/SSD Release configure/build passed; real RPC 2/2 and related HA 18/18 passed without skips, in 2.219s and 18.997s. The isolated 4 CPU / 32 GiB builder uses a separate source/build cache; the published maintenance image is unchanged.
  • CI run 38505158937 completed successfully for the exact main integration: all CPU/CUDA/ROCm/EFA build variants, four Ubuntu/Python wheel runtime jobs, format/docs/type checks, and SGLang integration passed. The two conditional unrelated jobs were skipped by workflow selection.
  • Maintenance native regressions: 2/2 RPC and 89/89 HA passed; six negative controls failed as expected.
  • Changed-line clang-format 20.1.8, diff whitespace, and applicable generic/spelling pre-commit checks passed.

The Windows C++ hook was verified with the same native formatter. The CMake hook rewrites a pre-existing promotion target block identically on baseline and candidate; that unrelated edit is excluded.

Checklist

  • I have performed a self-review of my own code.
  • I have formatted changed C++ lines using clang-format 20.1.8.
  • All pre-commit hooks pass without the documented baseline/platform exceptions.
  • I have added tests to prove the change is effective.
  • Documentation and RFC are not required for this narrow repair.

AI Assistance Disclosure

  • AI tools were used: OpenAI Codex authored the implementation and tests; an independent Codex review checked the frozen maintenance diff and test evidence. An independent Codex review also checked the main integration and its preservation of existing expiry/lifecycle protections.

The full metadata scan and other synchronous handlers remain unchanged. Sustained lock contention can still delay remount admission; this change does not establish an initiating expiry cause or a universal heartbeat deadline.

@zozyo
zozyo marked this pull request as ready for review October 14, 2026 03:48
@zozyo
zozyo merged commit 991e25f into main Oct 14, 2026
27 checks passed
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