Skip to content
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

tokio-epoll-uring: use it for on-demand downloads #6992

Merged

Conversation

problame
Copy link
Contributor

@problame problame commented Mar 1, 2024

Problem

On-demand downloads are still using tokio::fs, which we know is inefficient.

Changes

  • Add pagebench ondemand-download-churn to quantify on-demand download throughput
    • Requires dumping layer map, which required making history_buffer impl Deserialize
  • Implement an equivalent of tokio::io::copy_buf for owned buffers => owned_buffers_io module and children.
  • Make layer file download sensitive to io_engine::get(), using VirtualFile + above copy loop
    • For this, I had to move some code into the retry_download, e.g., sync_all() call.

Drive-by:

  • fix missing escaping in scripts/ps_ec2_setup_instance_store
  • if we failed in retry_download to create a file, we'd try to remove it, encounter NotFound, and abort() the process using on_fatal_io_error. This PR adds treats NotFound as a success.

Testing

Functional

  • The copy loop is generic & unit tested.

Performance

Future Work

Turn the manual performance testing described in the above results document into a performance regression test: #7146

We can probably get rid of the entire trait, but, that's for another time.
…uilder-refactor' into problame/integrate-tokio-epoll-uring/write-path/simplify-write-at-api
…r-blob-writer' into problame/integrate-tokio-epoll-uring/write-path/refactor-virtual-file
@problame problame requested a review from koivunej March 13, 2024 09:39
@problame problame marked this pull request as ready for review March 13, 2024 09:40
@problame problame requested a review from a team as a code owner March 13, 2024 09:40
@problame problame changed the title WIP: tokio epoll uring: use it for on-demand downloads tokio epoll uring: use it for on-demand downloads Mar 13, 2024
@problame problame changed the title tokio epoll uring: use it for on-demand downloads tokio-epoll-uring: use it for on-demand downloads Mar 13, 2024
Copy link
Contributor

@koivunej koivunej left a comment

Choose a reason for hiding this comment

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

OwnedAsyncWriter looks good, nice tests around the buffering.

problame added a commit that referenced this pull request Mar 13, 2024
The OwnedAsyncWrite stuff is based on the code in
tokio-epoll-uring on-demand download PR (#6992), which hasn't merged
yet.
…kio-epoll-uring/write-path/ondemand-downloads
@problame
Copy link
Contributor Author

Only got to do the performance testing now, but, I think this is a fairly safe change, so, merging it now, even if there's less soak time before the next release.

@problame problame enabled auto-merge (squash) March 15, 2024 17:27
@problame problame merged commit ad6f538 into main Mar 15, 2024
54 of 55 checks passed
@problame problame deleted the problame/integrate-tokio-epoll-uring/write-path/ondemand-downloads branch March 15, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-benchmarks Indicates to the CI that benchmarks should be run for PR marked with this label run-extra-build-macos When placed on a PR, tells the CI to run a build on macOS. No unit tests are run, though.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants