Skip to content

fix: replace H5AD only after successful write - #2570

Open
stanbot8 wants to merge 2 commits into
scverse:mainfrom
stanbot8:agent/issue-522-transactional-h5ad-overwrite
Open

fix: replace H5AD only after successful write#2570
stanbot8 wants to merge 2 commits into
scverse:mainfrom
stanbot8:agent/issue-522-transactional-h5ad-overwrite

Conversation

@stanbot8

@stanbot8 stanbot8 commented Jul 24, 2026

Copy link
Copy Markdown

Summary

HDF5 can truncate a live H5AD target before it reports a lock failure (h5py issue 1864, HDF5 issue HDFFV-11230). This change writes a complete replacement to a temporary file. It replaces the target only after the write succeeds. Closes #522.

Design

The live target is never opened with mode="w". A failed write leaves an existing target unchanged. A successful replacement publishes a complete new file. The replacement preserves symbolic links.

The implementation changes src/anndata/_io/h5ad.py. The design follows the reported truncation reproducer, the requested overwrite behavior, and the HDF5 lock analysis.

Limitations

  • The overwrite briefly needs additional free space approximately equal to the new H5AD file size. A 10 GB output can need approximately 10 GB of additional free space.
  • The replacement rejects a target with multiple hard links.
  • The replacement changes the inode and copies the file mode.
  • It does not copy ownership, ACLs, or extended attributes.
  • It does not add directory-sync steps.

Tests

  • Race tests cover file appearance, replacement, and concurrent readers.
  • Disk-full tests preserve existing HDF5 and non-HDF5 targets. New targets stay absent.
  • New files keep the direct-create POSIX file mode.
  • Focused tests passed on Windows and Ubuntu.
  • The full Windows read/write suite passed 256 tests and skipped 23.
  • Stress tests passed 60 reader cases and 10 rounds with 4 writers.
  • The installed wheel, 64 MiB replacement, and pre-commit checks passed.

All 11 CPU matrix jobs passed. The benchmark and patch coverage checks passed.

Commands
pytest -q tests/test_readwrite.py -k 'test_write_h5ad'

@stanbot8
stanbot8 force-pushed the agent/issue-522-transactional-h5ad-overwrite branch from c1561d0 to 12ae834 Compare July 24, 2026 08:51
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.90%. Comparing base (8660a9d) to head (2358fec).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/anndata/_io/h5ad.py 92.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2570      +/-   ##
==========================================
- Coverage   87.83%   85.90%   -1.94%     
==========================================
  Files          49       49              
  Lines        7799     7823      +24     
==========================================
- Hits         6850     6720     -130     
- Misses        949     1103     +154     
Files with missing lines Coverage Δ
src/anndata/_io/h5ad.py 93.13% <92.00%> (-0.17%) ⬇️

... and 8 files with indirect coverage changes

@stanbot8
stanbot8 marked this pull request as ready for review July 24, 2026 09:22
@flying-sheep flying-sheep added this to the 0.13.3 milestone Jul 24, 2026

@flying-sheep flying-sheep left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but the tests need slimming down

Comment thread tests/test_readwrite.py Outdated
Comment thread tests/test_readwrite.py Outdated
@stanbot8
stanbot8 force-pushed the agent/issue-522-transactional-h5ad-overwrite branch from 12ae834 to c66a734 Compare July 24, 2026 13:08
Stage each destructive write next to the target. Replace the target only after the new file is complete. Preserve the target after write and replacement failures.
@stanbot8
stanbot8 force-pushed the agent/issue-522-transactional-h5ad-overwrite branch from c66a734 to 68c7fa5 Compare July 24, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Opening file with "w" while it's open in backed mode elsewhere still deletes file contents

2 participants