Skip to content

refactor(fs): use builder for initialization of SequentialFileReader#9420

Merged
kskalski merged 1 commit into
anza-xyz:masterfrom
kskalski:ks/reader_builder
Dec 6, 2025
Merged

refactor(fs): use builder for initialization of SequentialFileReader#9420
kskalski merged 1 commit into
anza-xyz:masterfrom
kskalski:ks/reader_builder

Conversation

@kskalski
Copy link
Copy Markdown

@kskalski kskalski commented Dec 5, 2025

Problem

We need a more advanced way to create io_uring sequential file reader with several tuning options.
The nearest uses for this will be:

  • ability to enable/disable DIRECT_IO (we want to reserve ability to turn it off in case of bugs)
  • ability to enable/disable registering buffer with kernel as FIXED (it requires memlock ulimit that is not always available)
  • providing shared sqpoll FD
  • adjusting IO kernel workers, sqsize, etc. -> those can be tuned depending on whether sqpoll is used or if we use many readers across threads or pushing lots of ops into single threaded one, etc.

All those low level options would clutter the API when required to be passed as parameters to constructor functions.

Summary of Changes

  • add SequentialFileReaderBuilder, which uses defaults when created by new() and allows adjusting parameters and finally building the reader
  • extract some blocks of code used for initialization into separate functions
  • extract starting of reading into separate function (in the future we will allow building reader without providing the path, so start of reading will in fact be a separate flow)

@kskalski kskalski marked this pull request as ready for review December 5, 2025 05:33
@kskalski kskalski requested a review from brooksprumo December 5, 2025 05:33
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 5, 2025

Codecov Report

❌ Patch coverage is 98.64865% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.5%. Comparing base (e9052f0) to head (0537ccd).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9420   +/-   ##
=======================================
  Coverage    82.5%    82.5%           
=======================================
  Files         895      895           
  Lines      322338   322361   +23     
=======================================
+ Hits       266042   266097   +55     
+ Misses      56296    56264   -32     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread fs/src/io_uring/sequential_file_reader.rs
Comment thread fs/src/io_uring/sequential_file_reader.rs
Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

@kskalski kskalski enabled auto-merge December 5, 2025 15:48
@kskalski kskalski added this pull request to the merge queue Dec 6, 2025
Merged via the queue into anza-xyz:master with commit d8c1e2b Dec 6, 2025
47 checks passed
@kskalski kskalski deleted the ks/reader_builder branch December 6, 2025 01:33
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.

3 participants