fix(umbp): retry eviction demotions instead of dropping the only copy; count what is dropped by TianDi101 · Pull Request #763 · ROCm/mori

TianDi101 · 2026-10-13T15:35:53Z

Part of the #753 split; stacked on #741 (base feat/umbp-evict-converge). Retarget to main once #741 merges.

With #741 an eviction demotes asynchronously: the evicted DRAM copy is the only copy until it lands on SSD. The DRAM 300 GB → SSD 1.5 TB end-to-end run showed two ways that copy was lost.

1. A transient demotion failure dropped the key

Symptom: activity summaries showing (N moved, M failed); each failed demotion discarded the evicted copy.
Fix: TierTransitionResult::retryable, set for copy, commit and read-back failures; the transition worker re-queues retryable eviction demotions.

2. "No space" at the SSD target dropped keys readers had just found

Symptom: UMBP get failed … success=2364/2501: 137 keys lost in one 30 s window, each just after a reader had been told it was there.
Cause: for an SSD target, "no space" only means the staging arena is momentarily full of in-flight reads, yet the worker dropped the copy; a count-capped retry burned out in milliseconds.
Fix: no-space is retryable. Eviction demotions retry for a wall-clock budget from their first attempt (UMBP_EVICT_DEMOTION_RETRY_MS, default 30 s), pausing 20 ms only when nothing else is queued. A copy is still dropped once the budget is spent, or at once if the target can never take it (shape failure), so the tier still frees.

3. Count what was given up on

failed counts every failed attempt, including ones a retry rescues. New TierTransitionMetrics::dropped, exported as mori_umbp_client_tier_transitions_total{outcome="dropped"}. New retried counts eviction demotions re-queued after a retryable failure (outcome="retried"). With #683 in main, the INFO activity summary shows both: (N moved, M failed, R retried, K dropped).

Testing

Full tests/cpp/umbp suite: 51/53; the two failures are the known RDMA start-up aborts under ctest -j 8 (umbp_cross_node_smoke, umbp_pool_client_ranges). umbp_peer_pool passed 10/10 repeats.
New tests: a failed demotion is retried until it lands and dropped once its budget runs out; a demotion waits out a target with no room for now. The existing no-room-downstream test now uses a short budget and checks dropped.
End-to-end: passed on test/umbp-evict-e2e-v2 @ 2e1663d91 (main + feat(umbp): converge eviction onto one path; nodes evict locally by default #741 + fix(umbp): SSD staging no longer fails local loads; a local read holds its bytes until it is done #762 + this + feat(umbp): serve metrics from a masterless node, and summarise offload/load/evict/promote/demote at INFO #683): 1.47M demotions; 5,048 demotion failures (SSD staging full) all retried until they landed; 0 copies dropped. Details in the e2e comment below.

🤖 Generated with Claude Code

Eviction is decided in three places (master EvictKey, PageBackend's masterless round, PeerSsdManager's after-write round) and freed in two more ways than the medium's Evict (the page round's own copy of it, and the SPDK daemon's EvictLRU). The master's request also drops the tier it measured, so the peer frees by placement and can delete copies nobody asked about. The doc lays out the target: the master names (key, tier); PeerPool::Evict is the one routing entry point both the master and a background local eviction worker call; MediumBackend::Evict is the only code that frees a committed key; nothing evicts on the put path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rees that copy The master picks victims per (node, tier) but shipped only keys, so the peer freed by placement: on a tier without on_evict offload it deleted the key from every backend, and with copy-mode promotion (key in DRAM and SSD) SSD pressure freed the DRAM copy and left SSD as full as before. - EvictionVictim{key, tier}: SelectVictims and EvictKeyDispatcher carry the tier each victim was charged to. - EvictKeyRequest gains `repeated TierType tiers = 2`, parallel to keys. An older peer ignores it; an older master sends none and the peer frees each key from the medium that currently holds it. - PeerPool::Evict takes PoolEvictRequest{key, tier, backend_id} and frees exactly one copy: demote it when it is the placement and its tier has a downstream (any trigger -- on_evict vs watermark now only says whether the pool also offloads proactively), otherwise drop that copy only. Drops run outside the pool lock; lifecycle_mutex_ keeps ClearLocal out. - A transition that lands on a parked (draining) copy promotes it to the durable one, so a later eviction cannot delete it as still-draining. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rough PeerPool::Evict Before this, a committed key could be freed four ways: the master's EvictKey (through PeerPool and the medium's Evict), PageBackend's masterless round (its own copy of Evict's body, inline on commit), PeerSsdManager's round (inline after every SSD write, plus evict-then-retry on a failed write), and tier transitions. The two inline rounds bypassed PeerPool -- no on_evict demotion, stale placement, invisible to InstrumentedBackend -- and the page round ran before the pool's watermark check, so on a masterless node it deleted the cold keys the watermark offload was about to move. Now: - MediumBackend::Evict is the only code that frees a committed key for capacity. PageBackend::MaybeEvictToLowWatermark and PeerSsdManager::EvictToLowWatermark (with its watermarks, after-write checks and evict-and-retry) are gone; a write that does not fit fails. - MediumBackend::EnableLocalEviction is replaced by ordering hooks that free nothing: TrackEvictionOrder() and EvictionCandidates(max) -> EvictionOffer{key, bytes}, coldest first, skipping leased/pinned keys. - PeerPool::EnableLocalEviction(backend, high, low) starts one background worker. Commits, NO_SPACE allocations and finished transitions only wake it; it takes offers until their bytes cover the overage and calls PeerPool::Evict({key, backend_id}, kReclaim) -- the call the master's EvictKey makes -- so a tier with a downstream demotes instead of deleting. Lowest tier first, 64 keys per call, with a fixed byte budget as well as the live usage check. - PoolClient enables it for every medium without a master (paged media at local_evict_*, SSD at its own watermarks) and for SSD with a master, as before. - A demotion that loses a race to another transition no longer falls back to deleting the source, which could abort that transition and lose the key. - mori_umbp_client_local_evict_total{event=round|key|no_candidate|stalled}; the SSD medium's eviction_round event is gone (rounds are the pool's, keys and bytes are InstrumentedBackend's). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

