Skip to content

[Bugfix][Store] Recover Master NoF heartbeat connections after failures - #4806

Open
cage-goat wants to merge 5 commits into
kvcache-ai:mainfrom
cage-goat:codex/master-nof-heartbeat-recovery
Open

cage-goat wants to merge 5 commits into
kvcache-ai:mainfrom
cage-goat:codex/master-nof-heartbeat-recovery

Conversation

@cage-goat

@cage-goat cage-goat commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #4805

This PR depends on #4656, which introduces client-side NoF controller recovery. It reuses the qpair setup and safe teardown helpers introduced there.

Only the final commit contains the Master heartbeat recovery fix and its E2E tests. Earlier commits are inherited from the dependency branch. Please focus the review on the final commit.

Master NoF heartbeat probes can keep reusing failed controllers and qpairs after a connection failure. Subsequent probes may continue failing after the target recovers, eventually causing available segments to be removed.

This PR adds recovery for Master heartbeat connections:

  • Drain all probe qpairs on the affected controller before disconnecting it, keeping outstanding request buffers and contexts valid until cleanup completes.
  • Let subsequent heartbeats initiate controller reconnection, refresh namespace handles, and recreate qpairs as needed.
  • Use the existing heartbeat thread to advance asynchronous recovery and finish cleanup during shutdown.

Namespace registrations and the existing segment removal policy are preserved. Only successful probe reads refresh liveness.

Suggested Merge Priority

  • P0 — Urgent: Critical security vulnerability, data loss/corruption, or widespread production outage requiring immediate action
  • P1 — High: Significant correctness or stability issue in existing functionality, or a regression blocking users without a practical workaround
  • P2 — Normal: Routine bug fix, validated performance/compatibility improvement, or new feature for a confirmed use case
  • P3 — Low: Experimental feature, optional enhancement, refactoring, or cleanup with no urgent user impact

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake Conductor (mooncake-conductor)
  • 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?

Built and tested on Linux with SPDK v23.01.1, USE_NOF=ON, and BUILD_UNIT_TESTS=ON. Hugepages and access permissions were configured before running the E2E test.

Test commands:

cmake --build build --parallel 8 \
  --target mooncake_master nof_heartbeat_recovery_e2e nof_heartbeat_test

ctest --test-dir build \
  -R '^nof_heartbeat_test$' --output-on-failure

BUILD_DIR="$PWD/build" SPDK_DIR="$PWD/extern/spdk" \
  TARGET_CORE_MASK=0x10 \
  ./mooncake-store/tests/e2e/run_nof_heartbeat_recovery_tcp_e2e.sh

Test results:

  • Unit tests pass — all six existing NoF heartbeat tests passed
  • Integration tests pass — the new TCP heartbeat recovery E2E passed
  • Manual testing done — target pause/restart, persistent outage, and graceful Master shutdown were exercised

The E2E starts an independent mooncake_master process and a TCP malloc-bdev target with two namespaces sharing one controller. Both namespaces use 32 MiB capacity and 4096-byte sectors, with unchanged configuration across restart.

It registers both namespaces through RPC, pauses the target with SIGSTOP until a probe read times out, and restarts the target at the same endpoint. Both namespaces must resume successful heartbeat reads without restarting Master or registering the segments again. RPC queries verify that the original segment UUIDs and registration information are preserved.

A second, persistent outage verifies that both namespaces are removed through the existing heartbeat policy and that the registration list becomes empty. Master must then exit normally after SIGTERM.

Recovery evidence:

The following complete lines from one E2E run show the probe timeout, controller disconnect/reconnect, qpair creation, and successful heartbeat reads for both namespaces:

W20261020 17:44:46.888576 3813994 spdk_wrapper.cpp:716] nof_probe_recovery begin ctrlr=0x7c98a0003d40 namespaces=2
W20261020 17:44:46.888612 3813994 master_service.cpp:12554] segment_id=4269586136420895479-5314409991554274952, segment_name=heartbeat_recovery_ns2, endpoint=trtype:TCP adrfam:IPv4 traddr:127.0.0.1 trsvcid:33591 subnqn:nqn.2016-06.io.spdk:mooncake-heartbeat-3813918 ns:2, action=nof_heartbeat_failure, failure_count=1, latency_ms=1000, reason=completion_timeout
I20261020 17:44:46.888741 3813994 spdk_wrapper.cpp:761] nof_probe_recovery disconnected ctrlr=0x7c98a0003d40
I20261020 17:44:56.895581 3813994 spdk_wrapper.cpp:599] nof_probe_recovery reconnect ctrlr=0x7c98a0003d40
I20261020 17:44:56.958734 3813994 spdk_wrapper.cpp:781] nof_probe_recovery ready ctrlr=0x7c98a0003d40
I20261020 17:44:56.960387 3813994 spdk_wrapper.cpp:917] nof_qpair_async_connect_start ctrlr=0x7c98a0003d40 nsid=2 qpair=0x7c98a00812d0 pool=0 worker_idx=-1
I20261020 17:44:56.966828 3813994 master_service.cpp:12521] segment_id=4269586136420895479-5314409991554274952, segment_name=heartbeat_recovery_ns2, endpoint=trtype:TCP adrfam:IPv4 traddr:127.0.0.1 trsvcid:33591 subnqn:nqn.2016-06.io.spdk:mooncake-heartbeat-3813918 ns:2, action=nof_heartbeat_success, latency_ms=71
I20261020 17:45:01.971413 3813994 spdk_wrapper.cpp:917] nof_qpair_async_connect_start ctrlr=0x7c98a0003d40 nsid=1 qpair=0x7c98a00815a0 pool=0 worker_idx=-1
I20261020 17:45:01.976773 3813994 master_service.cpp:12521] segment_id=15799956214445689037-12485617706708953783, segment_name=heartbeat_recovery_ns1, endpoint=trtype:TCP adrfam:IPv4 traddr:127.0.0.1 trsvcid:33591 subnqn:nqn.2016-06.io.spdk:mooncake-heartbeat-3813918 ns:1, action=nof_heartbeat_success, latency_ms=6

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

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

Codex assisted with design analysis, implementation, code review, E2E testing, fault validation, and documentation. The human submitter is responsible for reviewing, understanding, and explaining all changes.

An error callback can finish a partially submitted task while it remains
on the QoS chain. The submit loop then completes it again when unlinking.
Require the task to be off the chain before publishing completion and
reclaiming it.
Add a per-worker control queue using the existing mutex and condition
variable. Expose worker-local state through WorkerContext.

Snapshot control and I/O queues under the same lock, then process control
tasks before I/O tasks outside the lock. This prevents newly queued I/O
from overtaking its preceding control task.

Release callback captures outside the lock and drain queued control
tasks before worker exit. Fail requests if task allocation fails after
dequeueing.

Signed-off-by: Cage Chen <cage@arcfra.com>
Detect connection failures through I/O and admin polling. Stop submission
and drain qpairs on their owner workers before reconnecting the controller,
then resume I/O after recovery.

Bind segments during Open and let their owner workers create qpairs,
advance asynchronous connections, and clean up on shutdown.

Signed-off-by: Cage Chen <cage@arcfra.com>
Master heartbeat probes keep reusing cached controllers and qpairs after
connection failures. Subsequent probes can keep failing after the target
recovers, eventually causing healthy segments to be removed.

Drain all probe qpairs on the affected controller before disconnecting
it. Reconnect on the next heartbeat, refresh namespace handles, and
recreate qpairs as needed. Use the existing heartbeat thread to advance
recovery and finish outstanding I/O safely before reusing probe buffers.

Signed-off-by: Cage Chen <cage@arcfra.com>
@cage-goat cage-goat changed the title Codex/master nof heartbeat recovery [Bugfix][Store] Recover Master NoF heartbeat connections after failures Oct 20, 2026

This branch has not been deployed

No deployments
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.

[Bug]: Master NoF heartbeat cannot recover after the target becomes available again

1 participant