Skip to content

feat(umbp): byte-sized eviction rounds; demote asynchronously instead of inside EvictKey - #742

Closed
TianDi101 wants to merge 11 commits into
feat/umbp-evict-convergefrom
feat/umbp-evict-byte-budget
Closed

TianDi101 wants to merge 11 commits into
feat/umbp-evict-convergefrom
feat/umbp-evict-byte-budget

Conversation

@TianDi101

@TianDi101 TianDi101 commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #741 (feat/umbp-evict-converge). Review 3df1165a9, acc4b97ee, 466570e5b and 3c6dd4771; 2c0f8349f only merges #741's doc and test cleanup in. This PR retargets to main once #741 merges. (acc4b97ee is titled WIP; 466570e5b completes it. Squash on merge.)

Summary

Two changes to how eviction work is sized and executed:

  1. Master rounds are sized in bytes, not rows. The round was capped at evict_batch_size = 32 candidates per over-full (node, tier) every 5 s. A round now frees the whole overage (usage above the low watermark), optionally throttled by UMBP_EVICT_MAX_BYTES_PER_ROUND.
  2. Demotion is asynchronous on the peer. PeerPool::Evict no longer copies a victim downstream inside the call. It queues the demotion to the pool's transition worker and returns, so an EvictKey RPC never blocks on data movement and needs no per-RPC byte limit.

Why

  • 32 rows per 5 s was too slow to matter. That is ~6 keys/s per tier. At the ~0.7 MB keys of the deployment eviction_manager.cpp cites (512 GiB, 155,157 keys per 0.9→0.7 band), that is ~4 MB/s, and draining one band took ~6.7 hours while puts hit NO_SPACE.
  • The cap saved nothing. Both metadata stores read every eligible row of a bucket before capping, then sort and truncate in C++. Each round paid a full scan to free 32 keys.
  • Nothing could tune it. No flag, env var or config path set evict_batch_size. The cap only became effective in refactor(umbp): make distributed mode backend- and transport-agnostic #540; before that, rounds were uncapped.
  • Sized rounds plus synchronous demotion would stall the RPC. With the row cap gone, one EvictKey can name gigabytes on a demoting tier. Copying all of it inside the call blows UMBP_EVICTKEY_DEADLINE_MS (1 s). Queuing the copy keeps the RPC short without capping it in bytes.

Changes

Master

  • EvictionConfig::evict_batch_size → max_evict_bytes_per_round (env UMBP_EVICT_MAX_BYTES_PER_ROUND, default 0 = whole overage). The overage is already at most (1 − low_watermark) of the tier; set this to throttle.
  • RunOnce asks the store for every eligible row again (max_per_bucket = 0), and the strategy trims to the byte budget. EvictKey RPCs are split by gRPC message size only, as before.
  • GetEnvUint64 added to env_time.h. runtime-env-vars.md documents the variable.

Peer

  • PeerPool::Evict returns one PoolEvictResult {key, bytes_freed, outcome} per request. PoolEvictOutcome is one of:
    • kFreed: the copy was dropped.
    • kDemotionQueued: a demotion job was enqueued. bytes_freed is 0, because the source is freed only when the copy lands.
    • kInFlight: a transition already owns the key.
    • kNone: nothing freed (leased, pinned, or gone).
  • Eviction demotions are TransitionJobs with evict = true. The worker runs them regardless of the tier's own watermark. If nothing downstream has room, it drops the evicted copy instead, unless another transition owns the key.
  • Local (masterless) reclaim counts queued and in-flight demotions as covered bytes, so a pass does not queue more than the overage while copies run.
  • A key mid-demotion holds a migration pin. PageBackend::EvictionCandidates still offers it, so the reclaimer can count it as in flight. It still skips SSD copy-through pins, and backend Evict still refuses every pinned key.

Does the master get slower? No added lock time. Both stores apply the cap after the scan, outside their locks. Removing it changes only post-scan work on the eviction thread: a full sort instead of a partial one, and more EvictKey RPCs. It also cuts scans, since a drain is one round instead of one round per 32 keys. The scan's own cost is unchanged and is the follow-up below.

Test plan

  • Master: a round frees the whole overage (77 keys, not 32); max_evict_bytes_per_round throttles a round to 10 keys; a 600-key budget is met in one strategy pass; victims keep their tier; the env var is read with the default when unset; GetEnvUint64 parses values above 4 GiB and rejects negatives and garbage.
  • Peer: EvictReturnsBeforeTheDemotionCopyRuns holds the copy behind a gated transfer engine and checks that Evict returns kDemotionQueued first. QueuedDemotionsCountTowardTheBudget checks that a pass queues exactly the overage (4 keys) while copies are held. The DRAM→SSD chain and peer-service tests now wait for the asynchronous move.
  • Full tests/cpp/umbp suite in rocm/mori-dev with RDMA: 50/52 under ctest -j 8. umbp_cross_node_smoke and umbp_pool_client_ranges abort on the RDMA device-open assertion under parallel load. Serially, umbp_cross_node_smoke passes. umbp_pool_client_ranges hits its GPU read-back flakes (PoolClientNuma.*), which also fail on unmodified main: an interleaved A/B gave main 2/4 failures and this branch 3/4.
  • Not measured: eviction throughput or master scan time at scale (1M–10M keys).

