fix: writer preference so budget-monopolizing copies are not starved#112
Merged
Merged
Conversation
8ef0aba to
9e9a706
Compare
A server-side COPY clamps its reservation to the whole budget so it runs exclusively (guards the multi-copy OOM #110 fixed). The admission gate is active_bytes + to_reserve <= limit, so a whole-budget request can only be admitted when active_bytes == 0. The limiter had no fairness: every release woke all waiters and a small request re-grabbed memory before the copy could, so active_bytes never drained to 0 under steady traffic. The copy backpressured the whole timeout (prod: MEMORY_BACKPRESSURE requested_mb == limit_mb) and 503'd, retried, starved again -- effectively never admitted while the pod had traffic. Add writer preference: track pending whole-budget (exclusive) waiters and hold back new non-exclusive admissions while one waits, so active_bytes drains and the copy gets its exclusive slot (bounded by in-flight request lifetimes). Copies still reserve the whole budget and run one-at-a-time -- the existing copy-concurrency/OOM regression tests are unchanged and still pass. Regression test: a pending exclusive copy must not be starved by a stream of small requests; the small ones queue behind it. Verified it fails against the old no-fairness limiter with the exact prod signature (requested_mb==limit_mb).
9e9a706 to
07115f6
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
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.
Problem
Prod logs showed a server-side COPY stuck on endless
MEMORY_BACKPRESSUREwithrequested_mb == limit_mb:A multi-GB manifest COPY deliberately clamps its reservation to the whole budget so it runs exclusively — this is the guard against the 3×4.7GB concurrent-copy OOM that #110 fixed (
tests/unit/test_copy_clamp_and_concurrency.py). That part is correct and unchanged.The bug is in the limiter's admission logic. The gate is
active_bytes + to_reserve <= limit, so a whole-budget reservation can only be admitted whenactive_bytes == 0. The limiter had no fairness: everyrelease()doesnotify_all(), and a small request re-grabs memory before the waiting copy can (it fits), soactive_bytesnever drains to 0 under steady backup traffic. The copy backpressures the fullBACKPRESSURE_TIMEOUT, 503s, retries, starves again — functionally never admitted while the pod has any other traffic.Fix
Writer preference, not a smaller clamp (lowering the clamp would re-introduce the multi-copy OOM). Track pending whole-budget ("exclusive") waiters; while one waits, hold back new non-exclusive admissions so
active_bytesdrains to 0 and the copy gets its exclusive slot — bounded by the lifetime of the already-running requests. Copies still reserve the whole budget and run one-at-a-time._can_admit(to_reserve, exclusive): an exclusive request ignores the pending gate (competes only onactive_bytes == 0); a non-exclusive request also waits while_pending_exclusive > 0._pending_exclusiveincremented/decremented around the wait, with anotify_all()on exit so held-back requests resume whether the copy acquired or gave up.Tests
All existing copy-concurrency / OOM regression tests are unchanged and pass (copies still exclusive, still one-at-a-time).
Added
test_pending_exclusive_copy_not_starved_by_small_requests: a pending exclusive copy must not be starved by a stream of small requests — they queue behind it, and the copy wins the slot the moment the in-flight blocker drains. Verified it fails against the old no-fairness limiter with the exact prod signature (requested_mb == limit_mbbackpressure while a small request slips ahead), and passes with writer preference.