feat(umbp): Redis/RESP master metadata store backend#468
Open
isytwu wants to merge 40 commits into
Open
Conversation
Adds a RESP-protocol-compatible IMasterMetadataStore backend so the master can run stateless with all metadata in an external store (Redis / Dragonfly / Valkey), selected at startup via UMBP_METADATA_BACKEND / UMBP_REDIS_URI. The one production wiring change is MasterServer's store construction, which now goes through MakeMasterMetadataStore(); router / eviction / reaper stay backend-agnostic behind IMasterMetadataStore&. Phase 1 vertical slice: - RespClient seam over hiredis (connection pool, EVALSHA + NOSCRIPT fallback, owned RespValue) isolating the client library from the store. - KeySchema (single deployment hash tag) + Lua scripts for the atomic hot-path mutations (register / heartbeat / route_get_batch / exists_batch / list_alive / unregister / expire). - RedisMasterMetadataStore implementing the six hot-path methods with semantics matching the in-memory backend (seq-CAS heartbeat, lease/access bump, full-sync replace), plus the methods the running master needs; external-KV and eviction candidate enumeration are Phase 2 stubs. - USE_REDIS_BACKEND CMake option (default OFF) wiring hiredis; setup.py passthrough. Default connection pool capped at 32 so a big host's CPU count cannot flood single-threaded Redis. - Conformance test (skips without a live store; passes on Redis + Dragonfly), a store-level microbench, docker-compose + local launcher for the backends, and the Phase 1 go/no-go benchmark report. Design: src/umbp/doc/design-redis-metadata-store.md
The RESP/Redis backend signals transport failures (and Phase-2-unimplemented methods) by throwing. Those exceptions escaped the gRPC handlers and the background loops and hit std::terminate, taking down the whole master on any Redis hiccup — observed under load as "terminate called after throwing an instance of mori::umbp::redis::RespError". - Wrap every gRPC handler body in GuardStore(), which converts any exception escaping the metadata store into grpc::UNAVAILABLE (the "degrade, don't fabricate" contract in design-redis-metadata-store.md §7). The in-memory backend never throws, so this is a no-op there. - Guard the reaper, hit-index GC, and eviction background loops so a transient store error is logged and retried on the next tick instead of killing the thread. Verified: with Redis killed mid-run the master now logs the error and keeps serving (returns UNAVAILABLE) instead of aborting; a normal run and the conformance suite are unaffected.
…lure CreateRdmaEndpoint read errno for logging/throwing only after intervening calls (a mutex lock/unlock and the spdlog sink). spdlog's console sink calls isatty(stderr), which sets errno=ENOTTY when stderr is not a terminal, so the logged/thrown error masked the real failure code — e.g. an ibv_create_qp rejection that is actually EINVAL (requested QP capacity, max_send_wr x the per-WQE size derived from sge + inline, exceeding the device's per-QP work-queue budget) was reported as "Inappropriate ioctl for device". Capture errno into a local right after each failing ibv_create_* call (comp channel, CQ, QP) and use it for both the log and the thrown message.
run_local_backends.sh: auto-fall-back for the Redis binary (PATH -> prebuilt in RUN_DIR -> apt-get -> build the official source with make) so the launcher works on images where the apt mirror is blocked but github over HTTPS is reachable, and resolve redis-server/redis-cli by absolute path instead of assuming they are on PATH.
route_get_batch already HGETALLs each block hash, so _acnt is in hand; write _lease/_lacc/_acnt in a single HSET instead of HSET + a separate HINCRBY. The single-slot Redis server is redis.call()-count bound, so dropping the per- touched-key writes from 2 to 1 cuts serial script cost ~11% (t1 b32 routeget: 290->260us p50, 3296->3657 ops/s) with identical semantics (_acnt still ends at old+1). Adds redis-backend-perf.md with the baseline + attribution runs (AOF is negligible; the bottleneck is serial Lua op-count) and this commit's before/after. Conformance: Redis + Dragonfly 10/10.
… verification Add --external-master / --node-id-prefix / --node-address to bench_kvevent_master_pressure so N OS processes (on one or many hosts) can share a single standalone umbp_master instead of each spawning its own in-process master. This reproduces the real "N-processes-per-machine + multi-machine" load on the master metadata backend (Redis) that the in-process kvevent bench (1 process, N clients) and the store microbench (1 process, N threads) cannot model. Add run_mp_redis_bench.sh (multi-process driver: starts one redis-backed umbp_master, fans out PROCS x CLIENTS client processes over the full Router/gRPC path, scrapes results) and parse_master_hist.py (per-RPC p50/p95/p99 from the master Prometheus histogram). Together they are the repeatable harness for judging Redis-backend optimizations: scale until the single-slot ceiling shows, then compare master BatchRouteGet/RoutePut/ Heartbeat latency + redis evalsha usec/call before vs after.
…efault 1)
Spread block-location keys across N hash-tag shards ({umbp:<ns>:b<shard>})
chosen by a stable FNV-1a hash of the user key, and fan the batched read hot
path (BatchLookupBlockForRouteGet / BatchExistsBlock) out to one single-slot
EVAL per shard via a new RespClient::EvalPipeline (one round trip; NOSCRIPT
retries only the failed calls so a lease/access bump never double-applies).
Write path stays a single atomic script: heartbeat / unregister / expire now
receive the fully composed block key from C++ and store it as the reverse-index
member, so Lua needs no shard math. seq-CAS, SEQ_GAP, and full_sync-atomic
semantics are unchanged.
UMBP_REDIS_BLOCK_SHARDS (Config::block_shards) controls N; default 1 keeps the
legacy single-tag layout with byte-identical keys and whole-batch-atomic reads,
so existing behavior is unchanged (verified: 32 tests pass on Redis + Dragonfly
with shards={1,16}; A/B shows shards=1 == base performance). N>1 relaxes a batch
read to per-key atomicity (matching the in-memory backend; interface only
promises per-key semantics).
Note: on a single instance the split gives no throughput win (single-threaded
Redis just runs more scripts; regresses under saturation) - the benefit needs
genuinely parallel backends (multiple Redis instances / cluster / dedicated-core
Dragonfly). The cross-slot write scripts are single-instance only; splitting
them per-shard for Redis Cluster + multi-endpoint fan-out is the next step.
Add UMBP_REDIS_SHARD_URIS: a comma-separated list of Redis instances, one per block shard, so block-lookup scripts run on independent server processes. This is the only way past a single instance's single-thread throughput ceiling (measured ~2.9x RouteGet at 4 instances on a dedicated host: M1 t16 ~5.2k -> M4 t16 ~15k ops/s; M1 stays flat at the single-slot ceiling at any concurrency). clients_[0] is the control instance (client records, alive set, peer view, extkv + block shard 0); shard s lives on clients_[s] with its own co-located per-node reverse index. Since no Lua script spans instances, the cross-store writes are split into a control step (seq-CAS + record + alive/peers) plus per-shard block steps (apply_block_events / wipe_node_blocks). Block ADD/REMOVE, full_sync clear+replay, and node-wipe are idempotent, so a partial failure is retried and healed by the peer's next SEQ_GAP -> full_sync; per-key/per-shard atomicity is preserved (same posture as the in-memory backend). Reads group keys by shard and issue one EvalPipeline per instance. Single-endpoint mode (no UMBP_REDIS_SHARD_URIS) is unchanged: the original single atomic scripts run, M==1 stays byte-identical to before. Fan-out is sequential per instance (throughput scales via concurrent RouteGets); a worker pool to also cut single-request latency on high-RTT remote stores is a noted follow-up (naive per-call std::async threads cost more churn than they save). Tests: the store suite now also runs a 3-endpoint mode (all endpoints on one physical Redis, disjoint tags) covering the split writes, per-shard reverse index, cross-shard full_sync/unregister/expire, and fan-out reads — 46 tests pass on Redis and Dragonfly.
Issue each Redis instance's read pipeline concurrently through a small persistent worker pool (redis/thread_pool.h) instead of sequentially, so a multi-endpoint BatchLookup/BatchExists costs one round trip rather than N. Measured ~3.3x RouteGet throughput at 4 instances on a dedicated host (t8 5.2k -> 17.3k ops/s, p50 1527us -> 462us) — better than sequential fan-out (13k) and far better than per-call std::async (8k, which is dominated by thread-creation churn). The win is larger on high-RTT remote stores where the N serial round trips dominate. The pool is persistent (no per-call churn), deadlock-free for this use (workers only run blocking Redis calls; only the caller waits on its futures), sized min(64, endpoints*8), and created only in multi-endpoint mode. Single-endpoint mode uses no pool and runs the one pipeline inline — byte-for-byte the prior path. Replies are scattered on the calling thread so the per-key decode stays lock-free. 46 store tests still pass on Redis and Dragonfly.
In multi-endpoint mode, a single Redis shard instance being down or erroring no longer fails the whole batch read: RunShardedRead captures that instance's transport/script failure, leaves its keys at the caller's default (a miss — empty locations / exists=false), logs a WARN, and still serves keys on the healthy shards. This keeps RouteGet/RoutePut-dedup working when one of several block instances is unavailable, instead of erroring the entire RPC. Single-endpoint mode is unchanged: with nothing to fall back to, a failure propagates so a total outage surfaces rather than masquerading as all-misses. The write path (heartbeat / unregister / expire) stays strict on purpose — a down shard makes block-apply throw so the peer retries instead of silently dropping locations; updates resume (gap heals via full_sync) when the shard returns. Test: RedisFaultToleranceTest points shard 1 at a closed port, seeds shard 0, and asserts a batch spanning both resolves the live key and reads the dead shard's key as a miss without throwing. 47 store tests pass.
…econds)
Add an optional observability hook so the Redis backend exports the latency of
each IMasterMetadataStore operation as a labeled histogram
mori_umbp_store_op_latency_seconds{op,backend="redis"}, making backend
round-trip cost (RTT + server-side script time) visible per store method on the
master's Prometheus endpoint — previously only inferable indirectly from redis
INFO commandstats.
- MetricsServer: add a labeled per-observation observe() overload (reuses the
labeled-histogram storage that observeAggregated already uses).
- IMasterMetadataStore: add a default-no-op SetMetricsSink() hook (in-memory
ignores it); RedisMasterMetadataStore stores the sink and a ScopedStoreOp RAII
timer records latency on the hot methods (RegisterClient, ApplyHeartbeat,
Unregister/Expire, LookupBlock, BatchLookup/BatchExists, ListAlive,
GetAlivePeerView). master_server wires the sink after the metrics server
starts. Null sink (e.g. the standalone microbench) = zero overhead.
Test: RedisStoreMetricsTest drives the store with a live MetricsServer and
scrapes /metrics, asserting the histogram appears with op/backend labels
(GPU-free; the kvevent bench that drives the master's RPC path needs a GPU).
48 store tests pass.
A down shard instance no longer aborts the write path. The control step (seq-CAS + record + nodes:alive/alive_peers) runs first, so the node is already marked ALIVE and the reaper won't expire it. If a per-shard block step then fails, ApplyHeartbeat returns SEQ_GAP instead of throwing — the master already converts SEQ_GAP into a full_sync request, so the peer re-ships a full snapshot and the node self-heals once the shard is back (block ADD/REMOVE and full_sync clear+replay are idempotent, so the replay is safe). Previously the exception propagated as a failed heartbeat RPC. UnregisterClient / ExpireStaleClients now wipe best-effort per shard: a down shard is skipped (its lingering locations point at a gone node and are filtered out of reads by GetAlivePeerView) and cleaned when it returns, instead of failing the whole cleanup. Single-endpoint mode is unchanged (one atomic script; failures still surface). Test: RedisWriteFaultToleranceTest points shard 1 at a closed port and asserts a heartbeat spanning both shards returns SEQ_GAP (not an exception), keeps the node ALIVE, and still applies the live shard's event. 49 store tests pass.
bench_umbp_kvevent_master_pressure set cfg.io_engine.host="0.0.0.0", so the published EngineDesc resolved to loopback — fine when every peer is on one host, but cross-machine peers then tried to reach the RDMA engine at 127.0.0.1 and got "ProtocolError: Connection reset by peer", making multi-machine --external-master runs crash. Advertise o.node_address instead (already the routable IP the peer registers with the master; default 127.0.0.1 keeps single-host runs unchanged). Verified: with this, 148+032 client machines drive a master+Redis on 039 over RDMA with no crash. The RDMA fabric itself was fine (ib_write_bw 039<->148 = 28.6 Gb/s over RoCEv2); the blocker was purely the advertised engine address.
…ent base Add sewenew/redis-plus-plus as a 3rdparty submodule and build it from source (static, PIC, C++17, tests off) behind USE_REDIS_BACKEND, using the system hiredis. This is the client library the Redis backend will standardize on for Redis Cluster failover + Sentinel + pooling (true-HA direction); building it from source keeps any build image working without an extra system package. The store code does not depend on redis++ yet — this only lands and de-risks the dependency (toolchain, -Werror, link against system hiredis) ahead of the seam migration. A skippable link/build smoke test (test_redispp_smoke) proves it compiles, links, and pings a reachable store. All 49 existing Redis-backend tests still pass.
…fly/cluster) Make the metadata store a sidecar the app container talks to over host networking instead of a redis-server built from source inside the app container. docker-compose.yml gains compose profiles for single Redis, Dragonfly, and a 3-master+3-replica Redis Cluster (with an idempotent cluster-init one-shot). A new store.sh wraps up/down/status/seeds and prints the exact UMBP_REDIS_* env to export; it auto-falls-back to run_local_backends.sh for single/dragonfly when docker is unavailable. run_local_backends.sh is demoted to the documented no-docker fallback. Verified on a host with docker: the cluster profile forms (cluster_state:ok, 6 nodes, 3 masters) and the redis++ cluster smoke test connects to it.
… backend The master used to build the Redis store and start "healthy" even when the store was unreachable or the config was contradictory, then return UNAVAILABLE on every RPC. The factory now: - Pings every configured endpoint at startup. UMBP_REDIS_REQUIRED (default true) turns an unreachable store into a clear immediate failure; set 0 to start degraded and rely on runtime reconnect. - Recognizes UMBP_REDIS_CLUSTER and rejects contradictory modes: cluster is mutually exclusive with UMBP_REDIS_SHARD_URIS, and (until the store's cluster path lands) is refused with a clear "not supported yet" message instead of silently running single-endpoint. Validated on umbp_master: cluster flag and dead-store default abort with a clear message; UMBP_REDIS_REQUIRED=0 starts degraded. All 49 backend tests still pass.
…iners) Update the design doc to reflect the productization decisions: true-HA target, redis-plus-plus as the client library (vendored submodule + source build on the system hiredis, §9), the UMBP_REDIS_CLUSTER (reserved, validated) and new UMBP_REDIS_REQUIRED readiness knobs (§10), and independent-container store deployment via tools/redis/store.sh. §13 flips from open questions to locked decisions.
Split RespValue/RespError and a new IRespClient interface (Command/Eval/ EvalPipeline/Ping) into resp_value.h; RespClient now implements IRespClient, and the store holds vector<unique_ptr<IRespClient>> instead of the concrete client. No behavior change (pure refactor) — this lets a redis-plus-plus cluster client implement the same seam for Redis Cluster mode while the hiredis client keeps driving single / multi-endpoint. All 49 backend tests still pass.
… seam RespClusterClient implements IRespClient on sw::redis::RedisCluster: it routes each command (by args[1]) and script (by KEYS[0]) to the node owning that hash tag's slot and delegates MOVED/ASK redirection, slot-map refresh, and master-failover reconnect to redis-plus-plus's redirection loop. Server errors are surfaced as Error RespValues (error-as-value preserved via a catch of ReplyError); transport failures throw RespError. EvalPipeline fans its single-slot groups out concurrently through a small worker pool, one redirection-aware EVAL each. Scripts use EVAL (Redis caches by SHA server-side) to avoid a per-node EVALSHA cache across failover/resharding. redis++_static is now linked into umbp_common; the store does not construct the cluster client yet (next: mode wiring). Compiles clean, all 49 tests still pass.
Add a cluster deployment mode driven by UMBP_REDIS_CLUSTER=1: the store builds a single RespClusterClient and uses the split control-script + per-shard-block write path (gated on a new split_writes() = mode != single, since cluster's control and block keys live in different slots and a single atomic script would CROSSSLOT). Block keys spread across the cluster's nodes via block_shards tags (factory default 16 in cluster mode). The four scripts that previously ran with empty KEYS (list_alive / expire_control / apply_block_events / wipe_node_blocks) now pass a same-slot routing key as KEYS[1] so the cluster client can route them and Redis fixes the slot; the Lua is unchanged and single/multi-endpoint stay byte-for-byte the same. Factory enables cluster (mutually exclusive with UMBP_REDIS_SHARD_URIS) with seeds from the UMBP_REDIS_URI comma list. Verified against a real 3-master+3-replica cluster: the full store conformance suite passes in a new cluster mode (no CROSSSLOT), alongside single / multi.
Add a "cluster" StoreMode to the parameterized RedisStoreTest: when UMBP_REDIS_CLUSTER_SEEDS is set it drives the store in cluster mode (16 block shard tags) and runs every existing assertion (register/heartbeat/routeget/ exists/full-sync/unregister/expire, incl. the cross-shard cases) against the cluster, so a stray cross-slot script surfaces as a CROSSSLOT failure. The probe is now an IRespClient (RespClusterClient for cluster, RespClient otherwise). Skips cleanly when no cluster is configured, so single-node CI is unaffected. Verified: 63 tests pass (49 single/multi + 14 cluster).
Update §9 (two IRespClient impls: hiredis RespClient for single/multi, redis-plus-plus RespClusterClient for cluster) and §10 (UMBP_REDIS_CLUSTER now selects a working RedisCluster client; UMBP_REDIS_BLOCK_SHARDS defaults to 16 in cluster mode). Verified against a real 3-master+3-replica cluster: conformance suite + failover + live reshard.
The cluster client now EVALSHAs (SHA loaded once and cached, with a SCRIPT LOAD + retry on NOSCRIPT for a promoted replica / resharded node) instead of shipping the ~1-2KB Lua body on every hot-path call, matching the hiredis client. A/B at a fixed slot mapping (fixed namespace, shards=6, t16, 3 runs each) shows a small but consistent win: routeget ~4478 -> ~4670 ops/s (+~4%), p50 ~3700 -> ~3557us. The earlier random-namespace comparison masked this under slot-mapping bimodality. The gain is larger on higher-RTT links where the body bytes dominate. Cluster conformance suite still passes.
In cluster mode, if UMBP_REDIS_BLOCK_SHARDS is not set, the factory now discovers the master count (RespClusterClient::DiscoverMasterCount via CLUSTER SLOTS) and sets block_shards = 2 x masters, falling back to 16 only if discovery fails. This replaces the fixed default of 16, which over-split each RouteGet batch into many tiny per-shard EVALs (throughput ~= single node). ~2x spreads a batch across the cluster's nodes while keeping per-batch scripts few; measured best in the shard sweep. An explicit UMBP_REDIS_BLOCK_SHARDS still wins. Verified: auto resolves to 6 on a 3-master cluster (matches explicit shards=6); cluster conformance suite still passes.
Cluster throughput was ~single-node and highly namespace-dependent because the
formulaic {umbp:<ns>:bS} tags hash to arbitrary slots — measured (per-node CPU +
commandstats) to pile most block shards onto one master (saturated) while others
sat idle. It was NOT redis++/client overhead: the busy node was CPU-bound and a
manually-balanced namespace already hit ~85% of multi-endpoint.
So place shards deterministically: at startup (cluster mode, when
UMBP_REDIS_BLOCK_SHARDS is unset) read CLUSTER SLOTS and pick one block-shard
hash tag per master whose CRC16 slot that node owns, so a RouteGet batch spreads
one EVALSHA per node.
- cluster_slots.h: CRC16 (bitwise) + SlotOfKey (hash-tag aware) + FindTagForRanges.
- RespClusterClient::DiscoverMasterSlotRanges: parse CLUSTER SLOTS into per-master
ranges (stable order).
- KeySchema gains an explicit-block-tags ctor; store BuildKeySchema uses it when
the factory supplies Config::cluster_block_tags, else the formulaic tags.
- Factory computes the balanced tags (one per master), falling back to formulaic
shards if discovery/search fails.
Measured on a 3-master glibc cluster: ~4.6k (bimodal 3.5-6.9k) -> ~11.7k stable
across random namespaces (~86% of multi-endpoint 13.5k), all masters evenly
saturated (~105k evalsha each). Correctness is independent of the local CRC16
(redis++ still routes by the real slot); only balance depends on it.
Single-endpoint and multi-endpoint (UMBP_REDIS_SHARD_URIS / hicache) paths are
untouched: cfg.cluster is false there, BuildKeySchema returns the identical
formulaic KeySchema, and the factory's cluster branch is skipped. All 63 backend
tests pass (single + Dragonfly + endpoints3 + cluster).
- ClusterSlotsTest (pure, always runs): CRC16/SlotOfKey against reference CLUSTER KEYSLOT values (foo=12182, bar=5061, 123456789=12739 the CRC16/XMODEM check value), the hash-tag rule (block key routes by its shard tag; empty braces are not a tag), and FindTagForRanges landing in / distinct across disjoint ranges. - A "clusterbalanced" StoreMode runs the full store conformance suite against a real cluster using the balanced per-master tags (same DiscoverMasterSlotRanges + FindTagForRanges the factory uses), so the balanced path — previously only bench-verified — now has automated coverage. MetaField rebuilds the block key from those tags. Skips unless UMBP_REDIS_CLUSTER_SEEDS is set. 81 tests pass (49 single/multi + 14 formulaic-cluster + 14 balanced-cluster + 4 slot unit).
…ackends run_mp_redis_bench.sh only drove a single Redis URI. Add REDIS_CLUSTER=1 (pass UMBP_REDIS_CLUSTER=1 + the comma seed list as UMBP_REDIS_URI) and SHARD_URIS=... (multi-endpoint) so the standalone-master proxy harness can benchmark all three deployments through the full Router/gRPC(+RDMA) path. Cluster/multi rely on the per-run unique namespace for isolation instead of a single-node FLUSHALL. Measured single-host, 32 clients, gap0, get=both: BatchRouteGet p50 single 6.6ms / cluster(balanced) 4.9ms / multi-3 2.8ms.
Single-endpoint block sharding (UMBP_REDIS_BLOCK_SHARDS>1) gave no speedup
on Dragonfly's multi-threaded architecture. Root cause: launching Dragonfly
with --default_lua_flags=allow-undeclared-keys forces EVERY Lua script into
global-transaction mode (a store-wide lock across all proactor threads), so
even the read hot path (route_get_batch / exists_batch) — which already
declares every key via KEYS[] — was serialized, erasing the sharding win.
(Confirmed by Dragonfly docs + a baseline: ~1.2k ops/s flat across shards
and client threads, the signature of a global lock.)
Fix (zero Redis impact): the two read hot-path scripts stay untouched (they
declare all keys, so Dragonfly runs them per-shard in lock-ahead mode,
parallel across threads). The 10 write/control scripts that derive auxiliary
same-slot keys from the hash tag each carry a first-line
"--!df flags=allow-undeclared-keys" directive so ONLY they run global on
Dragonfly. Redis honours only a "#!" shebang, so that line is a plain Lua
comment there — single-node and Cluster behaviour is byte-for-byte unchanged
(the read scripts are literally unmodified). Dragonfly is launched WITHOUT the
global flag (docker-compose.yml / run_local_backends.sh) so the per-script
directives take effect.
Scope: this optimizes the READ path only (routeget is the hicache hot path).
Write/control scripts still run global via --!df, which is acceptable
(heartbeat/register/expire are low-frequency and off the RouteGet path). A
follow-up to parallelize writes (declare keys -> lock-ahead) is tracked in the
handoff doc.
Measured (039, single Dragonfly v1.23.2, routeget batch32 keys50000):
- P8: ~1.2k -> ~13.3k ops/s peak (t32, shards=8)
- P16: ~22k, P32: ~43k ops/s peak (scales with --proactor_threads)
- single-node Redis unchanged (~4k, sharding still a no-op as expected)
- test_redis_master_metadata_store: 53 PASSED / 0 FAILED on Redis AND
Dragonfly (cluster cases skipped without seeds)
…gonfly - resp_cluster_client: extract SeedConnectionOptions() so ConnectCluster() and DiscoverMasterSlotRanges() share the one seed-URI -> ConnectionOptions mapping (host/port + password + timeouts). Pure refactor, no behavior change. - factory: when a single-endpoint backend reports itself as Dragonfly (best-effort INFO server probe for "dragonfly_version") AND UMBP_REDIS_BLOCK_SHARDS is left at the default 1, log a WARN recommending it be set to ~the --proactor_threads count so the RouteGet read hot path parallelizes across Dragonfly's threads. It does NOT silently change the default: block_shards is fixed for a deployment's lifetime, so auto-bumping it would strand existing block keys. Probe failure is ignored (the readiness Ping still surfaces a real outage). No functional change to the Redis single-node / Cluster paths; correctness unchanged (53/53 on Redis + Dragonfly, 32/32 cluster cases).
…ailures, retry stale reads - SHA cache: key by the Lua script object address instead of its ~1.5KB text, so the read hot path (route_get_batch / exists_batch) no longer rehashes and compares the whole script body under the cache mutex on every call. Scripts are now stable `inline const std::string` objects. Applies to both the single-node and cluster RESP clients; behaviour is unchanged. - Cluster reads: a single down or erroring node now degrades its keys to misses instead of failing the whole RouteGet/Exists batch, matching multi-endpoint mode. The tolerate-failures decision is passed in explicitly (a cluster is one client, so it could not be inferred from the client count). - Stale connections: RespClient::Command retries once on a fresh connection after a transport failure (a pooled socket dropped by the server or a load balancer). Read-only path only; scripted writes are never retried. - Metrics: optional cold-path counters/histograms (connection-pool waits, transport errors, NOSCRIPT reloads, degraded-shard misses), gated on a metrics sink and emitted only on rare paths, so the success path is unaffected. - Remove unused DiscoverMasterCount and num_endpoints(). 81 tests pass (single/multi and cluster/clusterbalanced against a real cluster); store-microbench shows no regression across single, multi-endpoint, cluster, and Dragonfly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two RESP client implementations (RespClient on hiredis, RespClusterClient on redis-plus-plus) had duplicated the EVALSHA SHA cache, the EVALSHA argv layout, and the const-char*/length splitting. A change to any of them (keying the SHA cache by script identity, or adding cold-path metrics) had to be made twice. Extract three header-only helpers used by both: ScriptCache (identity-keyed SHA cache: GetOrLoad + Invalidate), ToArgv (split a vector<string> into the parallel argv/len arrays hiredis wants), and BuildEvalshaArgv (assemble the EVALSHA argv). Pure refactor, no behaviour change. 81 tests pass (single/multi and cluster/clusterbalanced against a real cluster). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The batch bucketing + fan-out + fault-tolerance + reply-scatter logic (the trickiest, most order-sensitive part of the backend) lived in the store .cpp anonymous namespace, so it could only be exercised via the live integration test, which skips when no Redis is reachable. Move ShardedBatch / GroupKeysByShard / RunShardedRead into redis/sharded_read.h (no behaviour change) and add test_sharded_read: deterministic unit tests against a fake IRespClient covering reply scatter to caller order, multi-instance fan-out, and the tolerate-shard-failures degrade-to-miss vs propagate behaviour. These run without a backend. 8 new unit tests pass; the 81 integration tests still pass (single/multi and cluster/clusterbalanced). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, dragonfly-cluster Document four operational caveats surfaced while reviewing the backend: (1) the store is rebuildable soft state (projection of heartbeats), so disk persistence is optional and separate from replica HA; (2) route_get_batch makes every read a write, so synchronous persistence (AOF appendfsync=always) must stay off; (3) the control plane (clients_[0] / the control-tag node) does not scale horizontally; (4) Dragonfly-in-cluster underperformance is largely a client limitation (no cross-slot pipelining), not purely topological. Docs only; no code change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l tag
Replaces the Phase-1 `throw unimplemented` stubs for the external-KV
read/write family, its hit-count accounting, and hit GC with real
single-slot Lua on the control tag, so a real hicache PD
(kv_events_subscriber=true) can run on the Redis backend.
- extkv/hit/hit-index all hash under the control tag {umbp:<ns>}, so
RegisterExternalKvIfAlive / MatchExternalKv / Unregister* /
GetExternalKvHitCounts / GarbageCollectHits are each one keyed Lua on
the control instance in every mode (single / multi-endpoint control
instance / Redis Cluster control slot).
- Tier-set is a bitmask int per node (bit == 1<<TierType), computed with
plain arithmetic in Lua (no `bit`/bitop library) so the same script
runs on Redis / Dragonfly / Valkey.
- RegisterExternalKvIfAlive does the alive-check + write in one atomic
Lua (register-if-alive TOCTOU); MatchExternalKv's count_as_hit branch
increments hit:<hash>.c and stamps ls=now once per unique matched hash.
- GarbageCollectHits walks a hit:index reverse SET instead of SCAN
(SCAN has no key and cannot be routed per-node in cluster mode).
- GetExternalKvCount is O(1) SCARD of the per-node reverse index.
- Fix a latent cascade bug: UnregisterClient / ExpireStaleClients (and
the split-mode control scripts) previously only DEL'd the extkv
reverse index, orphaning each node's bits in extkv:<hash>; they now
clear the node from every hash it registered, matching the in-memory
backend's whole-node external-KV wipe.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hot-path change) Replaces the Phase-1 empty-map EnumerateEvictionCandidates so master-driven eviction works on the Redis backend, WITHOUT touching the read hot path. A per-(node,tier) LRU ZSET would have to be updated inside route_get_batch, but the lru:<node>:<tier> key is data-derived (the (node,tier) is only known after reading the block inside the script), so it cannot be pre-declared in KEYS[]; adding it under allow-undeclared-keys would force route_get_batch global on Dragonfly and lose the read-parallel win. Instead this reuses the existing per-node block reverse index (node:<id>:blocks) plus the _lease/_lacc already maintained on each block hash: - one keyed Lua per shard walks each requested node's reverse index, skips leased blocks (_lease > now), and emits (key,node,tier,size,_lacc) for the wanted (node,tier) pairs; - shards are fanned out and aggregated with the existing RunShardedRead (cluster-safe: every group routes by its shard-tag key); - ordering (kLeastRecentlyAccessed) and the per-bucket cap are applied in C++, mirroring the in-memory backend's scan-and-sort. route_get_batch / exists_batch and their store methods are byte-for-byte unchanged, so read throughput cannot regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…all modes Extends the parameterized RedisStoreTest (shards1 / shards16 / endpoints3 / cluster / clusterbalanced) with assertions for the newly-implemented methods, so each runs in single / multi-endpoint / Redis Cluster (cluster auto-skips without UMBP_REDIS_CLUSTER_SEEDS): - external-KV alive-gate + match, tier bitmask across HBM/DRAM, unregister by hash / by tier / by node, and O(1) GetExternalKvCount; - hit counting (count_as_hit accumulation, per-unique-hash increment), GetExternalKvHitCounts dedup/skip-missing, GarbageCollectHits by last_seen; - the cascade fix: UnregisterClient / ExpireStaleClients drop the node's external-KV entries, not just the reverse index; - eviction LRU order + per-bucket cap, skip-leased, only-requested-buckets, and tie-timestamp / cross-shard aggregation. Assertions verify the master_metadata_store.h contract, not in-memory internals (so the Redis-native choices — SCARD count, tier bitmask, hit:index, reverse-index eviction scan — are asserted as they behave). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… the design design-redis-metadata-store.md: mark the external-KV, hit-count, GC, and eviction methods implemented; add the hit:index reverse-set row; document extkv/hit control-tag placement and the tier bitmask; replace the "LRU ZSET" / "SCAN" descriptions with the reverse-index eviction scan and the keyed GC; update the §8 caveat accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a store-level microbench and wire up the proxy path to measure the external-KV hot-path IMasterMetadataStore methods (MatchExternalKv, RegisterExternalKvIfAlive, UnregisterExternalKv, GetExternalKvHitCounts) that SGLang KV-events integration drives, so per-interface latency and throughput can be compared across in-memory / single Redis / sharded Redis / Redis Cluster / single Dragonfly. - bench_master_metadata_store_extkv.cpp: factory-selected backend, no gRPC/RDMA; workloads match / match_nohit / report / revoke / hitcounts / mixed, with --nodes / --hit-ratio knobs. - run_extkv_store_bench.sh: sweeps all five topologies + prints a side-by-side ops/s + p50/p95/p99 comparison. - bench_kvevent_master_pressure: add --ext-kv-only (report+match only, no BatchPut/Get/RDMA) and --ext-kv-count-as-hit (default on; the real hot-path hit-count write, previously hardcoded off). - run_mp_redis_bench.sh: forward EXT_KV / EXT_KV_ONLY / EXT_KV_COUNT_AS_HIT to exercise the external-KV path end-to-end through the full Router/gRPC. Findings: external-KV/hit state lives on one control hash tag (single slot), so sharded Redis and Cluster do not scale this path; Dragonfly is worse still because every Lua script self-declares allow-undeclared-keys and thus runs global (all-proactor lock); the count_as_hit write roughly doubles MatchExternalKv cost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The external-KV hot path (MatchExternalKv + RegisterExternalKvIfAlive, driven by SGLang KV-events) hung entirely off the single control hash tag: one Redis slot / one instance / one Dragonfly proactor thread. So sharded and cluster deployments could not scale it, Dragonfly serialized every script under a store-wide GLOBAL lock (allow-undeclared-keys), and each match paid ~2x for the per-hash hit-count write. Two changes, keeping num_shards==1 byte-identical + atomic and the block read hot path (route_get_batch / exists_batch) untouched: 1. Lua cost + Dragonfly global lock. The match hit path drops from 5 to 3 redis.call() per hash (unconditional HSET ls; SADD hit-index only on the counter's first hit), and match / hitcounts / register / revoke now declare every touched key in KEYS[] (hashes deduped C++-side) so the scripts drop --!df flags=allow-undeclared-keys and Dragonfly runs them per-slot (lock-ahead) instead of global. 2. Shard the extkv keyspace. extkv:<hash> / hit:<hash> / the hit index / the per-node reverse index now live in the hash's OWN shard slot (KeySchema::ExtKv/Hit/HitIndex/ExtKvNode by ShardOf(hash)); reverse-index and hit-index members store full keys so the write scripts take only [node_id,tier] / [count,now] as shared ARGV and a whole batch fans out one single-slot script per shard via RunShardedRead / EvalPipeline (the block read path's fan-out). Cascades (UnregisterClient / ExpireStale) gate the inline extkv wipe on a wipe_extkv flag and, when sharded, wipe per shard via WipeNodeExtKvMulti; register does a control alive-check then a per-shard write (TOCTOU window healed by the cascade, consistent with the split block-write path). num_shards==1 collapses to one atomic control-slot script. Store microbench (threads=8 batch=32 keys=50k), match ops/s baseline->now: single 5195->8162 (+57%), sharded 5202->24364 (+369%), cluster 3716->13710 (+269%), dragonfly 395->3101 (+685%). Tests green in single / multi / cluster / dragonfly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bench_kvevent_master_pressure GET fetch (get-mode fetch/both) reads block data over RDMA from peer processes. With fixed --rounds the processes finish at slightly different times, so a faster one tore down its peer service while a straggler was still fetching -> "Connection reset by peer" on the worker thread -> std::terminate, losing that process's data and corrupting multi-process fetch/both runs. - Treat a BatchGet transport failure as a distinct fetch-error (not a miss, no latency sample) so hit/miss + p50 stay clean, and never abort the run. - Add a cross-process end-of-run file barrier (--barrier-dir/--barrier-size/ --barrier-tag): every client process reaches it before any tears down, so peer RDMA endpoints stay alive until all readers are done. Falls back to BENCH_DRAIN_MS when no barrier is configured; run_mp_redis_bench.sh wires PROCS. Verified on an 8-process both-mode run: 0 crashes, 0 fetch-errors, all 8 processes synced at the barrier, consistent per-RPC counts. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Redis/RESP-based master metadata store backend for UMBP, behind a client-agnostic seam, supporting single-node, Dragonfly, and Redis Cluster deployments, plus horizontal scaling via keyspace sharding and multi-endpoint fan-out.
Core backend
mori_umbp_store_op_latency_seconds)Horizontal scaling
Redis Cluster mode
IRespClientseam so the store is client-agnosticBuild / infra
Tests
Misc
Test plan
Perf
Two layers: store-direct microbench isolates the backend (cleanest); 8-process proxy is the full-link (master + Router/gRPC), ≈ SGLang's per-machine shape. batch=32, keys=50000, single multi-core host, loopback, glibc redis 7.2.5 / dragonfly v1.23.2.
routeget= read hot path (BatchLookupBlockForRouteGet, incl. lease write),heartbeat= write hot path (ApplyHeartbeat). ops/s = batch32 calls/s (higher better).Methodology: client threads
t= the applied load, held identical across backends (columns below). Only backend-internal config is tuned per backend (dragonflyproactor_threads/block_shards, redis instance count,pool, in-memoryindex_shards); that config + its resource footprint is listed per row.Store-direct microbench (
bench_umbp_master_metadata_store, no gRPC/RDMA)RouteGet (read) — cells =
ops/s (p50 ms), same loadtper column:index_shards=256proactor_threads=32, block_shards=64, pool=256Heartbeat (write) — cells =
ops/s (p50 ms):index_shards=256At the realistic ~8-process operating point (t8–t32) redis peaks immediately — multi-endpoint ~25–27k, cluster ~21k — while dragonfly is the worst at low concurrency (t8 5.4k): its 32 proactor threads are starved until enough requests are in flight, so it only overtakes multi past t128. Beyond the knee, extra load just grows latency (multi/cluster throughput stays flat, p50 climbs). Scaling knobs: cluster/multi scale with node count (cluster 3→6→8 masters = 9.7k→18.6k→22k read, ≈88% of same-count multi); dragonfly scales with
proactor_threadsbeyond P32: ~51k (P48, t384) → ~53k (P64, t512), diminishing. Writes: dragonfly collapses (see Takeaways).Reproduce (build + backends + microbench)
Full-link, 8-process proxy (
umbp_master+ Router/gRPC)8 client processes (each own conn pool + gRPC channel + heartbeat) hit one master; same total-client load per column, best internal config per backend.
GETMODE=exists (metadata read via
BatchLookup, no RDMA) — cells =get QPS / BatchLookup p50 (ms):index_shards=1GETMODE=both (real read hot path:
route_get_batch+ an RDMA block fetch) — cells =get QPS / BatchRouteGet p50 (ms)(get QPS counts lookup+fetch ≈ 2 ops/round, so compare only within this table):Full-link gap is smaller than store-direct: each RPC adds a backend-independent ~0.5ms fixed cost (gRPC + single-master CPU + net). redis throughput peaks at low concurrency then the single master saturates (latency grows, QPS falls); in-memory keeps scaling; dragonfly stays low (heartbeat writes take a global lock). Adding the RDMA block fetch (both) compresses the redis backends further — multi-endpoint ≈ cluster once the peer-to-peer data transfer is in the path — while dragonfly still trails. (both-mode multi-process runs required a bench fix — a cross-process end-of-run barrier — so a faster process no longer tears down its RDMA peer while a straggler is still fetching.)
Reproduce (full-link proxy)
Takeaways
KEYS[]→ run lock-ahead/parallel; write/control scripts derive keys from the hash tag so each carries a first-line--!df flags=allow-undeclared-keys→ global store-wide lock → write collapses to ~0.4k (more proactor threads don't help; also drags the full-link path via heartbeats). That directive is a plain comment on Redis. Read-heavy / light-write only.External-KV hot path (store-direct microbench,
bench_umbp_master_metadata_store_extkv)The external-KV interfaces SGLang's KV-events drive:
MatchExternalKv(count_as_hit=true)— "which live nodes hold these block hashes?", plus a per-hash hit-count write — is the read hot path;RegisterExternalKvIfAlive(BlockStored) is the write hot path;match_nohit(count_as_hit=false) isolates the pure read. The extkv keyspace is now sharded by hash (like the block index), so — unlike the original single-control-slot layout — it spreads across instances / slots / proactor threads.Tables are iso-load: each cell is
ops/s (p50 ms)at a fixed client-thread countt(the offered concurrency), so a row-to-row gap is the backend itself, not a different tuning point. Fixed per backend: multi-endpoint 8 instances pool=128; cluster 8 masters + 8 replicas balanced pool=128; dragonflyproactor=32, block_shards=64pool=256; singleblock_shards=1; in-memory in-process. batch=32 / keys=50000 / nodes=8. Dragonfly needs far more in-flight requests to fill its 32 proactors, so its t128/t256 points are shown separately. (All backend rows are from one consistent run so they're mutually comparable.)MatchExternalKv (read + per-hash hit-count write) — cells =
ops/s (p50 ms)dragonfly keeps climbing past t64: t128 9,313 (12.6) → t256 11,134 (15.9).
match_nohit (pure read, no hit write) — cells =
ops/s (p50 ms)dragonfly: t128 54,933 (2.1) → t256 63,113 (3.8).
RegisterExternalKvIfAlive (write) — cells =
ops/s (p50 ms)dragonfly: t128 13,968 (8.9) → t256 11,667 (20.7) (past its knee). in-memory
reportp50 is 27µs (0.03 ms) but p99 is 14–68 ms — the map-rehash-under-write-lock stalls (see below).Read vs write:
match_nohitshows a ~45–280× backend gap;match/reportdon't. The gap is not the backend "getting faster" for reads — it is where each side's bottleneck lies:match_nohitis the only path where in-memory does pure reads under ashared_lock: every calling thread reads the same in-process structure concurrently, no write lock, no network, no serialization. So it scales with cores — 1.67M (t16) → 3.91M (t64) — and exposes the in-memory ceiling. Redis, by contrast, pays a network RTT + RESP + Lua per op regardless, and each slot executes single-threaded; reads cap in the tens of thousands. That "in-process concurrent read" vs "per-op network/protocol tax" difference is the two-to-three orders of magnitude.match's per-hash hit-count bump, orreport's insert — in-memory takes aunique_lockand serializes:match/reportsaturate at t16 (~55k / ~27k) and no longer scale with threads. That drops in-memory from its ~3.9M read ceiling down into the same tens-of-thousands band where redis (RTT-bound) already sits, so the gap collapses. In short:match_nohitreveals the real backend gap;match/reportlook close only because in-memory is throttled by its own write lock.Cluster now ≈ multi-endpoint (scales with masters); dragonfly trails. It's the count of serving processes, not the protocol — see the footprint column:
tand only peaks at t256 (match_nohit 63,113 — actually the top pure-read there). But cross-proactor dispatch + fan-out barrier + high per-request latency (Match p50 ≈16ms @ t256) make it far less core-efficient (~0.35k/core vs multi's ~4k/core); once a write is in the op (Match), it peaks at only ~11k.reportis the one workload where in-memory is not the ceiling (~27k, at/just-below multi-8's ~28–31k, reproducible): the write serializes on a singleunique_lock, and becausereportinserts fresh hashes the keyspace grows → periodicunordered_maprehash under that lock stalls every thread (p99 14–68ms though p50 is 27µs), capping throughput ~27k. multi-8 instead spreads the writes across 8 independent processes (8-way parallel, no shared lock), so it ties or edges past in-memory here.Deployment headline: unlike
heartbeat(dragonfly ~0.4k, store-wide global lock), the external-KV write does NOT collapse on dragonfly (11.3k @ t64, 14.0k @ t128) — the sharded write scripts declare their keys inKEYS[]and run per-slot (lock-ahead) instead of taking the global lock.Reproduce (external-KV microbench)