[tensorcopy]

Description

Before this change, at base commit 19e0ab2, the HA master installed no SIGINT or SIGTERM handler. The only handlers were in the non-HA branch of main() (mooncake-store/src/master.cpp:1687-1692 at that commit; the current non-HA handlers are at master.cpp:1709), and MasterServiceSupervisor installed none. A SIGTERM therefore killed a serving leader at once: the RPC listener was not stopped, in-flight handlers were not finished, and the batch OpLog writer was not stopped.

This PR is the first half of the plan in #4710. It adds signal handling and a bounded, ordered drain. It makes no deliberate release of the leadership lease.

What changes:

• main() arms a shutdown watchdog and installs SIGINT/SIGTERM handlers in the HA branch before supervisor.Start(). If either fails, the master refuses to start. The handlers use sigaction with SA_ONSTACK, because the etcd and k8s HA backends are Go c-shared libraries in the same process. The handler does one lock-free atomic exchange on a sticky flag. A second signal exits with 128 + signo.
• The supervisor checks the request in every phase (standby, candidate, just acquired, producer-view claim, promotion, warmup, restore, serve preflight, monitor start, serve startup, readiness publication and serving) and, on its failure paths, before each lease release, service teardown and standby re-entry. After a request is seen, it starts no release, no standby re-entry and no catch-up cursor preservation. A release that an ordinary failure path already made is allowed to finish; its result is logged and not repeated.
• While serving, a small thread polls the request every 100 ms and fences the session through ServingStateGate with the same steps the leadership monitor uses: readiness off, leader-label removal requested, server.stop(). The serving exit tests the request, not which fence won the gate, so a request that races a lease loss still ends in a shutdown.
• The exit keeps today's serving-exit order and adds two steps: listener stop, RPC worker-pool join, leadership monitor stop, OpLog writer stop, serving state off, then admin HTTP server stop and standby replication stop. In earlier phases it stops whichever of the worker pool, monitor and writer already exist. The process then calls _exit(0).
• The watchdog bounds the whole shutdown at 15 s from the first observation of the request and ends the process with _exit(1) on expiry. The deadline starts once and nothing resets it. The watchdog only sleep-polls atomics; it takes no locks and does no I/O. Logs are flushed only on the supervisor or main thread, and only while the watchdog runs.
• Catch-up, promotion, restore and an already-started release cannot be interrupted. A request during one of them is handled when the call returns, within the 15 s bound.

Exit statuses: 0 = the selected drain completed; 130/143 = second SIGINT/SIGTERM; 1 = watchdog expiry; 255 = a request seen only after a fatal backend error made Start() return -1. Status 0 means the selected drain completed, not that every operation became durable or every response reached its client.

Why no lease release here, and why _exit. Releasing the lease is a separate, small change that is easy to review and to roll back on its own; the follow-up adds one ReleaseLeadership call at the single point marked by FinishShutdown, after the admin drain and immediately before the exit. Until then the process must exit without unwinding the supervisor loop: the loop creates the leader coordinator per iteration (master_service_supervisor.cpp:455), and ~EtcdLeaderCoordinator (etcd_leader_coordinator.cpp:155) revokes any held lease through ShutdownKeepAliveThread (:694). A normal return would revoke the lease as a side effect, and would also spend the shutdown budget destroying the index while the keepalive still runs.

Relation to the RFC text. This resolves RFC Q5 with terminal exit inside the supervisor loop, after explicit cleanup and before stack unwinding. It replaces the return-through-Start wording in RFC 2.4. PR A includes the watchdog and leaves the lease to TTL expiry; PR B adds the explicit release before the same exit. The watchdog has no disarm because every completed shutdown ends the process.

What this PR does not change. Failover after a SIGTERM still costs the lease TTL (5 s by default) plus the successor's warmup, as after a kill today. This PR makes the exit orderly and bounded; it does not make failover faster. In the worst case a planned restart takes up to 15 s longer than today, while the drain runs; a second signal ends it at once. The service's housekeeping threads, the metadata HTTP server and static destructors are not drained before _exit, and the Kubernetes leader-label change is only scheduled, so _exit can come before the label clears.

Answers to the questions on #4710:

1. Data-plane writes. On SIGTERM the leader stops its RPC listener, then joins the RPC worker pool (master_service_supervisor.cpp:980), so every master RPC handler admitted before the stop finishes before anything else is torn down; a handler that queues an OpLog entry still finds the writer accepting. Then the OpLog writer stops (:984): it rejects new work (mooncake-store/src/ha/oplog/ordered_oplog_writer.cpp:316-317), a healthy writer commits the entries already in its queue (:521-527), and the stop joins both the writer and the callback threads (:706-716); a writer already in retry leaves its batch (:563-565), as on lease loss today. On a planned shutdown with a healthy lease, PR A keeps renewal running through the drain and makes no deliberate revoke. The successor therefore waits for lease expiry after process exit, then performs the existing catch-up, warmup, and restore. Independent lease loss, external revocation, or a release already started by an ordinary failure path retains today's failure limits. The producer-view comparison fences OpLog batch commits after the successor's claim (master_service_supervisor.cpp:368-390, called at :591; compared at mooncake-store/src/ha/oplog/oplog_batch_storage.cpp:376-381, and only with --enable_oplog and a nonempty cluster_id); it does not fence direct client/store data transfers, arbitrary local work, or the plain admin quota Put. Admitted admin work drains before the planned terminal exit. Ordinary lease-loss admin races remain outside this change. Deleting master_view is not itself a write fence; the producer view is. A client can still lose the response to a request that completed during the drain.
2. Order. Yes. This PR is signal handling plus the drain, without the revoke. The follow-up adds the single release, the renewal-guard stop, a failover-latency e2e and the deployment-guide notes.