With ssd_backend=spdk, PeerSsdManager reaches the device through the spdk_proxy daemon, whose SpdkSsdTier retried a failed allocation by dropping its own oldest keys (EvictLRU). Nothing told PeerSsdManager: it kept listing those keys, no REMOVE reached the master, and the master kept routing reads to bytes that were gone. Fragmentation could trigger it well below the SSD watermark. A write that does not fit now fails, as it does on the file tier, and freeing space is the owner's local eviction, which reaches the tier through Evict(). PeerSsdManager is the daemon's only client. Not built by the default dev image (no SPDK); checked with a syntax-only compile against 3rdparty/spdk/include. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The PR description carries the design; the two code comments that pointed at the doc already explain themselves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Same coverage, fewer lines: - the SSD- and DRAM-scoped evicts of a promoted key share one test, as do the stale-tier and backend-scoped evicts; - the read-recency check rides on the drain test, and the bad-config check on the unordered-medium one; - the mismatched-tier-count case rides on the unscoped EvictKey test; - the two "writes never evict" cases share one fill; - dropped VictimCarriesTheTierItWasChargedTo (KeyOverBudgetInTwoTiers checks the tier on both victims) and the master-chain SSD-pressure test (the parked-source chain test checks the SSD victim end to end, and TierScopedEvictFreesOnlyThatMediumsCopy what the pool does with it); - removed the now-unused Keys() and HasRemove() helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Folds #742 into this PR. - Master: a round frees each (node, tier)'s overage in bytes, optionally capped by UMBP_EVICT_MAX_BYTES_PER_ROUND, instead of 32 candidates per bucket -- which freed a few MB per round and took hours to drain a 512 GiB tier's 0.9 -> 0.7 band. The store returns every eligible row and the strategy trims to the budget. - Peer: PeerPool::Evict queues a demotion on the transition worker instead of running the copy inside the EvictKey RPC, so the call never waits on bytes. Each result carries an outcome (freed, demotion queued, in flight). The worker runs an eviction's demotion whatever the tier's own watermark says, and drops the copy if nothing downstream can take it. - Local eviction counts queued demotions toward its budget (a migration pin no longer hides a key from EvictionCandidates), so a pass never queues more than the overage while the copies run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… tests Comments: drop history and restated rationale (the commit message has it). Tests, same coverage with fewer of them: - The two GetEnvUint64 tests become one. - Drop LruMasterEvictStrategy.ABudgetLargerThanAnyRowCapIsMetInOneRound: the strategy never had a row cap (the manager did), and it is unchanged. - The three MasterEvictionBytes tests become one, which also reads the limit through UMBP_EVICT_MAX_BYTES_PER_ROUND. - EvictReturnsBeforeTheDemotionCopyRuns folds into OnEvictMigratesBytesToConfiguredBackend, and QueuedDemotionsCountTowardTheBudget into the local-eviction demotion test (now ATierWithADownstreamDemotesJustTheOverage). - GatedEngine's hold times out after 10 s, so it needs no ReleaseOnExit guard to keep a failed test from hanging teardown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Comment-only. Drops narrative and rationale that restates the code, and the same "the pool decides, the medium only supplies order" paragraph that was repeated across several headers; field and helper docs become trailing one-liners. Added comment lines across the PR: ~400 -> 195. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

