Skip to content

fix(fs): handle reads for file with zero size / limit#10900

Merged
kskalski merged 1 commit into
anza-xyz:masterfrom
kskalski:ks/fix/zero_read
Mar 3, 2026
Merged

fix(fs): handle reads for file with zero size / limit#10900
kskalski merged 1 commit into
anza-xyz:masterfrom
kskalski:ks/fix/zero_read

Conversation

@kskalski
Copy link
Copy Markdown

@kskalski kskalski commented Mar 2, 2026

Problem

Files with 0 size or set to be read with limit 0 are not handled properly by SequentialFileReader.

This bug was introduced by #9701, which:

  • added limits for read file length (obviously not handling 0 properly)
  • added concept of reading a sequence of multiple files (causes a bit more complexity when handling switch between current and next file if current or next files have read limit 0)

Summary of Changes

The fix covers two areas:

  • skip obtaining full buffer if the current file state indicates no scheduled reads - this only happens for current file when read_limit == 0 (read ops are skipped and start_buf_index is never set)
  • when moving to read next file (i.e. discarding current file as finished reading from) we should:
    • only reclaim buffers if that file did any reads
    • make sure we look through all queued files to search for buffer index that was used for next scheduled read (some queued files might be zero limited) or fallback properly to the index to be used as such

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 96.36364% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.0%. Comparing base (b269166) to head (93d9121).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #10900     +/-   ##
=========================================
- Coverage    83.0%    83.0%   -0.1%     
=========================================
  Files         837      837             
  Lines      316505   316542     +37     
=========================================
+ Hits       262813   262823     +10     
- Misses      53692    53719     +27     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kskalski kskalski marked this pull request as ready for review March 2, 2026 13:59
Comment thread fs/src/io_uring/sequential_file_reader.rs Outdated
@brooksprumo
Copy link
Copy Markdown

Thanks!

r+ sme approval

@steviez
Copy link
Copy Markdown

steviez commented Mar 2, 2026

Per Discord discussion, sounds like we need/want a v4.0 backport as well. It sounded like kskalski was going to look at one more item so going to let him add the label after he checks whatever he needs

@kskalski
Copy link
Copy Markdown
Author

kskalski commented Mar 3, 2026

Confirmed this fixes the agave-fs internal panic on the problematic (empty) archive. Validator still doesn't like that file, but for the scope of this PR it works fine.

@kskalski kskalski added this pull request to the merge queue Mar 3, 2026
Merged via the queue into anza-xyz:master with commit 70a3438 Mar 3, 2026
51 checks passed
@kskalski kskalski deleted the ks/fix/zero_read branch March 3, 2026 02:47
@kskalski kskalski added the v4.0 Backport to v4.0 branch label Mar 3, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 3, 2026

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify Bot pushed a commit that referenced this pull request Mar 3, 2026
kskalski added a commit that referenced this pull request Mar 4, 2026
… of #10900) (#10927)

fix(fs): handle reads for file with zero size / limit (#10900)

(cherry picked from commit 70a3438)

Co-authored-by: Kamil Skalski <kamil.skalski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v4.0 Backport to v4.0 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants