Skip to content

Have Thruster send the parts of a file too - #362

Open
namespaceMarcello wants to merge 1 commit into
basecamp:mainfrom
namespaceMarcello:have-thruster-send-ranges-too
Open

namespaceMarcello wants to merge 1 commit into
basecamp:mainfrom
namespaceMarcello:have-thruster-send-ranges-too

Conversation

@namespaceMarcello

Copy link
Copy Markdown
Contributor

This sits on top of #359 and does nothing without it: it only acts when config.action_dispatch.x_sendfile_header is set, and on main it isn't.

With #359, Thruster sends a file that is asked for whole. A request with a Range header is still answered by the app with that part, and Puma writes it from the request's thread. A <video> asks that way, and past what the browser reads ahead it takes the file at the pace it plays. So a long video keeps a Puma thread for as long as it plays, and as many viewers as the worker has threads keep every other request waiting (measured on #361).

Thruster already has what it needs to send the part: it holds the browser's request, Range included, and it sends a file with Go's http.ServeFile, which answers ranges. It only needs the app to answer with the whole file. So when X-Sendfile is on, a before_action on ActiveStorage::DiskController#show drops Range from a GET. Rack::Files then answers with the whole file, Rack::Sendfile turns that into the X-Sendfile header, and Thruster sends the part.

Measurements

From our machine: a laptop with an AMD Ryzen 9 7940HX, Docker. The production image built from this branch plus the one line of #359, against the image of #359 alone; both with the day-long disk URL of #361, so that a video can be watched to its end. Thruster 0.1.23, Puma 8.0.2, Rack 3.2.7, one CPU and so one Puma worker with five threads, storage on a Docker volume (ext4), plain HTTP/1.1. Headless Chrome 155 on other CPUs of the same machine plays the <video> of a room; meanwhile a room page is asked for every second and given 4 seconds to answer. The video is 10 minutes, 296 MB, H.264 at 4 Mbit/s. One run of each; the runs on #359 are from earlier the same evening.

Five browsers play the video to its end:

#359 with #361's line this PR
room pages with no answer in 4 s, of 618 503, 295 of them in a row 0 (median 4 ms, slowest 83 ms)
the five videos 600 of 600 s 600 of 600 s
Puma threads held by the videos all five for 513 s, with up to 299 requests queued 0.08 s summed over the ten requests, no queue
memory of Puma's workers, before play and at its highest 237 and 386 MB 245 and 249 MB
Thruster: highest memory, CPU over the 619 s 19.3 MB, 1.58 s 20.2 MB, 1.26 s
Puma's CPU over the 619 s 4.71 s, 115 pages served 4.13 s, 618 pages served