With a master, a node can evict at its own watermarks instead of waiting for the master's EvictKey rounds, while still taking its victims from the master's cluster-wide recency. - Master: new GetEvictionCandidates RPC returns a node's keys on one medium (optionally one logical tier), least recently accessed first, until their sizes cover the requested bytes; master-leased keys are skipped. RegisterClient and every Heartbeat carry peer_driven_eviction, and EvictionManager leaves such nodes out of its rounds (in memory only). ClientRegistryConfig::advertise_peer_driven_eviction gates both. - Peer: local eviction asks a candidate source first (at most once per 200 ms per backend), rounds its value sizes up to the backend's pages, evicts through PeerPool::Evict, and covers the rest from the medium's own LRU. In peer-driven mode PoolClient runs local eviction for every medium, at a tiered policy's per-tier watermarks when there is one. - Opt-in: PoolClientConfig::peer_driven_eviction or UMBP_PEER_DRIVEN_EVICTION=1, and only once the master confirms support; otherwise behaviour is unchanged. New metric event master_key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Replaces the peer_driven_eviction switch with PoolClientConfig::eviction_mode (env UMBP_EVICTION_MODE), which says how a node with a master evicts: - local (default): every medium at the node's own watermarks, by its own LRU, without asking the master. - master_ranked: the same, with victims ranked by the master (GetEvictionCandidates), falling back to local LRU. - master: DRAM/HBM wait for the master's EvictKey rounds; SSD still evicts locally (the previous behaviour). A self-evicting node (local or master_ranked) asks the master to leave it out of its rounds; every mode still serves EvictKey, so the master can evict on demand. A node without a master evicts locally as before. Self-evicting nodes use a tiered policy's per-tier watermarks when there is one. Tests that fill tiny tiers on purpose (cross-node smoke, tier benchmark) now run in master mode, the behaviour they were written against. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The transition worker dropped an evicted copy on ANY failed demotion. That is right when nothing downstream has room -- the tier must still free -- but a copy, commit or read-back failure on a target that had room is a fault on the way, and the evicted copy is the only one: dropping it turned a burst of 3,934 transient SSD write failures into 3,934 lost keys in the e2e run. TierTransitionResult::retryable marks those failures; the worker requeues such an eviction demotion (up to 3 more attempts, at the back of the queue, so it runs after the backlog) and drops the copy only after that, or when no target had room. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…opping When a demotion's target answered "no space" the transition worker dropped the evicted copy at once -- the only copy. For an SSD target "no space" only ever means its staging arena is momentarily full of in-flight reads (BatchAllocate does not check device capacity), so under heavy read load the e2e run lost 137 keys in one 30 s window, each of which a reader had just been told was present: its next get failed. No space at a target is now retryable, and a retryable eviction demotion is retried for a wall-clock budget (UMBP_EVICT_DEMOTION_RETRY_MS, default 30 s, from its first attempt) instead of a fixed count, which a short queue burned in milliseconds. A retry pauses 20 ms only when nothing else is queued, so it neither spins nor holds up other transitions. The copy is still dropped once the budget is spent, or at once when the target can never take the key, so a source tier always frees. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

