Skip to content

[Bugfix][Store] Recover NoF controllers and reconnect I/O qpairs after connection failures - #4656

Open
cage-goat wants to merge 4 commits into
kvcache-ai:mainfrom
cage-goat:codex/nof-controller-recovery-v2
Open

cage-goat wants to merge 4 commits into
kvcache-ai:mainfrom
cage-goat:codex/nof-controller-recovery-v2

Conversation

@cage-goat

Copy link
Copy Markdown
Contributor

Fixes #3863

Description

The NoF client currently lacks a complete controller recovery and I/O qpair reconnection mechanism. After a connection failure or target restart, subsequent I/O cannot recover automatically. This PR adds coordinated controller recovery across multiple namespaces and workers.

Design

Before resetting a controller, all its I/O qpairs must stop submitting requests, drain outstanding I/O, and be freed. Since these qpairs may belong to different workers, recovery must coordinate all affected workers.

The controller coordinates recovery, while each owner worker creates, submits to, polls, and cleans up its own qpairs. A shared controller thread in SpdkWrapper polls admin queues and advances controller recovery.

Detect a connection failure through I/O or admin polling
  → Start controller recovery
  → Ask all owner workers to stop submission, finish requests, and free old qpairs
  → Wait for all owners to confirm cleanup
  → Disconnect and asynchronously reconnect the controller, retrying on failure
  → Refresh namespaces and notify workers to resume submission
  → Recreate qpairs on their owner workers when subsequent I/O arrives

Implementation highlights

  • Early owner binding: Bind segments to workers during Open and post initial connection tasks. Each worker owns its qpairs throughout their lifecycle.
  • Generic control queues: Add a control queue per worker, reusing the existing mutex and condition variable. Snapshot control tasks and I/O requests together, then execute control tasks first outside the lock.
  • Worker-local state: Workers use local recovery state to gate I/O submission and continue polling to advance asynchronous qpair connections.
  • Request and shutdown handling: Failed requests return errors without automatic replay. Recovery preserves the existing pool and segment handles. Workers clean up qpairs and unregister ownership before exiting.

This PR also includes a prerequisite fix that prevents task completion from being published before the task is removed from its QoS list, avoiding duplicate completion.

The scope is client-side NoF connection recovery. Master heartbeat and segment removal policies are unchanged.

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?

Test commands:

With USE_NOF=ON, BUILD_UNIT_TESTS=ON, and sufficient hugepages and permissions for the integration test:

cmake --build build --parallel 12 \
  --target nof_worker_control_queue_test nof_controller_recovery_e2e

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

./build/mooncake-store/tests/nof_worker_control_queue_test \
  --gtest_shuffle --gtest_repeat=20

BUILD_DIR="$PWD/build" SPDK_DIR="$PWD/extern/spdk" \
  ./mooncake-store/tests/e2e/run_nof_controller_recovery_tcp_e2e.sh

Test results:

  • Unit tests pass
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

The three control queue tests cover worker affinity and FIFO ordering, posting another task from a callback, and draining queued control tasks during shutdown. All passed, including 20 shuffled repetitions.

The TCP integration test uses an SPDK malloc-bdev target with two namespaces accessed by two workers. It stops the target with I/O outstanding, verifies that requests complete with errors, restarts the target, and verifies controller recovery, qpair reconnection, and successful reads/writes using the original pool and segment handles.

The existing worker-count test is also adjusted to account for the shared controller thread.

Selected complete lines from the latest integration run show fault handling, a retry while the target is unavailable, and successful recovery:

E20261014 18:31:06.966943 3752443 transfer_task.cpp:117] task_complete: I/O failedABORTED - SQ DELETION
E20261014 18:31:06.966985 3752443 transfer_task.cpp:689] poll completion error: ret -6
W20261014 18:31:06.966987 3752443 spdk_wrapper.cpp:656] nof_recovery begin traddr=127.0.0.1 trsvcid=44625 subnqn=nqn.2016-06.io.spdk:mooncake-recovery-3752384 owners=2
PASS failed_io_drained
W20261014 18:31:06.967414 3752441 spdk_wrapper.cpp:842] nof_recovery retry traddr=127.0.0.1 trsvcid=44625 subnqn=nqn.2016-06.io.spdk:mooncake-recovery-3752384 rc=-1 retry_after_ms=1000
I20261014 18:31:08.508172 3752441 spdk_wrapper.cpp:861] nof_recovery ready traddr=127.0.0.1 trsvcid=44625 subnqn=nqn.2016-06.io.spdk:mooncake-recovery-3752384
I20261014 18:31:08.519698 3752442 spdk_wrapper.cpp:719] nof_qpair_async_connect_start ctrlr=0x5e123aaab620 nsid=1 qpair=0x7040e8014b00 pool=0x7ffd21ae3160 worker_idx=0
I20261014 18:31:08.520373 3752443 spdk_wrapper.cpp:719] nof_qpair_async_connect_start ctrlr=0x5e123aaab620 nsid=2 qpair=0x7040f0016db0 pool=0x7ffd21ae3160 worker_idx=1
PASS controller_recovery

These results cover 92b53949. The subsequent merge with upstream has not yet been revalidated.

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 discussions, implementation, code review, and test validation. The human submitter is responsible for understanding and reviewing 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>

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]: NoF keeps reusing failed qpairs and cannot automatically recover I/O after target recovery

1 participant