Follow-ups (not in this PR)

  • Eviction efficiency: a master-maintained per-(node, tier) LRU, updated off the lookup path with deferred touches, so a round walks the cold end instead of scanning every key. Then trigger rounds from heartbeats instead of only the 5 s timer.
  • Dispatch to nodes in parallel; rounds are still sequential across nodes.

🤖 Generated with Claude Code

TianDi101 and others added 8 commits October 10, 2026 10:27
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>
EvictionManager capped each over-full (node, tier) at evict_batch_size =
32 candidates per round, every 5 s: about 6 keys/s per tier.  At the
~0.7 MB keys of the deployment the code itself cites, that is ~4 MB/s,
so a 512 GiB tier's 0.9 -> 0.7 band took hours to drain while puts hit
NO_SPACE -- and every round still paid the store's full scan, because
both stores read every eligible row before applying the cap.  Nothing
could tune it: no flag, env var or config path set evict_batch_size.

- evict_batch_size is replaced by two byte limits:
  * max_evict_bytes_per_round (default 0 = the whole overage, already at
    most (1 - low_watermark) of the tier) caps one round per (node, tier);
  * max_evict_bytes_per_rpc (default 1 GiB) splits a node's victims into
    EvictKey RPCs by bytes as well as gRPC size, so a demoting peer -- which
    copies inside the call -- stays within UMBP_EVICTKEY_DEADLINE_MS.
  Both are env-overridable (UMBP_EVICT_MAX_BYTES_PER_ROUND / _PER_RPC).
- The store is asked for every eligible row again (max_per_bucket = 0, as
  before #540); the strategy trims to the byte budget.
- EvictionVictim carries the bytes it was charged (identity stays
  key + tier); GetEnvUint64 reads the byte limits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y; drop per-RPC byte limit

PeerPool::Evict queues a demotion to the transition worker and returns
(PoolEvictOutcome::kDemotionQueued / kInFlight); the worker runs eviction
jobs regardless of the tier watermark and drops the copy if nothing
downstream has room. max_evict_bytes_per_rpc and EvictionVictim::bytes are
removed. Local eviction counts in-flight bytes toward its budget.

Known failing: PeerPoolLocalEviction.QueuedDemotionsCountTowardTheBudget
(queues 5, expects 4) -- a key mid-copy is migration-pinned and
PageBackend::EvictionCandidates skips pinned keys, so later passes cannot
count it as in flight. Fix: stop skipping migration pins there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A key whose demotion copy is running holds a migration pin, and
PageBackend::EvictionCandidates skipped every pinned key.  Later passes
of the local reclaimer therefore never saw it, could not count its bytes
as in flight, and reached past it for one more victim than the overage
needed (QueuedDemotionsCountTowardTheBudget: 5 queued, 4 expected).

Pins now record whether they are migration pins.  Candidates still skip
the SSD copy-through pin, but offer a migration-pinned key; PeerPool::Evict
reports it as kInFlight and the reclaimer counts it as covered.  Backend
Evict() still refuses every pinned key, so nothing frees pages mid-copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The locking note still described Evict running the demotion copy itself.
Removing the per-RPC row fused two runtime-env-vars.md rows onto one line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@TianDi101 TianDi101 changed the title feat(umbp): size master eviction rounds in bytes, not a 32-row cap feat(umbp): byte-sized eviction rounds; demote asynchronously instead of inside EvictKey Oct 12, 2026
@TianDi101

Copy link
Copy Markdown
Collaborator Author

Combined into #753 (with #741, #683 and the fixes from the DRAM→SSD end-to-end run). The WIP commit and its completion are squashed there. Leaving this open until #753 lands.

TianDi101 and others added 3 commits October 12, 2026 14:47
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>
# Conflicts:
#	src/umbp/doc/design-eviction-convergence.md
#	tests/cpp/umbp/distributed/test_master_evict_strategy.cpp
#	tests/cpp/umbp/distributed/test_peer_pool.cpp
TianDi101 added a commit that referenced this pull request Oct 13, 2026
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>
@TianDi101
TianDi101 force-pushed the feat/umbp-evict-converge branch from 7e49f72 to f19c52b Compare October 13, 2026 05:30
@TianDi101

Copy link
Copy Markdown
Collaborator Author

Folded into #741: this PR's change is d0faaa820 there, and f19c52b48 tightens its comments and folds its tests (same coverage). #741 is rebased on main. Closing this PR; please review the change in #741.

@TianDi101 TianDi101 closed this Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant