fix(api): stream file uploads and asset content instead of buffering them - #3346
Merged
Merged
Conversation
…them
Two API endpoints read a whole media file into RAM to serve one
request, so anthias-server's memory scaled with the asset:
* POST /api/v1|v2/file_asset without Content-Range did
`data = file_upload.read()` before writing anything to disk.
* GET /api/v1|v2/assets/<id>/content read the file, built a base64
copy, then let DRF render a third copy into the response.
Measured on anthias-server (RSS delta, real HTTP requests):
| path | file | before | after |
|-----------------------------|-------|---------|-------|
| upload, no Content-Range | 300MB | +302MB | +24MB |
| content, JSON | 300MB | +1197MB | +15MB |
| content, Accept: text/html | 50MB | +725MB | +7MB |
The browsable-API row is the worst case and was not in the report: a
browser pointed at the content URL renders the whole base64 blob into
an HTML page, ~14x the file size.
On a Raspberry Pi 3 B+ (787MB RAM) a single content request for a
150MB asset OOM-killed the server before this change:
Out of memory: Killed process 274861 (uvicorn) anon-rss:392792kB
and the API stayed down for ~40s. After, the same request completes in
+12MB with the server untouched.
Uploads now copy through `chunks()`, and the range-length check reads
the size the multipart parser already recorded rather than len(body).
Content is streamed as a StreamingHttpResponse that base64-encodes the
file in 3MB steps. The JSON envelope is rendered by DRF's own
JSONRenderer around a random sentinel and spliced, so filename escaping
stays with the renderer and only the (ASCII) base64 bypasses it. The
iterator is async on purpose: under ASGI, Django drains a *synchronous*
streaming_content through sync_to_async(list) before sending a byte,
which silently restores full buffering.
The wire format is unchanged — same fields, same order, same escaping,
same Content-Length — and is pinned by a test that compares the streamed
bytes against rendering the equivalent dict. File assets now always
answer application/json: the browsable renderer cannot serve one without
reintroducing the bug. URL assets are untouched.
A file that vanishes between the isfile() probe and the open is now a
clean 404 rather than a 200 whose body dies after the headers ship.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
vpetersson
previously approved these changes
Sep 21, 2026
Three defects found reviewing the streaming rewrite. The file handle was never released on an aborted download. `StreamingHttpResponse` registers a teardown closer only when the body object exposes `close`, and an async generator exposes `aclose` — so nothing was registered, and `response.close()` (the only cleanup Django runs for an abandoned streaming response) left the fd and its inode pinned until GC finalised the generator. A client that repeatedly starts and drops `/content` downloads would accumulate open fds, and a deleted asset would not give its disk space back. The body is now wrapped in a small object with a real `close`, which puts the handle under Django's own resource management rather than reaching into `response._resource_closers`. The new `open()` guard caught bare `OSError`, so a failing SD card (EIO/EUCLEAN), a permissions mistake (EACCES) or fd exhaustion (EMFILE) was reported as a clean 404 — invisible to Sentry, and silently skipped by a backup client. Narrowed to `FileNotFoundError`; everything else surfaces as a 500. This compounded with the leak above: leaked fds eventually raise EMFILE, which the broad guard would have disguised as "no such asset". The envelope was rendered with a bare `JSONRenderer()`, so `Accept: application/json; indent=4` was ignored for file assets while the URL branch — still a DRF `Response` — kept honouring it, leaving one endpoint formatting its two shapes differently. The negotiated media type is now forwarded. Each fix has a regression test, and each test was checked against the defect reintroduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
|
vpetersson
approved these changes
Sep 22, 2026
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.
What
Both endpoints called out in issue 3345 read a whole media file into RAM to serve one request, so
anthias-servermemory scaled with the asset:POST /api/v1|v2/file_assetwithoutContent-Rangediddata = file_upload.read()before writing anything to disk.GET /api/v1|v2/assets/<id>/contentread the file, built a base64 copy, then let DRF render a third copy into the response.Uploads now copy through
chunks(), and content is streamed as aStreamingHttpResponsethat base64-encodes the file in 3 MB steps.Measurements
Real HTTP requests against
anthias-server, RSS delta of the server process:Content-RangeAccept: text/htmlThe browsable-API row is the worst case and was not in the report: a browser pointed at the content URL renders the whole base64 blob into an HTML page, about 14x the file size.
Hardware validation
On the QA rig's Raspberry Pi 3 B+ (787 MB RAM, Anthias 2026.08.2), one
GET /contentfor a 150 MB asset OOM-killed the server before this change:The API returned nothing and stayed down for ~40 s while the container restarted. After the change the same request returns 200 with 209,715,292 bytes for +12 MB RSS, and the upload path costs +13 MB instead of +139 MB. Both v1 and v2 were checked on the device, including a non-ASCII filename round-trip. The device was restored to its original state afterwards.
Wire format
Unchanged, and deliberately so — same fields, same order, same escaping, same
Content-Length. A test pins the streamed bytes against rendering the equivalent dict through DRF'sJSONRenderer, so the v1/v2 contract can't drift as an accident of streaming.The envelope is built by rendering the real payload around a random sentinel and splicing, so
filenameescaping stays with the renderer and only the base64 (ASCII, needs no escaping) bypasses it.Two behaviour changes worth calling out:
application/json. The browsable renderer cannot serve one without reintroducing exactly the bug being fixed. URL assets keep the negotiatedResponseand are untouched.isfile()probe and the open is now a clean 404, rather than a 200 whose body dies after the status line has already shipped.The async iterator is load-bearing
The streaming iterator is
asyncon purpose. Under ASGI, Django'sStreamingHttpResponse.__aiter__funnels a synchronous iterator throughawait sync_to_async(list)(...), draining it in full before the first byte reaches the client. Handing it the sync generator looks correct and silently restores full buffering — measured +405 MB for a 300 MB asset versus +15 MB with the async one.test_asset_content_is_async_iterableguards that.Tests
Seven new tests in
test_v1_endpoints.pycovering the access pattern, exact byte round-trips around the 3-byte base64 boundary, the wire format under awkward filenames (empty, quoted, non-ASCII, JSON punctuation), bounded piece sizes, the async iterator, and the vanished-file 404.Each was checked against a deliberately reintroduced bug — slurping
read(), a sync iterator, one-piece output, and encoding off the 3-byte boundary — and each mutant is caught by the corresponding test.Full suite: 2198 passed, 3 skipped.
ruff check,ruff format --checkandmypy .are clean (the twotools/image_builderimport errors are pre-existing, from optional deps not installed in this venv).🤖 Generated with Claude Code