[GH-ISSUE #4710] [RFC]: [Store] HA: graceful leader shutdown on SIGTERM/SIGINT (release the lease before exit) #1199

New issue

Open

opened 2026-10-29 21:23:52 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented 2026-10-29 21:23:52 +00:00

Owner

Copy link

Originally created by @tensorcopy on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4710

Changes proposed

Summary

In HA mode, SIGTERM ends the master process with no cleanup. When that master is the leader, its etcd lease stays alive until it expires, and only then can a standby acquire leadership and begin its warmup. A planned restart (pod delete, rollout) therefore costs as much as a crash.

This RFC proposes that the HA master handle SIGTERM and SIGINT. It stops taking new work, waits for admitted admin handlers before the release and drains the healthy OpLog writer's ready queue (retaining today's failure behavior), revokes the leadership lease exactly once, and exits within a fixed deadline. A planned leader restart then costs about one warmup instead of lease expiry plus warmup. There are no data-plane changes, no new persistent state and no wire changes. The work is split into two PRs of about 520 and 440 lines (Section 9).

All code references are to main at 3aa0f716. Related: #3808 (HA roadmap), #4559, #4666, #4706.

#4601, merged after 3aa0f716, runs RPC handlers on a worker pool that the serving exit drains after the server future returns and before step 2 of 2.3; the ordered exit keeps that drain. It shifts later line numbers in master_service_supervisor.cpp and master.cpp.

1. Current behavior

• main() installs SIGINT/SIGTERM handlers only in the non-HA branch (mooncake-store/src/master.cpp:1653-1658). The HA branch builds a MasterServiceSupervisor and returns supervisor.Start() (master.cpp:1612-1618). In mooncake-store/src these are the only SIGINT/SIGTERM handlers on the master's path, so in HA mode SIGTERM takes its default action and ends the process without running destructors. The etcd backend's Go c-shared library does not install a SIGTERM handler either: Go installs handlers for asynchronous signals in that mode only when signal.Notify is called, and the wrapper does not call it.
• The supervisor loop (mooncake-store/src/ha/leadership/master_service_supervisor.cpp:330) has no stop condition. It leaves only through return -1: on a fatal backend error, or when the RPC listener fails to start (:667-680).
• The etcd coordinator grants the lease with DEFAULT_MASTER_VIEW_LEASE_TTL_SEC (5 s, mooncake-store/include/types.h:101) and creates the master_view key under that lease (mooncake-store/src/ha/leadership/backends/etcd/etcd_leader_coordinator.cpp:216-228), so a revoke deletes the key. For an acquired leadership session, normal cleanup revokes through ReleaseLeadership (etcd_leader_coordinator.cpp:567-613) or the destructor (:155, :653-704). SIGTERM's default termination does not run those cleanup paths.
• A new leader keeps renewing for one full lease TTL in warmup before it serves (WarmupLeadership, master_service_supervisor.cpp:189-217; the deadline is set at :191).
• A standby waits on an etcd watch of the master_view key (etcd_leader_coordinator.cpp:436-531), in 1 s slices (master_service_supervisor.cpp:31, :392-393, :409-410). It sees a delete at once (within one 1 s slice if the old leader was still warming up).

With lease TTL T, the two cases compare like this:

	Leader killed (today)	Leader releases the lease (proposed)
Successor can acquire	when the old lease expires, up to T after the kill	when the old leader revokes, at the end of its cleanup
Then	producer-view claim and catch-up/promotion, warmup T, then restore and serving startup	producer-view claim and catch-up/promotion, warmup T, then restore and serving startup

The saving is up to T per planned leader restart. It grows with the TTL, which #4706 makes configurable up to 60 s.

2. Proposed behavior

2.1 Signal intake

• main() installs SIGINT and SIGTERM handlers in the HA branch, before supervisor.Start(), so no lease can be acquired without them. If installation fails, the master refuses to start.
• The handler does one lock-free atomic exchange on a sticky "shutdown requested" flag (asserted lock-free at compile time). It does not log, lock or call a backend.
• The handlers are installed with sigaction and SA_ONSTACK. The etcd and k8s HA backends are Go c-shared libraries in the master process (mooncake-common/etcd/CMakeLists.txt:3, CMakeLists.txt:83), and Go requires SA_ONSTACK on any handler that non-Go code installs (os/signal docs). The std::signal calls in the non-HA branch (master.cpp:1657-1658) are not a model for this.
• A second signal calls _exit(128 + signo) (130 for SIGINT, 143 for SIGTERM), the convention the Transfer Engine watcher already follows (graceful_shutdown.cpp:151). This is the escape hatch for a shutdown that does not finish.
• The supervisor receives the request as a predicate (std::function<bool()>) that defaults to the process flag, so tests can inject their own.

The Transfer Engine already has a SIGTERM/SIGINT watcher that uses a self-pipe (mooncake-transfer-engine/src/graceful_shutdown.cpp:146-167, :181-211). The master's HA path does not construct a TransferEngine (in mooncake-store/src, only client_service.cpp does), and mooncake_master does not link transfer_engine (mooncake-store/src/CMakeLists.txt:712-725), so that watcher is not active here. Its design is a reference for open question 4.

2.2 Supervisor: a shutdown request is handled like a lease loss, then the loop exits

The loop already has a release path for every phase after acquisition, because each failure calls ReleaseLeadership (HandleLeadershipPhaseError, master_service_supervisor.cpp:179-187, and the LogLeadershipReleaseWarning call sites). The change makes each phase notice the request and then leaves the loop instead of retrying.

Phase	Today	With a shutdown request
Outer loop and coordinator creation (:330-338)	retries every 1 s on error	checked at the top of each outer iteration; exits with no lease held
Standby / candidate (:343-419)	waits on the view in 1 s slices	checks the request at the top of each iteration and before TryAcquireLeadership; exits with no lease held
Just acquired (:378-383)	proceeds to the claim	releases at once, without claim or promotion
Producer-view claim, catch-up and promotion (:428-458)	blocking calls	checked when each call returns, next to the existing renewal.Lost() check (:482)
Warmup (:494-516)	renews for T in 1 s sleeps	WarmupLeadership also checks the request each iteration and returns early
Restore (:556-597)	blocking	checked when it returns, next to renewal.Lost() (:586)
Serve preflight and startup (:599-705)		checked under the existing ServingStateGate (mooncake-store/include/ha/leadership/master_service_supervisor.h:14-45) before listener startup and local readiness activation; the backend publication stays outside the gate and uses a request check before the call
Serving (:725)	blocks on the server future	a small thread, started before async_start (:656) and joined before the serving objects are destroyed, polls the request and calls serving_state.RequestShutdown(...) with the same fence the leadership monitor uses (:632-639): readiness off, leader label cleared, state standby, server.stop()

The gate is sticky and both activation and the fence go through it (:656, :690), so a request that races readiness publication cannot leave the master serving. (PublishServiceReady at :688 runs outside the gate and can still write the ready address into master_view after the fence; a successful revoke deletes it after cleanup, and on forced exit before the revoke it can remain until TTL expiry.) If a lease-loss fence wins the gate first, the trigger's RequestShutdown returns false, so the serving exit tests the process flag, not which fence ran.

Catch-up, promotion and restore cannot be interrupted today. A request during one of them is handled when the call returns, and the deadline in 2.4 bounds the wait.

