lablup / lablup/backend.ai

Add admission control (concurrency limit) to the Storage Proxy upload path

Open
#11,785 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.