Skip to content

stream rustdoc html content from S3, use streaming rewriter, stream to client #2872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 28, 2025

Conversation

syphar
Copy link
Member

@syphar syphar commented Jul 27, 2025

This will start streaming also rustdoc html files from S3 to the client.

First for simple cases, release specific rustdoc assets, which can directly be streamed.

Then I also added a streaming HTML rewriter.

It's a little more complex than I wanted it to be due to

  1. the streaming design of the synchronous lol-html rewriter.
  2. the fact that I want to use our rendering threadpool to limit the concurrency for CPU-Bound requests.

Generally the first advantage is more paralelism, since we don't have to load the whole HTML into memory before handling it. Second is that we can (IMO) drop file size limits when the content is streamed.

Downside is that any error while streaming will just lead to a dropped connection on the client side, and not a classic 500. This means we would also not see a sentry error in this case.
Which is why I added error! calls in these cases.

Metrics don't show any rewrite OOMs in the metrics, so I don't think we'll have any production issues here.

@syphar syphar self-assigned this Jul 27, 2025
@syphar syphar requested a review from a team as a code owner July 27, 2025 14:38
@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Jul 27, 2025
@syphar syphar force-pushed the storage-stream-html branch from 911f820 to 548dd70 Compare July 27, 2025 14:44
@GuillaumeGomez
Copy link
Member

Looks all good to me, thanks!

@syphar syphar merged commit fc4cc8d into rust-lang:master Jul 28, 2025
9 checks passed
@syphar syphar deleted the storage-stream-html branch July 28, 2025 11:27
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Jul 28, 2025
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Jul 28, 2025
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