fix: stop copy clamp under-reserve for multi-GB manifest copies#110
Merged
Conversation
Upload-style routine-peak clamp (59MB) applied to UploadPartCopy reservations let three concurrent 4.7GB Scylla .sm_ manifest copies each hold ~175MB ciphertext while the governor tracked ~178MB total, OOMKilling pods at ~790MB RSS. - Add copy_governor_clamped_reserve: min(honest, budget), not routine peak - reserve_copy_memory / try_acquire(copy=True) for server-side copies - Regression tests for prod manifest size, exclusive budget, mixed workload Co-authored-by: Cursor <cursoragent@cursor.com>
- Tighten copy_pipeline_peak with measured part//7 transport slack at large parts - Add tests/integration/test_copy_memory_governor.py (subprocess e2e: 3× copy serialize, mixed Scylla uploads + copy, RSS bound) - Add unit tests: old upload-clamp regression, mixed workload, CopyObject path - Strict tracemalloc bounds at 512MB–4.7GB without slack fudge factor Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
RepeatingBody needs tell/seek for botocore request checksums. Assert SlowDown on concurrent copies rather than succeeded < 3 (sequential success after backpressure is valid). Co-authored-by: Cursor <cursoragent@cursor.com>
Split e2e tests by workload (memory/stress, core, multipart, copy+range, misc) so slow 512MB copy-governor tests don't block the full integration suite. Each shard gets its own MinIO/redis stack and pytest-xdist workers. Co-authored-by: Cursor <cursoragent@cursor.com>
The single memory shard bundled test_memory_usage (2GB multipart), test_memory_leak (20×256MB hammer), and copy governor (3×512MB uploads) — wall clock was sum of all three (~10+ min). - Split into memory_usage, memory_leak, memory_copy (7 parallel jobs) - Copy e2e: 256MB shared source (module fixture), one upload not three - 256MB still triggers exclusive 48MB governor slot (peak ~49.6MB) Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
UploadPartCopy decrypts+reencrypts the full object; 256MB copies took ~3min each serialized. Use 64MB/32MB governor (peak still 48MB), drop redundant RSS e2e (covered by unit tracemalloc), run shard with -n0. Co-authored-by: Cursor <cursoragent@cursor.com>
32MB governor left no room after the request gate's MIN_RESERVATION on top of a budget-monopolizing copy clamp; use 96MB and 5MB upload parts so one copy plus concurrent uploads can succeed while a second copy still gets SlowDown. Co-authored-by: Cursor <cursoragent@cursor.com>
BACKPRESSURE_TIMEOUT=2 let excess copies wait for a slot and all three succeeded serially. Use 0 so the third copy gets SlowDown; assert at most two succeed under the 96MB budget. Co-authored-by: Cursor <cursoragent@cursor.com>
ServerSideHannes
added a commit
that referenced
this pull request
Jul 7, 2026
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).
ServerSideHannes
added a commit
that referenced
this pull request
Jul 7, 2026
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).
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.
Summary
UploadPartCopyof 4.7GB Scylla.sm_manifests used upload-style routine-peak clamp (59MB reservation) while each copy held ~175MB ciphertext atcopy.py:420copy_governor_clamped_reserve()andreserve_copy_memory()so large copies monopolize the budget slot (min(honest, budget)) instead of sharing at 59MB eachRoot cause
PR #109's
streaming_governor_clamped_reserveapplied to alltry_acquire()calls. Copies with honest peak ~500MB were clamped to routine upload peak ~59MB. Prod logs:governed_active_mb=178,rss_mb=790,MEMORY_DEBUG_TOP #1: copy.py:420 → 522MB (3 allocations).Test plan
uv run pytest tests/unit/— 499 passedtests/unit/test_copy_clamp_and_concurrency.py: manifest clamp, 3× exclusive acquire, upload+copy serialization, tracemalloc at 4.7GB2026.7.6to staging; Scylla backup — confirm no OOMKilled pods during.sm_manifest copy phaseMEMORY_CLAMPED_TO_BUDGETfor copies showsreserved_mb=192(not 59) and max 1 large copy per pod at governor limitOps follow-ups (separate)
S3PROXY_MEMORY_DEBUGin ArgoCDMade with Cursor