Skip to content

[Store] Add HealthCheck RPC and /readyz probe for master RPC readiness - #4140

Open
Colors-111 wants to merge 5 commits into
kvcache-ai:mainfrom
Colors-111:feat/master-healthcheck-readyz
Open

Colors-111 wants to merge 5 commits into
kvcache-ai:mainfrom
Colors-111:feat/master-healthcheck-readyz

Conversation

@Colors-111

@Colors-111 Colors-111 commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

Background

A production incident (#3940) observed a window where the master's RPC plane became fully unresponsive: when a store client's lease expired, the master's ClientMonitorThreadFunc synchronously invoked ClearInvalidHandles → ClearStaleHandles to sweep all keys/replicas owned by that client. During the sweep, ClearStaleHandles held the metadata shard write lock, while data-plane RPCs such as PutStart / Get need the same shard's read lock — blocking all data-plane RPCs and driving inference-engine get/put throughput to zero for minutes.

In a K8s deployment, operators want a readinessProbe to automatically remove a blocked master from Service endpoints and shift traffic to a healthy standby, plus Prometheus alerts to surface the condition. But every existing liveness/readiness mechanism fails against this "process alive, port listening, RPCs fully blocked" scenario:

Probe method Behavior during the block Detects?
tcpSocket 50051 socket is up, TCP handshake succeeds No (false green)
httpGet /health (admin port 9003) admin thread takes no shard lock, always 200 No (false green)
Ping RPC takes only client_mutex_ shared lock, not a shard lock, returns normally No (false green)
ServiceReady RPC returns the version string, takes no lock No (false green)

Root cause: Ping takes only the client_mutex_ shared lock (MasterService::Ping), ServiceReady takes no lock, and ClearInvalidHandles releases client_mutex_ before calling ClearStaleHandles — none of these contend with the shard write lock, so none can be blocked by the sweep. Reliably detecting this condition requires a probe RPC that contends for the shard read lock.

Design

Add a new public RPC HealthCheck that walks all metadata shards (kNumShards = 1024) probing read-lock availability, paired with an in-process self-probe thread, Prometheus metrics, and a /readyz endpoint. No existing RPC interface is modified — Ping / ServiceReady / ExistKey keep their signatures and behavior; HealthCheck is purely additive.

1. New RPC HealthCheck

Signature (WrappedMasterService / MasterService, no parameters):

struct HealthCheckResponse {
    bool ok;               // true if all shards acquired the read lock within budget
    uint64_t shards_checked;
    uint64_t shards_blocked;
};
tl::expected<HealthCheckResponse, ErrorCode> HealthCheck();

Algorithm:

  • Walk all 1024 shards serially, calling try_lock_shared() (non-blocking) on each shard's mutex.
  • Acquire-and-release: hold at most one read lock at a time, so writers on other shards are not blocked — interference is negligible. Proceed to the next shard immediately.
  • When a shard's write lock is held by a long-running sweep, try_lock_shared fails → retry with ~10ms backoff until success or global budget exhaustion.
  • Global budget kShardLockBudget (default 2s, below the probe timeout of 3s).
  • All shards acquired within budget → ok=true, shards_checked=1024; any shard exceeds budget → return ok=false, shards_blocked=<first blocked shard index>.
  • Read-only, no OpLog, no side effects; works in both HA and non-HA modes. Under normal load 1024 non-contended try_lock calls complete in < 5ms; only a shard held for seconds triggers failure.

Rationale for walking all 1024 shards rather than a single key: a single key hits only one shard (key % kNumShards), so the chance of landing on the shard currently being swept is ~1/1024. With a 5s probe interval over a 7-minute window that is under 10% — it would mostly miss. HealthCheck covers all shards in one pass for a reliable verdict.

Register with one line in RegisterRpcService:

server.register_handler<&mooncake::WrappedMasterService::HealthCheck>(
    &wrapped_master_service);

2. Self-probe thread (RPC probe)

Add an independent rpc_probe_thread_ to MasterAdminServer (which already runs a 10s metrics-reporting thread):

  • Period rpc_probe_interval_seconds (default 5s); runs only when service_available_ is true.
  • Calls HealthCheck against rpc_address:<rpc_port> with timeout rpc_probe_timeout_seconds (default 3s). The probe target uses the RPC server's actual bind address: 0.0.0.0/:: is normalized to 127.0.0.1; otherwise rpc_address is used directly (when bound to a specific IP, 127.0.0.1 is unreachable).
  • Uses TCP (the TCP listener stays on rpc_port even when the RPC server enables RDMA).
  • Results cached under atomic guards: rpc_probe_ok_, rpc_probe_latency_ms_.
  • HA guard: on a standby, service_available_=false (RPC server not started) → the probe thread skips → /readyz returns 503 directly, which is correct.

3. Prometheus metrics + /readyz

New metrics in MasterMetricManager:

Metric Type Meaning
mooncake_master_rpc_responsive gauge 1 = probe completed within timeout, 0 = timed out
mooncake_master_rpc_probe_latency_ms gauge Latency of the most recent probe
mooncake_master_rpc_probe_failures_total counter Cumulative failed probes

New GET /readyz on MasterAdminServer: service_available_ && rpc_probe_ok_ → HTTP 200; otherwise HTTP 503. It only reads cached state — the admin thread takes no shard lock, so it returns instantly. The existing GET /health is unchanged (process liveness, always 200).

4. Configuration (new gflags, all tunable)

--enable_rpc_health_probe    (default true; when false, no probe thread starts and /readyz degrades to /health semantics)
--rpc_probe_interval_seconds (default 5)
--rpc_probe_timeout_seconds  (default 3)

Design Rationale

  • Add rather than modify existing interfaces: Ping / ServiceReady have callers relying on their current semantics (no shard lock, always fast). Changing their lock behavior would break client contracts. HealthCheck has a clear purpose ("all-shard read-lock liveness") and is probe-only — purely additive, zero regression risk.
  • RPC rather than in-process call: calling service_->HealthCheck() in-process saves the RPC client but bypasses the dispatch path, so it cannot detect "RPC worker pool exhausted by blocked RPCs" — a secondary symptom. Going over RPC exercises the full accept → dispatch → handler path, giving the most faithful readiness verdict — under pool exhaustion HealthCheck itself is delayed and judged unavailable, which is the desired behavior.
  • All shards rather than a single key: kNumShards = 1024; a single-key probe hits ~1/1024, unable to reliably catch a specific shard's long-held write lock.
  • Readiness vs liveness separation: /readyz (drives readinessProbe; failure only removes traffic, no restart) answers "can it serve"; /health (drives livenessProbe; failure restarts) answers "is the process alive". Do not use "RPC timeout" to trigger liveness restarts — it would interrupt a legitimate sweep, re-scan after restart, and likely recur.

Known Limitations

  • HealthCheck has no Python binding. The self-probe is an in-process C++ behavior needing no Python; a Python client wrapper for the public HealthCheck RPC is left as follow-up.
  • This PR only addresses "detect / drain traffic"; it does not fix the root cause. The root-cause fix (sweep duration under the shard write lock in ClearStaleHandles) is Approach A in [Bug][Store]: Lease-expired client cleanup blocks all master RPC for minutes under HA + SSD Offload #3940 and should proceed separately.
  • The probe thread occupies one admin thread; /readyz itself only reads cached state and takes no shard lock, so it never delays under blocking.

Testing

New unit tests:

  • HealthCheckReturnsOkWhenNoLockHeld: with no shard write lock held, HealthCheck returns ok=true, shards_checked=1024, shards_blocked=0, completing in milliseconds.
  • HealthCheckDetectsShardWriteLockHeld: the test fixture takes a write lock on shard 0 and holds it past the budget (2s); HealthCheck returns ok=false with shards_blocked=0 (the first blocked shard); after releasing the lock, HealthCheck recovers to ok=true, shards_checked=1024.

An integration test for the /readyz endpoint (ReadyzReflectsProbeResult) requires standing up a coro_rpc server + admin server + probe thread simultaneously, which is too heavy for the master_service_test unit tests; left as follow-up (can live in master_admin_service integration tests or e2e coverage). The /readyz logic is simple (read service_available_ && rpc_probe_ok_ cache → 200/503) and is indirectly covered by the two HealthCheck tests.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake Conductor (mooncake-conductor)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

cmake --build . --target master_service_test -j$(nproc)

./mooncake-store/tests/master_service_test \
    --gtest_filter='MasterServiceTest.HealthCheck*'

Test results:

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

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

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

AI assisted with root-cause analysis (reading the Ping / ServiceReady / ClearStaleHandles lock paths and the kNumShards constraint) and design drafting. The human submitter has reviewed every changed line and can explain end-to-end: why existing probes fail against the "process alive, port listening, RPCs fully blocked" scenario, why HealthCheck walks all 1024 shards rather than a single key, and why an RPC probe (vs. an in-process call) additionally covers the RPC worker-pool exhaustion symptom.

@ykwd

ykwd commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

FYI, @Aionw already has a PR that removes the per-shard locking granularity. This may make the shard-level health check here unnecessary.

@Colors-111

Copy link
Copy Markdown
Contributor Author

granularity

Thanks for the pointer. I took a look at #3593 (bounded write-lock batches for ClearStaleHandles). It does address the core of #3940 — the sweep no longer holds the per-shard write lock across an entire shard; it now picks candidates under a shared lock and takes the exclusive lock in batches of 64 (kStaleHandleCleanupBatchSize), releasing between batches so data-plane RPCs get a turn. That substantially shortens the blocking interval, and a shard with nothing to clean never takes the write lock at all.

But the /readyz + self-probe framework has independent value beyond the sweep-locking question:

So the probe framework is orthogonal to the sweep-locking work. Happy to adjust HealthCheck's internal implementation if the consensus is that #3593 makes the all-shard walk unnecessary, but the /readyz + probe + metrics surface can land now and protect production in the interim.

Open to the maintainers' call on sequencing.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants