refactor(umbp): make distributed mode backend- and transport-agnostic - #540
Open
TianDi101 wants to merge 31 commits into
Open
refactor(umbp): make distributed mode backend- and transport-agnostic#540TianDi101 wants to merge 31 commits into
TianDi101 wants to merge 31 commits into
Conversation
First step of the backend-agnostic refactor (see src/umbp/doc/design-backend-agnostic-refactor.md): cuts SSD out of the distributed PoolClient/PeerServiceServer/routing path so the data plane collapses to one shape (async RDMA page slots) ahead of generalizing it into a MediumBackend interface in later phases. - Delete the SSD read-staging lease coupling: ssd_read_lease.h, PrepareSsdRead/ReleaseSsdLease RPCs (proto tags reserved, not freed), the peer-side read-slot state machine, StagingMetrics, and the SSD metrics block in master_metrics.h. - PoolClient no longer builds a PeerSsdManager/SsdCopyPipeline or an SSD staging buffer; BatchGetPlan collapses from 4 buckets to 2 (remote_groups/local_indices). - PeerServiceServer's constructor drops to (dram_alloc, engine_desc_bytes, master_client). - distributed_client.cpp advertises DRAM-only tier_capacities — nothing serves SSD anymore, so advertising it would route into a dead path. - PeerSsdManager/SsdCopyPipeline move under distributed/peer/ssd/, stay compiled and tested (dormant), and PeerSsdManager drops its OwnedLocationSource base while keeping the same-shaped methods for a future SsdBackend adapter. - Delete the coupling-only tests (test_peer_ssd_read_rpc, test_ssd_read_lease_gating, test_ssd_reliability, test_peer_service — the latter's entire content turned out to be SSD lease RPC tests). Verified: umbp_common/umbp_master/umbp_client/umbp_standalone_server build clean; ctest 35/35 pass, including the local SSD suites and the 3 dormant adapter tests unmodified, plus the real-RDMA integration tests (umbp_cross_node_smoke, umbp_pool_client_batch_put). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…boundary
Adds include/umbp/distributed/peer/medium_backend.h: MediumBackend, its
value types, and BackendRegistry. Nothing implements it yet; the gate is
that the tree compiles.
Also revises the plan doc, which was missing the criterion that decides
where code belongs and so had allocation and registration sitting outside
the abstraction they belong to:
- §1 gains a fourth coupling: memory ownership lives entirely outside the
backend. Allocation is in DistributedClient, registration in
PoolClient::Init with MemoryLocationType::CPU hardcoded, the local copy
is a host memcpy over a buffer table with no tier dimension. Adding HBM
today needs four call-site edits before the registry line, which is what
put Phase 5's acceptance test out of reach.
- §2 (new) states the rule: descriptors are medium-agnostic, raw pointers
are medium-specific, registration is the conversion and belongs to
whoever allocated. This is why the remote path already works against an
HBM peer unmodified, and why the local fast path does not.
- §3 drops byte movement from the interface and records the three things
proposed and rejected (LocalCopyIn/Out, a staging pool, a "not ready"
resolve state) so they are not re-added.
- §4 (new) abstracts the transfer engine inside UMBP, with mori-io as one
unmodified implementation. Registration and transfer share one
TransferEngine type; CompositeTransferEngine fans registration out across
transports. Bounce buffers live here because this is the only layer that
can observe completion.
- §5 splits Phase 2 into map-split plus ownership move, gates real HBM on
the new Phase 6, and keeps the local fast path untouched until then.
- §8 (new) records what none of this fixes: Location{node_id, tier} cannot
describe shared media, so S3/3FS need control-plane work.
Effort: Phase 2 2-3 -> 4-5 days, Phase 6 added at 4-6, SSD re-add drops to
1-1.5 now that the staging pool moved to the transfer layer.
Verified: header is self-contained and warning-clean under the project's
clang++ flags, clang-format clean, umbp_core/umbp_common build unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PeerDramAllocator held an internal map<TierType, ...> but served exactly one
live tier: PoolClient::Init default-constructed an empty hbm_cfg, so HBM, its
routing rank and the tier map were unexercised scaffolding. Invert that map —
one backend instance per medium — and relocate the generality to a registry
where it is exercised.
PeerDramAllocator -> PageBackend : MediumBackend
Every method loses its TierType parameter (the tier is now the identity of the
object), which makes the file smaller.
Ownership moves down with it. Allocation, RDMA registration and capacity
advertisement lived outside the "backend", so it was a bookkeeper over memory
it did not own: DistributedClient called HostMemAllocator, PoolClient::Init
registered the buffer with MemoryLocationType::CPU hardcoded, and capacity was
a {DRAM, {size,size}} literal. PageBackend::Init(TransferEngine*) now
self-allocates its pool with its own hugepage/NUMA policy and registers it,
choosing the location type itself. Consequently:
- PoolClientConfig::dram_buffers and ExportableDram are deleted; no buffer
pointer crosses into PoolClientConfig
- BuildDramTierConfig is deleted
- tier capacities are aggregated over BackendRegistry::Capacity() after Init
instead of being passed in. This also drops the old mapped_size-vs-
allocatable-tail discrepancy: capacity is now bitmap-derived, so master's
view and the allocator's agree by construction.
TransferEngine is a ~30-line shim over IOEngine::RegisterMemory for now; Phase 6
replaces it with CompositeTransferEngine and no backend changes, which is why
Init's signature takes it from the start.
MockBackend is registered for HBM so the registry dispatches to more than one
backend. It advertises zero capacity, so routing never selects it and DRAM
behavior is unchanged.
Phase gate: DRAM behavior unchanged with a second backend registered; no buffer
pointer in PoolClientConfig. ctest 36/36, including the local SSD suites and
the 3 dormant SSD adapter tests Phase 0 requires to stay green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… go agnostic
The peer service held one typed PageBackend* and served every RPC from it, so
it could only ever talk to DRAM. It now holds a BackendRegistry* and dispatches
on the request's tier tag; no concrete backend type is named in peer_service at
all (it no longer even includes page_backend.h). Same for PoolClient's local
paths and MasterClient's heartbeat.
Commit/Abort carry no tier on the wire. Every backend numbers its slots from 1
independently, so a bare slot_id is ambiguous the moment a second medium is
live. umbp_peer.proto documents the id as opaque and echoed back, so the peer
now tags the tier into its high byte and strips it on return: dispatch works
with no proto change and no client change. The tag is peer-local — the local
fast path talks to a backend directly and never sees a tagged id.
Resolve and Evict carry no tier either, and are handled by shape rather than by
assumption: Resolve walks this peer's media and takes the first hit, Evict fans
out to every medium and sums the freed bytes, so a mirrored key is dropped
everywhere instead of only from DRAM.
MediumBackend absorbs OwnedLocationSource, which is deleted along with
AddOwnedLocationSource/owned_sources_ (design doc §3). MasterClient aggregates
capacity, owned-key counts, events, the clear gate and the auto-flush hook over
the registry, so a newly registered backend participates in the heartbeat with
no change there. Its aggregation tests move to test_mock_backend.cpp.
Single-key RPCs are served by one-element batches, so there is no separate
single-key path on the interface to keep in sync.
Two limits stay, deliberately, and are documented where they are decided:
- GetPeerInfo and BatchResolveKeys can express only ONE medium per response
(dram_memory_descs is a flat buffer_index space, dram_page_size a single
field). A second buffer-owning backend is reported, not silently merged
into a colliding index space. The tier dimension is a Phase 6 prerequisite.
- The local fast path still memcpys, so it needs a raw base pointer that
MediumBackend deliberately does not expose; PoolClient keeps one concrete
handle for it, deleted in Phase 6.
Also fixed while here, all three found by review of Phase 2:
- PageBackend::Init leaked RDMA MRs when a later buffer's allocation failed:
the unwind cleared owned_mem_descs_ without deregistering, and owns_memory_
never became true so Shutdown() could not clean up either.
- The local copy loop took the backend's allocator mutex once per page (the
same lock every Allocate/Commit/Resolve and the heartbeat snapshot contend
for). Bases are immutable after Init, so they are snapshotted once.
- MockBackend is no longer registered in the production PoolClient::Init. It
was inert only because nothing dispatched by tier; with Phase 3 an
HBM-tagged request would have reached it and "succeeded" with no pages and
no descs, publishing a key backed by nothing. Registry dispatch is proven
by tests instead.
New test_peer_service_dispatch.cpp drives the real gRPC surface with two
backends registered: per-tier routing, the tier-tagged slot_id round trip,
mixed-tier batch ordering, and the read-walk / evict fan-out.
Phase gate: bench_pool_client_batch_get throughput unchanged (measured against
the last committed state; deltas within run-to-run noise). ctest 37/37.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rders
Two tier orders were the last medium-specific knowledge in the routing plane:
TierPriorityRouteGetStrategy's HBM > DRAM > SSD, and put's kPutTierOrder =
{HBM, DRAM} with SSD excluded as a non-direct-put target.
Scope decision: delete them, do not replace them. The plan was to re-express
both as an advertised BackendProperties {read_rank, put_eligible}. But every
medium live in the system today is equivalent, so an advertised order would be
scaffolding nothing exercises — the same mistake §1 calls out about
PeerDramAllocator's unexercised tier map. So BackendProperties,
MediumBackend::Properties() and BackendRegistry::ByReadRank() are gone too; the
last was redundant with All(), which already iterates in a deterministic
ascending-TierType order.
TierPriorityRouteGetStrategy -> LocalPreferringRouteGetStrategy
It keeps the requester-local preference — the thing that makes
cache_remote_fetches pay off, since a node that re-cached a block reads its own
copy with no RDMA — and drops the ranking. The old name described only the
half that was deleted. RoutePut now scores every (node, tier) pair that has
room, on free space alone.
EvictionManager's `if (tier == SSD) continue` goes with them. It existed
because EvictKey only ever reached the peer's DRAM allocator, so evicting on an
SSD overload would have dropped the DRAM copy instead; since Phase 3 EvictKey
fans out to every backend by key, so an overloaded medium now evicts from
itself. The guard was a special case justified by a condition that no longer
held.
Four behavior changes, all invisible while DRAM is the only live medium:
- a requester's own replica wins even on a "slower" medium (was: remote
faster tier)
- on a node with HBM 10G / DRAM 400G, a put picks DRAM (was: HBM)
- a tier that is the only one with room is used, including SSD (was:
unroutable)
- under same-node affinity, a spill stays on the anchor node and changes tier
(was: jumped to a remote node's faster tier)
When SSD returns as an SsdBackend it must re-assert put_eligible=false itself:
the router no longer assumes it. Noted in the design doc's re-add path.
Phase gate, revised: the original — "test_tier_priority_route_get and
test_route_put_strategy pass unmodified" — cannot hold, because those tests ARE
the hardcode. test_tier_priority_route_get is replaced by
test_local_preferring_route_get, and 6 assertions across the put/get suites are
inverted; each inverted test names the expectation it replaces so the diff reads
as a deliberate contract change. Everything else passes untouched: ctest 37/37,
and bench_pool_client_batch_get is unchanged within run-to-run noise.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last medium-specific knowledge outside a backend was PoolClient's byte
moving: it held a raw base pointer per DRAM buffer and memcpy'd through it, so
the local path was host-DRAM-only, PoolClient had to name a concrete backend
type to obtain those pointers, and a second live medium would have needed a tier
branch in the copy loop. There is now exactly one byte-moving path, behind
TransferEngine, and engine selection is a function of the (src, dst) pair.
transfer/transfer_engine.h TransferRef, MemoryRegistrar, PeerDirectory,
TransferItem/Plan/Handle, TransferEngine
transfer/mori_io_engine RDMA; owns mori::io::IOEngine
transfer/local_copy_engine both endpoints local; NT-AVX2 block copy
transfer/composite_* fan-out registration, per-pair dispatch
mori-io is NOT modified, per §6: UMBP owns TransferRef, so mori-io never has to
learn a non-memory endpoint and its other consumers see no blast radius.
Moved out of PoolClient into MoriIoEngine, each because it had to know about the
wire: GroupTransfersByPair (now Plan()), the bounce buffer and its mutex, peer
engine registration, and the peer buffer-descriptor cache that used to live in
PeerConnection. RemoteDramScatterWrite/Read and GroupPagesByBuffer are deleted
outright — dead since the batch path landed, and a second byte-moving path is
what §6 forbids.
MediumBackend::LocalBufferViews() -> BufferRef(buffer_index) + BufferCount()
That is the change that lets the local path fold in. A TransferRef is
medium-agnostic (§2); a raw base pointer is not. ExecuteLocalPut/Get now build
TransferItems and hand them to the same planner as everything else, and
PoolClient drops local_copy_backend_, local_buffers_, CanCopyLocally,
LocalPutPages, LocalGetPages and LocalCopyBlock.
Four deliberate deviations from §4, each argued in the design doc:
- TransferRef is a struct of merged per-transport handles, not a variant.
Registration fans OUT — the same buffer is a raw pointer AND an RDMA MR at
once — and a variant makes the local path inexpressible. mori::io::
MemoryDesc already resolves this the same way (ipcHandle beside
fabricHandle).
- no FileRef/ObjectRef and no RegisterFile. No engine consumes one yet, and
§3 already refused an abstraction nothing exercises once (BackendProperties).
SsdBackend brings its own, as a one-file change to this header.
- Wait lives on the handle, not the engine: the RDMA backend holds raw
TransferStatus* into the handle's status vector, so the drain-on-early-
destroy safety net has to sit with the statuses.
- the schedulers stay in PoolClient. Submit-all-then-wait spans several
Submit calls with local reads in the gap, and UMBP_DRAM_{READ,WRITE}_THREADS
parallelize across the keys of a batch — a batch is not a concept the engine
has. What is in the base class is Transfer(), for callers with no overlap.
Phase 5 Rules A and C close here, as §5 predicted, both as types rather than
lint. MemoryRegistrar (register/deregister) and TransferEngine : MemoryRegistrar
(+ CanHandle/Plan/Submit) mean "a backend must not move bytes" is a compile
error; deleting LocalBufferViews() removed the last concrete-typed call, so
PoolClient::Init builds through MakePageBackend() -> unique_ptr<MediumBackend>.
Reaching mori-io's peer handshake needed a MoriIoEngine* at first; those six
calls became the PeerDirectory interface, so a second remote transport
implements an interface instead of editing pool_client.cpp. PoolClient::Init is
now the only file naming any concrete backend or engine. Rule B's lint tool is
still outstanding — it is not in tree.
Two behavior changes, both improvements, both asserted by tests:
- a batch mixing registered and unregistered caller buffers works (was: a
contract violation, failed wholesale)
- a staged batch larger than the bounce buffer is chunked into pool-sized
round trips (was: the whole peer batch failed)
Both fall out of moving staging into the engine. The old code reserved the
batch's staging up front and held one mutex from submit to wait, so a submit-all
over several staging peers would deadlock — hence permit_staging, the all-zc-or-
all-staging contract, and the two-armed fork in ExecuteBatch{Put,Get}Plan. All
gone: a plan needing the pool completes INSIDE Submit, so the lock is never held
across a return. test_cross_node_smoke's PutStagingOverflowFailsBatchCleanly
asserted the old behavior and is replaced by
PutStagingLargerThanPoolIsChunkedNotFailed plus
PutPageLargerThanStagingPoolFailsBatchCleanly — the failure that is still a
failure is a single page larger than the entire pool, which cannot be chunked.
Fixes a pre-existing correctness bug found while restructuring the path:
ExecuteLocalGet returned kSuccess for a key no local medium held, reporting a
HIT with an untouched dst. Reachable — PartitionBatchGetTargets sends a key
master has no route for down the local path as a fallback — and the caller
cannot tell the difference, so it hands stale bytes to its own caller. Now
returns kRetry, plus a resolved.size != size guard matching the remote path.
Layout: §3's three components are three directories, so the boundary is visible
in the tree and the Phase 5 lint rules can be scoped by directory rather than by
filename list. distributed/transfer/ is a SIBLING of peer/, not a child: the
include graph says so, since peer_service — the reason peer/ exists — references
the transfer layer zero times (a peer hands out descriptors; the initiator moves
the bytes). transfer/ depends on nothing but types.h while backend/ and
pool_client depend on it, so nesting the lowest layer inside a higher-level
sibling would invert the layering, and LocalCopyEngine settles it from the other
side — a memcpy between two of this node's own buffers has no peer in it.
backend/ does belong under peer/: PeerServiceServer dispatches every
Allocate/Commit/Resolve/Evict into BackendRegistry, and capacity, eviction and
the heartbeat outbox are statements about what this node holds for the cluster.
Also deletes distributed/pool_allocator.h: the pre-page-model byte-offset
allocator, superseded by PageBitmapAllocator and with no includer since 158c7e8.
New test_transfer_engine.cpp covers the planner (grouping, coalescing, bounds
rejection) and composite selection without gRPC or RDMA; MoriIoEngine stays
covered end-to-end by test_cross_node_smoke. ctest 38/38.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Delete src/umbp/include/umbp/common/error_code.h — never included anywhere in the repo. - Delete src/umbp/include/umbp/distributed/obs_counters.h — its macros (MORI_UMBP_OBS_INC/ADD/TEST_VIRTUAL) are never invoked. - Remove MasterClient::SetPeerSsdManager and the ssd_manager_ field — Phase 0 scaffolding for SSD capacity reporting that was never wired up; SsdBackend now reports capacity through the standard MediumBackend/BackendRegistry path in SnapshotAndCacheTierCapacities, making this dead. - Collapse the duplicate ctest target in tests/cpp/umbp/local/CMakeLists.txt: test_umbp_local_eviction and test_prefix_aware_eviction both built from the same source file and ran the same tests under two names; keep the latter, whose name matches the source file per this CMakeLists' own convention. Verified via full umbp_core/umbp_common rebuild + ctest (39/39 passing, was 40/40 before collapsing the duplicate target) inside the umbp-refactor-ditian12 dev container.
* feat(umbp): HBM + SSD backends, HbmCopyEngine, and the two how-to skills
Exercises the Phase 1-6 backend/transfer abstraction by actually adding a
medium and an engine through it, and writes down the resulting recipe. Two
findings came out of doing it rather than describing it.
FINDING 1: the interface was right, its one implementation was not.
MediumBackend was already medium-agnostic, and PageBackend's class comment
already claimed to serve "DRAM or HBM" — truthfully for the slot lifecycle,
bitmap allocator, event outbox, reaper, read leases and copy pins, none of
which consult the tier. But Init/Shutdown/BuildBufferRefs hardcoded
HostMemAllocator, MemoryLocationType::CPU and device=-1, so the claim did not
survive contact with a second medium.
PageMemorySource Allocate/Release/LocationType/Device/Name
Five methods holding exactly what differs per medium. HostPageMemorySource is
the old inline body, moved; the OwnershipConfig constructor now funnels into
the PageMemorySource one, so DRAM and HBM share ONE Init path rather than two
kept in step. LocationType/Device are the load-bearing pair: PageBackend
mirrors them into every published TransferRef, and that is what selects the
engine. Getting them wrong fails at neither build nor init.
So HbmPageMemorySource IS the HBM backend — no HbmBackend type, ~100 lines,
zero duplication of the 800 that already worked.
FINDING 2: HBM's local path was unservable by any engine in tree.
LocalCopyEngine requires BOTH endpoints loc == CPU
MoriIoEngine `if (src_remote == dst_remote) return false;`
A both-local pair with a GPU endpoint was claimed by nobody, so an HBM
backend's local Put/Get could not complete at all. The REMOTE side already
worked unmodified, exactly as §2 predicted ("why the remote path already works
against an HBM peer, and why the local fast path does not") — this closes the
local half.
HbmCopyEngine H2D + D2H + D2D, hipMemcpy, ~200 lines
No new TransferRef field needed: host_ptr is documented as the "process-local
view", and for hipMalloc'd memory the device pointer IS that view. The three
engines now partition (src, dst) with no overlap, so composite order stays
documentation rather than a tie-break — asserted, not assumed. Synchronous by
choice: the parallelism that matters is across keys and already exists in
PoolClient's executors, and a shared stream would need its own synchronization
for no win at KV-block sizes. Restores the caller's current device, since
Submit runs on threads that are not ours to leave re-pointed.
SSD: staged, and deliberately NOT a FileRef.
transfer_engine.h reserves a file endpoint for SsdBackend. Not taken. SSD bytes
are not addressable, and closing that gap in the transfer layer means a new
TransferRef kind, a PosixFileEngine, AND chaining in CompositeTransferEngine
for the remote reader — which that class explicitly does not implement. Instead
SsdBackend publishes ordinary registered host DRAM and spills behind it:
Allocate lends a staging page, Commit spills it to PeerSsdManager and returns
the page, Resolve fills a page under a read lease and the reaper reclaims it.
Cost is one host copy per side; benefit is SSD reaching the data plane with
ZERO transfer-layer change, and a remote peer needing no code at all. A FileRef
backend stays the right answer for GDS; this does not block it.
PeerSsdManager needed one accessor (SizeOf) — PrepareRead picks its staging
buffer before the read, so the reader must know the size while holding only the
key. Its header anticipated this adapter; Phase 0's dormancy ends here.
Two honest limits, both pinned by tests so a fix changes them deliberately:
- one key, one page. PrepareRead takes a single (ptr, cap), not a scatter
list. Fine while master's page_size IS the KV block size.
- staging exhaustion degrades a Get to found=false, which makes the client
retry another peer for a key this node does hold. This is the "not ready,
retry here" state medium_backend.h records as proposed and rejected.
Skills, written from what the work actually required:
.claude/skills/umbp-add-backend two shapes: 5-method
PageMemorySource, or full
MediumBackend + staging
.claude/skills/umbp-add-transfer-engine pair dispatch, disjointness,
Plan/Submit/Wait, PeerDirectory
Both carry the contracts with no compile-time protection (one event bundle per
seq, full-sync clearing the outbox in the same critical section, kFailedNoSpace
vs kFailed, Evict's positional results) and the build's real constraint: umbp
needs protoc/grpc_cpp_plugin from the mori image, not the bare host.
Verified: ctest 38/38 (36 pre-existing + 2 new), including test_page_backend
and test_transfer_engine unchanged. test_hbm_backend runs real H2D/D2H/D2D and
a Put/Get round trip on an MI355X; test_ssd_backend runs 21 cases against a
real posix SSD tier. clang-format clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(umbp-bench): --tier/--dst-loc flags and HBM wiring through DistributedClient
Migrated from the worktree-umbp-bench-tier branch (commit b634ae4a plus the
uncommitted work that sat on top of it).
- UMBPHbmConfig (enabled/device/capacity_bytes) on UMBPDistributedConfig,
lowered by ToPoolClientConfig into PoolClientConfig::hbm, so PoolClient::Init
registers an HBM PageBackend. Unlike dram/ssd there is no ownership struct to
thread through: hipMalloc has no hugepage/NUMA/prefault dimension.
- IUMBPClient::RegisterMemory grows loc/device describing the CALLER's
allocation, so a GPU-resident src/dst routes through HbmCopyEngine instead of
being assumed host memory. StandaloneProcessClient rejects non-CPU rather
than silently mis-registering.
- umbp_bench.py: --tier {dram,hbm,ssd}, --dst-loc {host,gpu}, --hbm-device.
DeviceBuffer allocates the read destination via raw HIP (ctypes on
libamdhip64) so --tier hbm --dst-loc gpu exercises the D2D hipMemcpy path
without pulling in a torch dependency.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(umbp): per-backend buffer indices (backend_id) for multi-medium peers
Migrated from the uncommitted work-in-progress on feat/umbp-hbm-ssd-backends
in the main checkout, so HBM/SSD can actually be benchmarked: PoolClient::Init
always registers DRAM, so --tier hbm/ssd produces a two-backend node, which is
exactly the case this fixes.
buffer_index is backend-local, but EnsurePeerServiceConnection folded every
backend's buffers into one flat dram_memory_descs list. One medium's buffers
were published, the rest unreachable -- yet HasRemoteBuffers still went true,
so the next resolve asked the peer to omit descriptors and the missing media's
pages were read against the published medium's memory. Now each desc names its
backend, and Build{Put,Get}Transfers snapshot per backend the batch touches
rather than once per peer.
pool_client.{h,cpp} carried changes from both this work and the bench-tier
migration; both are preserved (SlotPlan::backend_id plus the loc/device
RegisterMemory signature).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(umbp): wire the SSD tier through DistributedClient, and stop --tier lying
SsdBackend was already registered by PoolClient::Init from
PoolClientConfig::ssd -- the only thing missing was DistributedClient lowering
UMBPConfig::ssd into it, so the tier stayed dark however it was configured.
The bench's "SSD is unwired" refusal was describing the old PeerSsdManager
world and is now wrong, so it is gone.
Opt-in is UMBPDistributedConfig::enable_ssd_tier, deliberately NOT
UMBPConfig::ssd.enabled -- that defaults to true, so keying off it would make
every existing distributed deployment start advertising SSD capacity.
Also fixes the benchmark measuring the wrong medium: DRAM is always
registered, and routing has no tier order (kMostAvailable picks whichever
medium has the most free bytes), so --tier hbm against the 8 GiB DRAM default
and a 4 GiB HBM pool routed every put to DRAM and labelled it HBM. Non-DRAM
tiers now shrink DRAM to a 256 MiB floor so most-available can only pick the
requested medium.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(umbp-bench): size the SSD staging arena, and let tier runs disable re-cache
Two things that made a tier benchmark measure something other than the tier.
SSD reads stage through a registered host arena of ssd_staging_buffer_slots
pages; the default 16 caps read concurrency at 16 keys, and a slot stays
pinned for UMBP_SSD_READ_LEASE_MS when the reader's best-effort
ReleaseSsdLease is lost. A 1024-key sweep therefore returned hits=17
misses=1007 -- the overflow degrades to MISS, not to a slow hit, so it reads
as a correctness failure rather than a capacity limit. ssd_backend.cpp says it
directly: "Sizing the arena for read concurrency is the mitigation." Slots and
arena bytes are now env-tunable and default to 512 / 2 GiB, which takes the
same sweep to 1024 hits / 0 misses / 0 mismatches.
cache_remote_fetches defaults on, and the reader gets its own pool of the tier
under test, so after pass 1 most reads were served from the reader's own
memory instead of RDMA out of the writer's tier. UMBP_CACHE_REMOTE_FETCHES=0
pins every pass to a genuine remote read; the default keeps production
behavior.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…g::medium (#543) PoolClient::Init registered DRAM unconditionally and then added HBM/SSD beside it, so `--tier hbm` or `--tier ssd` produced a two-backend node. That reads like a tier stack and is not one: since Phase 4 the master treats every advertised tier as an equally valid put target, so the second medium is mirrored into by free capacity rather than sitting behind the first. A node's keys split across two pools for no gain, and a tier benchmark ends up measuring DRAM. Replace the per-medium `enabled` flags with a single selector. A node picks exactly one of DRAM / HBM / SSD; heterogeneity comes from different nodes picking differently, which the routing plane already handles. Adding a medium is now a `case` in one switch, not another `if`. - UMBPDistributedConfig::medium (new UMBPMedium enum) replaces enable_ssd_tier and UMBPHbmConfig::enabled. Defaults to DRAM, so an existing deployment is bit-identical. - UMBPConfig::Validate checks only the selected medium's sizing; the unselected blocks are ignored, not validated, so one deployment template can carry all three. - PoolClient caches the live tier as medium_ and exposes Medium(). Also removes the two DRAM literals the design doc listed as outstanding: - the re-cache installer hardcoded TierType::DRAM, which would have installed into a tier an HBM/SSD node does not have; - PartitionBatchPutTargets filtered remote routes to DRAM/HBM, silently dropping every put master routed to a peer's SSD even though SSD publishes ordinary registered staging pages. New test CrossNodeBatchPutLandsOnSsdPeer covers exactly this. Tests: 20/20 umbp ctest targets pass (incl. the new test_umbp_medium_selection: lowering, per-medium validation, one-backend registry, and the cross-node SSD put/get). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…izes nothing The DistributedClient init line printed "staging=<ssd_staging_buffer_size>MB/<slots>slots", but ssd_staging_buffer_size does not size the SsdBackend arena — that is staging_pages * page_size. A run with 2048 slots and a 2 MiB page logged "staging=6144MB" against a real 4096 MiB arena, i.e. it advertised capacity the node did not have. SsdBackend's own "[SsdBackend] Init ... arena_bytes=" line is authoritative and already correct. Print the slot count alone, since that is the number an operator needs: it is the SSD medium's read-concurrency limit, and a BatchGet wider than it degrades to MISSES rather than backpressure (ssd_backend.h reports staging exhaustion as found=false). Observed on n06-21: a 2048-key BatchGet against a 2048-slot arena all-missed in 4ms, and a 128-key sweep missed 49%, while batch<=32 was a clean 0-miss ~190 MiB/s. ssd_staging_buffer_size is now referenced only by the pre-refactor PeerSsdManager path; leaving it in place, but it should not appear in a line describing the live SSD backend. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CORRECTION to the previous commit's message: it blamed the SSD misses on
staging-arena exhaustion. That was wrong. The arena logs a warning on every
page-acquisition failure and the failing run had zero of them; reruns at the
same scale, the same batch width, and the same 2048-slot override all passed
with 0 misses. (The log-line fix itself stands — the printed number really was
wrong.)
The actual cause is here: _sync_recv_until defaulted to a flat 600s wait for
the reader's DONE. A 4 GiB SSD sweep reads at ~190 MiB/s and takes ~14 minutes,
so the writer gave up mid-run, and its exit killed the umbp_master it spawned
(PR_SET_PDEATHSIG) and unregistered its keys. The still-running reader then
missed every subsequent read. Timeline from the failing run: WRITER_READY at
07:51:34, writer gone at 08:01:34 — exactly 600s — and the reader's misses
began in the leg covering that moment. DRAM and HBM finish in seconds and never
reached it, which is why only SSD "failed".
The failure was invisible from both ends: the writer logged no error and never
printed READER_DONE, and the reader reported it only as hits=0 in a RESULT
line. So this does two things:
- The reader sends a keepalive every 30s for the whole read phase, so the
writer's wait is an IDLE timeout rather than a run-duration budget. No
number to tune per medium: the reader proves liveness however slow it is.
Sends are serialized with the final DONE under a lock, since interleaved
sendall() from two threads could split the token.
- A timeout now raises with a specific message naming the idle window,
instead of dying into a wall of unexplained misses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
segment::CrcUpdate was a bitwise, table-less CRC-32/ISO-HDLC -- 8 shift/XOR per byte, ~152 MB/s on one core -- run over every byte of every record on both the read and the write path. On the SSD tier it dominated everything else: the checksum, not the drive, set the tier's ceiling. Two changes, both confined to the segment layer: * CRC-32C via the SSE4.2 crc32 instruction, with __builtin_cpu_supports runtime dispatch and a byte-identical portable table fallback. The fallback matters for correctness, not just portability: a segment written on a host with hardware support must still verify on one without, so both paths are pinned to the same reflected polynomial (0x82F63B78) and tested against an independent bitwise reference. ~152 MB/s -> ~13.4 GB/s. No -msse4.2 on the translation unit; the dispatch is per-call and predicted. * Split segment::Writer::Prepare into Build (checksum + record assembly) and Reserve (index reservation). SSDTier now runs Build outside mu_ and only Reserve under it, so a write batch no longer blocks concurrent reads on the same drive for the whole of its CRC + copy time. Prepare is kept as Build+Reserve for callers already holding the lock. Build leaves `generation` zero and Reserve patches it in place, since it is the one header field that does not exist until the reservation is taken. kRecordVersion 1 -> 2: the polynomial changed, so v1 checksums must not be verified with the v2 routine. The scanner's existing version check drops them and the segment refills -- this tier is a cache, so its contents are always re-fetchable. Tests: new test_segment_crc pins the standard CRC-32C check value 0xE3069283 (which also catches an accidental revert to ISO-HDLC, whose check value is 0xCBF43926), verifies the selected path against an independent bitwise reference at every tail length mod 8 and at unaligned starts, checks streaming composition, and covers the Build/Reserve generation stamping. test_ssd_tier passes unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tic SSD path (#554) - Add multi-drive SSD backend
- add arena auto sizing for ssd backend benchmark
- Port tree connector changes
Remote BatchGetRanges and BatchPutRanges shared one scratch arena and a single mutex held across the whole network round-trip, so a remote ranged get and put serialized against each other. Allocate a separate RDMA- registered arena and mutex per direction (DistributedClient owns both, each sized by ranged_scratch_size) so load/offload overlap instead of blocking: mixed remote get+put throughput improves ~60% at 4-8 threads; same-direction behavior is unchanged. Also adds concurrency correctness tests: a separate-arena remote put-then-get round-trip and a concurrent get/put no-clobber check.
DistributedClient's constructor allocated the GET and PUT scratch arenas then threw on failure without freeing them. HostBufferHandle is not RAII and the destructor/Close() do not run on a constructor throw, so a partial allocation (get succeeds, put fails) leaked the get arena. Free both before throwing (Free() is a no-op on an invalid handle). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e and transfer Replace the per-medium counter mechanism (MediumBackend::Counters(), keyed by an SSD-specific header and a hand-copied dashboard JSON) with a MetricSource / MetricPublisher abstraction: an InstrumentedBackend decorator derives generic ops/bytes/latency series for any storage backend from the MediumBackend interface, and CompositeTransferEngine now measures its own per-engine bytes, plans, failures, and in-flight time at the one point that knows which transport carried which plan. A medium or transfer engine no longer needs its own metrics code or its own dashboard panel — tier/backend/ engine become labels instead of separate metric identifiers. Dashboards follow that split rather than collapsing into one. Only the backend-specific dashboard is merged: umbp_ssd_tier.json becomes umbp_backends.json, whose panels group by tier and engine so every medium and every transport shares them and a new one appears with no dashboard change. The four backend-agnostic dashboards — client data rate/bandwidth, RPC and call rates, master client RPC latency, and the external-KV index — measure the client, the master and the KV index, look the same whatever medium is underneath, and are left exactly as they were. All five stay plain Grafana JSON, editable in the UI and exported back. Also fix MetricsServer::addCounter truncating fractional deltas (e.g. *_seconds_total) by taking uint64_t deltas instead of double, and switch value serialization to print exact integers instead of ostream's six-significant-digit default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ranged (sub-object) I/O was DRAM-only: SSDTier had no override, so the
TierBackend all-false default made every SSD key look like a miss, and
LocalStorageManager sent ranged reads to DRAM unconditionally. With an SSD
tier configured, StandaloneClient refused ranged I/O outright and reported
SupportsRangedIO()=false, which is what made the sglang tree connector
reject ssd_enabled=true.
The v3 record layout already suits this: a value is one contiguous extent at
a kRecordAlign-anchored offset, padded to the same boundary, so a range is
just value_offset + object_offset handed to pread.
SSDTier::ReadBatchRangesIntoPtr resolves keys under mu_, then plans outside
it. Ranges contiguous in the object are merged into runs and served by a
single device read -- the shape a layer-group load produces, since
consecutive layers of a component are adjacent in the stored object. Runs go
out as one io_driver_->ReadBatch, with a per-run fallback if the driver
rejects the batch, and the scatter out of any bounce buffer fans across
tier_io_threads.
A run reads straight into the caller's buffer when it is a single host range
with, under O_DIRECT, an aligned offset, buffer and length. Otherwise it
bounces through an aligned window; the widening cannot escape the record
because values are padded on disk. Device destinations always bounce and
lift with DeviceCopy: the tree connector registers its device KV buffers
over IPC, so the resolved destination really can be GPU memory.
Checksums are not verified on this path. A record CRC covers the whole
value, so a partial read has nothing to check itself against; per-block
checksums would be a record-format change. Whole-object reads keep
verifying, so the guarantee narrows only for callers that opt into ranged
I/O.
LocalStorageManager now routes ranged reads by holding tier, the way
ReadBatchIntoPtr already did, and issues at most one call per tier per pass
-- including the stale-hint retry. Batching is not cosmetic here: it is what
lets the tier coalesce runs, submit one io_uring batch, and fan out across
drives.
Measured on n06-21 (Kioxia NVMe, ext4, O_DIRECT, io_uring), 61 layers x
36KiB, 512 pages, destinations laid out layer-major:
fetched ranged vs whole-object
1.6% 30.3x
6.6% 9.7x
13.1% 3.8x <- UMBP_LAYER_GROUP=8 default
26.2% 1.9x
52.5% 0.9x
100.0% 0.6x
Crossover is ~30-35% fetched, against ~12.5% previously measured on DRAM.
The loss at full coverage is the mandatory bounce and scatter; a caller that
needs per-layer destinations pays that either way.
Adds TierCapabilities::ranged_read so routing is capability-driven rather
than tier-hardcoded, a gtest suite covering both buffered and O_DIRECT (set
UMBP_TEST_SSD_RANGES_DIR to a block-backed path -- /tmp is usually overlayfs
and skips), and tests/python/umbp/umbp_ranged_bench.py.
Distributed SSD is unchanged: DistributedClient::SupportsRangedIO() still
excludes an SSD medium, whose reads stage whole objects through SsdBackend.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A batch put larger than the DRAM tier demotes its own earlier keys while it runs. MoveKey records LOCAL_SSD for those keys in the very index StandaloneClient consults -- and then BatchPut walked the results and stamped every written key CPU_DRAM, erasing it. Afterwards the index claimed the whole dataset was in DRAM when most of it was on SSD. Every later read then missed on the hinted tier and fell through to the slow path: per-key for whole-object reads, and (before the batched retry landed) per-key for ranged reads too. It cost roughly 5x on a 512-key batch and was invisible, because the fallback still returns the right bytes. Found while benchmarking: the tier log showed keys=1 per call on a batch of 512. With the fix the same run reports keys=498 runs=498, and the whole-object GET of a 1.1GiB dataset drops from 167ms to 91ms. Only claim CPU_DRAM for keys the write path did not already place. BatchPutRanges had the same pattern and gets the same guard. BatchPutWithDepth does not: it writes and indexes one key at a time, so a demote triggered by a later key cannot be overwritten by an earlier one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DistributedClient::SupportsRangedIO() answered false whenever the node's medium
was SSD, so on a pure-SSD deployment -- where every node sets medium: SSD -- it
was false cluster-wide and the sglang tree connector fell back to whole-object
I/O for every request.
The gate rested on a backend nobody built. Its comment justified itself by
saying ranged access maps object ranges onto pages a backend publishes as
in-process endpoints while SSD publishes storage refs instead. SsdBackend does
not do that: ssd_backend.h weighs a file endpoint against staging and picks
staging, so it publishes ordinary registered host pages and a resolved SSD key
reaches BuildLocalRangeTransfers looking exactly like a DRAM one.
All four ranged paths therefore already worked and were being refused:
- local get -- BatchResolve stages, ranges copy out of the staging page
- remote get -- ExecuteBatchGetPlan, which pool_client.cpp already documents
as reaching "any medium the owning peer publishes --
including SSD"
- local put -- BatchAllocate a staging page, write the ranges into it,
BatchCommit spills it; a ranged put tiles its object, so the
one whole-object write was always going to happen
- remote put -- assembled in the scratch arena, sent contiguous
The scratch arena remains the sole opt-in and still defaults to zero. Note the
flag is a declaration, not a guard: the ranged entry points forward to
PoolClient unconditionally, so this changes what the client advertises rather
than unblocking a code path. The tests are written to the conjunction -- each
asserts SupportsRangedIO() before exercising bytes -- because a test that only
drove the data path would pass against the old gate and pin nothing. All six
fail against it as written, including one that covers the standalone-server
hop where the capability crosses a Ping.
UMBP_DISTRIBUTED_RANGED_SCRATCH_BYTES was documented in neither
runtime-env-vars.md nor pure-ssd-mode.md, so the feature was reachable only by
reading standalone_server_main.cpp. Both now carry it.
umbp_bench.py (whole-object, distributed, Mooncake) and umbp_ranged_bench.py
(ranged, local, single process) each covered one corner and neither covered
two UMBP processes over RDMA. Now one script, four backends x three commands:
BACKEND umbp | umbp-local | umbp-server | mooncake
COMMAND correctness | batch_perf | ranged_perf
`umbp` and `mooncake` keep their exact CLI, so existing invocations -- the
mooncake-vs-umbp-bench skill's included -- still run verbatim. `umbp-local` is
the in-process StandaloneClient (no master, no network: the baseline that says
whether a distributed number is bounded by the medium or by the transport) and
`umbp-server` forwards to a standalone UMBP server, the deployment the tree
connector actually runs.
ranged_perf times ranged against whole-object over the same keys in the same
process and reports the RATIO per fetched fraction, which cancels the load of a
shared node in a way absolute MiB/s does not. It verifies fetched bytes before
timing anything: a fast wrong answer is the failure mode ranged I/O makes easy
and it is invisible in a bandwidth number.
SSD knobs (--ssd-dir with comma-separated multi-drive sharding, --direct-io,
--verify-crc, --io-backend, --queue-depth, --tier-io-threads, --segment-bytes,
--dram-bytes) are lowered onto the UMBP_* names UMBPBackend already reads
rather than into a second config path, so a flag and its env var cannot drift
and nothing bypasses the staging-arena sizing warnings that make an SSD number
trustworthy. An explicitly exported env var still wins.
Three fixes to existing bench behaviour found while porting:
* writes were read back before the master had them. A peer publishes ADDs on
the heartbeat and the size-based auto-flush only trips at
UMBP_AUTO_FLUSH_EVENT_THRESHOLD (default 128) puts -- and on SSD not even
then. Datasets under 128 objects came back as a TOTAL miss that looks
exactly like a broken read path; the old default of 2048 always cleared the
threshold, which is why nobody hit it. write_all() now polls to a barrier.
* start_server spawned umbp_master without the build tree on
LD_LIBRARY_PATH, dying as an unexplained "failed to start".
* HostBuffer.__del__ raised "Exception ignored" per buffer at shutdown.
n06-21, umbp_master + two standalone servers, object 32x36KiB. Same drive, same
access shape, only the read path differs:
umbp-local, 1 drive (true partial pread) 6.96-9.83x FASTER at 3.1% fetched
umbp-server over RDMA (stages whole object) 0.23-0.30x
umbp-local, 3 drives sharded every ranged read MISSES
So D1 is worth roughly an order of magnitude and the drive has already shown
it. Two things this turns up that change the plan, both recorded in the design
doc: the distributed ranged cost is per key and never amortizes (~0.69 ms/key
at batch 8/32/128) which makes X1 a prerequisite rather than a cleanup; and
multi-drive is not merely unoptimised but broken, since ShardedSsdTier neither
forwards ReadBatchRangesIntoPtr nor advertises ranged_read while
supports_ranged_io() still reports true on that node.
ctest: 51/51 (umbp_local_client's GPU case fails identically at the base commit
and is unrelated).
Add BatchPutRanges/BatchGetRanges e2e bandwidth histograms as their own metric families rather than folding them into the whole-object BatchPut/ BatchGet series -- a ranged call moves a subset of each object, so sharing a family would make bytes-per-call meaningless. Bytes counted are the range bytes delivered to (or committed from) the caller's buffers, matching what the sglang tree connector reports on its side. ExecuteLocalPutRangesBatch now optionally accumulates actually-committed bytes: a key already present in the medium succeeds without moving anything, and crediting it would inflate the histogram. Also add two opt-in debug facilities, both off by default: UMBP_RANGED_CALL_DEBUG splits a ranged call into resolve / route / xfer / lock / remote phases, with a per-component rollup keyed by object size. This is what shows the get and put paths are not symmetric: a get resolves locally and only routes on a miss, while a put issues BatchRoutePut on every call including fully-local ones. HbmCopyEngine records a per-plan GatherSkip reason so the log can say both which path carried a batch and why the other declined; the hot path never computes it unless UMBP_HBM_COPY_DEBUG is on. Grafana: four ranged bandwidth panels (local/remote put and get) plus two global aggregates in umbp_data_rate_bandwidth.json. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
argparse runs help text through %-formatting, so the bare "99%" in the two_bucket description raised ValueError on --help. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…stall counter Two follow-ups to #589, both code-health rather than behaviour. ExecuteRemoteBatchGetPlan was introduced as "the submit-all/wait-all half split out of ExecuteBatchGetPlan", but nothing was actually taken out of the latter: the two carried the same reserve, the same submit loop and the same wait loop, differing only in whether run_local() sat between them. Give the shared one an optional in-flight window and have ExecuteBatchGetPlan pass run_local into it. The overlap it exists for is preserved -- local reads still run after every peer is posted and before any is waited on -- and the ranged callers, which have no local half by construction, pass nothing. MORI_UMBP_METRIC_RANGED_REMOTE_INSTALL_FAILURES_TOTAL had narrowed to one branch. It used to fire on every failed install on the ranged path; after the locality rework it fired only from MaybeInstallCompleteArenaObject, which is now the least-taken of the three routes. A slot the medium refused, a slot that is not one addressable run, a refused commit, and every failure of the background pull were all silent apart from a debug log -- that is, the counter named for "the object did not become local" stopped covering most of the ways that happens, while the feature it measures grew two new ones. Route every such case through one helper. The background pass reports its total in one go rather than per key, since it is there to say how much locality is being lost, not to trace individual keys. Also drops commit_index, written and never read since the pull was batched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A read is two questions -- "who holds this key" and "give me the bytes".
When this node holds the key itself, its own backends answer the first,
and the routing RPC only confirms what a local resolve already knew. On a
single node that answer can never differ, so the round trip is pure
latency.
Reads now resolve locally first and contact the master ONLY for the keys
this node missed; a fully-local batch issues no RPC at all. A local hit is
conclusive (the backend owns the bytes), so this cannot invent a hit; a
local MISS is not conclusive, which is why the misses still go to the
master. Gated by PoolClientConfig::local_first (default on,
UMBP_DISTRIBUTED_LOCAL_FIRST), so route-first ordering can be restored.
BatchGetRanges already worked this way; the flag now gates it too, so all
three read paths share one shape.
The local half is batched on both axes: one BatchResolve per BACKEND, not
per key -- that mutex is shared with Allocate/Commit/Evict, and in
standalone-process mode every rank shares this client -- then ONE Transfer
for the whole batch, tagged by key index so the engine attributes a
failure to its key. This matters more than the saved RPC: a per-key local
half measured 1.4-1.6x SLOWER than route-first at batch >= 32 despite
issuing no RPC at all. Batching it, with no ParallelFor, made local-first
win at every batch size and let ExecuteLocalGet go away entirely. The same
core now serves the route-first arm, which is 1.1-1.4x faster for it.
master_config.master_address may now be empty. That is the single-node
deployment: the whole data plane -- backends, transfer engine, peer
service -- is built exactly as in a cluster, but nothing is routed,
registered or heartbeated, and a local miss is the final answer.
GetDeploymentMode() still reports Distributed; this is not a fallback to
the Local backend, and config alone decides which deployment a process is.
Puts place on this node, external-KV degrades to a no-op, and the metrics
sinks tolerate having nowhere to report.
Measured on one node, same client, 2048 x 1 MiB, best of 3:
BatchGet 20,751 -> 105,072 req/s at batch 128 vs the local backend
Exists (present) 3.5x - 34x faster than route-first
Ranged get constant ~50 us/call saved, 1.08x - 2.06x
Exists (ABSENT) 2.2-2.5x penalty with a master, gone without one --
on one node that lookup can never change the answer
Also fixes DistributedClient::Flush, which dereferenced Master()
unconditionally and would have null-derefed on a master-less client.
The eviction gap is known and deliberate: the only trigger is the master's
EvictionManager driving the peer's EvictKey, so a long-running master-less
node fills its pool and puts begin failing with kFailedNoSpace.
Tests: test_umbp_local_first (4) asserts the no-RPC claim by STOPPING the
master and reading anyway, with a local_first=false twin that must fail on
the same data; test_umbp_no_master (9) covers put/get, exists hit and miss,
ranged get and put, external-KV, and both deployments from one client
class, starting no master anywhere so reaching for one hangs or crashes
rather than passing quietly. test_ssd_ranged_io's publication barrier now
polls the reader rather than the holder: under local_first the holder
answers Exists from its own backend a heartbeat before the master knows,
so polling it stopped waiting for the thing that test needs.
umbp_bench.py gains exists_perf (present/absent populations, probed on the
holder as well as the reader) and UMBP_LOCAL_FIRST.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ement Lets one peer own several named backends and route between them, instead of serving exactly one medium. A peer registers up to 16 backends. Placement is a PoolPolicy (single, weighted, or tiered) compiled from a JSON logical-tier graph: one entry tier, each backend in exactly one tier, offload only to a later-declared tier. Watermark offload and promote-on-hits run as background transitions (plan under the pool lock, copy without it, finish under the lock). Reclaim demotes on eviction so the key stays reachable; discard drops every copy. Ranged put/get now allocate, commit, resolve and promote through PeerPool, so tree-connector traffic is placed by the policy and participates in watermark offload and promote-on-read. Previously those calls bypassed the pool, so a loaded policy was inert for the production I/O path. Follow-ups from review that landed here: ranged APIs, and a trigger-based promote policy (promote_trigger / promote_hits / promote_mode) instead of a boolean promote_on_read. Post-rebase production fixes: multi-page SSD staging, retry of transient SSD resolve pressure, SSD read-lease knobs independent of DRAM, Redis persistence of peak member utilization, and forwarding migration pins through InstrumentedBackend so metrics wrapping does not disable tier transitions. Verified with umbp_peer_pool (placement, migration, reclaim vs discard, master-to-peer eviction) and a 900s DeepSeek-V4-Pro E2E on eight GPUs across 256D, 192D+320S, 128D+640S, 128D+4x160S and 128D+128S. Hybrid arms complete transitions rather than spinning. Lock-granularity work is left for a follow-up, as discussed in review.
refactor(umbp)!: delete StandaloneClient, make an unconfigured client embedded UMBP had two client implementations for one job: StandaloneClient owned a private local storage stack, DistributedClient did the same work through peer backends but required a master. A DistributedClient with no master address is now the no-master deployment -- same backends, nothing routed or heartbeated, a local miss is final. The local stack is deleted (21 files, ~12k lines). Three things it could not do before, all of which the local stack did: - Evict. Evict() is a master decision, so a masterless pool filled up and then answered NO_SPACE forever. PageBackend now runs its own watermark loop. - The heartbeat outbox. Only MasterClient drains it, so with no master it grew one entry per put, forever. Publishing is off under the same condition. - Ranged I/O. SupportsRangedIO() required the remote scratch arenas, so a masterless client reported no ranged support and logged an arena ERROR on every ordinary miss. Both short-circuit when there is no master. Also: embedded page size fits the pool, one shared gRPC message limit with EvictKey chunked to it, next-fit page allocation, batched remote ranged assembly, and a PagePool seam under the DRAM/HBM allocator. BREAKING CHANGE: StandaloneClient and the umbp/local storage stack are removed -- construct a client with no master address instead. UMBP_SSD_ENABLED no longer selects a medium; use UMBP_DISTRIBUTED_MEDIUM or UMBP_BACKEND_POLICY. Cut the "Also:" paragraph too if you want it shorter — the three bullets are the part that justifies the deletion, so I'd keep those last.
5 tasks
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
4 participants
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.
Makes UMBP's distributed mode backend- and transport-agnostic, so adding a storage medium or a byte-moving path is a registration rather than an edit spread across the data plane.
Before this, four couplings meant a second medium could not be added without touching unrelated code: the peer allocator carried an internal
map<TierType, ...>, the routing plane hardcoded two tier orders,PoolClientmemcpy'd through raw per-buffer base pointers (so the local path was host-DRAM-only), and memory ownership lived entirely outside the "backend" — allocation inDistributedClient, registration inPoolClient::InitwithMemoryLocationType::CPUhardcoded.The two interfaces
MediumBackend— a backend OWNS bytes and PUBLISHES DESCRIPTORS for them. It does not move them. One instance == one medium, so no method takes aTierType; the tier is the identity of the object andBackendRegistrydispatches on it.TransferEngine— there is exactly one byte-moving path and it is here. Engine selection is a function of the(src, dst)PAIR, never of either endpoint alone.The rule connecting them: descriptors are medium-agnostic, raw pointers are medium-specific, and registration is the one-way conversion — which belongs to whoever allocated. That is why the remote path already worked against an HBM peer unmodified while the local fast path did not.
Phases
MediumBackendinterface + the backend/transfer boundaryPhase 5 has no commit of its own: it was the lint rules, and two of the three closed as types rather than lint in Phase 6 —
MediumBackend::Initreceives aMemoryRegistrar(which has noSubmit), so "a backend must not move bytes" is a compile error, and deletingLocalBufferViews()removed the last concrete-typed call soPoolClient::Initbuilds throughMakePageBackend() -> unique_ptr<MediumBackend>. Rule B's lint tool is still outstanding and is not in tree.Transfer layer (Phase 6)
mori-io is NOT modified. UMBP owns
TransferRef, so mori-io never has to learn a non-memory endpoint and its other consumers see no blast radius.transfer/transfer_engine.h—TransferRef,MemoryRegistrar,PeerDirectory,TransferItem/Plan/Handletransfer/mori_io_engine— RDMA; ownsmori::io::IOEnginetransfer/local_copy_engine— both endpoints local; NT-AVX2 block copytransfer/composite_*— fan-out registration, per-pair dispatchFour deliberate deviations from the design doc's §4 sketch, each argued in the doc:
TransferRefis a struct of merged per-transport handles rather than a variant (registration fans out — the same buffer is a raw pointer AND an RDMA MR at once, and a variant makes the local path inexpressible); noFileRef/RegisterFileuntil a backend consumes one;Waitlives on the handle, not the engine (the RDMA backend holds rawTransferStatus*into the handle's status vector); and the schedulers stay inPoolClient.Behavior changes
Two improvements, both asserted by tests:
Both fall out of moving staging into the engine — a plan needing the pool completes inside
Submit, so the lock is never held across a return, which removespermit_stagingand the all-zc-or-all-staging contract entirely.Also fixes a pre-existing correctness bug found while restructuring:
ExecuteLocalGetreturnedkSuccessfor a key no local medium held, reporting a HIT with an untouched dst. Reachable, and the caller could not tell the difference, so it handed stale bytes upward. Now returnskRetry.Verified
ctest36/36 with-E '^cco_' -LE integration. Newtest_transfer_engine.cppcovers the planner and composite selection without gRPC or RDMA;MoriIoEnginestays covered end-to-end bytest_cross_node_smoke, which isintegration-labeled and needs a real fabric, so it has not been run here.Design doc:
src/umbp/doc/design-backend-agnostic-refactor.md— §2 the descriptor/pointer rule, §3 the three-component split, §4 the transfer engine, §8 what none of this fixes (Location{node_id, tier}cannot describe shared media, so S3/3FS need control-plane work).Follow-up
A branch on top of this (
feat/umbp-hbm-ssd-backends) exercises the abstraction by adding an HBM backend, an SSD backend and anHbmCopyEnginethrough it, and is held for a separate PR.🤖 Generated with Claude Code