`failed` counts every failed transition attempt, including ones a retry later rescues, so it cannot tell a pressure burst from lost keys. The pool now counts each evicted copy it actually drops -- the demotion could not land within its retry budget, or never could -- as TierTransitionMetrics::dropped, exported as mori_umbp_client_tier_transitions_total{outcome="dropped"}. The activity summary's "K dropped" comes with #683, which adds that summary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Comment-only, plus one anonymous namespace instead of two back to back. The transient- and persistent-failure tests become one (retried until it lands; dropped once the budget runs out). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TianDi101 · 2026-10-13T23:53:44Z

End-to-end validation of the eviction PR chain: passed.

Branch test/umbp-evict-e2e-v2 @ 2e1663d91 = main (097bb470f) + merges of #741, #762, #763 and #683, nothing else. (The #683 merge drops PageBackend's local-evict counters, which #741 removed, and adds #763's K dropped summary line.) Workload: InferenceY single-node agentic (DeepSeek-V4-Pro fp4, TP8 + DP attention, MTP, conc 256, CI run 29816's config) on n06-21, 3600 s profiling. UMBP is a standalone server with no master (so default local eviction), with a 300 GB DRAM tier demoting to a 1.5 TB SSD tier on 3 drives.

	earlier e2e on #753	this chain (exact PR heads)