RFC: #4710.

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?

1. shutdown_request_test (new) runs all cases that change process signal or watchdog state in child processes; the two fake-sigaction cases run in the parent: the first SIGTERM/SIGINT sets the flag, the child keeps running and writes nothing to stderr; a second signal exits with 128 + signo; the handlers are installed with SA_ONSTACK (through a test seam and with a real install); install failure returns false with errno; the watchdog does not fire without a request; it ends a blocked cleanup within the deadline, also when stderr is a full pipe, when the exiting thread is blocked in a glog flush, and when a request was latched without a signal; and a flush that blocks late in a shutdown gets no new deadline.
2. high_availability_test gains child-process cases against a real etcd: a serving leader exits 0 on SIGTERM and its master_view key is still present right after exit, then expires by TTL; a standby exits 0 and leaves the leader's key unchanged; SIGTERM during the initial warmup exits 0 before any readiness publication; an OpLog-enabled leader's mounted segment is in the durable log after exit and a successor's restore counts it (a restored segment stays a placeholder until its client remounts, so the successor's log line is the observable, not its segment list); SIGTERM racing a lease revoke exits 0; the constructor predicate drives the same shutdown without a signal. With -DMOONCAKE_ENABLE_TEST_FAILPOINTS=ON, more cases pause the writer, the serve startup, the MountSegment handler and the step after a lease release at file-handshake fail points. They show that the exit waits for an admitted handler and for the writer, stops the writer before the listener starts, never reaches serving after the request, keeps the result of a release that was already made, ends by the watchdog when an injected request blocks in the drain, and that a second SIGTERM exits 143.
3. Before the fix, the two SIGTERM regression cases fail on main (ServingLeaderSigtermDrainsAndKeepsLease and OplogLeaderCommitsQueuedRecordsBeforeExit), with the same test file and a child that starts as main() does today: child was terminated by signal 15. With this change both pass.
4. Before/after with the real binary: an HA mooncake_master that reached serving received SIGTERM. On main it returned -15 after 0.01 s; with this PR it returned 0 after 0.02 s and 0.11 s in two runs.

Test commands (reproduction commands against an already-running local etcd on the default port 2379; the recorded runs used another local port, so these are not an exact command transcript):

cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DBUILD_UNIT_TESTS=ON -DBUILD_EXAMPLES=OFF -DBUILD_BENCHMARK=OFF \
  -DWITH_STORE=ON -DWITH_STORE_RUST=OFF -DUSE_ETCD=ON -DSTORE_USE_ETCD=ON
cmake --build build --target shutdown_request_test high_availability_test mooncake_master
./build/mooncake-store/tests/shutdown_request_test
./build/mooncake-store/tests/high_availability_test --etcd_endpoints=127.0.0.1:2379
# fail-point cases
cmake -S . -B build-fp -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DBUILD_UNIT_TESTS=ON -DBUILD_EXAMPLES=OFF -DBUILD_BENCHMARK=OFF \
  -DWITH_STORE=ON -DWITH_STORE_RUST=OFF -DUSE_ETCD=ON -DSTORE_USE_ETCD=ON \
  -DMOONCAKE_ENABLE_TEST_FAILPOINTS=ON
cmake --build build-fp --target high_availability_test
./build-fp/mooncake-store/tests/high_availability_test --etcd_endpoints=127.0.0.1:2379
git diff --name-only origin/main...HEAD | xargs pre-commit run --files

Test results:

• Unit tests pass: shutdown_request_test, 15 of 15 in the default build and in the fail-point build
• Integration tests pass (if applicable): high_availability_test against etcd 3.5.21: default build 26 passed and 6 fail-point cases skipped; fail-point build 32 passed, none skipped. Measured SIGTERM-to-exit time for the serving leader in the test: 40 ms.
• Manual testing done: the timing-only run returned -15 after 0.01 s on base commit 19e0ab2 and 0 after 0.02 s with this PR. A separate key-check run returned -15 after 0.01 s on the base and 0 after 0.11 s with this PR; in that run, each master's master_view key was present immediately after exit and absent 8 s later (TTL 5 s).

Who ran each step and where: the implementation and every test execution above (unit, integration and manual runs) were performed by an AI coding assistant (Claude Sonnet 5.5) in a local development environment against a local etcd 3.5.21. No result in this description comes from a test run by a human.

Checklist

• I have performed a self-review of my own code
• C/C++ changes were formatted with git clang-format (clang-format 20.1.8); ./scripts/code_format.sh --check --changed-lines --base origin/main passed after committing. Transcript:

