feat(umbp): Redis/RESP master metadata store backend#468
Open
isytwu wants to merge 37 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>
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 224-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). Best config per backend.Store-direct microbench (
bench_umbp_master_metadata_store, no gRPC/RDMA)RouteGet (read)
index_shards=1proactor_threads=32, block_shards=64, pool=256Heartbeat (write)
Reproduce (build + backends + microbench)
Full-link, 8-process proxy (
umbp_master+ Router/gRPC; GETMODE=exists, no RDMA)8 client processes (each own conn pool + gRPC channel + heartbeat) hit one master. Best config per backend; cells = get QPS / BatchLookup p50 (ms):
index_shards=1The 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), while in-memory keeps scaling. dragonfly stays low regardless of proactor count — the mixed workload's heartbeat writes take a global store-wide lock. (route_get+RDMA, GETMODE=both, is RDMA-fetch-bound ~3–6k for all backends and flaky here, so BatchLookup is the stable read signal.)
Reproduce (full-link proxy)
Takeaways
proactor_threads, 43k) but write collapses to ~0.5k (global store-wide lock per write script; also drags the full-link path via heartbeats). Read-heavy / light-write only.