Add admission control (concurrency limit) to the Storage Proxy upload path
- Dominant language
- Python
- Stars
- 670
- Forks
- 183
- Avg merge
- 17h 7m
- Merged PRs (30d)
- 358
Description
## Background
The Storage Proxy TUS upload path accepts an unbounded number of concurrent PATCH requests. All chunk file I/O runs on a single shared default ThreadPoolExecutor (run_in_executor with executor=None), and each chunk commit serializes on a per-session fcntl.flock while still occupying a pool thread. This is a pre-existing property of the proxy, not introduced by the BA-3974 chunk-store rewrite. The old single-file-append model masked it by rejecting concurrent chunks with 409 before doing any I/O.
## Problem
Under high upload concurrency on a small or over-committed cluster with a sync-mounted NFS export, the bounded thread pool saturates: streaming writes and lock-protected commits contend for the same handful of threads, throughput collapses, and requests cross nginx proxy_read_timeout (default 60s). nginx then resets the upstream connection, surfacing as ConnectionResetError ("Connection lost") while reading the request body. Per-request memory is bounded (256 KiB streaming), so this is a throughput/timeout problem, not memory exhaustion.
## Evidence
Reproduced on the BA-3974 multi-node (nginx + 2 storage-proxy replicas sharing NFS) rig. Without a client-side concurrency cap, 16 in-flight PATCHes per upload stalled indefinitely and produced ConnectionResetError on the proxies. Capping in-flight PATCHes (semaphore) made 50/50 uploads complete cleanly. The client-side cap is only a test workaround; the real fix belongs in the proxy.
## Proposed approaches
- Add a configurable concurrency limit (asyncio.Semaphore) around the upload handler so the proxy applies back-pressure (e.g., return 503/Retry-After or queue) instead of accepting unbounded work.
- Use a dedicated, sized ThreadPoolExecutor for upload file I/O instead of the shared default executor, so uploads cannot starve unrelated proxy work.
- Avoid holding a thread while blocked on fcntl.flock (e.g., bounded retry/backoff) so lock-waiters do not occupy the pool.
- Document recommended nginx tuning (proxy_read_timeout, limit_conn) for multi-replica deployments as a complementary measure.
## Out of scope
BA-3974 covers correctness (no data corruption under concurrent chunk uploads), which is verified independently. This issue is purely about load management / admission control and should not change upload correctness semantics.
## Acceptance criteria
- A configurable upper bound on concurrent uploads exists and is enforced at the proxy.
- Exceeding the bound yields a well-defined back-pressure response, not a stalled connection that nginx resets.
- A high-concurrency upload load test completes without ConnectionResetError on the proxies.
JIRA Issue: BA-6169
Contributor guide
Assessment
This issue has not been assessed yet.