facade: open_swmr reads live only files with the SWMR-write flag
A file opened with open_swmr whose superblock does not have the SWMR-write flag (its writer has closed it) is now read exactly as File::open reads it: bounded by its recorded end of file, through the chunk cache, each operation tried once, and is_swmr_read() is false. Before, every file opened with open_swmr ignored its recorded end of file, so a closed file whose end of file is below its length (h5clear_fsm_persist_less.h5, or the mid-write fixture with its flags cleared) listed and read objects that File::open and libhdf5's plain reader refuse. Opening such a file is not retried once the superblock has been read. libhdf5's SWMR reader is looser than either (it reads the fixture's chunk index past the end of file even with the flags cleared); a test pins what h5py does and docs/design/swmr.md says why we follow the plain reader. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -27,6 +27,10 @@ Design: `docs/design/swmr.md`.
|
|||||||
would read as fill where the writer has since written). A file open for
|
would read as fill where the writer has since written). A file open for
|
||||||
writing without SWMR is refused with `Error::Locked`, as libhdf5 refuses
|
writing without SWMR is refused with `Error::Locked`, as libhdf5 refuses
|
||||||
it.
|
it.
|
||||||
|
Only a file whose superblock has the SWMR-write flag when it is opened is
|
||||||
|
read this way; any other file (its writer has closed it) reads exactly as
|
||||||
|
`File::open` reads it, bounded by its recorded end of file, and
|
||||||
|
`is_swmr_read()` is `false`.
|
||||||
- **`Dataset::refresh()`** reads the dataset's object header again
|
- **`Dataset::refresh()`** reads the dataset's object header again
|
||||||
(`H5Drefresh`, h5py `Dataset.refresh()`), so `shape()` and later reads
|
(`H5Drefresh`, h5py `Dataset.refresh()`), so `shape()` and later reads
|
||||||
see the writer's appends; a handle keeps its extent until refreshed.
|
see the writer's appends; a handle keeps its extent until refreshed.
|
||||||
|
|||||||
@@ -142,7 +142,7 @@ impl FileData {
|
|||||||
/// bytes past the recorded end of file are not read, as in libhdf5.
|
/// bytes past the recorded end of file are not read, as in libhdf5.
|
||||||
fn new(mut backing: Backing) -> Result<(Self, Superblock), Error> {
|
fn new(mut backing: Backing) -> Result<(Self, Superblock), Error> {
|
||||||
if let Backing::Storage(storage) = backing {
|
if let Backing::Storage(storage) = backing {
|
||||||
return Self::new_storage(storage, false);
|
return Self::new_storage(storage, false, &mut None);
|
||||||
}
|
}
|
||||||
let whole = backing.whole_file().unwrap_or_default();
|
let whole = backing.whole_file().unwrap_or_default();
|
||||||
let (user_block, hdf5) = signature::split_user_block(whole)?;
|
let (user_block, hdf5) = signature::split_user_block(whole)?;
|
||||||
@@ -184,8 +184,19 @@ impl FileData {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// [`Self::new`] for a [`Storage`] backend: the same checks, through
|
/// [`Self::new`] for a [`Storage`] backend: the same checks, through
|
||||||
/// reads of the storage. A `live` file is never read as one slice.
|
/// reads of the storage.
|
||||||
fn new_storage(storage: SharedStorage, live: bool) -> Result<(Self, Superblock), Error> {
|
///
|
||||||
|
/// With `swmr` ([`File::open_storage_swmr`]) the file is read live when
|
||||||
|
/// its superblock has the SWMR-write flag (version 3), as libhdf5's SWMR
|
||||||
|
/// reader reads it: reads end where the file ends at the time of each
|
||||||
|
/// read, and nothing is read as one slice. Any other file is read as
|
||||||
|
/// without `swmr`, bounded by its recorded end of file. Once the
|
||||||
|
/// superblock has been read, `flagged` says which it was.
|
||||||
|
fn new_storage(
|
||||||
|
storage: SharedStorage,
|
||||||
|
swmr: bool,
|
||||||
|
flagged: &mut Option<bool>,
|
||||||
|
) -> Result<(Self, Superblock), Error> {
|
||||||
let file_len = storage.len();
|
let file_len = storage.len();
|
||||||
let base = signature::find_signature_in(&*storage)?;
|
let base = signature::find_signature_in(&*storage)?;
|
||||||
let mut data = Self {
|
let mut data = Self {
|
||||||
@@ -196,12 +207,16 @@ impl FileData {
|
|||||||
overlay: Vec::new(),
|
overlay: Vec::new(),
|
||||||
image_error: None,
|
image_error: None,
|
||||||
contiguous: None,
|
contiguous: None,
|
||||||
live,
|
// Until the superblock says otherwise: its own reads are not
|
||||||
|
// bounded by an end of file it has not read yet.
|
||||||
|
live: swmr,
|
||||||
};
|
};
|
||||||
// Worked out again below, once the end of file and any cache image
|
// Worked out again below, once the end of file and any cache image
|
||||||
// are known.
|
// are known.
|
||||||
data.contiguous = data.find_contiguous();
|
data.contiguous = data.find_contiguous();
|
||||||
let superblock = Superblock::parse_in(&data, 0)?;
|
let superblock = Superblock::parse_in(&data, 0)?;
|
||||||
|
data.live = swmr && superblock.version >= 3 && superblock.is_swmr_write();
|
||||||
|
*flagged = Some(data.live);
|
||||||
data.end = base + superblock.data_end(base, file_len)?;
|
data.end = base + superblock.data_end(base, file_len)?;
|
||||||
match superblock_ext::cache_image_state_in(&data, &superblock)? {
|
match superblock_ext::cache_image_state_in(&data, &superblock)? {
|
||||||
CacheImageState::Absent => {}
|
CacheImageState::Absent => {}
|
||||||
@@ -515,10 +530,20 @@ impl File {
|
|||||||
/// attempt in which every structure read verified, so a torn read is
|
/// attempt in which every structure read verified, so a torn read is
|
||||||
/// an error (after the last attempt), never data.
|
/// an error (after the last attempt), never data.
|
||||||
///
|
///
|
||||||
|
/// All of this applies only to a file whose superblock (version 3) has
|
||||||
|
/// the SWMR-write flag set when it is opened: a file a SWMR writer has
|
||||||
|
/// open, or had and did not close. libhdf5's SWMR writer does not keep
|
||||||
|
/// the superblock's recorded end of file current, so for such a file it
|
||||||
|
/// is ignored, as libhdf5's SWMR reader ignores it. Any other file (one
|
||||||
|
/// whose writer has closed it, or that was never written in SWMR mode)
|
||||||
|
/// is read exactly as [`File::open`] reads it — bounded by its recorded
|
||||||
|
/// end of file, through the chunk cache, each operation tried once —
|
||||||
|
/// and [`is_swmr_read`](Self::is_swmr_read) is `false`. Which of the two
|
||||||
|
/// a handle is does not change after it is opened.
|
||||||
|
///
|
||||||
/// A file whose superblock says it is open for writing without SWMR is
|
/// A file whose superblock says it is open for writing without SWMR is
|
||||||
/// refused with [`Error::Locked`], as libhdf5 refuses it: such a writer
|
/// refused with [`Error::Locked`], as libhdf5 refuses it: such a writer
|
||||||
/// does not order its writes for readers. Any other file opens (one
|
/// does not order its writes for readers.
|
||||||
/// whose writer has closed it reads like [`File::open`]).
|
|
||||||
pub fn open_swmr<P: AsRef<std::path::Path>>(path: P) -> Result<Self, Error> {
|
pub fn open_swmr<P: AsRef<std::path::Path>>(path: P) -> Result<Self, Error> {
|
||||||
let storage = crate::swmr::FileStorage::open(path.as_ref()).map_err(Error::Io)?;
|
let storage = crate::swmr::FileStorage::open(path.as_ref()).map_err(Error::Io)?;
|
||||||
let mut f = Self::open_storage_swmr(Arc::new(storage))?;
|
let mut f = Self::open_storage_swmr(Arc::new(storage))?;
|
||||||
@@ -532,7 +557,16 @@ impl File {
|
|||||||
/// `HttpStorage`), does not show the writer's appends.
|
/// `HttpStorage`), does not show the writer's appends.
|
||||||
pub fn open_storage_swmr(storage: SharedStorage) -> Result<Self, Error> {
|
pub fn open_storage_swmr(storage: SharedStorage) -> Result<Self, Error> {
|
||||||
let swmr = crate::swmr::Retries::default();
|
let swmr = crate::swmr::Retries::default();
|
||||||
let (data, superblock) = swmr.retry(|| FileData::new_storage(storage.clone(), true))?;
|
// Retried only while the superblock cannot be read, or says a SWMR
|
||||||
|
// writer has the file: an error opening any other file is final.
|
||||||
|
let (data, superblock) = swmr.retry(|| {
|
||||||
|
let mut flagged = None;
|
||||||
|
match FileData::new_storage(storage.clone(), true, &mut flagged) {
|
||||||
|
Ok(opened) => Ok(Ok(opened)),
|
||||||
|
Err(e) if flagged == Some(false) => Ok(Err(e)),
|
||||||
|
Err(e) => Err(e),
|
||||||
|
}
|
||||||
|
})??;
|
||||||
if superblock.version >= 3 && superblock.is_write_access() && !superblock.is_swmr_write() {
|
if superblock.version >= 3 && superblock.is_write_access() && !superblock.is_swmr_write() {
|
||||||
return Err(Error::Locked(
|
return Err(Error::Locked(
|
||||||
"the file is open for writing without SWMR (libhdf5: \"file is already open \
|
"the file is open for writing without SWMR (libhdf5: \"file is already open \
|
||||||
@@ -550,15 +584,19 @@ impl File {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Whether the file was opened with [`File::open_swmr`] or
|
/// Whether the file is read live: opened with [`File::open_swmr`] or
|
||||||
/// [`File::open_storage_swmr`].
|
/// [`File::open_storage_swmr`] while its superblock had the SWMR-write
|
||||||
|
/// flag set. `false` for every other file, including one opened with
|
||||||
|
/// `open_swmr` whose writer had already closed it (it reads as
|
||||||
|
/// [`File::open`] reads it).
|
||||||
pub fn is_swmr_read(&self) -> bool {
|
pub fn is_swmr_read(&self) -> bool {
|
||||||
self.data.live
|
self.data.live
|
||||||
}
|
}
|
||||||
|
|
||||||
/// How many times an operation on a SWMR-read file is tried (see
|
/// How many times an operation on a SWMR-read file is tried (see
|
||||||
/// [`File::open_swmr`]); 100 unless set. Files not opened for SWMR
|
/// [`File::open_swmr`]); 100 unless set. Files not read live
|
||||||
/// reading try every operation once.
|
/// ([`is_swmr_read`](Self::is_swmr_read) `false`) try every operation
|
||||||
|
/// once.
|
||||||
pub fn swmr_read_attempts(&self) -> u32 {
|
pub fn swmr_read_attempts(&self) -> u32 {
|
||||||
self.swmr.attempts
|
self.swmr.attempts
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -166,6 +166,94 @@ fn open_swmr_refuses_a_file_open_for_writing_without_swmr() {
|
|||||||
assert!(!f.swmr_writer_active().unwrap());
|
assert!(!f.swmr_writer_active().unwrap());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The outcome of reading every dataset of `f` in full, as text: the
|
||||||
|
/// values, or the error.
|
||||||
|
fn read_all(f: &File) -> Vec<String> {
|
||||||
|
["a", "b"]
|
||||||
|
.iter()
|
||||||
|
.map(|n| match f.dataset(n).and_then(|d| d.read_f64()) {
|
||||||
|
Ok(v) => format!("{n}: {} values", v.len()),
|
||||||
|
Err(e) => format!("{n}: error {e}"),
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn open_swmr_reads_a_file_without_the_swmr_write_flag_as_file_open_does() {
|
||||||
|
// The mid-write copy with its flags cleared: a closed file that
|
||||||
|
// records an end of file of 715 in 6 030 bytes, its chunk indexes past
|
||||||
|
// that end. File::open (and libhdf5's plain reader) refuse what lies
|
||||||
|
// past the recorded end; open_swmr must too, and must not retry (the
|
||||||
|
// file is not live). See the h5py test below for libhdf5's SWMR reader.
|
||||||
|
let bytes = mid_write_copy_with_flags(0);
|
||||||
|
let dir = tempfile::tempdir_in(env!("CARGO_TARGET_TMPDIR")).unwrap();
|
||||||
|
let path = dir.path().join("closed_short_eof.h5");
|
||||||
|
std::fs::write(&path, &bytes).unwrap();
|
||||||
|
|
||||||
|
let plain = File::open(&path).unwrap();
|
||||||
|
let swmr = File::open_swmr(&path).unwrap();
|
||||||
|
let swmr_storage = File::open_storage_swmr(Arc::new(bytes)).unwrap();
|
||||||
|
let want = read_all(&plain);
|
||||||
|
assert!(
|
||||||
|
want.iter().all(|r| r.contains("error")),
|
||||||
|
"File::open reads past the recorded end: {want:?}"
|
||||||
|
);
|
||||||
|
for f in [&swmr, &swmr_storage] {
|
||||||
|
assert!(!f.is_swmr_read());
|
||||||
|
assert!(!f.swmr_writer_active().unwrap());
|
||||||
|
let started = std::time::Instant::now();
|
||||||
|
assert_eq!(read_all(f), want);
|
||||||
|
assert!(started.elapsed() < std::time::Duration::from_millis(100));
|
||||||
|
assert_eq!(f.swmr_retries(), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
// The same bytes with the SWMR-write flag (and write access) set are
|
||||||
|
// read live, past the recorded end.
|
||||||
|
let live = File::open_storage_swmr(Arc::new(mid_write_copy_with_flags(0x05))).unwrap();
|
||||||
|
assert!(live.is_swmr_read());
|
||||||
|
check_mid_write_copy(&live);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn how_h5py_reads_the_closed_copy_past_its_recorded_end_of_file() {
|
||||||
|
skip_if_no_python!();
|
||||||
|
let dir = tempfile::tempdir_in(env!("CARGO_TARGET_TMPDIR")).unwrap();
|
||||||
|
let path = dir.path().join("closed_short_eof.h5");
|
||||||
|
std::fs::write(&path, mid_write_copy_with_flags(0)).unwrap();
|
||||||
|
// libhdf5's plain reader refuses `a` (its chunk index is past the
|
||||||
|
// recorded end of file), as File::open and open_swmr do above.
|
||||||
|
// libhdf5's SWMR reader skips its end-of-allocation check for every
|
||||||
|
// file it opens (H5FD_read), and reads it; it still refuses an object
|
||||||
|
// *header* past the end ("address of object past end of allocation",
|
||||||
|
// H5O_protect). open_swmr does not copy that half-way rule for files
|
||||||
|
// without the SWMR-write flag: they read as File::open reads them.
|
||||||
|
let script = format!(
|
||||||
|
r#"
|
||||||
|
import h5py
|
||||||
|
for swmr in (False, True):
|
||||||
|
try:
|
||||||
|
with h5py.File("{p}", "r", swmr=swmr, locking=False) as f:
|
||||||
|
f["a"][()]
|
||||||
|
except Exception as e:
|
||||||
|
print("refused", swmr)
|
||||||
|
else:
|
||||||
|
print("read", swmr)
|
||||||
|
"#,
|
||||||
|
p = path.display()
|
||||||
|
);
|
||||||
|
let out = Command::new(python())
|
||||||
|
.args(["-c", &script])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
let stdout = String::from_utf8_lossy(&out.stdout);
|
||||||
|
assert_eq!(
|
||||||
|
stdout.split_whitespace().collect::<Vec<_>>(),
|
||||||
|
["refused", "False", "read", "True"],
|
||||||
|
"{}",
|
||||||
|
String::from_utf8_lossy(&out.stderr)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// A storage whose next `torn` reads come back garbled, as a read racing a
|
/// A storage whose next `torn` reads come back garbled, as a read racing a
|
||||||
/// rewrite of the structure can see them: the middle byte changed, or with
|
/// rewrite of the structure can see them: the middle byte changed, or with
|
||||||
/// `invert` every byte (signatures included).
|
/// `invert` every byte (signatures included).
|
||||||
|
|||||||
@@ -76,6 +76,18 @@ writer keeps appending. What the format and the library guarantee:
|
|||||||
chunks it needs again. A cached index would hide new chunks, and a
|
chunks it needs again. A cached index would hide new chunks, and a
|
||||||
cached partial edge chunk would read as fill where the writer has since
|
cached partial edge chunk would read as fill where the writer has since
|
||||||
written data (unfiltered edge chunks are rewritten in place).
|
written data (unfiltered edge chunks are rewritten in place).
|
||||||
|
- Only a file whose superblock has the SWMR-write flag when it is opened
|
||||||
|
is read live. Any other file is read exactly as `File::open` reads it
|
||||||
|
(bounded by its recorded EOF, through the chunk cache, no retries), and
|
||||||
|
`is_swmr_read()` is `false`. libhdf5's SWMR reader is looser: it skips
|
||||||
|
the end-of-allocation check in `H5FD_read` for every file it opens,
|
||||||
|
flagged or not, yet still refuses an object header past the EOF
|
||||||
|
(`H5O_protect`, "address of object past end of allocation"). On a
|
||||||
|
closed file whose EOF is below its length (checked 2026-09-27, h5py
|
||||||
|
3.16 / HDF5 2.0, the mid-write fixture with its flags cleared) it
|
||||||
|
therefore reads a dataset whose chunk index lies past the EOF, which
|
||||||
|
its plain reader refuses. We follow the plain reader there: the EOF of
|
||||||
|
a file no SWMR writer has open is what the file says it is.
|
||||||
- A storage that caches blocks (`clawhdf5-remote`'s `BlockCache`) would
|
- A storage that caches blocks (`clawhdf5-remote`'s `BlockCache`) would
|
||||||
serve stale bytes; `HttpStorage` also pins a file by ETag and length and
|
serve stale bytes; `HttpStorage` also pins a file by ETag and length and
|
||||||
refuses a changed file. Remote SWMR is out of scope.
|
refuses a changed file. Remote SWMR is out of scope.
|
||||||
|
|||||||
Reference in New Issue
Block a user