[Store] HA: release the leader lease after the SIGTERM drain (part 2 of #4710)

docs/source/deployment/mooncake-store-deployment-guide.md

@@ -426,6 +426,55 @@ When the Primary fails, the Standby is promoted through the following steps:
 
 This fail-closed behavior is intentional. Older versions could log a restoration error and continue serving from empty or partially restored metadata. That behavior was a correctness bug, not a supported availability fallback: the serving state could disagree with the durable OpLog and poison later recovery attempts. Mooncake does not automatically discard snapshots, OpLog records, or metadata after a recovery error.
 
+### Graceful Shutdown
+
+An HA master (`--enable_ha=true`) handles SIGTERM and SIGINT with an ordered drain. The steps and timings below are for the etcd backend, which the tests cover.
+
+On the first signal the master:
+
+1. Stops accepting RPCs: readiness goes off, removal of the leader label is scheduled, and the RPC listener stops.
+2. Waits for the RPC handlers that were already admitted.
+3. Stops the leadership monitor.
+4. Stops the batch OpLog writer after it commits the entries already queued. A writer that is already retrying leaves its batch, as it does on lease loss.
+5. Stops the admin HTTP server and standby replication, and stops the remote metadata cleanup worker (its in-flight request finishes or times out) before the lease is released.
+6. Releases the leadership lease once.
+7. Exits.
+
+A standby or candidate skips the steps for resources it does not have, and it holds no lease to release.
+
+The whole shutdown, including the release, is bounded to 15 seconds from the first observation of the request. When the bound expires the process exits with status 1 without logging, and a lease that was not yet released expires by TTL. A second SIGTERM or SIGINT exits at once with status 143 or 130 and skips the remaining steps, including the release.
+
+| Exit status | Meaning |
+| --- | --- |
+| 0 | The selected drain completed and the release succeeded or was not needed (released, already released, or no lease held). It does not claim that all work is durable or that every task in the process was drained. |
+| 2 | The drain completed but the release was not confirmed. If the lease remains, it expires by TTL. A timeout can also mean that the lease had already expired, or that the release reached etcd and its reply was lost, so a nonzero status does not prove that the lease is still held. |
+| 1 | The 15 second bound expired. |
+| 130, 143 | A second SIGINT or SIGTERM arrived during the shutdown. |
+| 255 | A request was seen only after a fatal backend error made the supervisor return. |
+
+The confirmed-release semantics and the exit statuses 0 and 2 above describe the etcd backend; the Kubernetes backend reports only that the cancel request was accepted.
+
+With `T` the lease TTL and `W` the successor's warmup (`W = T`), a planned stop changes how long the old key lives:
+
+| Leader stop | Old key removed | Successor serves after about |
+| --- | --- | --- |
+| SIGKILL or crash | When the lease expires: up to `T` after the last successful keepalive, with no fixed minimum after the kill. | remaining TTL + acquisition + OpLog takeover + `W` + serve start |
+| SIGTERM without the release step (earlier versions) | After the drain, then as for SIGKILL. | drain + remaining TTL + acquisition + OpLog takeover + `W` + serve start |
+| SIGTERM with a confirmed release | During the shutdown, after the drain and before the process exits. | drain + release + acquisition + OpLog takeover + `W` + serve start |
+
+`T` is `--ha_leader_lease_ttl_sec` (default 5, range 2 to 60). Acquisition is the standby noticing the deletion through its watch, then granting a lease and creating the key; its retry loop uses a one-second watch timeout, and in local tests, with the standby already watching, the whole step took 4 to 20 ms. OpLog takeover applies when the OpLog is enabled: it is the producer claim, catch-up and promotion that run before warmup, and it is zero without the OpLog. Serve start is restore, preflight and serving activation. With `T` = 5 s in a healthy local setup, a confirmed release saves roughly the remaining TTL, typically 3 to 5 seconds per planned leader restart, minus the drain and release time; an unconfirmed release falls back to TTL expiry. This is an estimate, not a guarantee: with a slow drain or a slow release the saving is smaller, and it can be negative compared with a kill.
+
+On Kubernetes, keep `terminationGracePeriodSeconds` greater than any preStop time plus the 15 second bound plus a few seconds. The default of 30 seconds leaves room for a preStop of about 10 seconds, and the master needs no preStop hook. If the grace period is shorter, the kubelet's SIGKILL can arrive during the shutdown: a kill before the release is confirmed can leave the lease to TTL expiry, as in the crash row above, while a kill after the release does not. The leader-label removal is only scheduled, so the process can exit before it is applied. `/health` and metrics stop answering during the shutdown, because the admin server stops before the release.
+
+Limits:
+
+- Independent lease loss, external revocation and keepalive failure keep their earlier behavior.
+- A client can lose the response to a request that completed during the drain.
+- The successor still warms up for `T` after a clean release.
+- With the OpLog off there is no producer-view fence.
+- The Kubernetes backend's release runs on SIGTERM but has no test coverage, and it reports only that the cancel request was accepted: the wrapper returns no release result, and when no election monitor thread is running it does not wait for the election goroutine. A status of 0 therefore does not confirm a Kubernetes release.
+- The timings above were measured on etcd only.
+
 ### Example: HA Deployment with etcd
 
 Primary configuration (`primary.yaml`):

mooncake-store/include/ha/leadership/master_service_supervisor.h

@@ -53,6 +53,10 @@ class ServingStateGate {
 // process exit. main() arms the shutdown watchdog with it.
 inline constexpr std::chrono::seconds kHaShutdownDeadline{15};
 
+// Exit status of a completed HA shutdown whose lease release was not
+// confirmed. A lease that remains is then left to TTL expiry.
+inline constexpr int kHaShutdownReleaseFailedStatus = 2;
+
 class MasterServiceSupervisor {
    public:
     // shutdown_requested defaults to the process signal flag; tests inject

mooncake-store/include/master_service.h

@@ -976,6 +976,17 @@ class MasterService {
      */
     void setHttpMetadataRemoteUrl(const std::string& metadata_connstring);
 
+    /**
+     * @brief Stop the remote HTTP metadata cleanup worker: drop queued work,
+     * skip the rest of a running batch and join. Returns after at most one
+     * in-flight DELETE completes or times out: the wait is the HTTP client's
+     * connect and request timeouts (name resolution included), and the
+     * shutdown watchdog bounds the process. Later cleanup requests are
+     * ignored. Safe when the worker never started. Call from one thread;
+     * sequential calls are safe.
+     */
+    void StopHttpMetadataCleanup();
+
    private:
     tl::expected<void, ErrorCode> RestoreFromStandbyState(
         const std::vector<StandbyObjectEntry>* legacy_objects,

mooncake-store/include/rpc_service.h

@@ -24,6 +24,7 @@ class WrappedMasterService {
     void SetBatchOpLogTerminalCallback(
         OrderedOpLogWriter::TerminalCallback callback);
     void StopBatchOpLogWriter();
+    void StopHttpMetadataCleanup();
     // Constructor with optional metadata-cleanup-on-timeout configuration.
     // - http_metadata_server: in-process pointer used when the HTTP metadata
     //   server is co-located in the master process (nullptr = not co-located).

mooncake-store/src/ha/leadership/master_service_supervisor.cpp

@@ -174,29 +174,57 @@ bool ShutdownObserved(const std::function<bool()>& shutdown_requested) {
     return true;
 }
 
-// What the supervisor holds when a shutdown ends the process.
+// What the supervisor holds when a shutdown ends the process. session and
+// coordinator are set together, only where the term's lease is acquired.
 struct ShutdownLease {
-    // Set while this process holds the lease; it is left to expire by TTL.
+    // Set while this process holds the lease; shutdown releases it once.
     const LeadershipSession* session = nullptr;
+    LeaderCoordinator* coordinator = nullptr;
+    // The term's renewal polling guard, once it exists. It stops before the
+    // release so that a poll cannot race the revoke.
+    LeadershipRenewalGuard* renewal = nullptr;
     // Result of an ordinary release that already ran for the session.
     std::optional<ErrorCode> prior_release{};
 };
 
-// Last step before the terminal exit. Returns the exit status.
+// Last step before the terminal exit, after the admin drain. Releases a
+// held lease once and returns the exit status: 0 when no lease is left to
+// TTL expiry, kHaShutdownReleaseFailedStatus when a release was not
+// confirmed.
 int FinishShutdown(const ShutdownLease& lease) {
+    // An ordinary release already ran for this term. Never repeat it; a
+    // second revoke is not a no-op (it costs a round trip of up to 5 s).
     if (lease.prior_release.has_value()) {
-        LOG_IF(INFO, *lease.prior_release == ErrorCode::OK)
-            << "Shutdown complete after leadership release, result=OK";
-        LOG_IF(WARNING, *lease.prior_release != ErrorCode::OK)
-            << "Shutdown complete after leadership release, result="
-            << toString(*lease.prior_release);
-        return 0;
+        if (*lease.prior_release == ErrorCode::OK) {
+            LOG(INFO)
+                << "Shutdown complete after leadership release, result=OK";
+            return 0;
+        }
+        LOG(WARNING) << "Shutdown complete after leadership release, result="
+                     << toString(*lease.prior_release);
+        return kHaShutdownReleaseFailedStatus;
     }
     if (lease.session != nullptr) {
-        LOG(INFO) << "Shutdown complete, no release attempted; the lease is "
-                     "left to TTL expiry, view_version="
-                  << lease.session->view.view_version;
-        return 0;
+        if (lease.renewal != nullptr) {
+            lease.renewal->Stop();
+        }
+        LOG(INFO) << "Shutdown releasing leadership, view_version="
+                  << lease.session->view.view_version << ", renewal_lost="
+                  << (lease.renewal != nullptr && lease.renewal->Lost());
+        TestFailPoint::Wait("supervisor_before_shutdown_release");
+        const ErrorCode err =
+            lease.coordinator->ReleaseLeadership(*lease.session);
+        if (err == ErrorCode::OK) {
+            LOG(INFO) << "Shutdown complete, leadership released, view_version="
+                      << lease.session->view.view_version;
+            return 0;
+        }
+        LOG(WARNING) << "Shutdown complete, leadership release failed, result="
+                     << toString(err)
+                     << "; release not confirmed, any remaining lease expires "
+                        "by TTL, view_version="
+                     << lease.session->view.view_version;
+        return kHaShutdownReleaseFailedStatus;
     }
     LOG(INFO) << "Shutdown complete, no lease held";
     return 0;
@@ -567,7 +595,8 @@ int RunSupervisorLoop(const HABackendSpec& spec,
             continue;
         }
 
-        ShutdownLease term_lease{.session = &*leadership_session};
+        ShutdownLease term_lease{.session = &*leadership_session,
+                                 .coordinator = &leader_coordinator};
         if (ShutdownObserved(shutdown_requested)) {
             return shutdown_exit("acquired", term_lease);
         }
@@ -593,6 +622,7 @@ int RunSupervisorLoop(const HABackendSpec& spec,
         // TryAcquireLeadership, but etcd KeepAlive previously started only in
         // the warmup phase, so a large Oplog replay expired the lease first.
         LeadershipRenewalGuard renewal(leader_coordinator, *leadership_session);
+        term_lease.renewal = &renewal;
 
         auto claim_error = ClaimProducerViewForServing(
             spec, config, leadership_session->view.view_version);
@@ -727,6 +757,10 @@ int RunSupervisorLoop(const HABackendSpec& spec,
                 StopLeadershipMonitor(monitor);
                 wrapped_master_service->StopBatchOpLogWriter();
                 DeactivateServingState(admin_server, label_reconciler);
+                // Local cleanup requests finish or time out before the lease
+                // release; a server that accepted a request whose reply was
+                // lost may still apply it afterwards.
+                wrapped_master_service->StopHttpMetadataCleanup();
                 LOG(INFO) << "Shutdown stopped the term's workers, monitor "
                              "and OpLog writer";
             };
@@ -989,6 +1023,7 @@ int RunSupervisorLoop(const HABackendSpec& spec,
         StopLeadershipMonitor(leadership_monitor_handle);
         wrapped_master_service->StopBatchOpLogWriter();
         DeactivateServingState(admin_server, label_reconciler);
+        wrapped_master_service->StopHttpMetadataCleanup();
         auto err = release_term("serving");
         LOG(INFO) << "Release leadership: " << toString(err);
         auto current_view = leader_coordinator.ReadCurrentView();

mooncake-store/src/master_service.cpp

@@ -13386,6 +13386,19 @@ void MasterService::setHttpMetadataRemoteUrl(
     LOG(INFO) << "HTTP metadata cleanup worker thread started";
 }
 
+void MasterService::StopHttpMetadataCleanup() {
+    if (!http_metadata_cleanup_thread_.joinable()) {
+        return;
+    }
+    {
+        std::lock_guard<std::mutex> lk(http_metadata_cleanup_mutex_);
+        http_metadata_cleanup_running_ = false;
+        http_metadata_cleanup_queue_.clear();
+    }
+    http_metadata_cleanup_cv_.notify_all();
+    http_metadata_cleanup_thread_.join();
+}
+
 void MasterService::cleanupHttpMetadata(const std::string& segment_name) {
     // Co-located: remove in-process, safe to run inline (no network I/O).
     if (http_metadata_server_) {
@@ -13406,6 +13419,11 @@ void MasterService::cleanupHttpMetadata(const std::string& segment_name) {
     if (!http_metadata_remote_url_.empty()) {
         {
             std::lock_guard<std::mutex> lk(http_metadata_cleanup_mutex_);
+            // A stopped worker never drains the queue, and a late DELETE
+            // could land after the lease is released.
+            if (!http_metadata_cleanup_running_) {
+                return;
+            }
             http_metadata_cleanup_queue_.push_back(segment_name);
         }
         http_metadata_cleanup_cv_.notify_one();
@@ -13447,6 +13465,10 @@ void MasterService::HttpMetadataCleanupThreadFunc() {
         }
 
         for (const auto& segment_name : batch) {
+            // Stopping skips the rest of the batch.
+            if (!http_metadata_cleanup_running_) {
+                break;
+            }
             const std::string ram_key =
                 http_metadata_prefix_ + "ram/" + segment_name;
             const std::string rpc_key =
@@ -13455,7 +13477,10 @@ void MasterService::HttpMetadataCleanupThreadFunc() {
             // Each key attempted independently so one failure does not
             // prevent cleanup of the other.
             const bool ram_removed = removeRemoteHttpMetadataKey(ram_key);
-            const bool rpc_removed = removeRemoteHttpMetadataKey(rpc_key);
+            // Stopping also skips the second key, so at most one DELETE is
+            // in flight when the stop request arrives.
+            const bool rpc_removed = http_metadata_cleanup_running_ &&
+                                     removeRemoteHttpMetadataKey(rpc_key);
             LOG(INFO) << "Cleaned up remote HTTP metadata for segment: "
                       << segment_name << ", ram_key_removed=" << ram_removed
                       << ", rpc_key_removed=" << rpc_removed;

mooncake-store/src/rpc_service.cpp

@@ -104,6 +104,10 @@ void WrappedMasterService::StopBatchOpLogWriter() {
     master_service_.StopBatchOpLogWriter();
 }
 
+void WrappedMasterService::StopHttpMetadataCleanup() {
+    master_service_.StopHttpMetadataCleanup();
+}
+
 tl::expected<MasterMetricManager::CacheHitStatDict, ErrorCode>
 WrappedMasterService::CalcCacheStats() {
     return MasterMetricManager::instance().calculate_cache_stats();