Skip to content

fix(doc): Concurrency bug in striped_downloads.md guide#4731

Merged
coryan merged 2 commits intogoogleapis:mainfrom
fellhorn:fix/striped-downloads
Feb 19, 2026
Merged

fix(doc): Concurrency bug in striped_downloads.md guide#4731
coryan merged 2 commits intogoogleapis:mainfrom
fellhorn:fix/striped-downloads

Conversation

@fellhorn
Copy link
Copy Markdown
Contributor

@fellhorn fellhorn commented Feb 19, 2026 •
edited by coryan
Loading

The guide on how to implement striped downloads uses tokio::fs::File in a non-thread-safe way by concurrently seeking the file cursor across the different tasks. The downloaded file will usually be messed up as the chunks are written with the wrong cursor position.

This PR adds a reference implementation using pwrite for Unix systems.

Towards #4730

@fellhorn
fix(doc): Concurrent seek in striped download
6c9b698 
Signed-off-by: Dennis Keck <26092524+fellhorn@users.noreply.github.com>
@fellhorn fellhorn requested a review from a team as a code owner February 19, 2026 08:20
@fellhorn
Copy link
Copy Markdown
Contributor Author

fellhorn commented Feb 19, 2026

I don't have access to any Windows machines to verify that seek_write from std::os::windows::fs::FileExt would work and is more or less performant.

@coryan
Copy link
Copy Markdown
Collaborator

coryan commented Feb 19, 2026

I don't have access to any Windows machines to verify that seek_write from std::os::windows::fs::FileExt would work and is more or less performant.

First, thanks for the bug report and PR. I am quite embarrassed you had to find this problem yourself.

Second, I think we do need the example to work on Windows, even if we must caveat the code with "beware, this may be slower". Do you want to take a stab at making the Windows changes too? If you prefer, we can merge the code with a panic!() on Windows referencing the bug you filed.

@coryan
Copy link
Copy Markdown
Collaborator

coryan commented Feb 19, 2026

/gcbrun

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 19, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (095167f) to head (5a82eb8).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4731      +/-   ##
==========================================
- Coverage   95.05%   95.04%   -0.02%     
==========================================
  Files         198      198              
  Lines        7581     7581              
==========================================
- Hits         7206     7205       -1     
- Misses        375      376       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coryan
Copy link
Copy Markdown
Collaborator

coryan commented Feb 19, 2026

Some formatting problems, can you use mdformat on this file:


diff --git a/guide/src/storage/striped_downloads.md b/guide/src/storage/striped_downloads.md
--
  | index e8293a1..c471416 100644
  | --- a/guide/src/storage/striped_downloads.md
  | +++ b/guide/src/storage/striped_downloads.md
  | @@ -122,7 +122,8 @@ futures:
  |  
  | Once they complete, the file is downloaded.
  |  
  | -Now you should complete writing the `write_stripe()` function. First, start a download from Cloud Storage:
  | +Now you should complete writing the `write_stripe()` function. First, start a
  | +download from Cloud Storage:
  |  
  | ```rust,ignore,noplayground
  | {{#rustdoc_include ../../samples/tests/storage/striped.rs:write-stripe-reader}}
  | @@ -143,10 +144,11 @@ get inconsistent reads:
  | {{#rustdoc_include ../../samples/tests/storage/striped.rs:write-stripe-reader-generation}}
  | ```
  |  
  | -Then you read the data and write it to the local file. This example uses the Unix
  | -specific `write_all_at` to write the chunk with a specific cursor offset in a thread-safe way.
  | -On Windows use `seek_write` from `std::os::windows::fs::FileExt` instead.
  | -To not block within the future, we run the closure on a dedicated thread:
  | +Then you read the data and write it to the local file. This example uses the
  | +Unix specific `write_all_at` to write the chunk with a specific cursor offset in
  | +a thread-safe way. On Windows use `seek_write` from
  | +`std::os::windows::fs::FileExt` instead. To not block within the future, we run
  | +the closure on a dedicated thread:

@coryan
Copy link
Copy Markdown
Collaborator

coryan commented Feb 19, 2026

/gcbrun

coryan
coryan approved these changes Feb 19, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@coryan coryan left a comment

Choose a reason for hiding this comment

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

I will make the fixes for Windows in a second pass.

@coryan
Copy link
Copy Markdown
Collaborator

coryan commented Feb 19, 2026

Thanks again for the PR! I made some formatting changes and will fix the code on Windows shortly.

@coryan coryan merged commit fbfe347 into googleapis:main Feb 19, 2026
35 of 36 checks passed
@fellhorn
Copy link
Copy Markdown
Contributor Author

fellhorn commented Feb 20, 2026

Sorry, I was busy with other things yesterday. Thanks for taking over the Windows part!

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

Reviewers

@coryan coryan coryan approved these changes

@PhongChuong PhongChuong PhongChuong approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fellhorn @coryan @PhongChuong