Input throughput	281,015 tok/s	283,993 tok/s
Prefix hit (HBM + UMBP)	92.03%	91.99%
Failed loads / SSD read or write errors / sglang crashes	0 / 0 / 0	0 / 0 / 0
DRAM→SSD demotions / copies dropped	1.48M / not measured	1.47M / 0
Demotion failures retried until they landed (#763)	not measured	5,048 (SSD staging full), all landed
SSD keys deferred to a later round / staging released early (#762)	2.93M / 60.3M	2.83M / 60.3M
Local eviction rounds / stalls (#741)	19 / 0	19 / 0

The log signatures of the 7 bugs this e2e originally found all stay at 0: whole-batch staging failures, UMBP get failed, overdue local holds, SSD write failures, PrepareReadBatch read failed, dropped demotions, and any [error] line. An earlier pass on the pre-trim heads gave the same result (277,392 tok/s, 0 failures).

Notes:

The harness sets UMBP_STANDALONE_STARTUP_TIMEOUT_MS=1800000. Since fix(umbp): keep the standalone server serving when its GPU-registered DRAM pool is migrated #756 the server reports ready while its 300 GB pool is still being GPU-registered, and under concurrent model loading sglang's default 30 s connect timed out. That is not part of this chain.
One rerun was lost to an sglang GPU memory fault in one DP rank (its own kernel, no UMBP data being loaded at the time); the rerun passed.

🤖 Generated with Claude Code

The master ranks a node's keys by last_accessed_at, which only RouteGet updates. A local_first node serves its hits without the master, so under master_ranked (and master) the hottest keys looked the coldest and went first: in a 4 GB DRAM / 8 GB SSD smoke a hot set re-read throughout the fill survived 0/512 under master_ranked vs 512/512 under local, and the hot reads then served from SSD slowed demotion until DRAM sat at 100%. A node whose eviction follows the master's recency now ships the keys it reads locally (deduped, bounded) on its heartbeats, and the master records an access for each. Local mode sends nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Picks up #741's local-read reports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

master_ranked asked GetEvictionCandidates synchronously inside the eviction pass, at most every 200 ms per backend. The in-memory master answers by scanning its whole index; at e2e scale (500k DRAM + 2.7M SSD keys on one node) that is 0.66 s per DRAM query and 1.9 s per SSD query, so a DRAM pass stalled most of every second and every SSD query hit the 1 s deadline and was thrown away after the master had done the work. The pass now waits at most 20 ms for the query it sends (a small index still answers the same pass); a slower answer serves a later pass unless it is over 3 s old, and the medium's own order evicts meanwhile. One query per backend is in flight at a time. The deadline goes to 3 s to match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Picks up #741's non-blocking ranked queries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

One pass reclaims a backend's whole watermark band: in the e2e a single DRAM pass covered 108k keys, while one GetEvictionCandidates answer carries at most 4096 and arrives after the pass has moved on. The pass asked once, at its start, so master_ranked ranked 0 of those 108k victims (at most 4% even when it used to wait for the answer). The pass now takes the master's answer whenever it is ready, and asks again for what the budget still needs as each answer runs out (still at most one query per 200 ms per backend); the medium's own order evicts in between. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Picks up #741's whole-pass master ranking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The branch already carries #741's head e8b950a, which bb6af32 squashes onto 38a4fa3, so the tree is kept as is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ummary With #683 in main, the activity summary can carry this PR's numbers: "(N moved, M failed, R retried, K dropped)". retried counts eviction demotions re-queued after a retryable failure, so a run shows whether the retry path is doing anything; it is also exported as mori_umbp_client_tier_transitions_total{outcome="retried"}. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…790) fix(umbp): port test_peer_pool's FailingEngine to CreatePlanBuilder (#790) #763 added FailingEngine overriding Plan(), which #774 had made non-virtual, so test_peer_pool failed to build. Delegate the new pure-virtual CreatePlanBuilder() to the wrapped LocalCopyEngine, as GatedEngine does.

TianDi101 and others added 15 commits October 13, 2026 04:59

docs(umbp): drop the eviction-convergence design doc

1eaaab2

The PR description carries the design; the two code comments that pointed at the doc already explain themselves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TianDi101 mentioned this pull request Oct 13, 2026

feat(umbp): converge eviction onto one path; nodes evict locally by default #741

Merged

7 of 10 tasks

This was referenced Oct 13, 2026

fix(umbp): SSD staging no longer fails local loads; a local read holds its bytes until it is done #762

Merged

feat(umbp): serve metrics from a masterless node, and summarise offload/load/evict/promote/demote at INFO #683

Merged

TianDi101 marked this pull request as ready for review October 13, 2026 23:54

TianDi101 and others added 6 commits October 14, 2026 05:27

Merge branch 'feat/umbp-evict-converge' into fix/umbp-demotion-retry

4cfdd1d

Picks up #741's local-read reports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge branch 'feat/umbp-evict-converge' into fix/umbp-demotion-retry

a194fbe

Picks up #741's non-blocking ranked queries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge branch 'feat/umbp-evict-converge' into fix/umbp-demotion-retry

2e973da

Picks up #741's whole-pass master ranking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TianDi101 mentioned this pull request Oct 14, 2026

UMBP: master-backed standalone server halves e2e throughput vs masterless (DSv4 agentic, DRAM→SSD tiers) #767

Open

TianDi101 and others added 2 commits October 15, 2026 01:48

Merge main's 38a4fa3 into fix/umbp-demotion-retry

ddf9944

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge main (#741 squash bb6af32) into fix/umbp-demotion-retry

9044216

The branch already carries #741's head e8b950a, which bb6af32 squashes onto 38a4fa3, so the tree is kept as is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TianDi101 changed the base branch from feat/umbp-evict-converge to main October 15, 2026 01:58

This was referenced Oct 15, 2026

fix(umbp): SSD staging works without tuning #772

Closed

feat(umbp): SSD demote/promote skip staging; promoting reads land in DRAM; 4 GiB staging preset #776

Merged

TianDi101 and others added 2 commits October 15, 2026 09:46

Merge remote-tracking branch 'origin/main' into fix/umbp-demotion-retry

51c93eb

This was referenced Oct 15, 2026

refactor(umbp): one incremental plan builder shared by every transfer engine #774

Merged

fix(umbp): stop false put-failure noise (sampled allocation WARN; in-flight key counts as stored) #777

Merged

TianDi101 merged commit d6b23ed into main Oct 16, 2026
23 of 25 checks passed

TianDi101 mentioned this pull request Oct 16, 2026

fix(umbp): port test_peer_pool's FailingEngine to CreatePlanBuilder #790

Merged

Navigation Menu

Repository navigation

fix(umbp): retry eviction demotions instead of dropping the only copy; count what is dropped - #763

fix(umbp): retry eviction demotions instead of dropping the only copy; count what is dropped#763
TianDi101 merged 25 commits into
mainfrom
fix/umbp-demotion-retry

TianDi101 commented Oct 13, 2026 •

edited

Loading

Uh oh!

TianDi101 commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

TianDi101 commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. A transient demotion failure dropped the key

2. "No space" at the SSD target dropped keys readers had just found

3. Count what was given up on

Testing

Uh oh!

TianDi101 commented Oct 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

TianDi101 commented Oct 13, 2026 •

edited

Loading