./scripts/code_format.sh --check --changed-lines --base origin/main (exit 0)

$ ./scripts/code_format.sh --check --changed-lines --base origin/main
[INFO] Mooncake Code Format Script
[INFO] Using clang-format version 20.1.8
[INFO] Formatting only C/C++ line ranges changed against origin/main

- [ ] I have run pre-commit on the files changed in this PR and all hooks pass (every hook passes except cmake-format, which also rewrites existing, unrelated lines of `mooncake-store/tests/CMakeLists.txt`; those lines are left as they are) - [ ] I have updated the documentation (if applicable) — deployment notes come with the follow-up - [x] I have added tests to prove my changes are effective - [x] For changes >500 LOC: I have filed an RFC issue (#4710)

AI Assistance Disclosure

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

AI tools assisted the design/review and implementation; the human submitter is responsible for reviewing every changed line and validating the reported results. Tools used: the implementation and all test executions were done by an AI coding assistant (Claude Sonnet 5.5); the design was written by an AI coding assistant (Claude Opus 5.5); the design and implementation reviews were done by an AI assistant (an OpenAI GPT model).

[tensorcopy]

The single failure in CTest unit tests (Python 3.12) is TcpWriteVisibilityTest.ConcurrentEndpointRefreshNeverCombinesHostAndPortSnapshots (mooncake-transfer-engine/tests/tcp_write_visibility_test.cpp:4253): endpoint_b.acceptedCount() was 0 where 1 was expected — a timing-sensitive assertion in the TCP endpoint-refresh path.

This PR does not touch the transfer engine (the diff is confined to mooncake-store). The two test binaries it adds or changes passed in the same job (shutdown_request_test, high_availability_test), and the failing case finished before the HA suite even started, so it is not contention from the new tests; the other 52 cases of that suite passed on the same runner, and every other completed check in this run is green.

This looks like a runner-load flake. Could you re-run the failed job?

[he-yufeng]

Reviewed as the follow-through on the #4710 discussion. The part-1 shape lands exactly what we agreed: drain without a deliberate release, lease left to TTL, and the terminal in-loop exit so ~EtcdLeaderCoordinator never gets to revoke on the way out.

What I verified while reading:

• The signal handler is async-signal-safe (lock-free exchange, _exit(128+signo) on repeat, no logging or allocation), SA_ONSTACK | SA_RESTART for the Go c-shared backends is right, and the watchdog's sleep-poll on two atomics cannot be stopped by a stalled cleanup or a held glog mutex. ExitAfterShutdown only flushing when the watchdog exists is a nice touch.
• release_term gets the ordering right: a shutdown observed before the term's release skips it, one observed after keeps the result, and FinishShutdown reports accurately for both. The phase coverage is exhaustive across starting/standby/candidate/acquired/claim/promotion/warmup/restore/preflight/serve, and every shutdown_exit path is the no-unwind terminal exit.
• The serving trigger goes through the same ServingStateGate fence as a leadership loss, which is the answer you gave on the data-plane question made concrete.

Two non-blocking observations:

1. StartShutdownWatchdog's two-step check-then-store is only safe because callers are serial; the header says so, which is fine, but a one-line comment at the call site in master.cpp would keep a future second caller honest.
2. In ServingShutdownTrigger::Run, on_shutdown_ runs with the mutex unlocked, which is correct; the destructor's Stop() joining while on_shutdown_ is inside server.stop() is bounded by the watchdog anyway, so no issue, but the interlock is subtle enough to deserve a comment.

CI note: the red CTest leg is tcp_write_visibility_test timing out in the TCP transport's own lane (segment-descriptor retrieval failures), unrelated to this change; a rerun should clear it. The HA failpoint tests are the real signal here, and the sub-process SIGTERM assertions (exit 0, lease key present right after exit, gone by TTL) are exactly the proof shape this PR needed.

[tensorcopy]

Thanks for the thorough review, and for verifying the pieces that matter (the handler safety, the release_term ordering, and the gate fence).

Both suggestions are in 3e810a7 (comment-only): the watchdog arm in main() now states the serial, one-shot caller contract, and ServingShutdownTrigger::Run documents why on_shutdown_ runs with the mutex unlocked and that the watchdog, not the interlock, bounds Stop()'s join if the callback blocks.

The push retriggers CI on the new head; the rerun request on the previous run can be dropped if this one comes back green.

[he-yufeng]

Both read correctly on 3e810a7, and CI is green across the board on this head (the earlier CTest red was the unrelated TcpWriteVisibilityTest flake, cleared as expected).

The master.cpp note pins the watchdog's serial one-shot caller contract at the only call site, and the supervisor comment now states the interlock's actual role: on_shutdown_ runs unlocked so Stop() can signal and join, with the watchdog bounding the process if the callback blocks. That is the right level of documentation for both — the invariants live next to the code that relies on them.

From my side this is ready for the maintainers with merge rights. Part 2 (explicit ReleaseLeadership after drain) can build on this once it lands.

[Icedcoco]

LGTM