2.3 Ordered exit of a leader

On shutdown, a session that holds the lease runs this sequence once, on the supervisor thread. It is the full serving form; in earlier phases the exit helper skips the steps with nothing to act on:

1. The gate fences the session and stops the RPC listener (above).
2. The leadership monitor stops, as in today's serving exit (:728).
3. The batch OpLog writer stops (:729). A healthy writer commits its queued entries before its thread exits (mooncake-store/src/ha/oplog/ordered_oplog_writer.cpp:521-527); a writer already in retry abandons its batch (:563-564, :671-675), as it does on lease loss today.
4. DeactivateServingState clears serving state and the leader label (:235-240).
5. The admin HTTP server shuts down (mooncake-store/src/master_admin_service.cpp:328-337) before the revoke. Per the comment at master_admin_service.cpp:871-873, stop() drains the HTTP executor; the quota handlers are synchronous and run on that executor. This matters because an admitted admin request can still write to etcd without a lease check: the quota write routes (PUT and DEL, master_admin_service.cpp:1469-1478) reach EtcdTenantQuotaPolicyStore::Save, which is a plain EtcdHelper::Put (mooncake-store/src/tenant/quota_policy_store.cpp:431-444). Turning off readiness rejects new requests but does not finish admitted ones. PR A will test that an admitted quota write finishes before the revoke. If the stop path does not provide that guarantee, PR A must add the required wait before it can claim graceful release.
6. Immediately before the release, the renewal polling guard stops; the etcd keepalive continues until ReleaseLeadership cancels it. ReleaseLeadership runs exactly once, and its result is kept for the exit status. A second call is not a no-op: after the first call clears the keepalive state, it sends another RevokeLease for the same lease (etcd_leader_coordinator.cpp:574-612). The wrapper treats lease-not-found as success (etcd_wrapper.go:442-446), so the extra call only costs a round trip of up to 5 s inside the deadline, and a release after the lease already expired still counts as released.
7. With runtime-state updates already off, StopStandby ends standby replication (mooncake-store/src/ha/standby_controller.cpp:252-263) and the loop returns.

On a path where today's code releases and re-enters standby after a fence (for example the startup cancellation at :657-666), the shutdown check runs first, so those calls do not run on a shutdown.

Each session records any release attempt and its result, including an attempt already in progress when shutdown is requested. Shutdown consumes that result instead of releasing the same session again. Error exits check the request before service destruction and standby re-entry.

The successor fences the old producer through ClaimProducerView (master_service_supervisor.cpp:268-290) as it does today. The only new thing is that the lease is revoked instead of left to expire.

A shutdown exit does not re-enter standby and does not run the #4559 cursor preservation (return_to_standby, :460-480). That preservation restores an in-process baseline for the next standby start in the same process, and the process is about to exit.

2.4 Deadline

An independent watchdog thread bounds the whole shutdown. It is armed next to the handler installation and stays armed through the supervisor's shutdown and main()'s bounded cleanup after supervisor.Start() returns.

• It starts timing at the first observation of the request. The proposed default deadline D is 15 s.
• On expiry it calls _exit with a nonzero status. It writes nothing to stderr first, because a write to a stalled pipe could block and defeat the bound.
• It only sleep-polls two atomics. It takes no locks and does not log, so it cannot block in the cleanup it bounds.
• The second-signal handler and watchdog call _exit directly, without flushing or logging. Normal shutdown may call google::FlushLogFiles(google::GLOG_INFO) on the supervisor thread while the watchdog remains armed. Forced exit can lose buffered logs; preserving the exit bound takes priority.

If the watchdog fires before a confirmed revoke, the lease may remain until TTL expiry. If release has already succeeded, the successor need not wait for the old lease.

2.5 Exit status

Outcome	Status
Lease released (or none held) and cleanup finished	0
Release failed; the lease expires by TTL	nonzero
Watchdog expired	nonzero
Second signal	128 + signo (130 SIGINT, 143 SIGTERM)

A nonzero status means failure or deadline expiry; it does not by itself prove that the lease is still held.

3. Scope

• The HA master on the etcd backend: master.cpp (HA branch only), master_service_supervisor.{h,cpp}, and a small process-level shutdown utility.
• The k8s backend goes through the same supervisor code and its own ReleaseLeadership (mooncake-store/src/ha/leadership/backends/k8s/k8s_leader_coordinator.cpp:334-365). The tests in this RFC cover etcd only.
• The Redis coordinator cannot lead today: ReadCurrentView and TryAcquireLeadership both return UNAVAILABLE_IN_CURRENT_MODE (mooncake-store/src/ha/leadership/backends/redis/redis_leader_coordinator.cpp:140-150), so the supervisor exits at its first iteration. Nothing changes there.

4. Non-goals

• Data-plane changes. Clients, stores and the RPC protocol are unchanged.
• Index handoff across restarts. That is #4666's subject, for standalone masters; HA already keeps the index through the OpLog and the standby.
• Any change to the non-HA branch of main().
• Shortening the successor's warmup after a clean release. Warmup fences an old leader whose lease merely expired; skipping it after a clean release needs its own safety argument.
• Making catch-up, promotion or restore cancellable.
• The lease TTL value, which #4706 covers.
• Admin requests admitted before an ordinary lease loss (not a shutdown) can still reach the unfenced quota Put after the release. That window exists today and is out of scope here; it could be a follow-up.

5. Relationship to #4559

#4559 ("fix(store): keep HA leadership through large oplog catch-up") moved lease renewal to a side thread that starts at acquisition (LeadershipRenewalGuard, master_service_supervisor.cpp:92-154, started at :428), added renewal.Lost() checks after catch-up (:482) and after restore (:586), and keeps the promotion cursor for a retry in the same process (:460-480).

This design targets that structure directly:

• Lease renewal needs no change. The etcd keepalive thread keeps the lease alive through the whole shutdown; ReleaseLeadership stops it immediately before the single revoke (etcd_leader_coordinator.cpp:600-604, :612).
• The new checks sit next to the existing renewal.Lost() checks and reuse their exits.

6. Relationship to #4666

#4666 (shutdown handoff for a standalone master, by @Juhyun-Kim-Memphis) and this RFC are complementary. #4666 states that the handoff "cannot be enabled together with HA and does not change the HA path"; this RFC changes only the HA path. Their features do not overlap.

Both need a process-level SIGTERM path and a watchdog with a deadline: #4666's delivery plan lists "SIGTERM path, admission, watchdog" in A5. This RFC adds two parts that #4666 does not describe: an async-signal-safe request flag and a second-signal escape hatch. The fork flag table in the #4666 appendix (§E) lists --handoff_shutdown_deadline_sec with a default of 45; the appendix notes that upstream names and defaults are still open. To avoid two versions:

• Share: one small, mode-agnostic utility (request flag, second-signal exit, watchdog with the deadline as a parameter), placed outside ha/. Whichever PR lands first owns it and the other reuses it. If #4666's A5 lands first, this work adapts to it.
• Keep separate: the deadline value. A handoff write needs much longer than a lease release (the #4666 appendix, §A.3, reports 11.1-11.7 s for the write alone with the real binary at 25M keys), so each mode sets its own.
• Defer to #4666: everything about snapshot, quiesce, claim and restore, and any change to the standalone signal path at master.cpp:1653-1658.

One observation in the #4666 appendix (§F, item 5) affects this design: a graceful exit of an upstream standalone master with a 12M-object index took 21.9 s, mostly spent destroying the index. In the HA serving path the lease is released before the index is destroyed (master_service_supervisor.cpp:731 runs before :734), so teardown cannot delay failover. It can still run past D, which is open question 5.

7. Relationship to #4706

#4706 adds --ha_leader_lease_ttl_sec (2-60, default 5), carried from MasterConfig to the etcd coordinator's GrantLease and the session's lease_ttl. The two changes are independent in code: #4706 touches BuildHABackendSpec, the coordinator's acquire path and the flag sections of master.cpp, while this work touches the loop body, Start() and the HA branch of main() (master.cpp:1612-1618). Whichever lands second needs at most a small textual rebase. If #4706 lands first, the deployment notes in the second PR below refer to the flag instead of the constant.

8. Deployment notes

Let T be the lease TTL, D the shutdown deadline (15 s proposed), P the time of any preStop hook, and G the pod's terminationGracePeriodSeconds. On Kubernetes, G defaults to 30 s, the countdown includes the preStop hook, and the kubelet sends SIGKILL when it expires (Pod Lifecycle).

• Keep G greater than P + D plus a margin of a few seconds. With the defaults (G = 30, D = 15) that leaves room for a preStop of up to about 10 s. The master itself needs no preStop hook.
• T does not enter that inequality, because a clean release does not wait for T. T matters on fallback paths only if the old lease has not already been revoked: after forced exit the successor may wait for its remaining TTL.
• If G is too small, the kubelet kills the master during cleanup and the restart becomes today's crash failover. That is slow but safe.
• If cleanup hangs, the lease stays alive until the watchdog fires, because the keepalive thread keeps running. The worst case for a planned restart is therefore up to D longer than today. The second signal shortens it.
• The admin server stops before the revoke, so /health and metrics stop answering during the shutdown.
• No format or wire change: old and new binaries can run in the same cluster. An old binary simply keeps today's behavior.

9. Delivery plan

PR	Content	Size (estimate)	Useful alone?
A	Signal intake (2.1), supervisor handling for every phase (2.2), ordered exit with admin drain and a single release (2.3), exit status; tests that run the supervisor in a child process against a real etcd, as high_availability_test already does	~520 lines, about half tests	Yes. It fixes the common case, a serving leader on SIGTERM
B	Watchdog and deadline (2.4), its subprocess tests, a manual two-master e2e script against a local etcd, and a deployment-guide section on terminationGracePeriodSeconds	~440 lines, about half test script	Depends on A

Without B, a shutdown stuck in catch-up or restore waits until the phase returns, the second signal, or the kubelet's SIGKILL at G. That is why B follows A directly. If maintainers prefer, the two can be reviewed together or merged as one.

10. Test plan

PR A:

1. A unit test of the shutdown utility in a child process: the first SIGTERM sets the flag and the process keeps running; a second signal exits with 128 + signo.
2. Extensions of mooncake-store/tests/ha/leadership/high_availability_test.cpp, which already runs a supervisor in a child process against a real etcd (SupervisorChildProcess, :179-234; RunSupervisorChild, :264-301). Today that test ends the child only with SIGKILL (:224). New cases:
   • a serving leader receives SIGTERM: exit 0 within a bound, and the master_view key is gone right after exit, well before T;
   • a second child waiting as standby acquires right after that revoke;
   • a standby receives SIGTERM: exit 0, and the leader's key is unchanged;
   • SIGTERM during warmup: exit 0 and the key is released;
   • a release that fails: nonzero exit, and the key remains until TTL;
   • a request that races a lease loss or the serving startup: the process flag, not the fence winner, decides the exit;
   • a second SIGTERM: exit status 143.

The existing child harness runs with the OpLog disabled (high_availability_test.cpp:297). PR A adds an OpLog-enabled serving-leader case, with a nonempty cluster_id and actual queued records, verifying that the healthy writer commits those records before release and that the successor can replay them.

PR B:

1. Watchdog tests in a child process: a blocked cleanup is ended within D plus a small slack; a disarmed watchdog never fires; expiry still ends the process when stderr is a full pipe, and while the supervisor thread is blocked inside a log flush.
2. A manual e2e script in the style of mooncake-store/tests/ha/leadership/etcd_leader_hang_e2e.sh: two real mooncake_master processes and a local etcd. SIGTERM to the leader must exit 0 and let the standby acquire before the old lease's TTL would have run out; a SIGKILL control must wait for the lease's remaining TTL.
3. A docs build for the deployment-guide change.

11. Rollout risks

• SIGTERM no longer ends an HA master at once; it can take up to D. Scripts that kill a master and immediately reuse its ports may need to wait for exit.
• A stuck cleanup keeps the lease alive for up to D (Section 8).
• A clean shutdown now exits 0 instead of dying by signal. Supervisors that read the exit status will see the change.
• The k8s backend's release and label clearing are not covered by the tests in Section 10.
• On k8s, the leader label is cleared asynchronously (leader_label_reconciler.h:61-83); a shutdown exit can return before the clear is applied. PR A either waits for it or documents it.

12. Questions for maintainers

1. Where should the shared shutdown utility live, and should #4666's A5 reuse it (Section 6)? We propose a mode-agnostic header and source under mooncake-store/, not under ha/.
2. Should D be a flag or a constant? If a flag, should HA and the handoff share one flag with mode-specific defaults, or keep separate flags?
3. Is 128 + signo for the second signal acceptable (130 for SIGINT, 143 for SIGTERM, as the Transfer Engine watcher does at graceful_shutdown.cpp:151), or do you prefer one fixed status?
4. To wake the serving fence, is a polling thread (100 ms or less) acceptable, or do you prefer a self-pipe like the Transfer Engine watcher? (A sigwait thread copied from real_client.cpp:812-857 is not sufficient here: the Go runtime's threads start before main() can block anything, and the Go docs say non-Go code should not change the signal mask on Go-created threads.)
5. To avoid spending the shutdown budget destroying a large index, should the HA process call _exit(0) after the ordered shutdown steps complete and release succeeds (or no lease is held), instead of destroying the index? Normal log flushing would run while the watchdog remains armed. Watchdog expiry means "shutdown did not finish by the deadline"; it does not prove that the lease remains held. The alternative is normal destruction with a larger D.
6. For tests: are child-process tests against a real etcd enough, or would you like an injectable coordinator for deterministic phase-race tests? The injection point does not exist today and would add about 50 lines. (high_availability_test.cpp already has a FakeLeaderCoordinator at :111; it is not injectable into the supervisor.)

AI assistance disclosure

• No AI tools were used
• AI tools were used

An AI coding assistant (Claude, by Anthropic) helped read the code and draft this RFC. A human author checked every code reference against main at 3aa0f716 and is responsible for the design and, in each PR, for every changed line.

Before submitting a new issue...

• Make sure you already searched for relevant issues and read the documentation

Originally created by @tensorcopy on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4710 ### Changes proposed ### Summary In HA mode, SIGTERM ends the master process with no cleanup. When that master is the leader, its etcd lease stays alive until it expires, and only then can a standby acquire leadership and begin its warmup. A planned restart (pod delete, rollout) therefore costs as much as a crash. This RFC proposes that the HA master handle SIGTERM and SIGINT. It stops taking new work, waits for admitted admin handlers before the release and drains the healthy OpLog writer's ready queue (retaining today's failure behavior), revokes the leadership lease exactly once, and exits within a fixed deadline. A planned leader restart then costs about one warmup instead of lease expiry plus warmup. There are no data-plane changes, no new persistent state and no wire changes. The work is split into two PRs of about 520 and 440 lines (Section 9). All code references are to `main` at `3aa0f716`. Related: #3808 (HA roadmap), #4559, #4666, #4706. #4601, merged after `3aa0f716`, runs RPC handlers on a worker pool that the serving exit drains after the server future returns and before step 2 of 2.3; the ordered exit keeps that drain. It shifts later line numbers in `master_service_supervisor.cpp` and `master.cpp`. ### 1. Current behavior - `main()` installs SIGINT/SIGTERM handlers only in the non-HA branch (`mooncake-store/src/master.cpp:1653-1658`). The HA branch builds a `MasterServiceSupervisor` and returns `supervisor.Start()` (`master.cpp:1612-1618`). In `mooncake-store/src` these are the only SIGINT/SIGTERM handlers on the master's path, so in HA mode SIGTERM takes its default action and ends the process without running destructors. The etcd backend's Go `c-shared` library does not install a SIGTERM handler either: Go installs handlers for asynchronous signals in that mode only when `signal.Notify` is called, and the wrapper does not call it. - The supervisor loop (`mooncake-store/src/ha/leadership/master_service_supervisor.cpp:330`) has no stop condition. It leaves only through `return -1`: on a fatal backend error, or when the RPC listener fails to start (`:667-680`). - The etcd coordinator grants the lease with `DEFAULT_MASTER_VIEW_LEASE_TTL_SEC` (5 s, `mooncake-store/include/types.h:101`) and creates the `master_view` key under that lease (`mooncake-store/src/ha/leadership/backends/etcd/etcd_leader_coordinator.cpp:216-228`), so a revoke deletes the key. For an acquired leadership session, normal cleanup revokes through `ReleaseLeadership` (`etcd_leader_coordinator.cpp:567-613`) or the destructor (`:155`, `:653-704`). SIGTERM's default termination does not run those cleanup paths. - A new leader keeps renewing for one full lease TTL in warmup before it serves (`WarmupLeadership`, `master_service_supervisor.cpp:189-217`; the deadline is set at `:191`). - A standby waits on an etcd watch of the `master_view` key (`etcd_leader_coordinator.cpp:436-531`), in 1 s slices (`master_service_supervisor.cpp:31`, `:392-393`, `:409-410`). It sees a delete at once (within one 1 s slice if the old leader was still warming up). With lease TTL `T`, the two cases compare like this: | | Leader killed (today) | Leader releases the lease (proposed) | | -- | -- | -- | | Successor can acquire | when the old lease expires, up to `T` after the kill | when the old leader revokes, at the end of its cleanup | | Then | producer-view claim and catch-up/promotion, warmup `T`, then restore and serving startup | producer-view claim and catch-up/promotion, warmup `T`, then restore and serving startup | The saving is up to `T` per planned leader restart. It grows with the TTL, which #4706 makes configurable up to 60 s. ### 2. Proposed behavior #### 2.1 Signal intake - `main()` installs SIGINT and SIGTERM handlers in the HA branch, before `supervisor.Start()`, so no lease can be acquired without them. If installation fails, the master refuses to start. - The handler does one lock-free atomic exchange on a sticky "shutdown requested" flag (asserted lock-free at compile time). It does not log, lock or call a backend. - The handlers are installed with `sigaction` and `SA_ONSTACK`. The etcd and k8s HA backends are Go `c-shared` libraries in the master process (`mooncake-common/etcd/CMakeLists.txt:3`, `CMakeLists.txt:83`), and Go requires `SA_ONSTACK` on any handler that non-Go code installs ([os/signal docs](https://pkg.go.dev/os/signal#hdr-Non_Go_programs_that_call_Go_code)). The `std::signal` calls in the non-HA branch (`master.cpp:1657-1658`) are not a model for this. - A second signal calls `_exit(128 + signo)` (130 for SIGINT, 143 for SIGTERM), the convention the Transfer Engine watcher already follows (`graceful_shutdown.cpp:151`). This is the escape hatch for a shutdown that does not finish. - The supervisor receives the request as a predicate (`std::function<bool()>`) that defaults to the process flag, so tests can inject their own. The Transfer Engine already has a SIGTERM/SIGINT watcher that uses a self-pipe (`mooncake-transfer-engine/src/graceful_shutdown.cpp:146-167`, `:181-211`). The master's HA path does not construct a `TransferEngine` (in `mooncake-store/src`, only `client_service.cpp` does), and `mooncake_master` does not link `transfer_engine` (`mooncake-store/src/CMakeLists.txt:712-725`), so that watcher is not active here. Its design is a reference for open question 4. #### 2.2 Supervisor: a shutdown request is handled like a lease loss, then the loop exits The loop already has a release path for every phase after acquisition, because each failure calls `ReleaseLeadership` (`HandleLeadershipPhaseError`, `master_service_supervisor.cpp:179-187`, and the `LogLeadershipReleaseWarning` call sites). The change makes each phase notice the request and then leaves the loop instead of retrying. | Phase | Today | With a shutdown request | | -- | -- | -- | | Outer loop and coordinator creation (`:330-338`) | retries every 1 s on error | checked at the top of each outer iteration; exits with no lease held | | Standby / candidate (`:343-419`) | waits on the view in 1 s slices | checks the request at the top of each iteration and before `TryAcquireLeadership`; exits with no lease held | | Just acquired (`:378-383`) | proceeds to the claim | releases at once, without claim or promotion | | Producer-view claim, catch-up and promotion (`:428-458`) | blocking calls | checked when each call returns, next to the existing `renewal.Lost()` check (`:482`) | | Warmup (`:494-516`) | renews for `T` in 1 s sleeps | `WarmupLeadership` also checks the request each iteration and returns early | | Restore (`:556-597`) | blocking | checked when it returns, next to `renewal.Lost()` (`:586`) | | Serve preflight and startup (`:599-705`) | | checked under the existing `ServingStateGate` (`mooncake-store/include/ha/leadership/master_service_supervisor.h:14-45`) before listener startup and local readiness activation; the backend publication stays outside the gate and uses a request check before the call | | Serving (`:725`) | blocks on the server future | a small thread, started before `async_start` (`:656`) and joined before the serving objects are destroyed, polls the request and calls `serving_state.RequestShutdown(...)` with the same fence the leadership monitor uses (`:632-639`): readiness off, leader label cleared, state standby, `server.stop()` | The gate is sticky and both activation and the fence go through it (`:656`, `:690`), so a request that races readiness publication cannot leave the master serving. (`PublishServiceReady` at `:688` runs outside the gate and can still write the ready address into `master_view` after the fence; a successful revoke deletes it after cleanup, and on forced exit before the revoke it can remain until TTL expiry.) If a lease-loss fence wins the gate first, the trigger's `RequestShutdown` returns false, so the serving exit tests the process flag, not which fence ran. Catch-up, promotion and restore cannot be interrupted today. A request during one of them is handled when the call returns, and the deadline in 2.4 bounds the wait. #### 2.3 Ordered exit of a leader On shutdown, a session that holds the lease runs this sequence once, on the supervisor thread. It is the full serving form; in earlier phases the exit helper skips the steps with nothing to act on: 1. The gate fences the session and stops the RPC listener (above). 2. The leadership monitor stops, as in today's serving exit (`:728`). 3. The batch OpLog writer stops (`:729`). A healthy writer commits its queued entries before its thread exits (`mooncake-store/src/ha/oplog/ordered_oplog_writer.cpp:521-527`); a writer already in retry abandons its batch (`:563-564`, `:671-675`), as it does on lease loss today. 4. `DeactivateServingState` clears serving state and the leader label (`:235-240`). 5. The admin HTTP server shuts down (`mooncake-store/src/master_admin_service.cpp:328-337`) before the revoke. Per the comment at `master_admin_service.cpp:871-873`, `stop()` drains the HTTP executor; the quota handlers are synchronous and run on that executor. This matters because an admitted admin request can still write to etcd without a lease check: the quota write routes (PUT and DEL, `master_admin_service.cpp:1469-1478`) reach `EtcdTenantQuotaPolicyStore::Save`, which is a plain `EtcdHelper::Put` (`mooncake-store/src/tenant/quota_policy_store.cpp:431-444`). Turning off readiness rejects new requests but does not finish admitted ones. PR A will test that an admitted quota write finishes before the revoke. If the stop path does not provide that guarantee, PR A must add the required wait before it can claim graceful release. 6. Immediately before the release, the renewal polling guard stops; the etcd keepalive continues until `ReleaseLeadership` cancels it. `ReleaseLeadership` runs exactly once, and its result is kept for the exit status. A second call is not a no-op: after the first call clears the keepalive state, it sends another `RevokeLease` for the same lease (`etcd_leader_coordinator.cpp:574-612`). The wrapper treats lease-not-found as success (`etcd_wrapper.go:442-446`), so the extra call only costs a round trip of up to 5 s inside the deadline, and a release after the lease already expired still counts as released. 7. With runtime-state updates already off, `StopStandby` ends standby replication (`mooncake-store/src/ha/standby_controller.cpp:252-263`) and the loop returns. On a path where today's code releases and re-enters standby after a fence (for example the startup cancellation at `:657-666`), the shutdown check runs first, so those calls do not run on a shutdown. Each session records any release attempt and its result, including an attempt already in progress when shutdown is requested. Shutdown consumes that result instead of releasing the same session again. Error exits check the request before service destruction and standby re-entry. The successor fences the old producer through `ClaimProducerView` (`master_service_supervisor.cpp:268-290`) as it does today. The only new thing is that the lease is revoked instead of left to expire. A shutdown exit does not re-enter standby and does not run the #4559 cursor preservation (`return_to_standby`, `:460-480`). That preservation restores an in-process baseline for the next standby start in the same process, and the process is about to exit. #### 2.4 Deadline An independent watchdog thread bounds the whole shutdown. It is armed next to the handler installation and stays armed through the supervisor's shutdown and `main()`'s bounded cleanup after `supervisor.Start()` returns. - It starts timing at the first observation of the request. The proposed default deadline `D` is 15 s. - On expiry it calls `_exit` with a nonzero status. It writes nothing to stderr first, because a write to a stalled pipe could block and defeat the bound. - It only sleep-polls two atomics. It takes no locks and does not log, so it cannot block in the cleanup it bounds. - The second-signal handler and watchdog call `_exit` directly, without flushing or logging. Normal shutdown may call `google::FlushLogFiles(google::GLOG_INFO)` on the supervisor thread while the watchdog remains armed. Forced exit can lose buffered logs; preserving the exit bound takes priority. If the watchdog fires before a confirmed revoke, the lease may remain until TTL expiry. If release has already succeeded, the successor need not wait for the old lease. #### 2.5 Exit status | Outcome | Status | | -- | -- | | Lease released (or none held) and cleanup finished | 0 | | Release failed; the lease expires by TTL | nonzero | | Watchdog expired | nonzero | | Second signal | `128 + signo` (130 SIGINT, 143 SIGTERM) | A nonzero status means failure or deadline expiry; it does not by itself prove that the lease is still held. ### 3. Scope - The HA master on the etcd backend: `master.cpp` (HA branch only), `master_service_supervisor.{h,cpp}`, and a small process-level shutdown utility. - The k8s backend goes through the same supervisor code and its own `ReleaseLeadership` (`mooncake-store/src/ha/leadership/backends/k8s/k8s_leader_coordinator.cpp:334-365`). The tests in this RFC cover etcd only. - The Redis coordinator cannot lead today: `ReadCurrentView` and `TryAcquireLeadership` both return `UNAVAILABLE_IN_CURRENT_MODE` (`mooncake-store/src/ha/leadership/backends/redis/redis_leader_coordinator.cpp:140-150`), so the supervisor exits at its first iteration. Nothing changes there. ### 4. Non-goals - Data-plane changes. Clients, stores and the RPC protocol are unchanged. - Index handoff across restarts. That is #4666's subject, for standalone masters; HA already keeps the index through the OpLog and the standby. - Any change to the non-HA branch of `main()`. - Shortening the successor's warmup after a clean release. Warmup fences an old leader whose lease merely expired; skipping it after a clean release needs its own safety argument. - Making catch-up, promotion or restore cancellable. - The lease TTL value, which #4706 covers. - Admin requests admitted before an ordinary lease loss (not a shutdown) can still reach the unfenced quota `Put` after the release. That window exists today and is out of scope here; it could be a follow-up. ### 5. Relationship to #4559 #4559 ("fix(store): keep HA leadership through large oplog catch-up") moved lease renewal to a side thread that starts at acquisition (`LeadershipRenewalGuard`, `master_service_supervisor.cpp:92-154`, started at `:428`), added `renewal.Lost()` checks after catch-up (`:482`) and after restore (`:586`), and keeps the promotion cursor for a retry in the same process (`:460-480`). This design targets that structure directly: - Lease renewal needs no change. The etcd keepalive thread keeps the lease alive through the whole shutdown; `ReleaseLeadership` stops it immediately before the single revoke (`etcd_leader_coordinator.cpp:600-604`, `:612`). - The new checks sit next to the existing `renewal.Lost()` checks and reuse their exits. ### 6. Relationship to #4666 #4666 (shutdown handoff for a standalone master, by @Juhyun-Kim-Memphis) and this RFC are complementary. #4666 states that the handoff "cannot be enabled together with HA and does not change the HA path"; this RFC changes only the HA path. Their features do not overlap. Both need a process-level SIGTERM path and a watchdog with a deadline: #4666's delivery plan lists "SIGTERM path, admission, watchdog" in A5. This RFC adds two parts that #4666 does not describe: an async-signal-safe request flag and a second-signal escape hatch. The fork flag table in the #4666 appendix (§E) lists `--handoff_shutdown_deadline_sec` with a default of 45; the appendix notes that upstream names and defaults are still open. To avoid two versions: - Share: one small, mode-agnostic utility (request flag, second-signal exit, watchdog with the deadline as a parameter), placed outside `ha/`. Whichever PR lands first owns it and the other reuses it. If #4666's A5 lands first, this work adapts to it. - Keep separate: the deadline value. A handoff write needs much longer than a lease release (the #4666 appendix, §A.3, reports 11.1-11.7 s for the write alone with the real binary at 25M keys), so each mode sets its own. - Defer to #4666: everything about snapshot, quiesce, claim and restore, and any change to the standalone signal path at `master.cpp:1653-1658`. One observation in the #4666 appendix (§F, item 5) affects this design: a graceful exit of an upstream standalone master with a 12M-object index took 21.9 s, mostly spent destroying the index. In the HA serving path the lease is released before the index is destroyed (`master_service_supervisor.cpp:731` runs before `:734`), so teardown cannot delay failover. It can still run past `D`, which is open question 5. ### 7. Relationship to #4706 #4706 adds `--ha_leader_lease_ttl_sec` (2-60, default 5), carried from `MasterConfig` to the etcd coordinator's `GrantLease` and the session's `lease_ttl`. The two changes are independent in code: #4706 touches `BuildHABackendSpec`, the coordinator's acquire path and the flag sections of `master.cpp`, while this work touches the loop body, `Start()` and the HA branch of `main()` (`master.cpp:1612-1618`). Whichever lands second needs at most a small textual rebase. If #4706 lands first, the deployment notes in the second PR below refer to the flag instead of the constant. ### 8. Deployment notes Let `T` be the lease TTL, `D` the shutdown deadline (15 s proposed), `P` the time of any preStop hook, and `G` the pod's `terminationGracePeriodSeconds`. On Kubernetes, `G` defaults to 30 s, the countdown includes the preStop hook, and the kubelet sends SIGKILL when it expires ([Pod Lifecycle](https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/)). - Keep `G` greater than `P + D` plus a margin of a few seconds. With the defaults (`G` = 30, `D` = 15) that leaves room for a preStop of up to about 10 s. The master itself needs no preStop hook. - `T` does not enter that inequality, because a clean release does not wait for `T`. `T` matters on fallback paths only if the old lease has not already been revoked: after forced exit the successor may wait for its remaining TTL. - If `G` is too small, the kubelet kills the master during cleanup and the restart becomes today's crash failover. That is slow but safe. - If cleanup hangs, the lease stays alive until the watchdog fires, because the keepalive thread keeps running. The worst case for a planned restart is therefore up to `D` longer than today. The second signal shortens it. - The admin server stops before the revoke, so `/health` and metrics stop answering during the shutdown. - No format or wire change: old and new binaries can run in the same cluster. An old binary simply keeps today's behavior. ### 9. Delivery plan | PR | Content | Size (estimate) | Useful alone? | | -- | -- | -- | -- | | A | Signal intake (2.1), supervisor handling for every phase (2.2), ordered exit with admin drain and a single release (2.3), exit status; tests that run the supervisor in a child process against a real etcd, as `high_availability_test` already does | ~520 lines, about half tests | Yes. It fixes the common case, a serving leader on SIGTERM | | B | Watchdog and deadline (2.4), its subprocess tests, a manual two-master e2e script against a local etcd, and a deployment-guide section on `terminationGracePeriodSeconds` | ~440 lines, about half test script | Depends on A | Without B, a shutdown stuck in catch-up or restore waits until the phase returns, the second signal, or the kubelet's SIGKILL at `G`. That is why B follows A directly. If maintainers prefer, the two can be reviewed together or merged as one. ### 10. Test plan PR A: 1. A unit test of the shutdown utility in a child process: the first SIGTERM sets the flag and the process keeps running; a second signal exits with `128 + signo`. 2. Extensions of `mooncake-store/tests/ha/leadership/high_availability_test.cpp`, which already runs a supervisor in a child process against a real etcd (`SupervisorChildProcess`, `:179-234`; `RunSupervisorChild`, `:264-301`). Today that test ends the child only with SIGKILL (`:224`). New cases: - a serving leader receives SIGTERM: exit 0 within a bound, and the `master_view` key is gone right after exit, well before `T`; - a second child waiting as standby acquires right after that revoke; - a standby receives SIGTERM: exit 0, and the leader's key is unchanged; - SIGTERM during warmup: exit 0 and the key is released; - a release that fails: nonzero exit, and the key remains until TTL; - a request that races a lease loss or the serving startup: the process flag, not the fence winner, decides the exit; - a second SIGTERM: exit status 143. The existing child harness runs with the OpLog disabled (`high_availability_test.cpp:297`). PR A adds an OpLog-enabled serving-leader case, with a nonempty `cluster_id` and actual queued records, verifying that the healthy writer commits those records before release and that the successor can replay them. PR B: 1. Watchdog tests in a child process: a blocked cleanup is ended within `D` plus a small slack; a disarmed watchdog never fires; expiry still ends the process when stderr is a full pipe, and while the supervisor thread is blocked inside a log flush. 2. A manual e2e script in the style of `mooncake-store/tests/ha/leadership/etcd_leader_hang_e2e.sh`: two real `mooncake_master` processes and a local etcd. SIGTERM to the leader must exit 0 and let the standby acquire before the old lease's TTL would have run out; a SIGKILL control must wait for the lease's remaining TTL. 3. A docs build for the deployment-guide change. ### 11. Rollout risks - SIGTERM no longer ends an HA master at once; it can take up to `D`. Scripts that kill a master and immediately reuse its ports may need to wait for exit. - A stuck cleanup keeps the lease alive for up to `D` (Section 8). - A clean shutdown now exits 0 instead of dying by signal. Supervisors that read the exit status will see the change. - The k8s backend's release and label clearing are not covered by the tests in Section 10. - On k8s, the leader label is cleared asynchronously (`leader_label_reconciler.h:61-83`); a shutdown exit can return before the clear is applied. PR A either waits for it or documents it. ### 12. Questions for maintainers 1. Where should the shared shutdown utility live, and should #4666's A5 reuse it (Section 6)? We propose a mode-agnostic header and source under `mooncake-store/`, not under `ha/`. 2. Should `D` be a flag or a constant? If a flag, should HA and the handoff share one flag with mode-specific defaults, or keep separate flags? 3. Is `128 + signo` for the second signal acceptable (130 for SIGINT, 143 for SIGTERM, as the Transfer Engine watcher does at `graceful_shutdown.cpp:151`), or do you prefer one fixed status? 4. To wake the serving fence, is a polling thread (100 ms or less) acceptable, or do you prefer a self-pipe like the Transfer Engine watcher? (A `sigwait` thread copied from `real_client.cpp:812-857` is not sufficient here: the Go runtime's threads start before `main()` can block anything, and the Go docs say non-Go code should not change the signal mask on Go-created threads.) 5. To avoid spending the shutdown budget destroying a large index, should the HA process call `_exit(0)` after the ordered shutdown steps complete and release succeeds (or no lease is held), instead of destroying the index? Normal log flushing would run while the watchdog remains armed. Watchdog expiry means "shutdown did not finish by the deadline"; it does not prove that the lease remains held. The alternative is normal destruction with a larger `D`. 6. For tests: are child-process tests against a real etcd enough, or would you like an injectable coordinator for deterministic phase-race tests? The injection point does not exist today and would add about 50 lines. (`high_availability_test.cpp` already has a `FakeLeaderCoordinator` at `:111`; it is not injectable into the supervisor.) ### AI assistance disclosure - [ ] No AI tools were used - [x] AI tools were used An AI coding assistant (Claude, by Anthropic) helped read the code and draft this RFC. A human author checked every code reference against `main` at `3aa0f716` and is responsible for the design and, in each PR, for every changed line. ### Before submitting a new issue... - [x] Make sure you already searched for relevant issues and read the [documentation](https://kvcache-ai.github.io/Mooncake/)

bsakaguchi commented 2026-10-29 21:23:53 +00:00

Author

Owner

Copy link

@github-actions[bot] commented on GitHub (Oct 16, 2026):

Thanks for opening this issue, @tensorcopy!

Field	Value
Issue	#4710
GitHub user ID	10591164
Reporter	@tensorcopy

A maintainer will triage this when possible. To help us respond faster, please include:

• Mooncake version or commit SHA
• Environment (OS, CUDA/driver, RDMA stack if relevant)
• Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

<!-- gh-comment-id:6207544831 --> @github-actions[bot] commented on GitHub (Oct 16, 2026): Thanks for opening this issue, @tensorcopy! | Field | Value | |-------|-------| | **Issue** | #4710 | | **GitHub user ID** | `10591164` | | **Reporter** | @tensorcopy | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented 2026-10-29 21:23:53 +00:00

Author

Owner

Copy link

@he-yufeng commented on GitHub (Oct 18, 2026):

Confirmed the current-behavior claims against main: the only SIGINT/SIGTERM handlers on the master path sit in the non-HA branch of master.cpp (1691-1692 now; the line drift you flagged from #4601 checks out), and MasterServiceSupervisor installs none, so in HA mode a SIGTERM really does cost lease expiry plus warmup instead of just warmup. The direction is worth pursuing.

Two design questions worth answering in the first PR's description or here:

1. The revoke-once discipline covers the lease, but what bounds the split-brain window on the data plane? After revoke, the standby can acquire leadership and start serving while the old leader may still hold in-flight client writes that were admitted before the drain began. "Stops taking new work" plus "waits for admitted admin handlers" covers the admin surface; is the plan to also quiesce or fence the data-plane path before revoking, or is the argument that the lease fencing token already makes those writes harmless?

2. For the two-PR split, landing signal handling plus drain first (without revoke) is safe to merge and observable on its own; the revoke half then becomes a small behavioral delta that is easy to roll back. Is that the order you have in mind for the 520/440 split?

No concerns about no data-plane or wire changes as scoped. Happy to review the first PR when it is up.

<!-- gh-comment-id:6244431004 --> @he-yufeng commented on GitHub (Oct 18, 2026): Confirmed the current-behavior claims against main: the only SIGINT/SIGTERM handlers on the master path sit in the non-HA branch of master.cpp (1691-1692 now; the line drift you flagged from #4601 checks out), and MasterServiceSupervisor installs none, so in HA mode a SIGTERM really does cost lease expiry plus warmup instead of just warmup. The direction is worth pursuing. Two design questions worth answering in the first PR's description or here: 1. The revoke-once discipline covers the lease, but what bounds the split-brain window on the data plane? After revoke, the standby can acquire leadership and start serving while the old leader may still hold in-flight client writes that were admitted before the drain began. "Stops taking new work" plus "waits for admitted admin handlers" covers the admin surface; is the plan to also quiesce or fence the data-plane path before revoking, or is the argument that the lease fencing token already makes those writes harmless? 2. For the two-PR split, landing signal handling plus drain first (without revoke) is safe to merge and observable on its own; the revoke half then becomes a small behavioral delta that is easy to roll back. Is that the order you have in mind for the 520/440 split? No concerns about no data-plane or wire changes as scoped. Happy to review the first PR when it is up.

bsakaguchi commented 2026-10-29 21:23:53 +00:00

Author

Owner

Copy link

@tensorcopy commented on GitHub (Oct 18, 2026):

Thanks for checking the claims against main — answers to both questions:

1. Data plane before the revoke. The ordered exit keeps the serving-exit order that main already has, and releases only at the end. In master_service_supervisor.cpp on current main: server.stop() goes through the same ServingStateGate fence as the leadership monitor, so the listener admits no new request; worker_pool.reset() (:741, from #4601) then waits for handler work that was already admitted; StopBatchOpLogWriter() (:745) lets a healthy writer commit its ready batches and joins the writer and callback threads, and after that Commit returns UNAVAILABLE_IN_CURRENT_STATUS (ordered_oplog_writer.cpp:316-317). Only then does ReleaseLeadership run (:747). The graceful shutdown adds the admin drain before that release. So on the graceful path, the old leader has no admitted data-plane work left when a successor can acquire.

The fencing token is the second line, for exits where the drain does not finish (watchdog, second signal, SIGKILL). With the OpLog enabled, every batch write compares producer_view (oplog_batch_storage.cpp:376-381), and the successor claims it before catch-up and promotion (master_service_supervisor.cpp:269-290, called at :431). A late batch from the old term then fails its compare and the writer goes terminal. Warmup is unchanged, so a leader whose lease merely expired is covered as today. Deleting master_view is not itself a write fence: it does not fence direct client/store data transfers or the plain admin quota Put.

Residuals that stay as today: calls drained after server.stop() take effect, but their responses can be lost (same as on lease loss); with the OpLog off there is no producer fence. #4745's tests pin the ordering directly: a serving leader exits 0 on SIGTERM with master_view still present right after exit and gone by TTL, and the OpLog case shows queued entries committed in the durable log after exit and counted by a successor's restore.

2. Order. Adopted — with one addition. #4745 is the signals, the phase checks, the ordered drain and the watchdog, and it makes no deliberate lease release: the process exits 0 and the lease expires by TTL, so failover still costs the lease TTL plus warmup until the follow-up. The watchdog sits in this PR so the drain itself is bounded (15 s; a second signal ends it at once). One detail that made the explicit split necessary: ~EtcdLeaderCoordinator revokes any held lease (etcd_leader_coordinator.cpp:155, through ShutdownKeepAliveThread at :694), so a "drain without revoke" PR would either need new code to suppress that revoke or leave it implicit at destructor order. This PR therefore ends in the terminal in-loop exit that RFC Q5 asked about, after its explicit cleanup and before any unwinding; the follow-up inserts the single ReleaseLeadership at the one marked point immediately before that same exit, so reverting it restores this PR's behavior exactly.

The follow-up also stops the renewal guard and adds the SIGTERM-vs-TTL failover comparison and the deployment-guide notes. Thanks for offering to review — #4745 is up; @he-yufeng, it would be great if you could take a look.

<!-- gh-comment-id:6247136350 --> @tensorcopy commented on GitHub (Oct 18, 2026): Thanks for checking the claims against main — answers to both questions: **1. Data plane before the revoke.** The ordered exit keeps the serving-exit order that main already has, and releases only at the end. In `master_service_supervisor.cpp` on current main: `server.stop()` goes through the same `ServingStateGate` fence as the leadership monitor, so the listener admits no new request; `worker_pool.reset()` (`:741`, from #4601) then waits for handler work that was already admitted; `StopBatchOpLogWriter()` (`:745`) lets a healthy writer commit its ready batches and joins the writer and callback threads, and after that `Commit` returns `UNAVAILABLE_IN_CURRENT_STATUS` (`ordered_oplog_writer.cpp:316-317`). Only then does `ReleaseLeadership` run (`:747`). The graceful shutdown adds the admin drain before that release. So on the graceful path, the old leader has no admitted data-plane work left when a successor can acquire. The fencing token is the second line, for exits where the drain does not finish (watchdog, second signal, SIGKILL). With the OpLog enabled, every batch write compares `producer_view` (`oplog_batch_storage.cpp:376-381`), and the successor claims it before catch-up and promotion (`master_service_supervisor.cpp:269-290`, called at `:431`). A late batch from the old term then fails its compare and the writer goes terminal. Warmup is unchanged, so a leader whose lease merely expired is covered as today. Deleting `master_view` is not itself a write fence: it does not fence direct client/store data transfers or the plain admin quota `Put`. Residuals that stay as today: calls drained after `server.stop()` take effect, but their responses can be lost (same as on lease loss); with the OpLog off there is no producer fence. #4745's tests pin the ordering directly: a serving leader exits 0 on SIGTERM with `master_view` still present right after exit and gone by TTL, and the OpLog case shows queued entries committed in the durable log after exit and counted by a successor's restore. **2. Order.** Adopted — with one addition. #4745 is the signals, the phase checks, the ordered drain and the watchdog, and it makes no deliberate lease release: the process exits 0 and the lease expires by TTL, so failover still costs the lease TTL plus warmup until the follow-up. The watchdog sits in this PR so the drain itself is bounded (15 s; a second signal ends it at once). One detail that made the explicit split necessary: `~EtcdLeaderCoordinator` revokes any held lease (`etcd_leader_coordinator.cpp:155`, through `ShutdownKeepAliveThread` at `:694`), so a "drain without revoke" PR would either need new code to suppress that revoke or leave it implicit at destructor order. This PR therefore ends in the terminal in-loop exit that RFC Q5 asked about, after its explicit cleanup and before any unwinding; the follow-up inserts the single `ReleaseLeadership` at the one marked point immediately before that same exit, so reverting it restores this PR's behavior exactly. The follow-up also stops the renewal guard and adds the SIGTERM-vs-TTL failover comparison and the deployment-guide notes. Thanks for offering to review — #4745 is up; @he-yufeng, it would be great if you could take a look.

bsakaguchi referenced this issue 2026-10-29 21:32:31 +00:00

[PR #1199] [MERGED] [Store] add safe tensor API #2066

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

te-coro

staryxchen-tent-capability-research

feature/musa-ep-jit

zxpdemonio/fix-tent-bootstrap-admission

add-store-rs-tone-tests

scatter-rdma-completion-pr2

P2P-Mooncake-Store

release/v0.3.14

feat/tent-rdma-adaptive-adapter

fix/ep-local-counter-gpu-scope

codex/kvcs-oplog-eviction

give-me-a-star

cruz/mooncake-kvcs-lowlevel-pr1-20261008

ylt-handshake-limits

ci/rocm-e2e-tent

sima/docs-pypi-downloads

fix-ci/hicache-mooncake-metadata-loopback

sima/cuda-driver-loading

sima/ci-performance-gate

fix/tent-rdma-unknown-topology-rail-pairing

ci/tent-e2e-dual-job

mc-patch-1

codex/p2p-merge-prerequisites

sima/store-hca-numa-placement

codex/p2p-merge-cli

codex/p2p-merge-ipv6

copilot/fix-ctests-job-failure

czh/pg-device-collectives-pr6

dependabot/cargo/mooncake-store-rs/pyo3-0.29.0

czh/pg-device-collectives-pr5

codex/segment-pool-master-cutover

sima/nightly-ci-recovery

ci/tent-params

stmatengss-patch-1

dependabot/cargo/mooncake-store-rs/rand-0.8.8

dependabot/cargo/mooncake-store-rs/opentelemetry_sdk-0.32.1

dependabot/cargo/mooncake-store-rs/xxhash-rust-0.8.16

codex/migrate-store-rest-service

fix/tone-wheel-arch-selection

czh/pg-device-collectives-pr4

czh/pg-device-collectives-pr3

ketong/fix_moe_tc

copilot/fix-test-wheel-ubuntu-job

fix/ci-wheel-torch-pg-pin

codex/migrate-structured-object-storage

codex/migrate-allocator-python

codex/migrate-vllm-python

codex/migrate-shared-segment

ci/fix_sgl_eval_install

codex/segment-pool-nof-placement-cleanup

feat/pip-native-release

copilot/bugfix-ha-standby-promotion

copilot/mooncake-store-transport-settings

copilot/fix-tensor-support-issues

copilot/optimize-rl-direction

copilot/mooncake-storeagentic-workflow

copilot/fix-client-reconnect-timeout

copilot/mooncake-error-debugging

copilot/mooncake-ipv6-support

feat/nccl-host-python-api

dependabot/go_modules/mooncake-p2p-store/src/p2pstore/google.golang.org/grpc-1.82.1

copilot/migrate-and-move-operations

copilot/migrate-and-move-operations-again

refactor/centralize-env-vars

idhanani/mooncake-retain-groups

copilot/codeownerreviewer

copilot/give-feedback-on-issue-2123

copilot/create-issue-rfc

copilot/break-cuda-monopoly-logic

copilot/compare-cudamemcpy-cu12-cu13

copilot/report-ci-error-issue

copilot/research-commit-history-analysis

copilot/explain-mooncake-store-service

copilot/analyze-project-functionality

copilot/sub-pr-1680

copilot/review-pr-1680

copilot/sub-pr-1597

copilot/fix-github-actions-job-failure

copilot/move-test-sglang-integration-job

v0.3.14

v0.3.14-pre-4

v0.3.14-pre-3

v0.3.14-pre-2

v0.3.14-pre

v0.3.14-rc1

v0.3.13.post1

v0.3.13-rc2

v0.3.13-rc1

v0.3.13

v0.3.12.post1

v0.3.12

v0.3.11-rc1

v0.3.11.post1

v0.3.11

v0.3.10.post2

v0.3.10.post1

v0.3.10

v0.3.9

v0.3.8.post1

v0.3.8

v0.3.7.post2

v0.3.7.post1

v0.3.7

v0.3.6.post1

v0.3.6

v0.3.5

v0.3.4.post2

v0.3.4.post1

v0.3.4

v0.3.3.post2

v0.3.3.post1

v0.3.3

v0.3.2.post1

v0.3.2

v0.3.1

v0.3.0

v0.3.0-beta

v0.3.0-alpha

v0.2.0

Labels

Clear labels   

ascend

  

auto-closed

  

bug

  

bug

  

CI/Build

  

Common

  

documentation

  

enhancement

  

feature request

  

good first issue

  

help wanted

  

high priority

  

MetaX

  

nightly-failure

  

pull-request

Mirrored from GitHub Pull Request

  

PyTorch Backend

  

question

  

RFC

  

Roadmap

  

stale

  

Store

  

Tests

  

Transfer Engine

No labels

ascend

auto-closed

bug

bug

CI/Build

Common

documentation

enhancement

feature request

good first issue

help wanted

high priority

MetaX

nightly-failure

pull-request

PyTorch Backend

question

RFC

Roadmap

stale

Store

Tests

Transfer Engine

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

kvcache-ai/Mooncake#1199

No description provided.