Refactor/umbp drop standalone client - #620
Merged
TianDi101 merged 10 commits intoSep 1, 2026
Merged
Conversation
Three things a distributed client without a master could not do, all of which the local backend did, and all of which have to work before it can replace that backend. Eviction. Evict() on a peer backend is a MASTER decision -- it picks victims with a cluster-wide view and calls through the peer service -- and nothing else in the system calls it. A node with no master therefore never frees a page: the pool fills and Allocate answers NO_SPACE from then on, where LocalStorageManager would have evicted on a watermark. PageBackend now carries that watermark loop, in the shape SsdBackend's manager already uses: an LRU inserted at Commit and renewed at Resolve, a round on the committing thread when usage reaches the high watermark (and on a NO_SPACE allocate, the one state a commit-time round cannot have prevented), freeing oldest-first down to the low watermark and skipping leased or copy-pinned keys exactly as master-driven Evict does. It is opt-in -- PoolClient::Init enables it precisely when it builds no MasterClient -- because with a master a second policy would free keys master still indexes and hands out routes for. The LRU is maintained only when it is on, so the cluster read path pays nothing. The outbox. Every backend queues an ADD or REMOVE per operation and the only consumer is MasterClient's heartbeat. With no master nothing drains it, so the outbox grew by one entry per put, forever -- a leak on the one deployment with no reason to keep one. SetEventPublishing(false), under the same condition, drops what is queued and stops recording. Ranged I/O. The scratch arenas are a remote-path resource, but SupportsRangedIO() required them unconditionally, so a masterless client reported no ranged support however local its work was -- and every ordinary ranged miss logged a scratch-arena ERROR on its way to returning the correct answer. Both now short-circuit when there is no master: nothing can route, so nothing can be remote. Watermarks come from UMBPDramConfig, which is where the local backend read the same policy from.
A UMBPConfig that names no deployment used to select a third client class. It now gets a distributed client with the deployment filled in: WithEmbeddedDefaults synthesizes identity, leaves the master address, IO engine host and peer service port empty, and selects DRAM. Each omission removes a subsystem rather than disabling one -- no MasterClient is built, MoriIoEngine is never constructed, no gRPC server is bound -- so what remains is one medium plus the local copy engines, which is what the local backend was. What makes a deployment embedded is the absence of a master, not a different implementation. DRAM rather than a medium inferred from ssd.enabled: that flag defaults to true, so inferring would quietly turn every unconfigured caller into an SSD node. A node serves one medium (see UMBPMedium), so serving SSD is an explicit choice, and configuring an SSD tier that will not be served warns. DRAM+SSD on one node is the multi-backend work, not this path. The pool is paged, so a value smaller than a page still occupies one. The default is 2 MiB, matching every other deployment; a caller storing KV pages should name the page's exact byte size the way the distributed deployments do, and one storing small values wants it smaller. UMBP_EMBEDDED_DRAM_PAGE_SIZE reaches it for callers that configure nothing else. UMBPDeploymentMode::Local is kept, and kept at 0, so out-of-tree callers that name it still compile and STANDALONE_BACKEND_LOCAL keeps its meaning on the wire. Nothing returns it.
Local was never a third kind of client -- it was a deployment. With the
factory synthesizing an embedded deployment for an unconfigured config,
the class that used to serve it has no callers, and neither does the
storage stack private to it: LocalStorageManager, LocalBlockIndex,
DRAMTier and the DRAM->SSD CopyPipeline.
What stays is the tier layer under local/: SSDTier, ShardedSsdTier, the
SPDK tiers, the segment log and HostMemAllocator. PeerSsdManager builds
the distributed SSD medium on exactly those classes, so this deletion
takes the client and its bookkeeping, not the storage.
Three capabilities go with it, all deliberate:
* DRAM+SSD tiering inside one process. UMBP's routing plane does not
tier within a node, so a node serves one medium; tiering across media
is the multi-backend work.
* Depth-aware (prefix-aware) eviction. DistributedClient already
discarded the depth hint -- master-as-advisor stopped tracking
per-key depth -- and this was the only implementation left.
* SharedSSDLeader / SharedSSDFollower. A shared pool is a deployment
now (a master, or a standalone server), not a role a private client
plays over a shared directory. UMBPRole and follower_mode stay as
inert compatibility fields so existing callers still compile and
UMBP_ROLE is still accepted; the distinction survives where it
belongs, in SSDTier's ReadOnlyShared access mode.
Consequently the standalone server always has a distributed backend: it
resolves the deployment itself, so the config it keeps describes the
backend it built, and its read-concurrency and GPU-IPC gates key on the
live medium rather than on ssd.enabled -- a flag that defaults to true
and now describes a tier the backend may not serve.
The server also stops demanding a node identity and an IO engine host
when it has no master, since without one nothing registers, nothing
routes and no peer can dial in. UMBP_DISTRIBUTED_MEDIUM alone is
therefore a complete configuration for a local SSD server, and
UMBP_SSD_ENABLED without a medium is now an error rather than a server
that quietly serves DRAM.
Tests follow the same split: the local-client and DRAM-tier suites are
gone, the SSD-tier suites are ported onto SSDTier directly (including the
shared-reader case, which is what the follower role tested), and the
ranged bench's local arm now deploys the same client embedded -- so it
measures a deployment rather than a second implementation.
The standalone-process design doc listed three deployment "shapes" keyed on which client class the factory returned; two of the three are now the same class with a different configuration, so the table names the configuration instead. The env-var reference gains what actually changed for an operator: UMBP_MASTER_ADDRESS is optional and is the switch that decides whether a backend evicts for itself and records heartbeat events at all; UMBP_SSD_ENABLED no longer selects a medium; a masterless server needs no node identity or IO engine host; and UMBP_EMBEDDED_DRAM_PAGE_SIZE sizes the pages of a deployment that configures nothing else.
A paged pool smaller than one page holds zero pages, and then every put fails with NO_SPACE -- silently, because a failed put is a legal answer rather than an error. The 2 MiB default therefore broke any caller whose pool was smaller than that, which the deleted local backend served fine by allocating exact sizes. WithEmbeddedDefaults now shrinks the page until the pool holds at least eight of them, and says so. Found by test_standalone_shm_ipc, whose server has a 1 MiB pool. That test also encoded two things this branch changed: it asserted the server's backend reports Local, and it stood up an "SSD-backed" server by setting ssd.enabled. Serving SSD is a medium now -- a server can carry SSD sizing and still serve DRAM, on which GPU IPC is perfectly fine -- so the test names the medium instead of the flag.
Every gRPC surface that talks to the MASTER set its message limit to 64 MB. Every surface that talks to a PEER left it at gRPC's 4 MiB default. Nothing made that visible, because the limit was written at each call site rather than shared -- four sites remembered, four did not, and the four that did not are all on the peer path. EvictKey is the message that outgrows it. An eviction round frees a FRACTION of the tier (high 0.9 -> low 0.7), so the victim list scales with the deployment while the limit does not: a 512 GiB tier selected 155,157 victims, which is 18.7 MB of `repeated string`. The receiving peer refused it whole -- protobuf has no partial delivery -- on all 81 rounds of a 30-minute run. Not one key was freed, the tier stayed full, and writes then failed with 2,088 NO_SPACE. Under the same load with no master, where the backend evicts for itself, there were zero. Two things follow, and neither is sufficient alone. The limit moves into common/grpc_limits.h as one value, read once per process, overridable with UMBP_GRPC_MAX_MESSAGE_BYTES (default 64 MiB, floor 1 MiB -- below that, registration itself starts failing, which presents as a node that cannot join rather than as a size limit). All eight channels and servers now apply it through the same two overloads; there is no longer a SetMax* literal in the tree. This lowers no existing limit: 64 MiB is what the master surfaces already used. Raising a ceiling does not remove a wall, though, since the batch grows with the tier and the limit is fixed. So EvictionManager splits its per-node list into as many EvictKey calls as it takes, sized from the actual key lengths rather than an assumed per-key constant. Splitting is safe here in a way it is not for most batch APIs: peer Evict is idempotent, master state is unchanged at dispatch (REMOVE events on the peer's next heartbeat shrink the index), and the round re-runs every 5 s. N chunks therefore reach the state one message would have, and a round that fails partway still makes progress instead of freeing nothing. The cost is that dispatch is synchronous, so a round can now take up to N x UMBP_EVICTKEY_DEADLINE_MS against an unresponsive peer. At the default limit the 155,157-key round is still a single call, so nothing changes unless the limit is lowered; dispatching chunks concurrently would remove even that, and is left alone because it changes this class's threading model for a failure we have not seen. Verified against the list that failed: at 4 MiB it now splits into 5 chunks of 31,031 keys, each under the ~34.8k the limit allows; at 16 MiB, 2 chunks; at the default, 1.
umbp_bench.py's local backend names its deployment explicitly, because a --tier ssd run that let the factory choose would silently measure DRAM. That also means it never reaches WithEmbeddedDefaults -- the path a caller that configures nothing actually takes, and the one this branch added. UMBP_LOCAL_FACTORY_DEFAULTS=1 leaves the config unnamed so the factory fills it in. DRAM only, since that is what the factory picks; asking for another tier with it set is an error rather than a quietly wrong run.
FindContinuousFreeRun and CollectFirstNFree both restarted their linear scan at index 0 on every Allocate. That is O(total_pages) per call no matter how much is already allocated, and it degrades as the pool fills because the first free run moves rightward. Invisible on a coarse pool, crippling on a fine one. sglang's direct linker requires the pool page to not exceed the smallest per-layer object -- 1728 bytes for DSv4-Pro -- so a 64 GiB tier is 39.8 MILLION page entries, and a remote put allocating 31 pages per object walked a growing fraction of them every time. Measured on an 8-rank embedded deployment with a master, 256K prompt at 100% hit, same node and session: PUT total 73.3 s -> 18.3 s mean call 905 ms -> 215 ms remote share 85.4% -> 48.2% commit share 29.7% -> 0.3% 256K load 8517 ms -> 453 ms The commit collapse is the clearest signal: that phase is the wait loop, and it was long only because peers were still grinding their own scans. Fix the scan and the waiting goes with it. A resume cursor per buffer, with a wrapping second pass. Correctness is unchanged: the two passes together still visit every page, so a run is found exactly when the old scan would have found one. Only WHICH free pages come back differs, and no caller depends on that -- the allocator's contract is "n free pages", not "the lowest n".
TianDi101
force-pushed
the
refactor/umbp-drop-standalone-client
branch
from
September 1, 2026 06:38
5f3c2ec to
7c59b72
Compare
The remote arena loop called CopyRangesToContiguous (put) and CopyContiguousToRanges (get) once PER KEY, and each of those is a blocking transfer_engine_->Transfer -- its own kernel launch and stream synchronize. The local path has always accumulated items across every request and transferred once; the remote path now does the same. Measured 369 transfers carrying 369 segments, 14.5 us apiece for ~5 us of copy, which starved the gather kernel to 30 segments per launch; batching took it to 817 segments per launch and 1.24 -> 16.64 GiB/s. Both copy helpers are now thin wrappers over item-builders, so the single and batched paths cannot drift. Objects are tagged so one unassemblable object fails alone rather than failing the batch, and a partially appended object rolls back -- both properties the per-key call gave for free. Honest about the size of this one: assembly turned out to be ~0.5% of a remote put, so end-to-end it lands inside run-to-run noise. It is a correctness-preserving cleanup that makes remote match local, not the fix for the regression -- that was the allocator scan.
PageBackend held its allocator as a concrete PageBitmapAllocator, so the page-allocation strategy for both paged media was welded to one implementation. Introduce PagePool, hold the allocator as that interface, and make PageBitmapAllocator the default implementation. No behaviour change: same bitmap, same next-fit cursor, same numbers. Only DRAM/HBM get this seam, deliberately. The SSD side already has its plug-in point one level up in TierBackend, with five implementations, and its allocation models would not fit this contract anyway -- the SPDK OffsetAllocator hands out variable-size contiguous byte extents from a single space, and the segment log is an append cursor reclaimed by whole-segment GC, not per-record free. A single interface over all three would be their union, which is not a contract. This one earns its keep because the opposite holds here: one implementation today, several credible ones (bitmap, binned free-list, slab, hierarchical bitmap), and exactly one call site to switch them at. The interface deliberately does NOT re-export Buffers(). PageBackend needed only each buffer's extent to build its TransferRefs, so it now asks NumBuffers()/BufferPageCount() instead of borrowing a reference to the bitmap's own BufferState vector. Handing out that reference would have tied the interface to one implementation's bookkeeping and made a free-list or slab awkward to write. Buffers() stays on the concrete class for diagnostics. Two contracts are written down rather than left to be rediscovered: implementations hold no internal lock (the caller serializes -- PageBackend::mutex_ on the peer, ClientRegistry::mutex_ on master), and a failed Allocate must leave the pool exactly as it found it, because callers treat nullopt as "no space" and do not clean up after it. Why now: the page allocator is on the measured critical path. A page sweep on DSv4-Pro at 256K/100% hit, 8-rank TP, shows load cost tracking pages-per-object almost exactly -- halving the page to 864 B doubles the cost in all four deployment combos (1.92x-2.25x), and one page per object at 53,568 B is 3.2x-6.5x better than the 1,728 B that sglang's linker picks by default. That makes the allocation strategy worth being able to replace, which is what this commit buys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TianDi101
force-pushed
the
refactor/umbp-drop-standalone-client
branch
from
September 1, 2026 07:02
7c59b72 to
3f7ebe0
Compare
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.
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist