Skip to content

chunkers: only use the direct read path for known regular files - #10065

Merged
ThomasWaldmann merged 1 commit into
borgbackup:masterfrom
ThomasWaldmann:reader-direct-regular-only
Aug 9, 2026
Merged

chunkers: only use the direct read path for known regular files#10065
ThomasWaldmann merged 1 commit into
borgbackup:masterfrom
ThomasWaldmann:reader-direct-regular-only

Conversation

@ThomasWaldmann

@ThomasWaldmann ThomasWaldmann commented Aug 9, 2026

Copy link
Copy Markdown
Member

Fixes the deterministic NetBSD CI hang introduced with PR #10060's direct read path.

The hang: test_create_read_special_symlink (FIFO via create --read-special) hung NetBSD 10.1 forever, at the same spot in both attempts of the PR #10060 CI run — one pytest-xdist worker stuck in the test from minute 2, the suite stalled at 88%. The earlier branch push without the direct-read commit passed NetBSD in 39 min, and FreeBSD/OpenBSD/OmniOS/Linux pass with it, so it bisects cleanly to the direct path reading a FIFO. Special files are opened blocking, and NetBSD was seen never returning from the direct path's big readv() on the FIFO, while the buffered block reader's 1 MiB os.read() calls have always worked there.

The fix: only known regular files take the direct path.

  • FileReader requires S_ISREG for direct. chunkify() (ChunkerBase, ChunkerFixed, ChunkerFailing + all .pyi stubs) accepts an optional st (os.stat_result) for callers that already have one.
  • process_file() passes the stat result it already has — the create hot path adds no stat syscall. Regular files keep the zero-copy direct path; --read-special FIFOs/devices route onto the buffered path via their stat mode.
  • Callers without a stat at hand still get the direct path where safe: FileReader fstats fh / fd.fileno() itself (one cheap syscall per file). Objects without an OS-level fd (BytesIO, recreate/transfer chunk-iterator wrappers) stay on the buffered path, as before fixed chunker: read directly into per-chunk buffers #10060.
  • New tests: direct path for a regular file with st given (data + EOF verified); direct path via FileReader's own fstat (fh and file object); buffered for file-likes without an OS fd; direct refused for a FIFO's st.

Local runs: chunkers suite 114 passed, create_cmd + benchmark tests 72 passed, mypy unchanged (same 12 pre-existing unrelated errors).

Note: with PR #10064's PYTEST_TIMEOUT=300, a hang like this would fail in 5 minutes with a stack dump instead of stalling CI for hours — the two PRs complement each other.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.76%. Comparing base (f9e6756) to head (800a4b8).
⚠️ Report is 6 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10065   +/-   ##
=======================================
  Coverage   86.75%   86.76%           
=======================================
  Files          98       98           
  Lines       17098    17100    +2     
  Branches     2590     2589    -1     
=======================================
+ Hits        14834    14837    +3     
  Misses       1570     1570           
+ Partials      694      693    -1     

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

The direct read path (big readv directly into the caller's buffer) was
seen blocking forever on NetBSD 10 when reading a FIFO via
create --read-special (test_create_read_special_symlink hung NetBSD CI
deterministically, at the same spot in two runs), while the buffered
block reader path's 1 MiB os.read() calls work fine there.

Restrict the direct path to known regular files: FileReader only goes
direct for S_ISREG modes. chunkify() accepts the file's os.stat_result
if the caller has it anyway (process_file does, so the create hot path
adds no stat syscall); otherwise FileReader fstats fh / fd.fileno()
itself, staying on the buffered path for objects without an OS-level
fd. Special files (--read-special) route onto the buffered path via
their stat mode, exactly as before this branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ThomasWaldmann
ThomasWaldmann force-pushed the reader-direct-regular-only branch from 130232d to 800a4b8 Compare August 9, 2026 16:50
@ThomasWaldmann
ThomasWaldmann merged commit a5d833e into borgbackup:master Aug 9, 2026
20 checks passed
@ThomasWaldmann
ThomasWaldmann deleted the reader-direct-regular-only branch August 9, 2026 17:44
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.

1 participant