One browser, this PR:

  • 120 s of the video: Puma writes no 206 and the request takes 0.00 s of a thread, where on Have Thruster send whole files from disk again #359 it held one for all 120 s. 120 room pages of 120 answered.
  • The whole video: it plays 600 of 600 s. Thruster cuts the response at 300 s (HTTP_WRITE_TIMEOUT), Chrome asks for the rest at 321 s and Thruster sends it. 0.05 s of a Puma thread in all.
  • A pause of 120 s, then play: it goes on, with a second request and no error.
  • A jump to 400 s and back to 100 s: the element fires seeked after 25 and 33 ms and is moving again after 819 and 829 ms (on Have Thruster send whole files from disk again #359: 818 and 815 ms).

What a curl gets, for 31 shapes of Range on a 256 MB attachment, a PDF, an empty file and an index.html: 21 answers have the same status and the same bytes as on #359, among them every request for a single range of the video with no other condition (0-, N-, N-M, -N), which is what a <video> sends, and the one with an If-Range that matches the file's date, which Chrome sends after a seek. Their Content-Range, Content-Length, Content-Type, Content-Disposition, Cache-Control and Last-Modified are the same too, and they gain Accept-Ranges: bytes.

What a client sees change

The other 10 answers are Go's now, not Rack's:

Request #359 (Puma) this PR (Thruster)
a range past the end of the file 416, 25 bytes, video/mp4, Cache-Control 416, 33 bytes of plain text, no Cache-Control; the same Content-Range: bytes */N
two ranges 206, a multipart body sent as Content-Type: video/mp4 206, multipart/byteranges with its boundary
If-Range that doesn't match (another date, an ETag): 2 206, the part 200, the whole file
If-Unmodified-Since an older date, If-Match: 2 206, the part 412 with no body, and with the whole file's Content-Length
a range from a client that accepts gzip: 3 206 gzipped by Rack::Deflater; 256 MB in 6.1 s 206 as it is on disk; 256 MB in 0.2 s
a range of an attachment named index.html 206 301 to ./, which answers 404

Limits

  • Thruster has to be in front, sending files. Have Thruster send whole files from disk again #359 already says so for whole files. With this PR it holds for parts too: with X_SENDFILE_ENABLED=false every GET with a Range gets a 200 with no bytes, where on Have Thruster send whole files from disk again #359 it still got its part. That is measured with curl; a browser there is not, but a video could not start. The same should hold for Puma asked directly, which is read from the code and not run.
  • An attachment named exactly index.html already can't be downloaded whole with Have Thruster send whole files from disk again #359 (ReverseProxy read error when serving index.html from /public thruster#57, measured in a comment on #359); with this PR a ranged request for it fails the same way. A fix is on Thruster's sendfile-index-bug branch, not merged. With Thruster's main and that branch merged, built and mounted over the gem's binary in this PR's image, the three index.html requests answer with the file again (200, 200, and 206 for the part), the two 412s above carry Content-Length: 0, and the other answers stay as they are here.
  • A response in progress is no longer closed by Puma when the client stops reading for 10 seconds. Thruster keeps it until HTTP_WRITE_TIMEOUT (300 s), as it already does for whole files with Have Thruster send whole files from disk again #359. A client that stopped reading for 40 s and then went on got all 268,435,456 bytes; on Have Thruster send whole files from disk again #359 it got 44,724,333 of them.
  • Rails logs a 200 for these requests. The 206 is in Thruster's log.
  • One browser (headless Chrome 155 on Linux). Firefox and Safari are not measured; for Safari's bytes=0-1 probe only the answer is, with curl. No TLS and no HTTP/2. Client and server on one machine. Synthetic constant-bitrate video. One Puma worker, not three. One run of each case.
  • Not measured: a file removed from disk while it is being watched, a server restart in the middle of a video, Thruster 0.1.27 (Update Thruster to 0.1.27 #360).

Checks

  • test/controllers/ranged_downloads_test.rb: a ranged GET for a disk URL answers 206 with the part when X-Sendfile is off, as today; 200 with the whole file when it is on; and a HEAD still answers 206. The second fails without the change (Expected response to be a <200: ok>, but was a <206: Partial Content>). Without the GET check the third fails, and without the check on the setting the first does.
  • The 40 other requests of Have Thruster send whole files from disk again #359's comparison (whole files, thumbnails, avatars, conditional requests): 37 the same as on Have Thruster send whole files from disk again #359; the 3 that differ are ranged requests of the kinds in the table above.

Written by Claude Code (AI assistant) on behalf of @namespaceMarcello, who directs this work.

With X-Sendfile on, Thruster sends a file that is asked for whole. A request
with a Range header is still answered by the app with that part, and Puma
writes it. A browser asks for a video that way and, past what it reads ahead,
takes it at the pace it plays, so a long video keeps one of the worker's
threads for as long as it plays.

Thruster still holds the browser's request, Range included. So when
X-Sendfile is on, the app drops the header from a GET for a file on disk and
answers with the whole file. Rack::Sendfile hands that to Thruster, and
Thruster sends the part.

Nothing changes unless config.action_dispatch.x_sendfile_header is set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 21:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Correctness depends on the stacked PR and external Thruster behavior while intentionally changing several HTTP range semantics.

0 open findings

What changed in this PR

Routes Active Storage ranged GETs through Thruster when X-Sendfile is enabled, preventing long-lived downloads from occupying Puma threads.

Changes:

  • Removes Range from eligible disk-service GET requests.
  • Adds coverage for ranged GET and HEAD behavior.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File Description
config/​initializers/​active_storage.rb Delegates ranged file serving to Thruster.
test/​controllers/​ranged_downloads_test.rb Tests ranged responses with and without X-Sendfile.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants