Skip to content

[Bugfix][Store] Interrupt in-flight snapshot children during shutdown - #3942

Open
waizuichougou wants to merge 1 commit into
kvcache-ai:mainfrom
waizuichougou:fix/snapshot-child-shutdown
Open

waizuichougou wants to merge 1 commit into
kvcache-ai:mainfrom
waizuichougou:fix/snapshot-child-shutdown

Conversation

@waizuichougou

@waizuichougou waizuichougou commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Description

MasterSnapshotManager::Stop() stopped future snapshot scheduling, but it did
not interrupt a snapshot child that was already running. The manager destructor
then joined the snapshot thread while that thread remained in
WaitForSnapshotChild(). A standalone master shutting down during a slow or
stalled periodic snapshot could therefore block until the configured child
timeout (five minutes by default).

This change makes the child wait responsive to Stop() and bounds child
termination:

  • replace the fixed two-second polling sleep with the manager's existing
    condition variable, preserving the normal polling interval while allowing
    shutdown to wake it immediately;
  • terminate and reap an in-flight snapshot child when the manager stops;
  • poll waitpid() during the existing five-second SIGTERM grace period so a
    responsive child is reaped promptly, then retain SIGKILL as the fallback;
  • restore the default SIGINT/SIGTERM dispositions in the forked child. The
    standalone master installs handlers that only update parent-process state;
    inheriting those handlers otherwise prevents the child from terminating on
    SIGTERM.

Trigger

  1. Run a standalone master with periodic snapshots enabled.
  2. Let a snapshot start and make the configured snapshot backend slow or
    unavailable so the child remains in progress.
  3. stop the master (for example, during a pod termination or process restart).

Before this change, MasterService::~MasterService() called Stop() and later
joined the snapshot thread, but the in-flight child wait continued until the
snapshot completed or snapshot_child_timeout_seconds elapsed. With the
default configuration, that can delay shutdown by up to 300 seconds.

Behavior

Path Before After
Normal snapshot Child status polled every 2 seconds Same 2-second polling interval
Shutdown with no snapshot in flight Snapshot scheduler wakes immediately Unchanged
Shutdown with a responsive child Wait until completion or child timeout Wake immediately, send SIGTERM, reap promptly
Unresponsive child SIGTERM, fixed 5-second sleep, then SIGKILL SIGTERM, poll for up to 5 seconds, then SIGKILL and reap

The payload and publication order are unchanged: metadata, segments, task
state, and the manifest are uploaded before the snapshot descriptor/latest
marker is published. Cancelling before publication can leave unreferenced
partial objects, as the existing timeout path already can, but restore does not
select them. Cancelling after publication is safe because all snapshot payloads
and the manifest have already completed.

No steady-state transfer, allocation, snapshot serialization, or restore code
is changed. This path is used by standalone periodic snapshots; HA batch-OpLog
snapshot ownership is not changed.

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?

The regression test forks a real child, runs the production wait path on a
separate thread, calls Stop(), and verifies both bounded completion and that
the child has already been reaped. The existing timeout test verifies the shared
termination helper, and the auto-snapshot test verifies that a normal child can
still serialize, publish, exit, and be reaped.

Test commands:

cmake --build build --target snapshot_child_process_test -j 4

build/mooncake-store/tests/snapshot_child_process_test

build/mooncake-store/tests/snapshot_child_process_test \
  --gtest_filter='SnapshotChildProcessTest.StopInterruptsRunningSnapshotChild' \
  --gtest_repeat=20 --gtest_break_on_failure

pre-commit run --files \
  mooncake-store/include/master_snapshot_manager.h \
  mooncake-store/src/master_snapshot_manager.cpp \
  mooncake-store/tests/ha/snapshot/snapshot_child_process_test.cpp

Test results:

  • Unit tests pass: all 26 snapshot_child_process_test tests passed.
  • Regression stress passes: 20/20 repeated stop/terminate/reap runs passed.
  • Integration tests pass (not applicable; the affected lifecycle is covered
    with real fork, signals, and waitpid in the unit test).
  • Manual testing done: responsive children were reaped during the SIGTERM
    grace period; the SIGKILL fallback was also exercised with a child that
    ignored SIGTERM.
  • PR-scoped pre-commit passes, including clang-format 20 and codespell.

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 (not applicable; no user-facing API or
    configuration changes)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: not applicable (well below 500 LOC)

AI Assistance Disclosure

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

Codex assisted with this contribution.

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.

1 participant