[gongwei-130]

Problem

Once multi-tenancy partitions the pool, the background evictor stops protecting most tenants.

EvictionThreadFunc gates on a pool-global ratio:

double used_ratio = segment_manager_.GetMemoryUsage().used_ratio();
if (used_ratio > eviction_high_watermark_ratio_ ||
    (need_mem_eviction_ && eviction_ratio_ > 0.0)) {

BuildEffectiveQuotaAssignmentsImpl apportions effective = capacity * requested / sum(requested), so effective quotas sum to exactly capacity. At most one tenant's share can therefore exceed eviction_high_watermark_ratio. Every other tenant reaches its own ceiling while the pool is still under the pool-wide watermark, and BatchEvict never runs on its behalf.

The quota path cannot arm it either: need_mem_eviction_ is only set next to inc_put_start_alloc_failures(), and a quota rejection returns before allocation is attempted.

What remains is the synchronous fallback in the admission retry loop:

for (int attempt = 0; attempt <= kMaxTenantQuotaEvictionRetries; ++attempt) {
    ...
    EvictTenantMemoryForQuota(object_id.tenant_id, quota_deficit_bytes);
}

quota_deficit_bytes is exactly the shortfall of the object being admitted — no headroom is ever built, so under concurrency every writer redoes the work and races for the same freed extent, and after 2 retries the write is rejected with TENANT_QUOTA_EXCEEDED.

Measured

A 7168 GiB two-tenant cluster on 0.3.13 with a 90/10 split against the default 0.90 watermark. The tenant ceiling and the pool trigger landed 11 MB apart:

tenant effective quota = 900/(900+100) x 7168 GiB = 6451.20 GiB
pool eviction trigger  =        0.90    x 7168 GiB = 6451.20 GiB

Over the following 13 hours:

signal	value
batch put items rejected	5-13%, 12,456 TENANT_QUOTA_EXCEEDED total
mooncake_tenant_quota_charged_bytes	frozen at the ceiling, 21 MB under it, for the whole window
mooncake_tenant_evict_bytes_total	25-198 MB/s throughout — the tenant path was working, it just could never get below the ceiling
master_attempted_evictions_total	unchanged — BatchEvict never fired
master_put_start_alloc_failures_total	0
mooncake_tenant_quota_over_quota	0 — a tenant pinned at its ceiling is not over it

Raising that tenant's share above the watermark moved the same cluster to 90-105 put items/s at a 0.005% reject rate. Twenty times the write pressure, two thousand times fewer rejections, same day and same hardware.

That workaround only helps one tenant, though, which is why this wants a mechanism rather than a runbook entry. Protecting N actively-writing tenants by lowering the pool watermark would need eviction_high_watermark_ratio < 1/N — under 0.50 for two, under 0.33 for three — which strands more of the pool than it saves. It would also be wrong: BatchEvict is global LRU, so at a 0.45 watermark a hot tenant's admissions would evict a cold tenant's data while the hot one still could not pass its own ceiling.

Change

Adds tenant_eviction_high_watermark_ratio: a background pass that evicts any tenant over its own watermark down to (watermark - eviction_ratio) of its effective quota. This gives the tenant path the same create-headroom-ahead-of-demand behaviour the pool path already has, instead of freeing exactly one object's deficit on the write path.

void MasterService::EvictTenantsOverWatermark() {
    if (!enable_multi_tenants_ ||
        tenant_eviction_high_watermark_ratio_ <= 0.0) {
        return;
    }
    const double target_ratio = std::max(
        0.0, tenant_eviction_high_watermark_ratio_ - eviction_ratio_);
    for (const auto& snapshot : tenant_quota_table_.ListTenantSnapshots()) {
        ...
        EvictTenantMemoryForQuota(snapshot.tenant_id, target_bytes);
    }
}

Notes on the shape of it:

• EvictTenantMemoryForQuota is reused unchanged, so lease, hard-pin and soft-pin handling, the two-pass soft-pin escalation and the offload path all behave exactly as they do today. The only new thing is when it is called and how much it is asked for.
• Called outside the pool-level branch, deliberately. The case this exists for is a tenant over its own watermark while the pool is under the pool-wide one, so it must not be gated on the pool condition having fired.
• Throttled to 1 Hz. The eviction thread wakes every 10 ms (kEvictionThreadSleepMs), and this walks every registered tenant and locks every quota shard. Quota pressure builds over seconds; the synchronous admission fallback still covers the gap in between.
• No new metric. EvictTenantMemoryForQuota already increments mooncake_tenant_evict_bytes_total.

Default

0.0, i.e. disabled — no behaviour change for anyone who does not opt in. 0.9 mirrors the pool default and is what we run. Happy to flip the default if you would rather this be on out of the box; it seemed better to make the first version obviously safe to merge and let you decide the policy.

The flag is added as an optional config parameter rather than a RequiredParam, so existing master.json / master.yaml files keep loading unchanged.

Tests

In master_service_tenant_quota_test.cpp, using the existing MasterServiceTenantQuotaTest friend fixture:

• TenantEvictionWatermarkIsDisabledByDefault — the pass is a no-op at 0.0
• TenantEvictionWatermarkLeavesTenantsBelowItAlone
• TenantEvictionWatermarkEvictsDownToTargetRatio — 0.96 -> at most 0.85
• TenantEvictionWatermarkOnlyTouchesTenantsOverIt — a quiet tenant does not pay for a noisy one

The test config sets default_kv_lease_ttl(0) (eviction skips live leases, so with the 10 s default these would pass for the wrong reason) and eviction_high_watermark_ratio(1.0) (keeps the pool-wide evictor from accounting for the freed bytes).

Not covered by tests: the contention that produces the 5-13% loss needs concurrent writers at a tenant ceiling. That part is from production measurement, not a unit test.

Caveat

I have not built this locally — the machine I wrote it on lacks boost, glog, gflags and yalantinglibs, so CI will be the first real compile. The change is mechanical plumbing plus one new function, but please treat the build as unverified on my side. I can iterate quickly on anything CI turns up, and can test a built image against the cluster above, where the failure reproduces on demand by setting the tenant share to 90% with the watermark at 0.90.

Refs #3741, where the same measurement is written up alongside the silent-eviction branch of the same code path.

[Copilot]

Pull request overview

Adds an optional per-tenant eviction high-watermark to the master’s background eviction loop so that quota pressure can create headroom proactively (per tenant) instead of relying on synchronous per-object eviction/retry and eventual TENANT_QUOTA_EXCEEDED rejections.

Changes:

• Introduces tenant_eviction_high_watermark_ratio config/gflag (default 0.0, disabled) and threads it through master config plumbing.
• Implements MasterService::EvictTenantsOverWatermark() and schedules it (1 Hz) from EvictionThreadFunc when multi-tenancy is enabled.
• Adds unit tests covering the disabled-by-default behavior, no-op when below watermark, eviction down to (watermark - eviction_ratio), and isolation between tenants.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
mooncake-store/tests/master_service_tenant_quota_test.cpp	Adds tenant-watermark-specific config helper and unit tests for the new background pass.
mooncake-store/src/master.cpp	Adds gflag + validator + config load/override wiring for the tenant watermark ratio.
mooncake-store/src/master_service.cpp	Implements and schedules the tenant watermark eviction pass in the eviction thread.
mooncake-store/include/types.h	Adds default constant DEFAULT_TENANT_EVICTION_HIGH_WATERMARK_RATIO.
mooncake-store/include/master_service.h	Declares the new private eviction-pass method and check interval constant.
mooncake-store/include/master_config.h	Threads the new config value through the various config wrapper/builder types.
mooncake-store/conf/master.yaml	Documents the new setting and adds it to the example config.
mooncake-store/conf/master.json	Adds the new setting to the example config.

Suppressed comments (5)

mooncake-store/tests/master_service_tenant_quota_test.cpp:1269

• This direct call to EvictTenantsOverWatermark() will not compile because the method is private on MasterService and TEST_F creates a derived fixture type (friendship isn't inherited). Call the fixture helper (e.g. RunTenantEvictionWatermarkPass(service)) instead.

    service.EvictTenantsOverWatermark();

mooncake-store/tests/master_service_tenant_quota_test.cpp:1288

• This direct call to EvictTenantsOverWatermark() will not compile because the method is private on MasterService and TEST_F creates a derived fixture type (friendship isn't inherited). Call the fixture helper (e.g. RunTenantEvictionWatermarkPass(service)) instead.

    service.EvictTenantsOverWatermark();

mooncake-store/tests/master_service_tenant_quota_test.cpp:1308

• This direct call to EvictTenantsOverWatermark() will not compile because the method is private on MasterService and TEST_F creates a derived fixture type (friendship isn't inherited). Call the fixture helper (e.g. RunTenantEvictionWatermarkPass(service)) instead.

    service.EvictTenantsOverWatermark();

mooncake-store/tests/master_service_tenant_quota_test.cpp:1333

• This direct call to EvictTenantsOverWatermark() will not compile because the method is private on MasterService and TEST_F creates a derived fixture type (friendship isn't inherited). Call the fixture helper (e.g. RunTenantEvictionWatermarkPass(service)) instead.
  mooncake-store/src/master_service.cpp:9155
• Same as the trigger log above: this INFO log will repeat every check interval for every tenant remaining over watermark. Prefer VLOG(1) (or a rate-limited INFO) to avoid log flooding under sustained quota pressure.

        LOG(INFO) << "[TENANT-EVICT-DONE] tenant=" << snapshot.tenant_id
                  << " freed_bytes=" << result.freed_bytes
                  << " evicted_objects=" << result.evicted_objects;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gongwei-130]

CI is green — following up on the "not built locally" caveat in the description, since that turned out to matter.

build-flags and all eight wheel builds passed on the first run, so the config plumbing and EvictTenantsOverWatermark itself compiled clean. The only failure was in my own test file: TEST_F bodies are a class derived from the fixture and friendship does not inherit, so service.EvictTenantsOverWatermark() was inaccessible even though MasterServiceTenantQuotaTest is a friend. Fixed by routing it through a fixture method, which is what the neighbouring helpers in that file already do for the same reason.

Now: build-flags pass, CTest unit tests pass (154/154, master_service_tenant_quota_test included), Check code format pass, all wheel builds pass.

I also verified the format check locally this time with clang-format 20.1.8 and the repo's own scripts/code_format.sh --check --changed-lines, rather than guessing.

[he-yufeng]

This is the right mechanism for the concern I raised on #3788. Arming global BatchEvict on a tenant-quota shortfall forces eviction of other tenants' objects while the pool sits far below the pool-wide watermark, and it still cannot relieve the constrained tenant. A per-tenant watermark makes quota pressure evict the tenant's own data, which is precisely the safe shape for the collision regime you measured in #3741.

I read the implementation against that concern and the mechanics hold: the (watermark - eviction_ratio) target mirrors the pool path's hysteresis, used_ratio is computed per tenant against its own effective quota, EvictTenantMemoryForQuota is reused unchanged so pin/lease/offload semantics are preserved, the pass is deliberately outside the pool-level branch (a tenant over its own watermark while the pool is under the pool-wide one is exactly the case that needs it), and the 1 Hz throttle plus the default-off flag make it opt-in and backward compatible.

Coordination question for you and huangxiaobai-ydmy rather than a blocker: #3788 fixes the same 13-hour incident from the admission side (arming need_mem_eviction_ on quota shortfall, deficit+headroom eviction, global metrics coverage, the reject flag). This PR is the cleaner fix for the collision regime, so I would rather land this mechanism and keep from #3788 the parts that are orthogonal (headroom eviction, metrics coverage, the config warnings), dropping its arming line. If you two see them as composable instead, the arming should at least be scoped the way I suggested there, but a per-tenant watermark makes that line unnecessary.

[gongwei-130]

Thanks @he-yufeng — agreed on the split, and I'd like to land it exactly the way you described.

On the coordination question. #3788 and this PR are not composable as written, and the reason is the one you identified on #3788: need_mem_eviction_ arming is unconditional on every non-final over-quota attempt, while BatchEvict picks victims globally with no tenant weighting. That is correct only in the collision regime and actively harmful in the opposite one — a small-quota tenant streaming over-quota writes keeps the flag armed, and every armed pass eats roughly eviction_ratio of other tenants' hot objects below the pool watermark without ever relieving the constrained tenant, because its charged bytes only drop when its own objects get picked. So the case that motivated the line is the only case it helps, and a per-tenant watermark covers that case directly by construction: the pass reads used_ratio against the tenant's own effective quota and routes through EvictTenantMemoryForQuota, so the bytes freed are always the constrained tenant's own. Keeping both would mean the tenant pass creates the headroom and the arming line separately fires a global pass to do it again, less precisely.

Concretely, what I'd propose to @huangxiaobai-ydmy:

• Drop from [Bugfix][Store] Fix tenant-quota eviction treadmill and watermark collision #3788: the two need_mem_eviction_.store(true, ...) lines in PutStart/UpsertStart, and the TenantQuotaWatermarkCollisionArmsNeedMemEviction test that pins that behaviour. Neither of your two scoping options is needed once the tenant watermark exists — option 1 (effective_quota_bytes >= watermark_bytes) is a proxy for "the tenant pass would have handled it anyway", and option 2 you already noted misses the regime I measured.
• Keep in [Bugfix][Store] Fix tenant-quota eviction treadmill and watermark collision #3788, all orthogonal to this PR: the headroom target in EvictTenantMemoryForQuota (deficit + eviction_ratio * effective_quota with the overflow saturation), the global inc_eviction_success/inc_mem_eviction_* coverage so master_attempted_evictions_total is not silent on the tenant path, MaybeWarnTenantQuotaConfig, and -tenant_quota_reject_on_exceed.

The headroom change is worth calling out as genuinely complementary rather than redundant: it fixes the synchronous path, which still runs whenever admission outruns the 1 Hz background pass, and it is what stops the treadmill inside a single burst. This PR fixes the steady state; #3788's headroom fixes the transient. They compose cleanly once the arming line is gone.

One ordering note: the tenant_quota_reject_on_exceed semantics get sharper with a tenant watermark in place, since "reject immediately" then means "reject and let the background pass make room for the next attempt" rather than "reject and hope the pool crosses its watermark". No code conflict either way — the two PRs touch disjoint parts of EvictTenantMemoryForQuota's caller set — so whichever lands first, the other rebases without a merge conflict beyond the master_config.h plumbing blocks.

Copilot review, addressed in 58bb9a5. Both remaining items:

• [TENANT-EVICT-TRIGGER]/[TENANT-EVICT-DONE] dropped from LOG(INFO) to VLOG(1). The pass runs at 1 Hz and emits two lines per over-watermark tenant, so sustained pressure across many tenants floods the log for what is by then routine steady-state behaviour. The pool-level [EVICT-TRIGGER]/[EVICT-DONE] pair stays at INFO — one line, gated on a pool-wide threshold, genuinely an event.
• Rewrapped the test comment that split need_mem_eviction_ across a line break.

The earlier private-method finding was already fixed in the previous push, and CI was green on it.

I have not built this locally — it is a log-level change plus a comment, and VLOG is already in use in the same translation unit — so I am relying on CI, but the format check passes locally against scripts/code_format.sh --check --changed-lines with clang-format 20.1.8.

[gongwei-130]

CI note on the previous run (33443257136): two jobs failed, and I do not believe either is caused by this PR. Retriggered with an empty commit since I only have read access on this repo and cannot re-run the failed jobs directly.

1. CTest unit tests — MasterServiceSSDSnapshotTest.EvictObject, 153/154 otherwise green.

Pre-existing flake, not specific to this branch. The same test failed today on two unrelated branches: vin/store-upload-planning and czh/pg-enq-fix, neither of which touches tenant quotas.

The failure is CompareServiceState(state_before, state_after) at master_service_test_for_snapshot_base.h:799, preceded by ~30 replica_list mismatch for key: test_keyNNNN lines. state_before is captured with CaptureServiceState(service.get()) on the live service while its eviction thread is still running — the surrounding log shows [EVICT-TRIGGER] memory_ratio=1 high_watermark=0.9 firing throughout — so any object the live service evicts between the snapshot persist and the capture diverges from the restored copy. Inherent race in a test that deliberately runs under eviction pressure.

For completeness, on why it is not this PR: EvictTenantsOverWatermark is gated on enable_multi_tenants_ && tenant_eviction_high_watermark_ratio_ > 0.0, and the && short-circuits on enable_multi_tenants_ before the steady_clock::now() read, so with multi-tenancy off the pass adds one boolean test per loop iteration and nothing else. kEvictionThreadSleepMs is unchanged, so the eviction thread's cadence in that test is byte-for-byte what it was before.

2. test-wheel-ubuntu (ubuntu-22.04, 3.12) — test_pg_elastic.TestMooncakePGElasticCPU.test_recovery_p2p_after_graceful_group_destroy.

This is in mooncake-pg; the diff here is mooncake-store only. The failure is rank 2: TimeoutError -- timed out waiting to start replacement plus rank 0: P2P with rank 1 failed — a 4-rank elastic-recovery test over TCP loopback, timing out on a shared runner. The ubuntu-24.04, 3.10 leg of the same suite passed on this same commit.

Happy to file a separate issue for the EvictObject race if it is not already tracked — it is hitting several PRs a day right now. Keeping it out of this PR to avoid mixing an unrelated test fix into the change.

[gongwei-130]

Follow-up, and the evidence is now conclusive rather than circumstantial.

The retrigger was an empty commit, so runs 33443257136 and 33455330017 ran against a byte-identical tree (42e436e7). They failed different tests:

run	tree	CTest failure
33443257136	42e436e7	MasterServiceSSDSnapshotTest.EvictObject
33455330017	42e436e7	MasterServiceSnapshotTest.CleanupStaleHandlesTest

Each test passed in the run where the other failed, 153/154 both times. Same code in, different answer out — the ha/snapshot suite is non-deterministic; this is not a defect in this PR.

The mooncake-pg failure from the first run (test_recovery_p2p_after_graceful_group_destroy) passed on the second, as expected for a loopback timing flake.

On the new one specifically: CleanupStaleHandlesTest fails in the fixture's TestSnapshotAndRestore, on CompareSnapshotDirectories — metadata is 71 bytes in the live service's backup vs 47 in the re-snapshot of the restored service. The test body unmounts a segment twice and relies on stale replicas being reaped lazily, so whether those entries are still present when the teardown snapshot is taken depends on when the background cleanup runs. Not a config difference: the snapshot metadata payload is MasterSnapshotCodec::Encode output over object/replica state, and tenant_eviction_high_watermark_ratio is not part of it.

And the same gating argument as before applies to both: the test constructs a bare MasterService(), so enable_multi_tenants is false and tenant_eviction_high_watermark_ratio is DEFAULT_TENANT_EVICTION_HIGH_WATERMARK_RATIO = 0.0. EvictTenantsOverWatermark is unreachable, and the && in the eviction loop short-circuits on enable_multi_tenants_ before the steady_clock::now() read, so the loop does not even acquire a timestamp it did not acquire before.

Retriggered once more. If the suite keeps rolling dice I would rather fix the flake than keep re-rolling — happy to open a separate PR for it, but not to fold an unrelated test fix into this one.

[catyans]

LGTM. The default-off watermark is scoped to each tenant's effective quota and reuses the existing pin/lease/offload-aware eviction path while preserving hysteresis. Focused CTest coverage passes; the remaining Ubuntu 22.04 wheel failures are in unrelated mooncake-pg recovery tests.

[gongwei-130]

@he-yufeng — could I ask you for a re-run of the failed jobs on this PR when you have a moment? I only have triage on this repo, and POST /actions/runs/:id/rerun-failed-jobs comes back 403 Must have admin rights to Repository, so the only lever I have is force-pushing an empty commit, which is getting noisy on the PR.

The reason I am asking rather than re-rolling again: three runs so far, all against a byte-identical tree (42e436e7 — the retriggers were empty commits, no tree change), and each produced a different failure set.

run	CTest unit tests	test-wheel-ubuntu (22.04, 3.12)
33443257136	❌ MasterServiceSSDSnapshotTest.EvictObject	❌ test_pg_elastic…test_recovery_p2p_after_graceful_group_destroy
33455330017	❌ MasterServiceSnapshotTest.CleanupStaleHandlesTest	✅
33538917197	✅ 154/154	❌ same pg test + test_pg_init_functional…test_destroy_and_reinit

Every individual test that failed also passed in at least one of the other runs, so none of them is a deterministic consequence of this diff. Currently 19 pass / 2 fail, where one of the two is CI Gate aggregating the other.

The two mooncake-pg failures are the easy call: both are failing on main itself, independently of this branch —

• main run 33600536644 → test_recovery_p2p_after_graceful_group_destroy
• main run 33600300214 → test_destroy_and_reinit

which is exactly the pair that failed here. This diff is mooncake-store only and does not touch mooncake-pg at all.

For the two ha/snapshot ones, in case it is useful when someone gets around to that suite: EvictObject fails on CompareServiceState after ~30 replica_list mismatch lines, with state_before captured from the live service while [EVICT-TRIGGER] memory_ratio=1 is still firing — anything evicted between the persist and the capture diverges from the restored copy. CleanupStaleHandlesTest fails earlier, on CompareSnapshotDirectories (metadata 71 bytes vs 47), in a test whose body unmounts a segment twice and relies on stale replicas being reaped lazily. Both are timing-dependent by construction. EvictObject has also been failing on vin/store-upload-planning and czh/pg-enq-fix.

And to close off the obvious suspicion — the new pass cannot reach any of these: all four tests construct a bare MasterService() or run outside mooncake-store entirely, so enable_multi_tenants is false and tenant_eviction_high_watermark_ratio is 0.0. EvictTenantsOverWatermark is unreachable, and the && in EvictionThreadFunc short-circuits on enable_multi_tenants_ before the steady_clock::now() read, so the eviction loop does not even take a timestamp it was not taking before. kEvictionThreadSleepMs is unchanged.

Separately, if you think the ha/snapshot races and the two mooncake-pg timeouts are worth fixing rather than living with, I am happy to take them — as their own PRs, not folded into this one.

[gongwei-130]

Thanks for the re-run @he-yufeng. That one is a genuine break, but not this PR's and not a flake — and it is already fixed on main.

The re-run failed differently from all three earlier attempts: instead of a timing flake, all six mooncake-pg test modules failed to import:

ModuleNotFoundError: No module named 'mooncake.pg_2_14_0'
ImportError: Mooncake PG was not built against torch==2.14.0.

mooncake/pg.py dispatches to a per-torch-version backend via importlib.import_module("mooncake.pg" + version_suffix), and _build-wheel.yaml was building ep_versions='2.11.0;2.12.0;2.12.1;2.13.0'. Once torch 2.14.0 landed on PyPI, the Run all PG CPU tests with latest PyTorch step started resolving to a torch the wheel has no backend for. That step is gated on ubuntu-22.04 && python 3.12, which is exactly why only that leg failed while ubuntu-24.04, 3.10 stayed green.

It was hitting everyone, not just this branch — same pg_2_14_0 error on dev/oplog-ha-prs/N07 and dev/add-metadata-version.

@UNIDY2002 already fixed it in #3842 (886feb8b, merged 02:53 UTC today), which adds 2.14.0 to ep_versions. The re-run did not pick it up because re-running reuses the merge ref computed when the run was first created, and this branch was still based on 73471dc2.

So I have rebased onto current main (3c841395) rather than re-rolling again, and dropped the empty retrigger commit while I was at it. Diff is unchanged — still +299 across the same 8 files, clang-format --check --changed-lines clean.

Recapping the four attempts, since the picture is now complete:

attempt	tree	outcome
33443257136	42e436e7	EvictObject + pg elastic — both flakes
33455330017	42e436e7	CleanupStaleHandlesTest — flake
33538917197	42e436e7	pg elastic + pg init — both flakes, also failing on main
re-run of the above	42e436e7	torch 2.14.0 import break — real, repo-wide, fixed by #3842
now	rebased on 3c841395	picks up #3842

The three ha/snapshot / mooncake-pg timing flakes are still out there and unrelated to this change; my earlier offer to fix them in separate PRs stands if you think they are worth chasing.

[ykwd]

Thanks for the change — the per-tenant watermark is the right shape for quota pressure that never crosses the pool-wide trigger.

I have two suggestions for this issue, the second of which is out of scope for this PR.

1. Align the multi-tenant path with the single-tenant path.

• Default tenant_eviction_high_watermark_ratio to 0.9, matching eviction_high_watermark_ratio, instead of leaving it at 0.0 / opt-in.
• On PutStart / UpsertStart quota shortfall, return TENANT_QUOTA_EXCEEDED immediately and drop the EvictTenantMemoryForQuota + retry loop.

That is the same contract as enable_multi_tenants=false: this put fails, the background evictor builds headroom, the next put can succeed. Keeping a distinct quota error (rather than NO_AVAILABLE_HANDLE) is still useful; the important part is that the write path stops doing synchronous per-object eviction. As it stands, even with this PR, the admission retry remains the hot path for any burst that outruns the 1 Hz pass, so the treadmill this PR is fixing is only half-addressed.

2. Out of scope, but worth discussing: sequential tenant eviction.

EvictTenantsOverWatermark walks over-watermark tenants one by one on the eviction thread and waits for EvictTenantMemoryForQuota to finish before starting the next. With a large working set, evicting tenant A down to (watermark - eviction_ratio) can take long enough that B and C have already run far past their own watermarks.

If a pass finds N tenants over watermark, could we evict them concurrently — e.g. start N threads, one per tenant — instead of serialising them?

[gongwei-130]

Thanks @ykwd — both done in the head commit, and your instinct on the write path was right for a sharper reason than I had appreciated. Raising one consequence below rather than fixing it here, since it touches how this composes with #3788.

1a. tenant_eviction_high_watermark_ratio now defaults to 0.90, matching eviction_high_watermark_ratio. It stays inert unless enable_multi_tenants is set, and 0.0 remains available as an opt-out. One knock-on: I dropped the set without multi-tenancy startup warning I had added, because with a non-zero default it would fire on every single-tenant master.

1b. PutStart/UpsertStart no longer evict inline. Over-quota returns TENANT_QUOTA_EXCEEDED immediately; the reject metric still fires.

Worth recording why this was more than a tidiness fix. EvictTenantMemoryForQuota walks up to kNumShards — 1024 — taking each shard's write lock and scanning that tenant's whole metadata map per shard, with a second full pass when allow_evict_soft_pinned_objects is on. The retry loop invoked it twice before giving up. So one rejected admission could take ~4k shard write-lock acquisitions and still reject — and the state that triggers it is precisely the state where every other writer is doing the same scan and racing for the same freed extent. That is the treadmill in #3741, and it lived on the write path.

Removing it let me delete the plumbing that only existed to feed it: kMaxTenantQuotaEvictionRetries, the quota_deficit_bytes out-parameter threaded through AllocateAndInsertMetadata, and the deficit_bytes out-parameter on ChargeTenantQuota. EvictTenantMemoryForQuota itself is untouched and still backs the background pass.

One committed behaviour changed, so please look at this specifically. TenantAdmissionEvictsOnlyThatTenantsExpiredObject asserted that a tenant at its ceiling could still be admitted, because admission synchronously evicted its own expired object. That contract is gone by construction. I retargeted the test at the new one as TenantOverQuotaRejectsAdmissionAndBackgroundPassReclaims: the rejected admission leaves the expired object untouched, the pass then reclaims it, the retry is admitted, and a tenant whose data is all hard-pinned keeps rejecting. Also added an EvictTenants() scenario-DSL action and RunTenantEvictForTesting so a test can drive one pass instead of racing the 1 Hz tick.

---

The consequence I want your and @he-yufeng's call on

Dropping the synchronous fallback opens a window for small-quota tenants, because the headroom this PR creates is a proportion of the quota, not an absolute:

reachable iff  ingest_rate > (1 - watermark) x quota / pass_interval
                           = 0.10 x quota / 1s

tenant quota	headroom	ingest needed to exhaust it in one pass interval
6451 GiB (the #3741 cluster)	645 GiB	645 GiB/s — not physically reachable
100 GiB	10 GiB	10 GiB/s — marginal on a fast fabric
10 GiB	1 GiB	1 GiB/s — trivially reachable

For the cluster I measured this is unreachable by orders of magnitude, and the production data backs that up: mooncake_tenant_evict_bytes_total ran 25–198 MB/s for 13 hours while charged_bytes stayed frozen at the ceiling. Eviction throughput was never the constraint — the tenant was pinned at 1.0 because deficit-only eviction never builds headroom. That is what this PR fixes. But a 10 GiB tenant can punch through its 1 GiB of headroom and then hard-reject until the next tick.

The cheap fix is to arm the pass instead of evicting inline: an atomic flag set on quota shortfall, OR'd into the 1 Hz gate in EvictionThreadFunc, so the pass runs on the next 10 ms tick rather than up to a second later. The write path still does zero eviction, which is your point; the window closes from 1 s to 10 ms; and because it arms a tenant-scoped pass it only ever frees that tenant's own data — so it does not reintroduce what @he-yufeng objected to about the global need_mem_eviction_ arming in #3788. It would want a floor on the arming interval so a tenant parked at its ceiling cannot drive the pass every 10 ms.

I have deliberately not put that in this PR, because it largely decides whether #3788's headroom change is still needed, and that is your and @he-yufeng's call rather than mine. Happy to add it here or send it separately.

On sequential tenant eviction (your point 2)

Deferring, and I would rather have a measurement first. In the one production window I have numbers for, the tenant path sustained 25–198 MB/s and was not the bottleneck — the missing piece was headroom, not eviction bandwidth. Fanning out N threads across tenants also means N concurrent walks contending on the same 1024 shard write locks, which is the cost that motivated 1b, so I would want evidence that head-of-line blocking is real before paying it. Happy to take it as a follow-up if you have a workload where a slow tenant demonstrably starves the others.

Verification

Built and tested this time, rather than relying on CI — a container on a 224-core box, BUILD_UNIT_TESTS=ON USE_CUDA=OFF WITH_EP=OFF USE_ETCD=OFF:

• everything compiles, no new warnings in the changed files
• full ctest: 176/181
• every tenant/eviction suite green: master_service_tenant_quota_test 35/35, master_service_evict_scenario_test 22/22, plus master_service_tenant_scenario_test, offload_on_evict_test, tenant_quota_test, tenant_quota_ledger_test, tenant_id_test — 8/8 suites
• the 5 ctest failures are all mine-the-environment, and I chased each: memory_location_test (numa_move_pages: Operation not permitted), posix_file_test and storage_backend_test (io_uring_queue_init: Operation not permitted) all pass once the container is given --privileged; tcp_transport_test and transfer_metadata_test fail on Unable to find metadata storage plugin etcd, an artefact of my USE_ETCD=OFF config, in a subsystem this diff does not touch.
• scripts/code_format.sh --check --changed-lines clean with clang-format 20.1.8

Rebased onto current main. #2991 collided with this in master_service.h — additive on both sides, so the resolution is just both declaration blocks; the build and the numbers above are from after the rebase.

[catyans]

Thanks for digging into the write-path cost — the ~4k shard-lock acquisitions per rejected admission make the tradeoff much more concrete. LGTM on the current head.

Moving tenant eviction out of PutStart/UpsertStart avoids turning quota pressure into a lock-amplification loop. With the tenant watermark now on by default (and 0.0 retained as an opt-out), the reject → background pass → retry contract is coherent, and the rewritten scenario test pins it directly. CI is green.

On the small-quota window: I like the tenant-scoped wakeup as a follow-up, with coalescing plus a minimum per-tenant interval so repeated rejects cannot drive a 10 ms full-shard scan. Once that exists, I agree #3788 should drop its global need_mem_eviction_ arming; its metrics/config diagnostics can still stand independently. I would not hold this PR for that latency improvement, especially now that the one-second behavior is explicit and tested.

@he-yufeng, does that split also match how you see the two PRs composing?

[Aionw]

Thanks for PR and dig into this problem.

There is a currently undergoing PR which change metadata manage struct, after that PR #3822 merged, per-tenant eviction should be much easier to implement, I wonder would you mind delaying this PR a little after that merged.

[stmatengss]

Thanks for PR and dig into this problem.

There is a currently undergoing PR which change metadata manage struct, after that PR #3822 merged, per-tenant eviction should be much easier to implement, I wonder would you mind delaying this PR a little after that merged.

Consider merging this PR first. I think this PR is simpler.