diff --git a/CHANGELOG.md b/CHANGELOG.md index 252855c..0b6b3b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,112 @@ # Changelog +## Unreleased + +### Added + +- `H5File::set_libver_latest` opts datasets created after the call into the + latest file format, the equivalent of libhdf5's + `H5Pset_libver_bounds(low = H5F_LIBVER_V200)`: filtered chunked datasets + get a version-5 data layout message whose chunk indexes store on-disk + chunk sizes in fixed 8-byte fields, removing the overflow risk when a + filter expands a chunk. Off by default — a v5 file needs libhdf5 ≥ 2.0 + (h5py bundling hdf5 1.14 rejects it), while the default v4 stays readable + everywhere. Independent of the knob, a chunk larger than 4 GiB forces + version 5, matching libhdf5, because v4 cannot represent its size field. + The version read from an existing file is preserved on reopen, so + appending to a v5 file never silently downgrades it. In-memory chunk-size + fields in the fixed-array and v2-B-tree indexes widened from `u32` to + `u64` to carry the 8-byte field. (issue #8) + +### Fixed + +- Concurrent operations on the *same* dataset serialize wholly under the + `threadsafe` feature. Each dataset now carries an operation lock beside + its metadata slot; every public write entry (`write_chunk*`, + `write_slice`, `append*`, `extend`/`set_extent`, `flush`) holds it for + the operation's full duration. The per-slot mutex only serialized each + individual acquisition, so two threads appending to one dataset could + interleave between the buffer take, the chunk writes and the extend — + losing or doubling rows. Same-dataset concurrency was previously + documented as unsupported; it is now correct, and writes to different + datasets still never contend. + +- Chunk geometry is validated at every dataset create, the rule libhdf5 + applies in `H5D__chunk_construct`: the chunk rank must match the dataspace, + no chunk dimension may be zero, and a chunk dimension may not exceed a + fixed maximum dimension unless that dimension's current size is zero. The + extensible-array and compressed-vlen creators previously accepted any + geometry, and a chunk wider than a fixed dimension made appends land rows + at the chunk stride — `[1, 2, 3, 4]` read back as `[1, 2, 0, 0]`. + + **Behavior change:** `SwmrWriter::create_streaming_dataset_tiled` no longer + accepts a chunk tile larger than the frame. libhdf5 refuses to create that + geometry, so no libhdf5-based writer (including the NDFileHDF5 tiling + controls the API mirrors) can produce such a file; previously the frame was + zero-padded up to the tile. + +- `append_vlen_strings` now applies the same character-set rule as + `write_vlen_strings_slice` — non-ASCII strings are rejected when the + dataset declares ASCII, instead of being stored mislabeled (libhdf5 + stores the bytes unvalidated; h5py raises on the same mismatch) — and + refuses a dataset whose elements are not variable-length strings, which + it previously overwrote with vlen references as raw bytes. + +- The append buffer records the absolute row its frames belong to, and + every operation that writes rows the buffer holds flushes it to the + chunks first. Before this, the buffer's position was derived from the + current extent, and two operations broke the derivation: a typed + `write_slice` into the buffered tail was silently overwritten by the + flush at close (write 99, read back 50), and an `extend` with buffered + appends made the flush land them at the extended end instead of where + they were appended. `write_vlen_strings_slice` drops its + patch-the-buffer path for the same flush-first rule. + +- Appends work on every chunk index and chunk shape. The append paths' + chunk writes required the extensible-array index, so appending to a + fixed-array or v2 B-tree dataset buffered fine and then failed at + `close()` with "not a chunked dataset", losing the buffered rows. They + also packed rows at the frame stride, so a chunk row narrower than the + frame — legal, libhdf5-creatable geometry — corrupted the first chunk + and errored on the second. Append writes now go through the same + index-generic hyperslab engine as `write_slice`, which also makes + appends to reopened 0.4.0 files with a wider-than-row chunk land + correctly instead of reading back `[1, 2, 0, 0]` for `[1, 2, 3, 4]`. + +- Every dataset creator checks the new name is unique before registering + the dataset. The vlen creators and `create_chunked_dataset_compressed` + skipped the check, so creating two datasets under one name silently + emitted an invalid file with two same-named links. The check-then-push + pair is now a witness type (`begin_create` → `push_dataset`), so a + creator cannot skip it. + +- Chunk slots are computed against the **maximum-extent** chunk grid, the + libhdf5 rule (`max_down_chunks` in H5Dfarray.c/H5Dearray.c/H5Dnone.c; + the fixed array is sized from `max_nchunks`). Slots were computed from + the *current* extent, which coincides only while every dimension after + the first sits at its maximum — any other geometry wrote files libhdf5 + reads differently, and extending re-scrambled the mapping. One owner + (`io::chunk_grid`) now serves the writer, the reader, and the dataset + API. Fallout fixed with it: + - The builder silently dropped a finite `max_shape` on the fixed-array + path (no unlimited dimension): the array was sized from the current + shape and the stored dataspace had no maximum, so the dataset could + never grow. `create_fixed_array_dataset_with_max` sizes the array + from the maximum's grid and such datasets now extend/append up to + their maximum. The fixed-shape creators keep their signatures and + now store `max_dims == dims` explicitly. + - **Behavior change:** growing a dataset past its stored maximum — or + growing one with *no* stored maximum at all — is rejected by + `extend_dataset`/`set_dataset_extent`, matching `H5Dset_extent` + (libhdf5 defaults maxdims to dims at creation). Previously the grow + succeeded and writes failed later, or scrambled chunk slots. + - **Behavior change:** creating an extensible-array dataset whose + unlimited dimension is not dimension 0 is rejected. Its chunks have + no fixed linear slot without libhdf5's swizzling (not implemented); + the geometry previously re-indexed — i.e. silently lost — chunks on + every extend. Reading such a file (libhdf5-written) now errors + instead of returning wrong data. + ## 0.4.1 ### Added diff --git a/docs/threadsafe-fine-grained-locking.md b/docs/threadsafe-fine-grained-locking.md index 7ce34cc..d6a3671 100644 --- a/docs/threadsafe-fine-grained-locking.md +++ b/docs/threadsafe-fine-grained-locking.md @@ -140,17 +140,28 @@ chunked dataset) becomes: lock the *one* target `DatasetSlot`, do the short allocate+write+record, unlock. Different datasets run fully in parallel. -Scope of the same-dataset guarantee: a *single* slot operation — one -`write_chunk` / `record_*` — serializes on that slot's `Mutex`, so it -cannot tear the chunk index. It does **not** make a multi-step sequence -atomic. `append` deliberately drops the slot between phases (read dims → -take `append_buffer` → compress + write each chunk → `extend_dataset`) so -compression and disk I/O run unlocked; two threads appending to the *same* -dataset can therefore interleave those phases and corrupt the -buffer/dimension accounting. The supported concurrency is across -**distinct** datasets (one writer per dataset, e.g. a rayon `par_iter` -over datasets). Concurrent writes to a *single* dataset are unsupported; -callers must serialize them externally. +Scope of the same-dataset guarantee: the metadata slot's `Mutex` +serializes a *single* acquisition — one `record_*`, one buffer take — so +it cannot tear the chunk index, but it does not make a multi-step +sequence atomic. `append` deliberately drops the slot between phases +(read dims → take `append_buffer` → compress + write each chunk → +`extend_dataset`) so compression and disk I/O run unlocked. Whole +operations are serialized one level up: each `DatasetCell` carries an +**op lock** beside its metadata slot, every public write entry +(`write_chunk*`, `write_slice`, `append*`, `extend`/`set_extent`, +`flush`) takes it for the operation's full duration and delegates to a +`_inner` variant, and multi-acquisition compositions in `dataset.rs` +take it around their whole sequence. `_inner` variants and the +`pub(crate)` helpers (`write_append_frames`, `flush_append_buffer*`, +`write_chunk_at_coords`) require the caller to hold it — or to hold the +writer exclusively via `&mut`, as close and the SWMR wrapper do. The op +lock is not reentrant; the single-thread build's `RefCell` panics on a +nested acquisition, so a missed entry/inner split fails in every test +run. Lock order: `create_lock → op → registry spine → metadata slot`, +never two datasets' op locks at once. Concurrent operations on the +*same* dataset are therefore supported and serialize wholly +(`tests/threadsafe_same_dataset_append.rs`); the parallelism win remains +across distinct datasets, whose op locks never contend. ### 4.4 `SharedInner` — drop the outer `Mutex` diff --git a/src/dataset.rs b/src/dataset.rs index 6e0dddd..c3af66f 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -319,20 +319,13 @@ impl DatasetBuilder { // must use the fixed-array index — libhdf5 // rejects an extensible-array index here. A // compressed fixed-shape dataset uses a *filtered* - // fixed array (FA client id 1). - if wants_filter { - writer.create_fixed_array_dataset_with_pipeline( - &full_name, - datatype, - &dims_u64, - &chunk_u64, - explicit_pipeline(), - )? - } else { - writer.create_fixed_array_dataset( - &full_name, datatype, &dims_u64, &chunk_u64, - )? - } + // fixed array (FA client id 1). The maximum shape + // sizes the array, so a finite max above the + // current shape stays growable. + let pipeline = wants_filter.then(explicit_pipeline); + writer.create_fixed_array_dataset_with_max( + &full_name, datatype, &dims_u64, &max_u64, &chunk_u64, pipeline, + )? } else if let Some(pipeline) = self.custom_pipeline { writer.create_chunked_dataset_with_pipeline( &full_name, datatype, &dims_u64, &max_u64, &chunk_u64, pipeline, @@ -976,6 +969,14 @@ impl H5Dataset { )) } }; + // Whole-operation guard: the flush, the grid snapshot and the chunk + // writes below must not interleave with a concurrent same-dataset + // operation. + let cell = writer.ds(index); + let _op = cell.op.lock(); + // A buffered append tail would flush over the image at close; hand + // it to the chunks first, the image below overwrites everything. + writer.flush_append_buffer(index)?; let chunk_dims = writer .dataset_chunk_dims(index) .ok_or_else(|| Hdf5Error::InvalidState("dataset has no chunk info".into()))? @@ -994,7 +995,11 @@ impl H5Dataset { } let total_chunks: u64 = grid.iter().product(); - // Decode a linear chunk index into row-major grid coordinates. + // Decode the iteration counter into row-major coordinates over the + // *current* image's chunk grid. This is only an odometer over the + // chunks the image spans — the slot a chunk is recorded under comes + // from the index grid (`Hdf5Writer::chunk_slot`), which the maximum + // extent decides. let coords_of = |linear: u64| -> Vec { let mut rem = linear; let mut coords = vec![0u64; rank]; @@ -1013,7 +1018,7 @@ impl H5Dataset { let coords = coords_of(linear); let chunk_buf = Self::gather_chunk(bytes, &dims, &chunk_dims, &coords, element_size); - writer.write_chunk_btree_v2(index, &coords, &chunk_buf)?; + writer.write_chunk_btree_v2_inner(index, &coords, &chunk_buf)?; } } else { // Extensible array and fixed array both compress each chunk through @@ -1028,24 +1033,26 @@ impl H5Dataset { let mut start = 0u64; while start < total_chunks { let end = (start + BATCH_WINDOW).min(total_chunks); - let items: Vec<(u64, Vec, Vec)> = (start..end) - .map(|linear| { - let coords = coords_of(linear); + let items: Vec<(Vec, Vec)> = (start..end) + .map(|counter| { + let coords = coords_of(counter); let buf = Self::gather_chunk(bytes, &dims, &chunk_dims, &coords, element_size); - (linear, coords, buf) + (coords, buf) }) .collect(); if fixed_array { let pairs: Vec<(&[u64], &[u8])> = items .iter() - .map(|(_, c, d)| (c.as_slice(), d.as_slice())) + .map(|(c, d)| (c.as_slice(), d.as_slice())) .collect(); - writer.write_chunks_fixed_array_batch(index, &pairs)?; + writer.write_chunks_fixed_array_batch_inner(index, &pairs)?; } else { - let pairs: Vec<(u64, &[u8])> = - items.iter().map(|(l, _, d)| (*l, d.as_slice())).collect(); - writer.write_chunks_batch(index, &pairs)?; + let mut pairs: Vec<(u64, &[u8])> = Vec::with_capacity(items.len()); + for (c, d) in &items { + pairs.push((writer.chunk_slot(index, c)?, d.as_slice())); + } + writer.write_chunks_batch_inner(index, &pairs)?; } start = end; } @@ -1161,46 +1168,17 @@ impl H5Dataset { let inner = borrow_inner(&self.file_inner); match &*inner { H5FileInner::Writer(writer) => { + // One op: the slot decode and the write see the same + // extents. + let cell = writer.ds(*index); + let _op = cell.op.lock(); if *fixed_array { - // Fixed-array dataset: convert the linear chunk - // index into row-major grid coordinates. - let chunk_dims = writer - .dataset_chunk_dims(*index) - .ok_or_else(|| { - Hdf5Error::InvalidState("dataset has no chunk info".into()) - })? - .to_vec(); - let dims = writer.dataset_dims(*index).to_vec(); - let mut grid = vec![0u64; dims.len()]; - for d in 0..dims.len() { - grid[d] = if chunk_dims[d] > 0 { - dims[d].div_ceil(chunk_dims[d]) - } else { - 1 - }; - } - // A zero-extent dimension yields a grid of 0 - // chunks — there is no chunk to write. - if grid.contains(&0) { - return Err(Hdf5Error::InvalidState( - "dataset has a zero-extent dimension and no chunks".into(), - )); - } - let mut rem = chunk_idx as u64; - let mut coords = vec![0u64; dims.len()]; - for d in (0..dims.len()).rev() { - coords[d] = rem % grid[d]; - rem /= grid[d]; - } - // A leftover means chunk_idx exceeded the grid. - if rem != 0 { - return Err(Hdf5Error::InvalidState(format!( - "chunk index {chunk_idx} is out of range for this dataset" - ))); - } - writer.write_chunk_fixed_array(*index, &coords, data)?; + // Fixed-array dataset: decode the index-grid slot + // into row-major grid coordinates. + let coords = writer.chunk_coords_from_slot(*index, chunk_idx as u64)?; + writer.write_chunk_fixed_array_inner(*index, &coords, data)?; } else { - writer.write_chunk(*index, chunk_idx as u64, data)?; + writer.write_chunk_inner(*index, chunk_idx as u64, data)?; } Ok(()) } @@ -1266,51 +1244,22 @@ impl H5Dataset { let inner = borrow_inner(&self.file_inner); match &*inner { H5FileInner::Writer(writer) => { + // One op: the slot decode and the write see the same + // extents. + let cell = writer.ds(*index); + let _op = cell.op.lock(); if *fixed_array { - // Fixed-array dataset: convert the linear chunk - // index into row-major grid coordinates. - let chunk_dims = writer - .dataset_chunk_dims(*index) - .ok_or_else(|| { - Hdf5Error::InvalidState("dataset has no chunk info".into()) - })? - .to_vec(); - let dims = writer.dataset_dims(*index).to_vec(); - let mut grid = vec![0u64; dims.len()]; - for d in 0..dims.len() { - grid[d] = if chunk_dims[d] > 0 { - dims[d].div_ceil(chunk_dims[d]) - } else { - 1 - }; - } - // A zero-extent dimension yields a grid of 0 - // chunks — there is no chunk to write. - if grid.contains(&0) { - return Err(Hdf5Error::InvalidState( - "dataset has a zero-extent dimension and no chunks".into(), - )); - } - let mut rem = chunk_idx as u64; - let mut coords = vec![0u64; dims.len()]; - for d in (0..dims.len()).rev() { - coords[d] = rem % grid[d]; - rem /= grid[d]; - } - // A leftover means chunk_idx exceeded the grid. - if rem != 0 { - return Err(Hdf5Error::InvalidState(format!( - "chunk index {chunk_idx} is out of range for this dataset" - ))); - } - writer.write_compressed_chunk_fixed_array( + // Fixed-array dataset: decode the index-grid slot + // into row-major grid coordinates. + let coords = writer.chunk_coords_from_slot(*index, chunk_idx as u64)?; + writer.write_compressed_chunk_fixed_array_inner( *index, &coords, data, filter_mask, )?; } else { - writer.write_compressed_chunk( + writer.write_compressed_chunk_inner( *index, chunk_idx as u64, data, @@ -1420,6 +1369,11 @@ impl H5Dataset { )) } }; + // Whole-operation guard: the dims snapshot, the chunk write + // and the extend below must not interleave with a concurrent + // same-dataset operation. + let cell = writer.ds(*index); + let _op = cell.op.lock(); let chunk_dims = writer .dataset_chunk_dims(*index) .ok_or_else(|| Hdf5Error::InvalidState("dataset has no chunk info".into()))? @@ -1463,10 +1417,10 @@ impl H5Dataset { // Fixed-array (fixed-shape) dataset: no dimension growth. match bytes { ChunkBytes::Unfiltered(data) => { - writer.write_chunk_fixed_array(*index, &coords, data)? + writer.write_chunk_fixed_array_inner(*index, &coords, data)? } ChunkBytes::Prefiltered { data, filter_mask } => writer - .write_compressed_chunk_fixed_array( + .write_compressed_chunk_fixed_array_inner( *index, &coords, data, @@ -1479,40 +1433,31 @@ impl H5Dataset { if btree2 { match bytes { ChunkBytes::Unfiltered(data) => { - writer.write_chunk_btree_v2(*index, &coords, data)? + writer.write_chunk_btree_v2_inner(*index, &coords, data)? } ChunkBytes::Prefiltered { data, filter_mask } => writer - .write_compressed_chunk_btree_v2(*index, &coords, data, filter_mask)?, + .write_compressed_chunk_btree_v2_inner( + *index, + &coords, + data, + filter_mask, + )?, } } else { - // Extensible array: linearize the chunk-grid coordinates - // (row-major) into the array's chunk index. - let mut linear = 0u64; - for d in 0..dims.len() { - let grid = if chunk_dims[d] > 0 { - dims[d].div_ceil(chunk_dims[d]) - } else { - 1 - }; - linear = linear - .checked_mul(grid) - .and_then(|l| l.checked_add(coords[d])) - .ok_or_else(|| { - Hdf5Error::InvalidState( - "chunk coordinates overflow the array index".into(), - ) - })?; - } + // Extensible array: the chunk's index-grid slot (row-major + // against the maximum extent). + let linear = writer.chunk_slot(*index, &coords)?; match bytes { - ChunkBytes::Unfiltered(data) => writer.write_chunk(*index, linear, data)?, - ChunkBytes::Prefiltered { data, filter_mask } => { - writer.write_compressed_chunk(*index, linear, data, filter_mask)? + ChunkBytes::Unfiltered(data) => { + writer.write_chunk_inner(*index, linear, data)? } + ChunkBytes::Prefiltered { data, filter_mask } => writer + .write_compressed_chunk_inner(*index, linear, data, filter_mask)?, } } if new_dims != dims { - writer.extend_dataset(*index, &new_dims)?; + writer.extend_dataset_inner(*index, &new_dims)?; } Ok(()) } @@ -1612,6 +1557,13 @@ impl H5Dataset { } }; + // Whole-operation guard: the buffer take, the frame writes, + // the re-buffer and the extend below are separate slot + // acquisitions that a concurrent same-dataset append must not + // interleave with. + let cell = writer.ds(ds_index); + let _op = cell.op.lock(); + let chunk_dims = writer .dataset_chunk_dims(ds_index) .ok_or_else(|| Hdf5Error::InvalidState("dataset has no chunk info".into()))? @@ -1650,59 +1602,57 @@ impl H5Dataset { std::slice::from_raw_parts(data.as_ptr() as *const u8, data.len() * es) }; - // Merge buffered data with new data. Scope the slot guard: the - // loop below calls `write_chunk`, which re-locks the same slot. - let (buffered_frames, mut combined) = { - let ds = writer.ds(ds_index); - let mut m = ds.lock(); - let buffered_frames = m.append_buffered_frames as usize; - let combined = std::mem::take(&mut m.append_buffer); - m.append_buffered_frames = 0; - (buffered_frames, combined) + // Merge the buffer with the new frames when it is the + // dataset's tail; a buffer left mid-extent (the extent moved + // past it) keeps its recorded place — flush it and start + // fresh at the current end. + let taken = { writer.ds(ds_index).lock().append.take() }; + let (base_dim0, buffered_frames, mut combined) = match taken { + Some(b) if b.base + b.frames == current_dim0 as u64 => { + (b.base as usize, b.frames as usize, b.bytes) + } + Some(b) => { + writer.write_append_frames(ds_index, b.base, b.frames, &b.bytes)?; + (current_dim0, 0, Vec::new()) + } + None => (current_dim0, 0, Vec::new()), }; combined.extend_from_slice(raw); let total_frames = buffered_frames + n_new_frames; - let total_bytes = combined.len(); - - // Base chunk index: account for buffered frames - let base_dim0 = current_dim0 - buffered_frames; - let mut byte_pos = 0usize; - let mut frame_pos = 0usize; - - while frame_pos < total_frames { - let abs_frame = base_dim0 + frame_pos; - let chunk_idx = abs_frame / chunk_dim0; - let remaining_frames = total_frames - frame_pos; - let frames_to_fill = chunk_dim0 - (abs_frame % chunk_dim0); - - if remaining_frames >= frames_to_fill { - // These frames complete the chunk's remaining span. - let end = byte_pos + frames_to_fill * frame_bytes; - let offset_in_chunk = (abs_frame % chunk_dim0) * frame_bytes; - writer.append_frames_into_chunk( - ds_index, - chunk_idx as u64, - offset_in_chunk, - &combined[byte_pos..end], - )?; - byte_pos = end; - frame_pos += frames_to_fill; - } else { - // Partial chunk — buffer for next append - let ds = writer.ds(ds_index); - let mut m = ds.lock(); - m.append_buffer = combined[byte_pos..total_bytes].to_vec(); - m.append_buffered_frames = remaining_frames as u64; - frame_pos = total_frames; - } + + // Rows up to the last chunk boundary are written now; the + // tail that does not complete a chunk goes back in the + // buffer for the next append (or the flush at close). The + // boundary can precede `base_dim0` — a reopened file's + // flushed partial chunk leaves the base mid-chunk — in + // which case everything is tail. + let last_boundary = ((base_dim0 + total_frames) / chunk_dim0) * chunk_dim0; + let write_frames = last_boundary.saturating_sub(base_dim0); + let tail_frames = total_frames - write_frames; + if write_frames > 0 { + writer.write_append_frames( + ds_index, + base_dim0 as u64, + write_frames as u64, + &combined[..write_frames * frame_bytes], + )?; + } + if tail_frames > 0 { + let ds = writer.ds(ds_index); + let mut m = ds.lock(); + m.append = Some(crate::io::writer::AppendBuffer { + base: (base_dim0 + write_frames) as u64, + frames: tail_frames as u64, + bytes: combined[write_frames * frame_bytes..].to_vec(), + }); } // Extend dims to include all frames (buffered + new) let logical_dim0 = base_dim0 + total_frames; let mut new_dims: Vec = dims; new_dims[0] = logical_dim0 as u64; - writer.extend_dataset(ds_index, &new_dims)?; + writer.extend_dataset_inner(ds_index, &new_dims)?; Ok(()) } @@ -4997,11 +4947,11 @@ mod tests { std::fs::remove_file(&path).ok(); } - /// Elements the append buffer still holds are not on disk yet, so the - /// update has to land in the buffer or the flush at close would write the - /// pre-update reference over it. + /// Elements the append buffer still holds are not on disk yet; the + /// update flushes them to their chunks first, so the flush at close has + /// nothing left to write the pre-update reference over. #[test] - fn write_vlen_strings_slice_reaches_the_append_buffer() { + fn write_vlen_strings_slice_updates_buffered_elements() { let path = temp_path("vlen_slice_buffered"); { let file = H5File::create(&path).unwrap(); @@ -5321,4 +5271,296 @@ mod tests { "20 buffered updates left {size} bytes, one collection per update" ); } + + /// Regression: a typed `write_slice` into rows the append buffer still + /// held wrote the chunks, and the flush at close wrote the stale buffered + /// rows back over it — write 99, read 50. The slice now flushes the + /// buffer first, making the chunks the single authority for those rows. + #[test] + fn write_slice_into_the_buffered_tail_survives_close() { + let path = temp_path("slice_into_buffered_tail"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0]) + .chunk(&[4]) + .max_shape(&[None]) + .create("d") + .unwrap(); + // 6 rows: 4 land in chunk 0, rows 4 and 5 stay buffered. + ds.append(&[10, 11, 12, 13, 50, 51]).unwrap(); + ds.write_slice(&[4], &[1], &[99]).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.read_raw::().unwrap(), vec![10, 11, 12, 13, 99, 51]); + } + std::fs::remove_file(&path).ok(); + } + + /// Extending a dataset while appends sit in the buffer must not move + /// them: the buffer records the absolute row its frames belong to, so + /// the flush at close lands them there, and the grown region reads as + /// fill. + #[test] + fn extend_does_not_move_buffered_appends() { + let path = temp_path("extend_keeps_buffered_rows"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0]) + .chunk(&[4]) + .max_shape(&[None]) + .create("d") + .unwrap(); + ds.append(&[10, 11, 12, 13, 50, 51]).unwrap(); // rows 4, 5 buffered + ds.extend(&[10]).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!( + ds.read_raw::().unwrap(), + vec![10, 11, 12, 13, 50, 51, 0, 0, 0, 0] + ); + } + std::fs::remove_file(&path).ok(); + } + + /// Regression: appends to a v2 B-tree indexed dataset (two unlimited + /// dimensions) buffered fine but close() failed "not a chunked dataset" + /// and lost the buffered rows — the append's chunk writes required the + /// extensible-array index. They now go through the index-generic + /// hyperslab engine. + #[test] + fn append_to_a_btree_v2_dataset_survives_close() { + let path = temp_path("append_bt2_close"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 3]) + .chunk(&[4, 3]) + .max_shape(&[None, None]) + .create("d") + .unwrap(); + // One buffered row, then a batch that crosses the chunk + // boundary: 4 rows fill chunk band 0, one row stays buffered + // for the flush at close. + ds.append(&[1, 2, 3]).unwrap(); + ds.append(&(4..=15).collect::>()).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![5, 3]); + assert_eq!( + ds.read_raw::().unwrap(), + (1..=15).collect::>() + ); + } + std::fs::remove_file(&path).ok(); + } + + /// A chunk row narrower than the frame row is legal geometry (libhdf5 + /// creates it); appended frames must be scattered across the row's + /// tiles at the chunk stride, not packed at the frame stride. + #[test] + fn append_scatters_frames_across_narrow_chunk_tiles() { + let path = temp_path("append_narrow_chunks"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 8]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(8)]) + .create("d") + .unwrap(); + // 3 rows of 8: rows 0..2 complete chunk band 0 (two tiles), + // row 2 is flushed partial at close. + ds.append(&(0..24).collect::>()).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![3, 8]); + assert_eq!(ds.read_raw::().unwrap(), (0..24).collect::>()); + } + std::fs::remove_file(&path).ok(); + } + + /// A fixed-array dataset has no room to grow: appending must surface an + /// error naming the chunk grid, not lose rows silently. (Before the + /// index-generic append it failed as "not a chunked dataset".) + #[test] + fn append_to_a_full_fixed_array_dataset_errors() { + let path = temp_path("append_fa_errors"); + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 3]) + .chunk(&[2, 3]) + .create("d") + .unwrap(); + let err = ds.append(&(0..6).collect::>()).unwrap_err(); + assert!( + err.to_string().contains("chunk grid"), + "unexpected error: {err}" + ); + file.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// A finite max_shape above the current shape used to be dropped on the + /// fixed-array path: the array was sized from the current dims and the + /// stored dataspace had no maximum, so growth failed. The array is now + /// sized from the maximum's chunk grid (libhdf5 `max_nchunks`), so a + /// fixed-max dataset appends up to its maximum and roundtrips. + #[test] + fn fixed_array_with_a_larger_max_shape_grows_and_survives_close() { + let path = temp_path("fa_growable_dim0"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 3]) + .chunk(&[2, 3]) + .max_shape(&[Some(10), Some(3)]) + .create("d") + .unwrap(); + ds.write_raw(&(0..12).collect::>()).unwrap(); + ds.append(&(12..18).collect::>()).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![6, 3]); + assert_eq!(ds.read_raw::().unwrap(), (0..18).collect::>()); + } + std::fs::remove_file(&path).ok(); + } + + /// The multiplier-dimension boundary: growing a dimension other than 0 + /// changes the current chunk grid but not the index grid. Chunk slots + /// must come from the maximum's grid (libhdf5 `max_down_chunks`), or the + /// chunks written before the extend are looked up under different + /// indices after it. + #[test] + fn fixed_array_growable_inner_dimension_keeps_chunk_slots() { + let path = temp_path("fa_growable_dim1"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 3]) + .chunk(&[2, 3]) + .max_shape(&[Some(4), Some(9)]) + .create("d") + .unwrap(); + ds.write_raw(&(0..12).collect::>()).unwrap(); + ds.extend(&[4, 6]).unwrap(); + ds.write_slice(&[0, 3], &[4, 3], &(12..24).collect::>()) + .unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![4, 6]); + // Row-major [4,6]: row r is [r*3 .. r*3+3) from the first write + // then [12 + r*3 ..) from the second. + let mut expect = Vec::new(); + for r in 0i32..4 { + expect.extend((r * 3)..(r * 3 + 3)); + expect.extend((12 + r * 3)..(12 + r * 3 + 3)); + } + assert_eq!(ds.read_raw::().unwrap(), expect); + } + std::fs::remove_file(&path).ok(); + } + + /// Growth boundaries: past the stored maximum is rejected, and a dataset + /// without a stored maximum is fixed at its extent (libhdf5 defaults + /// maxdims to dims at creation). + #[test] + fn extend_beyond_the_maximum_is_rejected() { + let path = temp_path("extend_beyond_max"); + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 3]) + .chunk(&[2, 3]) + .max_shape(&[Some(6), Some(3)]) + .create("d") + .unwrap(); + ds.extend(&[6, 3]).unwrap(); + let err = ds.extend(&[8, 3]).unwrap_err(); + assert!( + err.to_string().contains("exceeds the maximum"), + "unexpected error: {err}" + ); + file.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// An unlimited dimension other than 0 has no fixed linear slot without + /// libhdf5's extensible-array swizzling, which is not implemented; + /// creating the geometry silently re-indexed chunks on every extend, so + /// it is rejected at create. + #[test] + fn builder_rejects_an_unlimited_inner_dimension() { + let path = temp_path("unlimited_inner_dim"); + let file = H5File::create(&path).unwrap(); + let err = match file + .new_dataset::() + .shape([4, 0]) + .chunk(&[2, 2]) + .max_shape(&[Some(4), None]) + .create("d") + { + Ok(_) => panic!("create accepted an unlimited inner dimension"), + Err(e) => e, + }; + assert!( + err.to_string().contains("not the first"), + "unexpected error: {err}" + ); + file.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// Regression: a chunk wider than a fixed max dimension used to be + /// accepted, and appends then packed rows at the chunk stride — writing + /// [1, 2, 3, 4] and reading back [1, 2, 0, 0]. libhdf5 rejects the + /// geometry at create (`H5D__chunk_construct`); so do we now. + #[test] + fn builder_rejects_a_chunk_wider_than_a_fixed_max_dimension() { + let path = temp_path("builder_chunk_wider_than_max"); + let file = H5File::create(&path).unwrap(); + let err = match file + .new_dataset::() + .shape([0, 2]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(2)]) + .create("v5") + { + Ok(_) => panic!("create accepted a chunk wider than the fixed max dimension"), + Err(e) => e, + }; + assert!( + err.to_string().contains("maximum dimension size"), + "unexpected error: {err}" + ); + file.close().unwrap(); + std::fs::remove_file(&path).ok(); + } } diff --git a/src/file.rs b/src/file.rs index cc980bf..cf175df 100644 --- a/src/file.rs +++ b/src/file.rs @@ -166,6 +166,32 @@ impl H5File { H5FileOptions::default() } + /// Opt in to the latest file format for datasets created after this call — + /// the equivalent of libhdf5's `H5Pset_libver_bounds(low = H5F_LIBVER_V200)`. + /// + /// With `latest` set, filtered chunked datasets get a version-5 data layout + /// message, whose chunk indexes store on-disk chunk sizes in fixed-width + /// (`sizeof_size`, i.e. 8-byte) fields instead of fields sized from the + /// uncompressed chunk size. That removes the overflow risk when a filter + /// *expands* a chunk, but the file is only readable by libhdf5 ≥ 2.0 + /// (h5py bundling hdf5 1.14 rejects it with "bad version number"). + /// + /// Off by default; unfiltered and contiguous datasets are unaffected. + /// Independent of this setting, a chunk larger than 4 GiB forces version 5 + /// because version 4 cannot represent its size field, matching libhdf5. + /// + /// Errors in read mode. + pub fn set_libver_latest(&self, latest: bool) -> Result<()> { + let mut inner = borrow_inner_mut(&self.inner); + match &mut *inner { + H5FileInner::Writer(writer) => { + writer.set_libver_latest(latest); + Ok(()) + } + _ => Err(Hdf5Error::InvalidState("cannot write in read mode".into())), + } + } + /// Return a handle to the root group. /// /// The root group can be used to create datasets and sub-groups. diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index 3eb46bb..1134c97 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -83,8 +83,11 @@ pub struct Bt2FilteredChunkRecord { pub scaled_offsets: Vec, /// File address of the chunk data. pub chunk_address: u64, - /// Size of the chunk after filtering (compressed size). - pub chunk_size: u32, + /// Size of the chunk after filtering (compressed size). `u64` because the + /// encoded field is `chunk_size_len` bytes wide — up to 8 under a + /// version-5 layout (or a v4 layout whose uncompressed chunk derives an + /// 8-byte width). + pub chunk_size: u64, /// Filter mask (bit i set = skip filter i). pub filter_mask: u32, } @@ -981,7 +984,7 @@ impl Bt2ChunkIndex { &mut self, scaled_offsets: Vec, chunk_address: u64, - chunk_size: u32, + chunk_size: u64, filter_mask: u32, ) { match self @@ -1068,7 +1071,7 @@ impl Bt2ChunkIndex { // filter mask, then the scaled offsets. buf.extend_from_slice(&rec.chunk_address.to_le_bytes()[..sa]); buf.extend_from_slice( - &(rec.chunk_size as u64).to_le_bytes()[..self.chunk_size_len as usize], + &rec.chunk_size.to_le_bytes()[..self.chunk_size_len as usize], ); buf.extend_from_slice(&rec.filter_mask.to_le_bytes()); for &offset in &rec.scaled_offsets { @@ -1193,7 +1196,7 @@ impl Bt2ChunkIndex { // mask, then the scaled offsets. let chunk_address = read_addr(&record_data[pos..], sa); pos += sa; - let chunk_size = read_size(&record_data[pos..], chunk_size_len) as u32; + let chunk_size = read_size(&record_data[pos..], chunk_size_len); pos += chunk_size_len; let filter_mask = u32::from_le_bytes([ record_data[pos], @@ -1815,7 +1818,7 @@ mod tests { // record_size = 8 + 2 + 4 + 16 = 30, so a leaf holds (2048-10)/30 = 67. let mut idx = Bt2ChunkIndex::new_filtered(2, 2); for i in 0..500u64 { - idx.insert_filtered(vec![i, 0], 0x10_000 + i * 0x100, (i % 400) as u32 + 1, 0); + idx.insert_filtered(vec![i, 0], 0x10_000 + i * 0x100, (i % 400) + 1, 0); } let tree = idx.build_tree(&ctx); assert!(tree.depth() >= 1); @@ -1828,7 +1831,7 @@ mod tests { for (i, r) in records.iter().enumerate() { assert_eq!(r.scaled_offsets, vec![i as u64, 0]); assert_eq!(r.chunk_address, 0x10_000 + i as u64 * 0x100); - assert_eq!(r.chunk_size, (i as u32 % 400) + 1); + assert_eq!(r.chunk_size, (i as u64 % 400) + 1); } } diff --git a/src/format/chunk_index/fixed_array.rs b/src/format/chunk_index/fixed_array.rs index 4eab0f5..fbbc28d 100644 --- a/src/format/chunk_index/fixed_array.rs +++ b/src/format/chunk_index/fixed_array.rs @@ -217,7 +217,10 @@ pub struct FixedArrayChunkElement { #[derive(Debug, Clone, PartialEq)] pub struct FixedArrayFilteredChunkElement { pub address: u64, - pub chunk_size: u32, + /// Stored (on-disk) chunk size. `u64` because the encoded field is + /// `chunk_size_len` bytes wide — up to 8 under a version-5 layout (or a + /// v4 layout whose uncompressed chunk derives an 8-byte width). + pub chunk_size: u64, pub filter_mask: u32, } @@ -448,7 +451,7 @@ impl FixedArrayDataBlock { for _ in 0..num_elmts { let address = read_addr(&buf[pos..], sa); pos += sa; - let chunk_size = read_size(&buf[pos..], chunk_size_len) as u32; + let chunk_size = read_size(&buf[pos..], chunk_size_len); pos += chunk_size_len; let filter_mask = u32::from_le_bytes([buf[pos], buf[pos + 1], buf[pos + 2], buf[pos + 3]]); @@ -609,7 +612,7 @@ pub fn encode_filtered_page( let mut buf = Vec::with_capacity(elems.len() * elem_size + 4); for e in elems { buf.extend_from_slice(&e.address.to_le_bytes()[..sa]); - buf.extend_from_slice(&(e.chunk_size as u64).to_le_bytes()[..chunk_size_len]); + buf.extend_from_slice(&e.chunk_size.to_le_bytes()[..chunk_size_len]); buf.extend_from_slice(&e.filter_mask.to_le_bytes()); } let cksum = checksum_metadata(&buf); @@ -705,7 +708,7 @@ pub fn decode_filtered_page( for _ in 0..nelmts { let address = read_addr(&page_buf[pos..], sa); pos += sa; - let chunk_size = read_size(&page_buf[pos..], chunk_size_len) as u32; + let chunk_size = read_size(&page_buf[pos..], chunk_size_len); pos += chunk_size_len; let filter_mask = u32::from_le_bytes([ page_buf[pos], @@ -940,7 +943,7 @@ mod tests { let mut buf = Vec::new(); for e in elems { buf.extend_from_slice(&e.address.to_le_bytes()[..sa]); - buf.extend_from_slice(&(e.chunk_size as u64).to_le_bytes()[..chunk_size_len]); + buf.extend_from_slice(&e.chunk_size.to_le_bytes()[..chunk_size_len]); buf.extend_from_slice(&e.filter_mask.to_le_bytes()); } let cksum = checksum_metadata(&buf); diff --git a/src/format/messages/data_layout.rs b/src/format/messages/data_layout.rs index 643fb97..144d82a 100644 --- a/src/format/messages/data_layout.rs +++ b/src/format/messages/data_layout.rs @@ -19,14 +19,17 @@ //! D 4-byte LE dimension sizes (chunk dims; last is the element size). //! The chunk index is always a version-1 B-tree. //! -//! Binary layout (version 4, chunked only): -//! Byte 0: version = 4 +//! Binary layout (versions 4 and 5, chunked only): +//! Byte 0: version = 4 or 5 //! Byte 1: layout class = 2 (chunked) //! flags(1) + ndims(1) + enc_bytes_per_dim(1) //! + dim_sizes(ndims * enc_bytes_per_dim, each LE) //! + index_type(1) //! + [for earray: 5 param bytes] //! + index_address(sizeof_addr) +//! +//! Version 5 (libhdf5 2.0) differs from version 4 only in the version byte; +//! see [`VERSION_5`] for its effect on filtered chunk indexes. use crate::format::bytes::{read_le_addr as read_addr, read_le_uint as read_size}; use crate::format::{FormatContext, FormatError, FormatResult, UNDEF_ADDR}; @@ -149,8 +152,16 @@ pub enum DataLayoutMessage { /// Address of the version-1 B-tree that indexes the chunks. b_tree_address: u64, }, - /// Version 4 chunked storage. + /// Version 4 chunked storage. Version 5 shares this exact wire format — + /// only the version byte differs — so both decode into this variant. ChunkedV4 { + /// Message version byte: 4 or 5. Version 5 (libhdf5 2.0, + /// `H5O_LAYOUT_VERSION_5`) declares that the chunk index encodes + /// filtered-chunk sizes in a fixed `sizeof_size`-byte field instead + /// of the width derived from the chunk byte count, so a filter may + /// expand a chunk without overflowing the field. Readers older than + /// libhdf5 2.0 reject version 5. + version: u8, flags: u8, /// Chunk dimension sizes. chunk_dims: Vec, @@ -203,11 +214,13 @@ impl DataLayoutMessage { /// For example, for a 2D dataset with chunk=(1,4) and element_size=8, /// pass chunk_dims = [1, 4, 8]. pub fn chunked_v4_earray( + version: u8, chunk_dims: Vec, earray_params: EarrayParams, index_address: u64, ) -> Self { Self::ChunkedV4 { + version, flags: 0, chunk_dims, index_type: ChunkIndexType::ExtensibleArray, @@ -222,11 +235,13 @@ impl DataLayoutMessage { /// /// `chunk_dims` should include the trailing element-size dimension. pub fn chunked_v4_farray( + version: u8, chunk_dims: Vec, farray_params: FixedArrayParams, index_address: u64, ) -> Self { Self::ChunkedV4 { + version, flags: 0, chunk_dims, index_type: ChunkIndexType::FixedArray, @@ -240,8 +255,9 @@ impl DataLayoutMessage { /// Version 4 chunked layout with B-tree v2 index. /// /// `chunk_dims` should include the trailing element-size dimension. - pub fn chunked_v4_btree_v2(chunk_dims: Vec, index_address: u64) -> Self { + pub fn chunked_v4_btree_v2(version: u8, chunk_dims: Vec, index_address: u64) -> Self { Self::ChunkedV4 { + version, flags: 0, chunk_dims, index_type: ChunkIndexType::BTreeV2, @@ -257,6 +273,7 @@ impl DataLayoutMessage { /// `chunk_dims` should include the trailing element-size dimension. pub fn chunked_v4_single(chunk_dims: Vec, index_address: u64) -> Self { Self::ChunkedV4 { + version: VERSION_4, flags: 0, chunk_dims, index_type: ChunkIndexType::SingleChunk, @@ -307,6 +324,7 @@ impl DataLayoutMessage { buf } Self::ChunkedV4 { + version, flags, chunk_dims, index_type, @@ -323,8 +341,9 @@ impl DataLayoutMessage { let max_dim = chunk_dims.iter().copied().max().unwrap_or(1); let enc_bytes = enc_bytes_for_value(max_dim); + debug_assert!(matches!(*version, VERSION_4 | VERSION_5)); let mut buf = Vec::with_capacity(64); - buf.push(VERSION_4); + buf.push(*version); buf.push(CLASS_CHUNKED); buf.push(*flags); buf.push(ndims); @@ -674,6 +693,7 @@ impl DataLayoutMessage { Ok(( Self::ChunkedV4 { + version: buf[0], flags, chunk_dims, index_type, @@ -852,7 +872,7 @@ mod tests { #[test] fn roundtrip_chunked_v4_earray() { let params = EarrayParams::default_params(); - let msg = DataLayoutMessage::chunked_v4_earray(vec![1, 256, 256], params, 0x2000); + let msg = DataLayoutMessage::chunked_v4_earray(4, vec![1, 256, 256], params, 0x2000); let encoded = msg.encode(&ctx8()); assert_eq!(encoded[0], 4); // version 4 assert_eq!(encoded[1], 2); // class chunked @@ -864,13 +884,34 @@ mod tests { #[test] fn roundtrip_chunked_v4_earray_ctx4() { let params = EarrayParams::default_params(); - let msg = DataLayoutMessage::chunked_v4_earray(vec![1, 128], params, 0x1000); + let msg = DataLayoutMessage::chunked_v4_earray(4, vec![1, 128], params, 0x1000); let encoded = msg.encode(&ctx4()); let (decoded, consumed) = DataLayoutMessage::decode(&encoded, &ctx4()).unwrap(); assert_eq!(consumed, encoded.len()); assert_eq!(decoded, msg); } + /// A version-5 layout differs from v4 only in the version byte; the body + /// encodes identically and the version must survive the round trip (a + /// reopen that dropped it would silently downgrade the file to v4 while + /// its filtered index keeps 8-byte size fields). + #[test] + fn roundtrip_chunked_v5_earray() { + let params = EarrayParams::default_params(); + let v5 = DataLayoutMessage::chunked_v4_earray(5, vec![1, 256, 256], params.clone(), 0x2000); + let encoded = v5.encode(&ctx8()); + assert_eq!(encoded[0], 5); // version 5 + assert_eq!(encoded[1], 2); // class chunked + let (decoded, consumed) = DataLayoutMessage::decode(&encoded, &ctx8()).unwrap(); + assert_eq!(consumed, encoded.len()); + assert_eq!(decoded, v5); + + // Same message at v4: only byte 0 differs. + let v4 = DataLayoutMessage::chunked_v4_earray(4, vec![1, 256, 256], params, 0x2000); + let encoded_v4 = v4.encode(&ctx8()); + assert_eq!(encoded[1..], encoded_v4[1..]); + } + #[test] fn roundtrip_chunked_v4_single() { let msg = DataLayoutMessage::chunked_v4_single(vec![100, 200], 0x3000); @@ -888,6 +929,7 @@ mod tests { fn roundtrip_chunked_v4_single_filtered() { for ctx in [ctx8(), ctx4()] { let msg = DataLayoutMessage::ChunkedV4 { + version: 4, flags: 0x02, chunk_dims: vec![100, 200, 4], index_type: ChunkIndexType::SingleChunk, @@ -921,7 +963,7 @@ mod tests { fn chunked_v4_enc_bytes() { // chunk dims [1, 256, 256]: max=256, needs 2 bytes let params = EarrayParams::default_params(); - let msg = DataLayoutMessage::chunked_v4_earray(vec![1, 256, 256], params, 0x2000); + let msg = DataLayoutMessage::chunked_v4_earray(4, vec![1, 256, 256], params, 0x2000); let encoded = msg.encode(&ctx8()); // version(1) + class(1) + flags(1) + ndims(1) + enc_bytes(1) // + 3*2 dim bytes + index_type(1) + 5 earray params + 8 addr = 25 @@ -1000,7 +1042,7 @@ mod tests { fn chunked_v4_large_dims() { // Large dims requiring 4 bytes each let params = EarrayParams::default_params(); - let msg = DataLayoutMessage::chunked_v4_earray(vec![1, 65536], params, 0x4000); + let msg = DataLayoutMessage::chunked_v4_earray(4, vec![1, 65536], params, 0x4000); let encoded = msg.encode(&ctx8()); assert_eq!(encoded[4], 3); // enc_bytes_per_dim = 3 (65536 = 0x10000, needs 3 bytes) } diff --git a/src/io/chunk_grid.rs b/src/io/chunk_grid.rs new file mode 100644 index 0000000..ad9ec90 --- /dev/null +++ b/src/io/chunk_grid.rs @@ -0,0 +1,236 @@ +//! Row-major chunk linear indexing against the maximum-extent grid. +//! +//! libhdf5 indexes a chunk by its row-major position in the chunk grid of the +//! *maximum* dataspace extent, never the current one: `H5D__chunk_set_info_real` +//! (H5Dchunk.c) computes `max_chunks[d] = ceil(max_dims[d] / chunk_dims[d])` +//! and every linearly-addressed index — fixed array (H5Dfarray.c), extensible +//! array (H5Dearray.c), implicit (H5Dnone.c) — feeds `max_down_chunks` to +//! `H5VM_array_offset_pre`. That makes a chunk's index permanent: extending +//! the dataset never re-addresses chunks already written. +//! +//! Row-major offsets never multiply by the slowest dimension's own extent, so +//! dimension 0 may grow — or be unlimited — without entering the arithmetic. +//! An *unlimited* dimension other than 0 has no finite multiplier; libhdf5 +//! handles it by swizzling that dimension to the slowest position +//! (H5Dearray.c), which this crate does not implement, so such coordinates +//! are rejected here and such dataspaces are rejected at dataset create. +//! +//! This module is the single owner of that arithmetic for the writer, the +//! reader, and the high-level dataset API. + +use crate::io::{IoError, IoResult}; + +/// Chunk count along each dimension of the *index* grid: the maximum extent +/// where one is declared (absent maximum means the shape is fixed, so the +/// current extent is the maximum), the current extent for an unlimited +/// dimension. Rejects a zero chunk dimension and a rank mismatch. +pub(crate) fn index_grid( + dims: &[u64], + max_dims: Option<&[u64]>, + chunk_dims: &[u64], +) -> IoResult> { + let ndims = dims.len(); + if chunk_dims.len() != ndims { + return Err(IoError::InvalidState(format!( + "dataset chunk shape has {} dimensions but the dataspace has {}", + chunk_dims.len(), + ndims + ))); + } + if let Some(max) = max_dims { + if max.len() != ndims { + return Err(IoError::InvalidState(format!( + "dataset maximum shape has {} dimensions but the dataspace has {}", + max.len(), + ndims + ))); + } + } + let mut grid = Vec::with_capacity(ndims); + for d in 0..ndims { + if chunk_dims[d] == 0 { + return Err(IoError::InvalidState(format!( + "chunk dimension {d} is zero" + ))); + } + let extent = match max_dims { + Some(max) if max[d] != u64::MAX => max[d], + _ => dims[d], + }; + grid.push(extent.div_ceil(chunk_dims[d])); + } + Ok(grid) +} + +/// Whether dimension `d`'s maximum extent is unlimited. +fn is_unlimited(max_dims: Option<&[u64]>, d: usize) -> bool { + max_dims.is_some_and(|m| m[d] == u64::MAX) +} + +/// Row-major linear index of the chunk at grid `coords` — the slot an +/// extensible or fixed array records the chunk under. +/// +/// Bounds every coordinate by the index grid (an unlimited dimension is +/// unbounded); an out-of-grid coordinate on a bounded dimension would +/// otherwise silently alias another chunk's slot. +pub(crate) fn linear_index( + dims: &[u64], + max_dims: Option<&[u64]>, + chunk_dims: &[u64], + coords: &[u64], +) -> IoResult { + let ndims = dims.len(); + if coords.len() != ndims { + return Err(IoError::InvalidState(format!( + "chunk_coords has {} entries but the dataset has {} dimensions", + coords.len(), + ndims + ))); + } + let grid = index_grid(dims, max_dims, chunk_dims)?; + let mut linear = 0u64; + for d in 0..ndims { + if is_unlimited(max_dims, d) { + if d != 0 { + return Err(IoError::InvalidState(format!( + "unlimited dimension {d} is not the first: its chunks have \ + no fixed linear index (extensible-array swizzling is not \ + supported)" + ))); + } + } else if coords[d] >= grid[d] { + return Err(IoError::InvalidState(format!( + "chunk coordinate {} in dimension {} is outside the chunk grid (0..{})", + coords[d], d, grid[d] + ))); + } + // The multiplier for coords[d] is the grid extent of the dimensions + // after it; dimension 0's own extent is never multiplied in, which is + // what lets it grow without re-indexing. + linear = if d == 0 { + coords[0] + } else { + linear + .checked_mul(grid[d]) + .and_then(|l| l.checked_add(coords[d])) + .ok_or_else(|| { + IoError::InvalidState("chunk coordinates overflow the array index".into()) + })? + }; + } + Ok(linear) +} + +/// Grid coordinates of the chunk at row-major `linear` — the inverse of +/// [`linear_index`]. Dimension 0 takes the leftover quotient, so an index +/// beyond the current extent (a slot written before a shrink, or one that +/// only becomes visible after an extend) still decodes to its true position. +pub(crate) fn coords_of( + dims: &[u64], + max_dims: Option<&[u64]>, + chunk_dims: &[u64], + linear: u64, +) -> IoResult> { + let ndims = dims.len(); + let grid = index_grid(dims, max_dims, chunk_dims)?; + let mut coords = vec![0u64; ndims]; + let mut rem = linear; + for d in (1..ndims).rev() { + if is_unlimited(max_dims, d) { + return Err(IoError::InvalidState(format!( + "unlimited dimension {d} is not the first: its chunks have \ + no fixed linear index (extensible-array swizzling is not \ + supported)" + ))); + } + if grid[d] == 0 { + return Err(IoError::InvalidState(format!( + "chunk grid is empty in dimension {d}" + ))); + } + coords[d] = rem % grid[d]; + rem /= grid[d]; + } + if ndims > 0 { + if !is_unlimited(max_dims, 0) && rem >= grid[0] { + return Err(IoError::InvalidState(format!( + "chunk index {linear} is outside the chunk grid" + ))); + } + coords[0] = rem; + } else if rem != 0 { + return Err(IoError::InvalidState(format!( + "chunk index {linear} is outside the chunk grid" + ))); + } + Ok(coords) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The multiplier for every coordinate comes from the maximum extent, + /// so an index assigned while the dataset was small stays valid after + /// an extend — the libhdf5 `max_down_chunks` rule. + #[test] + fn indices_come_from_the_maximum_grid() { + // dims [4,3], max [10,9], chunks [2,3]: max grid is [5,3]. + let dims = [4, 3]; + let max = [10, 9]; + let chunks = [2, 3]; + let li = |c: &[u64]| linear_index(&dims, Some(&max), &chunks, c).unwrap(); + assert_eq!(li(&[0, 0]), 0); + assert_eq!(li(&[0, 2]), 2); + assert_eq!(li(&[1, 0]), 3); // 3 columns of chunks in the MAX grid, not 1 + assert_eq!(li(&[4, 2]), 14); + assert_eq!( + coords_of(&dims, Some(&max), &chunks, 14).unwrap(), + vec![4, 2] + ); + } + + /// Without a stored maximum the shape is fixed, so the current extent is + /// the maximum and the grids coincide. + #[test] + fn absent_maximum_means_the_current_extent() { + let dims = [4, 4]; + let chunks = [2, 2]; + assert_eq!(linear_index(&dims, None, &chunks, &[1, 1]).unwrap(), 3); + assert_eq!(coords_of(&dims, None, &chunks, 3).unwrap(), vec![1, 1]); + let err = linear_index(&dims, None, &chunks, &[2, 0]).unwrap_err(); + assert!(err.to_string().contains("outside the chunk grid")); + } + + /// Dimension 0 never enters the multiplication, so an unlimited first + /// dimension is indexable and its coordinate is unbounded; decoding an + /// index beyond the current extent recovers the true position. + #[test] + fn unlimited_dimension_zero_is_unbounded() { + let dims = [4, 8]; + let max = [u64::MAX, 8]; + let chunks = [2, 4]; + assert_eq!( + linear_index(&dims, Some(&max), &chunks, &[100, 1]).unwrap(), + 201 + ); + assert_eq!( + coords_of(&dims, Some(&max), &chunks, 201).unwrap(), + vec![100, 1] + ); + } + + /// An unlimited dimension other than 0 has no finite multiplier; libhdf5 + /// swizzles it to the slowest position, which this crate does not + /// implement, so both directions reject it. + #[test] + fn unlimited_inner_dimension_is_rejected() { + let dims = [4, 0]; + let max = [4, u64::MAX]; + let chunks = [2, 2]; + let err = linear_index(&dims, Some(&max), &chunks, &[0, 0]).unwrap_err(); + assert!(err.to_string().contains("not the first")); + let err = coords_of(&dims, Some(&max), &chunks, 0).unwrap_err(); + assert!(err.to_string().contains("not the first")); + } +} diff --git a/src/io/mod.rs b/src/io/mod.rs index fca1985..7225082 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -5,6 +5,7 @@ //! and SWMR (Single Writer Multiple Reader) protocol support. pub mod allocator; +pub(crate) mod chunk_grid; pub mod file_handle; pub(crate) mod hyperslab; pub mod locking; diff --git a/src/io/reader.rs b/src/io/reader.rs index 61a5ec2..2955375 100644 --- a/src/io/reader.rs +++ b/src/io/reader.rs @@ -1607,50 +1607,41 @@ impl Hdf5Reader { return Ok(()); } - // Total chunk count across all dimensions. - let chunks_dim0: u64 = (0..dims.len()) - .map(|d| { - if chunk_dims[d] > 0 { - dims[d].div_ceil(chunk_dims[d]) - } else { - 0 - } - }) - .fold(1u64, |acc, n| acc.saturating_mul(n)); + // Total slot count of the index grid. The maximum extent + // decides the multipliers (libhdf5 max_down_chunks); an + // unlimited dimension 0 is bounded by the current extent for + // this read — a slot beyond it (written before a shrink) is + // not visible. + let max_dims = info.dataspace.max_dims.clone(); + let grid = + crate::io::chunk_grid::index_grid(&dims, max_dims.as_deref(), chunk_dims)?; + let chunks_total: u64 = grid.iter().fold(1u64, |acc, &n| acc.saturating_mul(n)); let chunk_entries = self.collect_ea_chunk_entries( index_address, params, &dims, + max_dims.as_deref(), chunk_dims, element_size, )?; - let n_chunks = std::cmp::min(chunks_dim0 as usize, chunk_entries.len()); - - // Chunks are placed N-dimensionally: the linear chunk index - // maps (row-major) to chunk-grid coordinates, so sub-frame - // chunks (a chunk smaller than a full frame) land correctly. - let ndims = dims.len(); - let chunks_per_dim: Vec = (0..ndims) - .map(|d| { - if chunk_dims[d] > 0 { - dims[d].div_ceil(chunk_dims[d]) - } else { - 0 - } - }) - .collect(); - let chunk_coords = |mut i: u64| -> Vec { - let mut c = vec![0u64; ndims]; - for d in (0..ndims).rev() { - if chunks_per_dim[d] > 0 { - c[d] = i % chunks_per_dim[d]; - i /= chunks_per_dim[d]; - } - } - c - }; + let n_chunks = std::cmp::min(chunks_total as usize, chunk_entries.len()); + + // Chunks are placed N-dimensionally: each slot decodes + // (row-major, against the index grid) to chunk-grid + // coordinates, so sub-frame chunks (a chunk smaller than a + // full frame) land correctly. + let mut slot_coords = Vec::with_capacity(n_chunks); + for i in 0..n_chunks as u64 { + slot_coords.push(crate::io::chunk_grid::coords_of( + &dims, + max_dims.as_deref(), + chunk_dims, + i, + )?); + } + let chunk_coords = |i: u64| -> &[u64] { &slot_coords[i as usize] }; // Build one read job per chunk (no I/O yet), then read + // decompress them together (in parallel where positioned reads @@ -1669,7 +1660,7 @@ impl Hdf5Reader { || nbytes == 0 || addr >= file_size || nbytes > file_size - || !target.overlaps(&chunk_coords(i as u64), chunk_dims) + || !target.overlaps(chunk_coords(i as u64), chunk_dims) { None } else { @@ -1688,7 +1679,7 @@ impl Hdf5Reader { .enumerate() .map(|(i, &(addr, nbytes, _))| { if addr == UNDEF_ADDR - || !target.overlaps(&chunk_coords(i as u64), chunk_dims) + || !target.overlaps(chunk_coords(i as u64), chunk_dims) { None } else { @@ -1714,7 +1705,7 @@ impl Hdf5Reader { output, &dims, chunk_dims, - &coords, + coords, element_size, ); } @@ -1854,7 +1845,7 @@ impl Hdf5Reader { let elems = decode_filtered_page(&page_buf, &self.ctx, page_nelmts, chunk_size_len)?; for e in elems { - chunk_entries.push((e.address, e.chunk_size as u64, e.filter_mask)); + chunk_entries.push((e.address, e.chunk_size, e.filter_mask)); } } else { let addrs = decode_unfiltered_page(&page_buf, &self.ctx, page_nelmts)?; @@ -1881,7 +1872,7 @@ impl Hdf5Reader { chunk_size_len, )?; for e in &fa_dblk.filtered_elements { - chunk_entries.push((e.address, e.chunk_size as u64, e.filter_mask)); + chunk_entries.push((e.address, e.chunk_size, e.filter_mask)); } } else { let fa_dblk = @@ -1892,25 +1883,22 @@ impl Hdf5Reader { } } - // Compute number of chunks per dimension. A zero chunk dimension - // from a malformed layout message would divide by zero. - if chunk_dims.contains(&0) { - return Err(crate::io::IoError::InvalidState( - "fixed-array layout has a zero chunk dimension".into(), - )); + // Index-grid slot -> chunk-grid coordinates (row-major, against the + // maximum extent — the array was sized from its chunk grid, so a slot + // beyond the current extent still decodes to its true position and + // then simply falls outside the read target). A zero chunk dimension + // from a malformed layout message is rejected inside. + let max_dims = info.dataspace.max_dims.clone(); + let mut slot_coords = Vec::with_capacity(chunk_entries.len()); + for i in 0..chunk_entries.len() as u64 { + slot_coords.push(crate::io::chunk_grid::coords_of( + &dims, + max_dims.as_deref(), + chunk_dims, + i, + )?); } - let chunks_per_dim: Vec = (0..ndims) - .map(|d| dims[d].div_ceil(chunk_dims[d])) - .collect(); - // Linear chunk index -> chunk-grid coordinates (row-major). - let chunk_coords = |mut i: u64| -> Vec { - let mut c = vec![0u64; ndims]; - for d in (0..ndims).rev() { - c[d] = i % chunks_per_dim[d]; - i /= chunks_per_dim[d]; - } - c - }; + let chunk_coords = |i: u64| -> &[u64] { &slot_coords[i as usize] }; // Build one read job per chunk (no I/O yet). Filtered chunks carry // their exact compressed size (read at-most, since a zero size means @@ -1922,7 +1910,7 @@ impl Hdf5Reader { .enumerate() .map(|(linear_idx, &(addr, comp_size, mask))| { if addr == UNDEF_ADDR - || !target.overlaps(&chunk_coords(linear_idx as u64), chunk_dims) + || !target.overlaps(chunk_coords(linear_idx as u64), chunk_dims) { None } else if pipeline.is_some() { @@ -1960,7 +1948,7 @@ impl Hdf5Reader { output, &dims, chunk_dims, - &coords, + coords, element_size, ); } @@ -2648,6 +2636,7 @@ impl Hdf5Reader { index_address: u64, params: &data_layout::EarrayParams, dims: &[u64], + max_dims: Option<&[u64]>, chunk_dims: &[u64], element_size: u64, ) -> IoResult> { @@ -2662,17 +2651,13 @@ impl Hdf5Reader { return Ok(vec![]); } - // Total chunk count across every dimension (sub-frame chunks make - // this larger than the dim-0 chunk count alone). - let chunks_dim0: usize = (0..dims.len()) - .map(|d| { - if chunk_dims[d] > 0 { - dims[d].div_ceil(chunk_dims[d]) as usize - } else { - 0 - } - }) - .fold(1usize, |acc, n| acc.saturating_mul(n)); + // Slot count of the index grid, bounding the collection walk. The + // maximum extent decides the multipliers (sub-frame chunks make this + // larger than the dim-0 chunk count alone); the unlimited dimension 0 + // is bounded by the current extent. + let chunks_dim0: usize = crate::io::chunk_grid::index_grid(dims, max_dims, chunk_dims)? + .iter() + .fold(1usize, |acc, &n| acc.saturating_mul(n as usize)); let geo = EaGeometry::new( params.idx_blk_elmts, params.data_blk_min_elmts, diff --git a/src/io/writer.rs b/src/io/writer.rs index 2ee3b07..3d9a2e9 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -184,6 +184,17 @@ impl Slot { } } +/// Proof that the create gate (`create_lock`) is held and the new dataset's +/// name passed the uniqueness check. Only [`Hdf5Writer::begin_create`] +/// constructs one and [`Hdf5Writer::push_dataset`] demands one, so a creator +/// cannot reach the dataset registry while skipping either step. +pub(crate) struct CreateGuard<'a> { + #[cfg(not(feature = "threadsafe"))] + _gate: std::cell::RefMut<'a, ()>, + #[cfg(feature = "threadsafe")] + _gate: std::sync::MutexGuard<'a, ()>, +} + /// Reference-counted shared pointer, feature-selected. The single-thread /// build uses `Rc` (no atomics); the `threadsafe` build uses `Arc` so a /// dataset/group slot can be cloned out of the registry and locked on its @@ -195,18 +206,84 @@ pub(crate) type Shared = std::rc::Rc; #[cfg(feature = "threadsafe")] pub(crate) type Shared = std::sync::Arc; -/// A single dataset's metadata behind its own [`Slot`], reference-counted so -/// a writer can clone it out of the registry (releasing the registry lock) -/// and then lock just this one dataset. Two threads writing different -/// datasets take different `DatasetRef` locks and never contend; the same -/// dataset's writes serialize, which is required because one chunk index is -/// not concurrently mutable. -pub(crate) type DatasetRef = Shared>; +/// One dataset's cell in the registry: its metadata slot plus the operation +/// lock that serializes whole logical operations on it. Both live in one +/// allocation so they cannot fall out of step — every dataset has its op +/// lock by construction. +pub(crate) struct DatasetCell { + /// Serializes one *whole* logical operation on this dataset. + /// + /// The metadata slot below serializes each individual acquisition, but a + /// multi-acquisition operation — take the append buffer → write chunks → + /// re-buffer the tail → extend, or flush-then-overwrite in a slice write + /// — would interleave with a concurrent same-dataset operation *between* + /// its acquisitions under `threadsafe`. Public write entries take this + /// lock and delegate to `_inner` variants; `_inner` variants and the + /// `pub(crate)` write helpers require the caller to hold it (or to hold + /// the writer exclusively via `&mut`, as close and the SWMR wrapper do). + /// + /// Not reentrant: the single-thread build's `RefCell` panics instantly + /// on a nested acquisition, so a missed entry/inner split fails loudly + /// in every test run rather than deadlocking only under `threadsafe`. + /// + /// Lock order: `create_lock → op → registry spine → metadata slot`. An + /// op lock is never held across another dataset's op lock, and no + /// op-lock holder takes `create_lock`, so the order is acyclic. + pub(crate) op: Slot<()>, + info: Slot, +} + +impl DatasetCell { + pub(crate) fn new(info: DatasetInfo) -> Self { + DatasetCell { + op: Slot::new(()), + info: Slot::new(info), + } + } + + /// Borrow the metadata slot (a single acquisition; see [`Self::op`] for + /// whole-operation serialization). + #[cfg(not(feature = "threadsafe"))] + pub(crate) fn lock(&self) -> std::cell::RefMut<'_, DatasetInfo> { + self.info.lock() + } + + /// Lock the metadata slot (a single acquisition; see [`Self::op`] for + /// whole-operation serialization). + #[cfg(feature = "threadsafe")] + pub(crate) fn lock(&self) -> std::sync::MutexGuard<'_, DatasetInfo> { + self.info.lock() + } +} + +/// A single dataset's [`DatasetCell`], reference-counted so a writer can +/// clone it out of the registry (releasing the registry lock) and then lock +/// just this one dataset. Two threads writing different datasets take +/// different `DatasetRef` locks and never contend; the same dataset's writes +/// serialize, which is required because one chunk index is not concurrently +/// mutable. +pub(crate) type DatasetRef = Shared; /// A single group's metadata behind its own [`Slot`], reference-counted like /// [`DatasetRef`]. pub(crate) type GroupRef = Shared>; +/// Appended frames held back until they complete a chunk. +/// +/// The buffer is the sole authority for rows `base .. base + frames`: the +/// file's chunks do not hold them yet, and any operation that writes those +/// rows must go through [`Hdf5Writer::flush_append_buffer`] first. `base` is +/// recorded when the frames are buffered — never derived from the current +/// extent, which an `extend_dataset` can move independently. +pub struct AppendBuffer { + /// Absolute row of the first buffered frame. + pub base: u64, + /// Number of buffered frames. + pub frames: u64, + /// The frames' bytes, `frames` whole rows, row-major. + pub bytes: Vec, +} + /// Metadata for a dataset being written. /// /// The whole struct lives behind a per-dataset [`Slot`] (via [`DatasetRef`]). @@ -233,10 +310,8 @@ pub struct DatasetInfo { pub fixed_array: Option, /// B-tree v2 chunked storage info. pub btree_v2: Option, - /// Buffer for partially filled chunks during append. - pub append_buffer: Vec, - /// Number of frames accumulated in `append_buffer`. - pub append_buffered_frames: u64, + /// Appended frames not yet written to chunks, `None` when empty. + pub append: Option, /// Attributes attached to this dataset. pub attributes: Vec, /// File offset where the dataset object header was written (for SWMR in-place rewrites). @@ -251,6 +326,12 @@ pub struct DatasetInfo { /// means default zero-fill; `Some` is emitted as a `fill_defined = 2` /// fill-value message in the dataset object header. pub fill_value: Option>, + /// Layout message version for chunked storage: 4, or 5 when the chunk + /// index encodes stored chunk sizes in a fixed `sizeof_size` field + /// (libhdf5 2.0). Chosen at create by `Hdf5Writer::chunk_layout_version`, + /// preserved from the file on reopen, and emitted verbatim at finalize. + /// Contiguous datasets ignore it. + pub layout_version: u8, } /// Runtime metadata for a chunked dataset. @@ -320,69 +401,89 @@ impl ChunkGeometry { self.chunk_dims.iter().product::() * self.element_size } - /// Number of chunks spanning each dimension at the current extent. - fn grid(&self) -> Vec { - self.dims - .iter() - .zip(&self.chunk_dims) - .map(|(&d, &c)| if c > 0 { d.div_ceil(c) } else { 0 }) - .collect() - } - /// Row-major position of `coords` in the chunk grid — the linear index an - /// extensible or fixed array records the chunk under. - /// - /// The grid extents multiplied here come from the *current* dims, which is - /// how every chunk written so far was indexed. The bound a coordinate is - /// checked against comes from `max_dims`, so an unlimited dimension — the - /// one an extensible array exists to grow — is unbounded, while a fixed - /// dimension still rejects an out-of-grid coordinate that would otherwise - /// silently alias another chunk's slot. + /// extensible or fixed array records the chunk under, computed against + /// the maximum-extent grid by [`crate::io::chunk_grid::linear_index`]. fn linear_index(&self, coords: &[u64]) -> IoResult { - let ndims = self.dims.len(); - if coords.len() != ndims { + crate::io::chunk_grid::linear_index( + &self.dims, + self.max_dims.as_deref(), + &self.chunk_dims, + coords, + ) + } +} + +/// Validate caller-supplied chunk geometry at dataset definition, the rule +/// libhdf5 applies in `H5D__chunk_construct` (H5Dchunk.c): the chunk rank +/// must match the dataspace rank, no chunk dimension may be zero, and a +/// chunk dimension may not exceed a fixed maximum dimension — except in a +/// dimension whose current size is zero, which libhdf5 exempts. +fn validate_chunk_geometry(dims: &[u64], max_dims: &[u64], chunk_dims: &[u64]) -> IoResult<()> { + let ndims = dims.len(); + if chunk_dims.len() != ndims { + return Err(crate::io::IoError::InvalidState(format!( + "chunk shape has {} dimensions but the dataspace has {}", + chunk_dims.len(), + ndims + ))); + } + if max_dims.len() != ndims { + return Err(crate::io::IoError::InvalidState(format!( + "maximum shape has {} dimensions but the dataspace has {}", + max_dims.len(), + ndims + ))); + } + for d in 0..ndims { + if chunk_dims[d] == 0 { return Err(crate::io::IoError::InvalidState(format!( - "chunk_coords has {} entries but the dataset has {} dimensions", - coords.len(), - ndims + "chunk dimension {d} is zero" ))); } - if self.chunk_dims.len() != ndims { + if dims[d] != 0 && max_dims[d] != u64::MAX && max_dims[d] < chunk_dims[d] { return Err(crate::io::IoError::InvalidState(format!( - "dataset chunk shape has {} dimensions but the dataspace has {}", - self.chunk_dims.len(), - ndims + "chunk dimension {} is {} but the maximum dimension size is {}", + d, chunk_dims[d], max_dims[d] ))); } - let grid = self.grid(); - let mut linear = 0u64; - for d in 0..ndims { - if self.chunk_dims[d] == 0 { - return Err(crate::io::IoError::InvalidState(format!( - "chunk dimension {d} is zero" - ))); - } - let extent = self.max_dims.as_ref().map_or(self.dims[d], |m| m[d]); - if extent != u64::MAX { - let bound = extent.div_ceil(self.chunk_dims[d]); - if coords[d] >= bound { - return Err(crate::io::IoError::InvalidState(format!( - "chunk coordinate {} in dimension {} is outside the chunk grid (0..{})", - coords[d], d, bound - ))); - } - } - linear = linear - .checked_mul(grid[d]) - .and_then(|l| l.checked_add(coords[d])) - .ok_or_else(|| { - crate::io::IoError::InvalidState( - "chunk coordinates overflow the array index".into(), - ) - })?; + } + Ok(()) +} + +/// An extensible-array index linearizes chunk coordinates, which requires +/// every unlimited dimension to be dimension 0: any later dimension is a +/// multiplier in the row-major index and must be finite. libhdf5 supports +/// other positions by swizzling the unlimited dimension to the slowest one +/// (H5Dearray.c), which this crate does not implement. +fn ensure_unlimited_is_leading(max_dims: &[u64]) -> IoResult<()> { + for (d, &m) in max_dims.iter().enumerate().skip(1) { + if m == u64::MAX { + return Err(crate::io::IoError::InvalidState(format!( + "unlimited dimension {d} is not the first: extensible-array \ + swizzling is not supported; reorder the dimensions so the \ + unlimited one comes first" + ))); + } + } + Ok(()) +} + +/// Reject strings the dataset's declared character set cannot label. +/// +/// A Rust `&str` is always UTF-8, so only an ASCII declaration (charset 0) +/// can be violated. libhdf5 stores the bytes unvalidated — its vlen write +/// path has no cset check anywhere — which mislabels them for every reader +/// that trusts the declaration (h5py raises on the same mismatch). +fn ensure_vlen_charset(charset: u8, strings: &[&str]) -> IoResult<()> { + if charset == 0 { + if let Some((i, s)) = strings.iter().enumerate().find(|(_, s)| !s.is_ascii()) { + return Err(crate::io::IoError::InvalidState(format!( + "string {i} ({s:?}) is not ASCII, but the dataset's character set is" + ))); } - Ok(linear) } + Ok(()) } /// Runtime metadata for a fixed-array-indexed chunked dataset. @@ -519,6 +620,14 @@ pub struct Hdf5Writer { /// outermost lock a create acquires (create_lock → spine → slot), and no /// write path takes it, so it cannot deadlock with the registry locks. pub(crate) create_lock: Slot<()>, + /// Target the libhdf5 2.0 file format (`H5Pset_libver_bounds` with low + /// bound `H5F_LIBVER_V200`): filtered chunked datasets created while + /// this is set get layout message version 5, whose chunk indexes encode + /// stored chunk sizes in a fixed `sizeof_size` field, so an expanding + /// filter cannot overflow the size field. Off by default — version-5 + /// files are rejected by libhdf5 before 2.0, including the 1.14-based + /// h5py wheels. + libver_latest: bool, closed: bool, /// Set once `finalize_for_swmr` has published a readable file. /// @@ -580,6 +689,7 @@ impl Hdf5Writer { hard_links: Slot::new(Vec::new()), root_attributes: Slot::new(Vec::new()), create_lock: Slot::new(()), + libver_latest: false, closed: false, swmr_active: false, root_group_addr: None, @@ -587,6 +697,43 @@ impl Hdf5Writer { }) } + /// Target the libhdf5 2.0 file format for datasets created after this + /// call: filtered chunked datasets get layout message version 5, whose + /// chunk indexes store chunk sizes in a fixed `sizeof_size`-byte field + /// with no overflow limit (see [`Self::chunk_layout_version`]). Off by + /// default, because readers older than libhdf5 2.0 — including the + /// 1.14-based h5py wheels — reject version 5. + pub fn set_libver_latest(&mut self, latest: bool) { + self.libver_latest = latest; + } + + /// Layout message version for a new chunked dataset — the + /// `H5D__chunk_set_info` rule (H5Dchunk.c): version 5 is *required* for + /// a chunk over 4 GiB (pre-2.0 readers cannot handle one even though the + /// v4 wire format could express it) and *preferred* for filtered chunks + /// when the file targets the 2.0 format; everything else stays at + /// version 4, which every 1.10+ reader accepts. + fn chunk_layout_version(&self, filtered: bool, chunk_bytes: u64) -> u8 { + if chunk_bytes > u32::MAX as u64 || (filtered && self.libver_latest) { + 5 + } else { + 4 + } + } + + /// Width of the stored-chunk-size field in a filtered chunk index: + /// version 5 uses the fixed `sizeof_size`; version 4 derives it from the + /// uncompressed chunk byte count (one spare byte included), the + /// `H5D_*_COMPUTE_CHUNK_SIZE_LEN` rule shared by the extensible-array, + /// fixed-array and v2-B-tree indexes. + fn chunk_size_len_for(&self, layout_version: u8, chunk_bytes: u64) -> u8 { + if layout_version >= 5 { + self.ctx.sizeof_size + } else { + compute_chunk_size_len(chunk_bytes) + } + } + /// Provide public access to the format context. pub fn ctx(&self) -> &FormatContext { &self.ctx @@ -620,13 +767,25 @@ impl Hdf5Writer { Shared::clone(&self.groups.lock()[index]) } + /// Enter the create gate: take `create_lock` and check that `name` is not + /// already taken. The returned witness is what [`Self::push_dataset`] + /// requires, so the uniqueness check and the registry push are atomic + /// (see `create_lock`) at every creator by construction. + pub(crate) fn begin_create(&self, name: &str) -> IoResult> { + let gate = self.create_lock.lock(); + self.ensure_unique_dataset_name(name)?; + Ok(CreateGuard { _gate: gate }) + } + /// Push a freshly-built dataset into the registry and return its index. /// Takes the registry lock only for the push, so it does not block an /// in-flight write that already cloned its own [`DatasetRef`] out. - pub(crate) fn push_dataset(&self, info: DatasetInfo) -> usize { + /// The [`CreateGuard`] proves the caller entered through + /// [`Self::begin_create`] and still holds the gate. + pub(crate) fn push_dataset(&self, _create: &CreateGuard<'_>, info: DatasetInfo) -> usize { let mut reg = self.datasets.lock(); let idx = reg.len(); - reg.push(Shared::new(Slot::new(info))); + reg.push(Shared::new(DatasetCell::new(info))); idx } @@ -815,14 +974,21 @@ impl Hdf5Writer { chunked: None, fixed_array: None, btree_v2: None, - append_buffer: Vec::new(), - append_buffered_frames: 0, + append: None, attributes: attrs, obj_header_written_addr: Some(*obj_addr), obj_header_encoded_size: 0, filter_pipeline: fp, deleted: false, fill_value, + // Preserve the on-disk layout version so finalize re-encodes + // what it read: a v5 file reopened and appended to must not be + // silently downgraded to v4 (the filtered indexes keep their + // 8-byte size fields, which v4 readers would mis-derive). + layout_version: match &dl { + DataLayoutMessage::ChunkedV4 { version, .. } => *version, + _ => 4, + }, }; // Reconstruct storage-specific metadata @@ -1034,7 +1200,7 @@ impl Hdf5Writer { // only the final hand-off needs the `Shared>` shape. let datasets = existing_datasets .into_iter() - .map(|i| Shared::new(Slot::new(i))) + .map(|i| Shared::new(DatasetCell::new(i))) .collect(); let groups = groups .into_iter() @@ -1050,6 +1216,7 @@ impl Hdf5Writer { hard_links: Slot::new(Vec::new()), root_attributes: Slot::new(root_attributes), create_lock: Slot::new(()), + libver_latest: false, closed: false, swmr_active: false, root_group_addr: None, @@ -1549,10 +1716,7 @@ impl Hdf5Writer { datatype: DatatypeMessage, dims: &[u64], ) -> IoResult { - // Hold the create gate across the uniqueness check and the registry - // push so the two are atomic (see `create_lock`). - let _create = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; + let create = self.begin_create(name)?; let total_elements: u64 = if dims.is_empty() { 1 } else { @@ -1574,25 +1738,28 @@ impl Hdf5Writer { DataspaceMessage::simple(dims) }; - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, // set during finalize - data_addr, - data_size, - chunked: None, - fixed_array: None, - btree_v2: None, - append_buffer: Vec::new(), - append_buffered_frames: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: None, - deleted: false, - fill_value: None, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, // set during finalize + data_addr, + data_size, + chunked: None, + fixed_array: None, + btree_v2: None, + append: None, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: None, + deleted: false, + fill_value: None, + layout_version: 4, + }, + ); Ok(idx) } @@ -1610,10 +1777,11 @@ impl Hdf5Writer { max_dims: &[u64], chunk_dims: &[u64], ) -> IoResult { - // Hold the create gate across the uniqueness check and the registry - // push so the two are atomic (see `create_lock`). - let _create = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; + let create = self.begin_create(name)?; + validate_chunk_geometry(dims, max_dims, chunk_dims)?; + ensure_unlimited_is_leading(max_dims)?; + let chunk_bytes = chunk_dims.iter().product::() * datatype.element_size() as u64; + let layout_version = self.chunk_layout_version(false, chunk_bytes); let earray_params = EarrayParams::default_params(); let ndblk_addrs = compute_ndblk_addrs(earray_params.sup_blk_min_data_ptrs)?; let nsblk_addrs = compute_nsblk_addrs( @@ -1661,37 +1829,40 @@ impl Hdf5Writer { max_dims: Some(max_dims.to_vec()), }; - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: None, - deleted: false, - fill_value: None, - fixed_array: None, - btree_v2: None, - chunked: Some(ChunkedDatasetInfo { - chunk_dims: chunk_dims.to_vec(), - max_dims: max_dims.to_vec(), - earray_params, - ea_header_addr, - ea_iblk_addr, - ndblk_addrs, - ea_header, - ea_iblk, - chunks_written: 0, - filt_iblk: None, - chunk_size_len: 0, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr: UNDEF_ADDR, + data_size: 0, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: None, + deleted: false, + fill_value: None, + layout_version, + fixed_array: None, + btree_v2: None, + chunked: Some(ChunkedDatasetInfo { + chunk_dims: chunk_dims.to_vec(), + max_dims: max_dims.to_vec(), + earray_params, + ea_header_addr, + ea_iblk_addr, + ndblk_addrs, + ea_header, + ea_iblk, + chunks_written: 0, + filt_iblk: None, + chunk_size_len: 0, + }), + append: None, + }, + ); Ok(idx) } @@ -1703,6 +1874,7 @@ impl Hdf5Writer { /// creation time. pub fn write_dataset_raw(&self, index: usize, data: &[u8]) -> IoResult<()> { let ds = self.ds(index); + let _op = ds.op.lock(); let data_addr = { let g = ds.lock(); if g.chunked.is_some() { @@ -1736,6 +1908,19 @@ impl Hdf5Writer { /// /// `data` must be exactly chunk_size bytes (product of chunk_dims * element_size). pub fn write_chunk(&self, index: usize, chunk_idx: u64, data: &[u8]) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_chunk_inner(index, chunk_idx, data) + } + + /// [`Self::write_chunk`] body; the caller holds the dataset's op lock or + /// the writer exclusively. + pub(crate) fn write_chunk_inner( + &self, + index: usize, + chunk_idx: u64, + data: &[u8], + ) -> IoResult<()> { let ds = self.ds(index); // Read the chunk geometry and filter pipeline under one brief lock, // then drop it: compression runs *outside* the lock, and @@ -2106,6 +2291,20 @@ impl Hdf5Writer { starts: &[u64], counts: &[u64], data: &[u8], + ) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_slice_inner(index, starts, counts, data) + } + + /// [`Self::write_slice`] body; the caller holds the dataset's op lock or + /// the writer exclusively. + pub(crate) fn write_slice_inner( + &self, + index: usize, + starts: &[u64], + counts: &[u64], + data: &[u8], ) -> IoResult<()> { let ds_ref = self.ds(index); let ds = ds_ref.lock(); @@ -2156,6 +2355,10 @@ impl Hdf5Writer { drop(ds); if is_chunked { + // Rows the append buffer holds are not in the chunks yet; writing + // them there anyway would be undone when the buffer flushes at + // close. Hand them to the chunks first. + self.flush_append_buffer_if_intersecting(index, starts[0], starts[0] + counts[0])?; return self.write_slice_chunked(index, starts, counts, data); } if base_addr == UNDEF_ADDR { @@ -2321,6 +2524,7 @@ impl Hdf5Writer { use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; use crate::format::messages::datatype::DatatypeMessage; + let create = self.begin_create(name)?; let num_strings = strings.len() as u64; // Build a global heap collection with all strings @@ -2359,25 +2563,28 @@ impl Hdf5Writer { let dataspace = crate::format::messages::dataspace::DataspaceMessage::simple(&[num_strings]); - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr, - data_size: data_size as u64, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: None, - deleted: false, - fill_value: None, - chunked: None, - fixed_array: None, - btree_v2: None, - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr, + data_size: data_size as u64, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: None, + deleted: false, + fill_value: None, + layout_version: 4, + chunked: None, + fixed_array: None, + btree_v2: None, + append: None, + }, + ); Ok(idx) } @@ -2394,6 +2601,7 @@ impl Hdf5Writer { use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; use crate::format::messages::datatype::DatatypeMessage; + let create = self.begin_create(name)?; let num_items = items.len() as u64; // Build a global heap collection with all byte arrays. @@ -2432,25 +2640,28 @@ impl Hdf5Writer { let datatype = DatatypeMessage::vlen_bytes(); let dataspace = crate::format::messages::dataspace::DataspaceMessage::simple(&[num_items]); - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr, - data_size: data_size as u64, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: None, - deleted: false, - fill_value: None, - chunked: None, - fixed_array: None, - btree_v2: None, - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr, + data_size: data_size as u64, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: None, + deleted: false, + fill_value: None, + layout_version: 4, + chunked: None, + fixed_array: None, + btree_v2: None, + append: None, + }, + ); Ok(idx) } @@ -2470,7 +2681,9 @@ impl Hdf5Writer { use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; use crate::format::messages::datatype::DatatypeMessage; + let create = self.begin_create(name)?; let num_strings = strings.len() as u64; + validate_chunk_geometry(&[num_strings], &[num_strings], &[chunk_size as u64])?; // Build a global heap collection with all strings let mut gcol = GlobalHeapCollection::new(); @@ -2506,7 +2719,8 @@ impl Hdf5Writer { let dims: Vec = vec![num_strings]; let max_dims: Vec = vec![num_strings]; let chunk_bytes = chunk_size as u64 * element_size; - let chunk_size_len = compute_chunk_size_len(chunk_bytes); + let layout_version = self.chunk_layout_version(true, chunk_bytes); + let chunk_size_len = self.chunk_size_len_for(layout_version, chunk_bytes); let earray_params = EarrayParams::default_params(); let ndblk_addrs = compute_ndblk_addrs(earray_params.sup_blk_min_data_ptrs)?; @@ -2557,37 +2771,40 @@ impl Hdf5Writer { nsblk_addrs, ); - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: Some(pipeline), - deleted: false, - fill_value: None, - fixed_array: None, - btree_v2: None, - chunked: Some(ChunkedDatasetInfo { - chunk_dims: chunk_dims.clone(), - max_dims: max_dims.clone(), - earray_params, - ea_header_addr, - ea_iblk_addr, - ndblk_addrs, - ea_header, - ea_iblk, - chunks_written: 0, - filt_iblk: Some(filt_iblk), - chunk_size_len, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr: UNDEF_ADDR, + data_size: 0, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: Some(pipeline), + deleted: false, + fill_value: None, + layout_version, + fixed_array: None, + btree_v2: None, + chunked: Some(ChunkedDatasetInfo { + chunk_dims: chunk_dims.clone(), + max_dims: max_dims.clone(), + earray_params, + ea_header_addr, + ea_iblk_addr, + ndblk_addrs, + ea_header, + ea_iblk, + chunks_written: 0, + filt_iblk: Some(filt_iblk), + chunk_size_len, + }), + append: None, + }, + ); // Write chunks of vlen references with compression let chunk_byte_size = chunk_bytes as usize; @@ -2645,11 +2862,34 @@ impl Hdf5Writer { /// references, and appends them as new chunks to the dataset. pub fn append_vlen_strings(&self, ds_index: usize, strings: &[&str]) -> IoResult<()> { use crate::format::global_heap::{encode_vlen_reference, GlobalHeapCollection}; + use crate::format::messages::datatype::DatatypeMessage; if strings.is_empty() { return Ok(()); } + // Whole-operation guard: buffer take, frame writes, re-buffer and + // extend below are separate slot acquisitions that a concurrent + // same-dataset append must not interleave with. + let cell = self.ds(ds_index); + let _op = cell.op.lock(); + + // The elements about to be written are vlen references; any other + // element type would be overwritten with them as raw bytes. + let charset = { + let ds = self.ds(ds_index); + let m = ds.lock(); + match m.datatype { + DatatypeMessage::VarLenString { charset } => charset, + _ => { + return Err(crate::io::IoError::InvalidState( + "append_vlen_strings is only for variable-length string datasets".into(), + )) + } + } + }; + ensure_vlen_charset(charset, strings)?; + // Build a new global heap collection for this batch let mut gcol = GlobalHeapCollection::new(); let mut obj_indices = Vec::with_capacity(strings.len()); @@ -2686,55 +2926,55 @@ impl Hdf5Writer { let chunk_dim0 = chunk_dims[0] as usize; let frame_bytes = ref_size; - // Merge buffered data with new data. Scope the slot guard: the loop - // below calls `write_chunk`, which re-locks the same slot. - let (buffered_frames, mut combined) = { - let ds = self.ds(ds_index); - let mut m = ds.lock(); - let buffered_frames = m.append_buffered_frames as usize; - let combined = std::mem::take(&mut m.append_buffer); - m.append_buffered_frames = 0; - (buffered_frames, combined) + // Merge the buffer with the new frames when it is the dataset's tail; + // a buffer left mid-extent (the extent moved past it) keeps its + // recorded place — flush it and start fresh at the current end. + let taken = { self.ds(ds_index).lock().append.take() }; + let (base_dim0, buffered_frames, mut combined) = match taken { + Some(b) if b.base + b.frames == current_dim0 as u64 => { + (b.base as usize, b.frames as usize, b.bytes) + } + Some(b) => { + self.write_append_frames(ds_index, b.base, b.frames, &b.bytes)?; + (current_dim0, 0, Vec::new()) + } + None => (current_dim0, 0, Vec::new()), }; combined.extend_from_slice(&raw); let total_frames = buffered_frames + n_new_frames; - let total_bytes = combined.len(); - let base_dim0 = current_dim0 - buffered_frames; - let mut byte_pos = 0usize; - let mut frame_pos = 0usize; - - while frame_pos < total_frames { - let abs_frame = base_dim0 + frame_pos; - let chunk_idx = abs_frame / chunk_dim0; - let remaining_frames = total_frames - frame_pos; - let frames_to_fill = chunk_dim0 - (abs_frame % chunk_dim0); - - if remaining_frames >= frames_to_fill { - let end = byte_pos + frames_to_fill * frame_bytes; - let offset_in_chunk = (abs_frame % chunk_dim0) * frame_bytes; - self.append_frames_into_chunk( - ds_index, - chunk_idx as u64, - offset_in_chunk, - &combined[byte_pos..end], - )?; - byte_pos = end; - frame_pos += frames_to_fill; - } else { - let ds = self.ds(ds_index); - let mut m = ds.lock(); - m.append_buffer = combined[byte_pos..total_bytes].to_vec(); - m.append_buffered_frames = remaining_frames as u64; - frame_pos = total_frames; - } + + // Rows up to the last chunk boundary are written now; the tail that + // does not complete a chunk goes back in the buffer for the next + // append (or the flush at close). The boundary can precede + // `base_dim0` — a reopened file's flushed partial chunk leaves the + // base mid-chunk — in which case everything is tail. + let last_boundary = ((base_dim0 + total_frames) / chunk_dim0) * chunk_dim0; + let write_frames = last_boundary.saturating_sub(base_dim0); + let tail_frames = total_frames - write_frames; + if write_frames > 0 { + self.write_append_frames( + ds_index, + base_dim0 as u64, + write_frames as u64, + &combined[..write_frames * frame_bytes], + )?; + } + if tail_frames > 0 { + let ds = self.ds(ds_index); + let mut m = ds.lock(); + m.append = Some(AppendBuffer { + base: (base_dim0 + write_frames) as u64, + frames: tail_frames as u64, + bytes: combined[write_frames * frame_bytes..].to_vec(), + }); } // Extend dims let logical_dim0 = base_dim0 + total_frames; let mut new_dims = dims; new_dims[0] = logical_dim0 as u64; - self.extend_dataset(ds_index, &new_dims)?; + self.extend_dataset_inner(ds_index, &new_dims)?; Ok(()) } @@ -2751,9 +2991,8 @@ impl Hdf5Writer { /// does: `H5T__vlen_disk_write` deletes the reference it read into the /// conversion background buffer before storing the new one. /// - /// Elements the append buffer still holds are patched in the buffer rather - /// than on disk, because that buffer — not the file — is their current - /// content until it is flushed. + /// Elements the append buffer still holds are flushed to their chunks + /// first, so the whole range is on disk and one write path covers it. pub fn write_vlen_strings_slice( &self, ds_index: usize, @@ -2772,9 +3011,15 @@ impl Hdf5Writer { return Ok(()); } + // Whole-operation guard: the flush, the old-reference reads and the + // slice write below must not interleave with a concurrent + // same-dataset operation. + let cell = self.ds(ds_index); + let _op = cell.op.lock(); + // Snapshot what the write needs, then drop the guard: `write_slice` // below re-locks the same slot. - let (charset, dims, buffered_frames) = { + let (charset, dims) = { let ds = self.ds(ds_index); let m = ds.lock(); let charset = match m.datatype { @@ -2786,11 +3031,7 @@ impl Hdf5Writer { )) } }; - ( - charset, - m.dataspace.dims.clone(), - m.append_buffered_frames as usize, - ) + (charset, m.dataspace.dims.clone()) }; if dims.len() != 1 { @@ -2806,15 +3047,7 @@ impl Hdf5Writer { dims[0] ))); } - // charset 0 is ASCII. A Rust `&str` is UTF-8, so anything non-ASCII - // would be stored as UTF-8 under a datatype that declares otherwise. - if charset == 0 { - if let Some((i, s)) = strings.iter().enumerate().find(|(_, s)| !s.is_ascii()) { - return Err(crate::io::IoError::InvalidState(format!( - "string {i} ({s:?}) is not ASCII, but the dataset's character set is" - ))); - } - } + ensure_vlen_charset(charset, strings)?; // One collection for the batch, as `append_vlen_strings` does. let mut gcol = GlobalHeapCollection::new(); @@ -2824,28 +3057,17 @@ impl Hdf5Writer { } let ref_size = vlen_reference_size(&self.ctx); - // The append buffer holds the tail of the dataset, so the range splits - // into an on-disk prefix and a buffered suffix. An append that is - // between publishing its buffered count and extending the dims (two - // separate lock acquisitions on its side) can make the count exceed - // the extent; report that instead of wrapping. - let buffer_base = dims[0].checked_sub(buffered_frames as u64).ok_or_else(|| { - crate::io::IoError::InvalidState(format!( - "the append buffer holds {buffered_frames} frames but the dataset has {} elements", - dims[0] - )) - })?; - let split = end.min(buffer_base).max(start); + // Elements the append buffer holds are not in the chunks yet: hand + // them to the chunks first so the whole range is on disk and the one + // write path below covers it. + self.flush_append_buffer_if_intersecting(ds_index, start, end)?; // The on-disk references about to be overwritten, read before anything // moves. libhdf5 reads the same bytes into the conversion background // buffer (`H5D__scatgath_write` gathers the file's current elements // when `need_bkg` is set) and hands them to `H5T__vlen_disk_write`, - // which deletes them before storing the new reference. The buffered - // suffix is not on disk yet; its references are read further down, - // under the same lock that patches them. - let mut superseded = - self.current_element_bytes(ds_index, start, split - start, ref_size)?; + // which deletes them before storing the new reference. + let superseded = self.current_element_bytes(ds_index, start, end - start, ref_size)?; let gcol_encoded = gcol.encode(&self.ctx); let gcol_addr = self.allocator.allocate(gcol_encoded.len() as u64); @@ -2861,41 +3083,7 @@ impl Hdf5Writer { )); } - if split > start { - let n = (split - start) as usize; - self.write_slice(ds_index, &[start], &[n as u64], &refs[..n * ref_size])?; - } - if end > split { - // One lock acquisition owns the buffered suffix: the offset is - // derived, the superseded references read and the new ones patched - // against the same buffer. The snapshot above can be stale under - // the `threadsafe` feature — a concurrent append may have flushed - // the buffer and moved its base between the two acquisitions, - // putting the suffix on disk where this patch no longer reaches - // it — so a moved base is reported, not applied. A base that is - // unchanged means nothing flushed (appends only push it up), and - // then the buffer can only have grown past our range. - let ds = self.ds(ds_index); - let mut m = ds.lock(); - m.dataspace.dims[0] - .checked_sub(m.append_buffered_frames) - .filter(|&b| b == buffer_base) - .ok_or_else(|| { - crate::io::IoError::InvalidState(format!( - "the append buffer moved while elements {split}..{end} were being replaced" - )) - })?; - let off = ((split - buffer_base) as usize) * ref_size; - let tail = &refs[(split - start) as usize * ref_size..]; - if off + tail.len() > m.append_buffer.len() { - return Err(crate::io::IoError::InvalidState(format!( - "buffered elements {split}..{end} run past the {}-byte append buffer", - m.append_buffer.len() - ))); - } - superseded.extend_from_slice(&m.append_buffer[off..off + tail.len()]); - m.append_buffer[off..off + tail.len()].copy_from_slice(tail); - } + self.write_slice_inner(ds_index, &[start], &[strings.len() as u64], &refs)?; // Only now that the new references are in place: a failure above must // not leave the file naming objects this already freed. @@ -2984,9 +3172,21 @@ impl Hdf5Writer { /// existing size with the recovered bytes given to the free-space marker, /// and a collection that ends up empty returns its block to the allocator. /// A nil reference (address 0 or `UNDEF_ADDR`) names no object. The - /// sequence length does not decide: this crate's writers store even the - /// empty string as a real heap object, so a zero-length reference with a - /// defined address still holds one that must be released. + /// address decides, not the sequence length: this crate's writers store + /// even the empty string as a real heap object, so a zero-length reference + /// with a defined address still holds one that must be released. libhdf5 + /// diverges here against itself — `H5T__vlen_disk_delete` returns before + /// `H5HG_remove` when the sequence length is zero, yet its write path + /// (`H5VL__native_blob_put`) inserts a heap object even for an empty + /// sequence, stranding it forever. The address rule frees those objects. + /// + /// Heap objects carry no reference count on this path, matching libhdf5: + /// its vlen code never calls `H5HG_link` (only the virtual-dataset layer + /// does). Releasing the same reference twice is absorbed by the + /// missing-index check below, but a crafted file in which two elements + /// share one heap object would lose it for the survivor when either is + /// replaced — the same exposure the file has under libhdf5. This crate's + /// writers never share: each element write inserts its own object. /// /// Under SWMR nothing is freed and no collection is rewritten: a reader may /// be following those references, the same reason `place_chunk` keeps a @@ -3021,10 +3221,15 @@ impl Hdf5Writer { } for (addr, indices) in per_collection { - let head = self.handle.read_at_most(addr, 64)?; - let declared = GlobalHeapCollection::decode_size(&head, &self.ctx)?; - let image = self.handle.read_at(addr, declared)?; - let (mut gcol, _) = GlobalHeapCollection::decode(&image, &self.ctx)?; + // A collection is at least 4096 bytes (H5HG_MINALLOC) and most are + // exactly that, so one read usually covers the whole image; only + // an oversized collection needs a second read at its declared size. + let mut image = self.handle.read_at_most(addr, 4096)?; + let declared = GlobalHeapCollection::decode_size(&image, &self.ctx)?; + if declared > image.len() { + image = self.handle.read_at(addr, declared)?; + } + let (mut gcol, _) = GlobalHeapCollection::decode(&image[..declared], &self.ctx)?; let mut removed_any = false; for idx in indices { removed_any |= gcol.remove_object(idx); @@ -3247,56 +3452,86 @@ impl Hdf5Writer { crate::format::messages::fill_value::tiled_fill(chunk_bytes, fv) } - /// Place `frames` in chunk `chunk_idx`, `offset_in_chunk` bytes from its - /// start, keeping every byte outside that span. + /// Write `n_frames` whole frames whose first row is `base_frame`, for + /// whichever chunk index the dataset uses and whatever its chunk shape. + /// + /// The single owner of an append's chunk writes. The frames are one + /// hyperslab — rows `base_frame .. base_frame + n_frames` over the full + /// row shape — so the write goes through + /// [`write_slice_chunked`](Self::write_slice_chunked), the same engine + /// `write_slice` uses: a chunk the span covers completely is written + /// straight through, a partial one is read-modify-write on top of what + /// is stored (or the fill value), and a chunk row narrower or wider + /// than the frame row is scattered at the chunk stride. The previous + /// owner required the extensible-array index and packed rows at the + /// frame stride, so appends to a fixed-array or v2 B-tree dataset + /// failed at close and lost the buffered rows. /// - /// The single owner of an append's chunk write. A span covering the whole - /// chunk is written straight through; a narrower one is read-modify-write, - /// because the bytes around it belong to frames appended earlier. Building - /// a fresh fill-value buffer for a chunk that already holds frames is what - /// erased them. A chunk the index does not reach has had no write land in - /// it, so the fill value *is* its content — the rule - /// [`write_slice`](Self::write_slice) already applies to a partially - /// covered chunk. - pub(crate) fn append_frames_into_chunk( + /// The caller holds the dataset's op lock or the writer exclusively. + pub(crate) fn write_append_frames( &self, ds_index: usize, - chunk_idx: u64, - offset_in_chunk: usize, + base_frame: u64, + n_frames: u64, frames: &[u8], ) -> IoResult<()> { - let chunk_bytes = { - let ds = self.ds(ds_index); - let m = ds.lock(); - let chunked = m - .chunked - .as_ref() - .ok_or_else(|| crate::io::IoError::InvalidState("not a chunked dataset".into()))?; - chunked.chunk_dims.iter().product::() as usize * m.datatype.element_size() as usize - }; - if offset_in_chunk + frames.len() > chunk_bytes { + if n_frames == 0 { + return Ok(()); + } + let geo = self.chunk_geometry(ds_index)?; + let mut starts = vec![0u64; geo.dims.len()]; + starts[0] = base_frame; + let mut counts = geo.dims.clone(); + counts[0] = n_frames; + let expected = counts.iter().product::() * geo.element_size; + if frames.len() as u64 != expected { return Err(crate::io::IoError::InvalidState(format!( - "{} bytes at offset {offset_in_chunk} do not fit chunk {chunk_idx}, \ - which is {chunk_bytes} bytes", + "{n_frames} frames at rows {base_frame}.. need {expected} bytes, got {}", frames.len() ))); } - if offset_in_chunk == 0 && frames.len() == chunk_bytes { - return self.write_chunk(ds_index, chunk_idx, frames); + self.write_slice_chunked(ds_index, &starts, &counts, frames) + } + + /// Write the dataset's append buffer (if any) into its chunks and clear + /// it. The single owner of the buffer-to-chunks transition: the flush at + /// close, an append meeting a non-contiguous buffer, and any operation + /// about to write rows the buffer holds all come through here. + /// + /// The caller holds the dataset's op lock or the writer exclusively — + /// the take and the frame writes are separate acquisitions. + pub(crate) fn flush_append_buffer(&self, ds_index: usize) -> IoResult<()> { + let taken = { self.ds(ds_index).lock().append.take() }; + match taken { + Some(b) => self.write_append_frames(ds_index, b.base, b.frames, &b.bytes), + None => Ok(()), } - let mut chunk_buf = match self.read_chunk_if_present(ds_index, chunk_idx)? { - Some(existing) if existing.len() == chunk_bytes => existing, - Some(existing) => { - return Err(crate::io::IoError::InvalidState(format!( - "stored chunk {chunk_idx} is {} bytes but the chunk shape needs \ - {chunk_bytes}", - existing.len() - ))) - } - None => self.new_chunk_buffer(ds_index, chunk_bytes), + } + + /// Flush the append buffer when rows `start_row .. end_row` intersect + /// the buffered range — those rows' current content is the buffer, and + /// writing them on disk while the buffer still holds them would be + /// undone by the flush at close. + /// + /// The caller holds the dataset's op lock or the writer exclusively. + pub(crate) fn flush_append_buffer_if_intersecting( + &self, + ds_index: usize, + start_row: u64, + end_row: u64, + ) -> IoResult<()> { + let intersects = { + let ds = self.ds(ds_index); + let m = ds.lock(); + m.append + .as_ref() + .is_some_and(|b| start_row < b.base + b.frames && end_row > b.base) }; - chunk_buf[offset_in_chunk..offset_in_chunk + frames.len()].copy_from_slice(frames); - self.write_chunk(ds_index, chunk_idx, &chunk_buf) + if intersects { + self.flush_append_buffer(ds_index) + } else { + Ok(()) + } } /// Read an already-written chunk's *decompressed* bytes when the chunk @@ -3481,10 +3716,16 @@ impl Hdf5Writer { chunk_coords: &[u64], ) -> IoResult>> { let geo = self.chunk_geometry(ds_index)?; - let linear = geo.linear_index(chunk_coords)?; + // Only the linearly-addressed indexes compute a slot; a v2 B-tree is + // keyed by the coordinates themselves (and may hold unlimited inner + // dimensions, which have no linear slot). match geo.kind { - ChunkIndexKind::ExtensibleArray => self.read_chunk_if_present(ds_index, linear), + ChunkIndexKind::ExtensibleArray => { + let linear = geo.linear_index(chunk_coords)?; + self.read_chunk_if_present(ds_index, linear) + } ChunkIndexKind::FixedArray => { + let linear = geo.linear_index(chunk_coords)?; let ds = self.ds(ds_index); let m = ds.lock(); let pipeline = m.filter_pipeline.clone(); @@ -3492,7 +3733,7 @@ impl Hdf5Writer { let lidx = linear as usize; let (addr, nbytes, mask) = if pipeline.is_some() { match fa.fa_dblk.filtered_elements.get(lidx) { - Some(e) => (e.address, e.chunk_size as u64, e.filter_mask), + Some(e) => (e.address, e.chunk_size, e.filter_mask), None => return Ok(None), } } else { @@ -3515,7 +3756,7 @@ impl Hdf5Writer { let found = if bt2.index.filtered { bt2.index .lookup_filtered(chunk_coords) - .map(|r| (r.chunk_address, r.chunk_size as u64, r.filter_mask)) + .map(|r| (r.chunk_address, r.chunk_size, r.filter_mask)) } else { bt2.index .lookup(chunk_coords) @@ -3540,6 +3781,8 @@ impl Hdf5Writer { /// [`read_chunk_at_coords`](Self::read_chunk_at_coords). Unlike the /// dataset-level `write_chunk_at`, this never grows the dataspace — a /// hyperslab write is bounded by the current extent by definition. + /// + /// The caller holds the dataset's op lock or the writer exclusively. pub(crate) fn write_chunk_at_coords( &self, ds_index: usize, @@ -3550,12 +3793,14 @@ impl Hdf5Writer { match geo.kind { ChunkIndexKind::ExtensibleArray => { let linear = geo.linear_index(chunk_coords)?; - self.write_chunk(ds_index, linear, data) + self.write_chunk_inner(ds_index, linear, data) } ChunkIndexKind::FixedArray => { - self.write_chunk_fixed_array(ds_index, chunk_coords, data) + self.write_chunk_fixed_array_inner(ds_index, chunk_coords, data) + } + ChunkIndexKind::BtreeV2 => { + self.write_chunk_btree_v2_inner(ds_index, chunk_coords, data) } - ChunkIndexKind::BtreeV2 => self.write_chunk_btree_v2(ds_index, chunk_coords, data), } } @@ -3587,104 +3832,43 @@ impl Hdf5Writer { }) } - /// Define a chunked dataset indexed by a fixed array (no unlimited dimensions). - /// - /// `dims` and `max_dims` should be the same (all fixed). `chunk_dims` defines the - /// chunk shape. Returns the dataset index. - pub fn create_fixed_array_dataset( + /// Index-grid slot of the chunk at grid `coords` (see + /// [`crate::io::chunk_grid`]). + pub(crate) fn chunk_slot(&self, ds_index: usize, coords: &[u64]) -> IoResult { + self.chunk_geometry(ds_index)?.linear_index(coords) + } + + /// Grid coordinates of the chunk recorded under index-grid slot `linear` + /// — the inverse of [`Self::chunk_slot`]. + pub(crate) fn chunk_coords_from_slot( &self, - name: &str, - datatype: DatatypeMessage, - dims: &[u64], + ds_index: usize, + linear: u64, + ) -> IoResult> { + let geo = self.chunk_geometry(ds_index)?; + crate::io::chunk_grid::coords_of( + &geo.dims, + geo.max_dims.as_deref(), + &geo.chunk_dims, + linear, + ) + } + + /// Define a chunked dataset indexed by a fixed array, fixed at its + /// current shape (`max_dims == dims`). `chunk_dims` defines the chunk + /// shape. Returns the dataset index. + pub fn create_fixed_array_dataset( + &self, + name: &str, + datatype: DatatypeMessage, + dims: &[u64], chunk_dims: &[u64], ) -> IoResult { - // Hold the create gate across the uniqueness check and the registry - // push so the two are atomic (see `create_lock`). - let _create = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; - // Compute total number of chunks. `chunk_dims` is caller-supplied; - // validate it before any indexing or division. - let ndims = dims.len(); - if chunk_dims.len() != ndims { - return Err(crate::io::IoError::InvalidState(format!( - "chunk shape has {} dimensions but the dataspace has {}", - chunk_dims.len(), - ndims - ))); - } - let mut num_chunks: u64 = 1; - for d in 0..ndims { - if chunk_dims[d] == 0 { - return Err(crate::io::IoError::InvalidState(format!( - "chunk dimension {d} is zero" - ))); - } - num_chunks = num_chunks - .checked_mul(dims[d].div_ceil(chunk_dims[d])) - .ok_or_else(|| { - crate::io::IoError::InvalidState("chunk count overflows u64".into()) - })?; - } - - // Create FA header - let mut fa_header = FixedArrayHeader::new_for_chunks(&self.ctx, num_chunks); - let hdr_encoded = fa_header.encode(&self.ctx); - let fa_header_addr = self.allocator.allocate(hdr_encoded.len() as u64); - - // Create FA data block. libhdf5 switches to a paged layout once - // num_elmts exceeds dblk_page_nelmts; both layouts allocate space - // for `num_chunks` chunk addresses up front, but the paged layout - // also reserves the page-init bitmap and a per-page checksum. - let fa_dblk = FixedArrayDataBlock::new_unfiltered(fa_header_addr, num_chunks as usize); - let dblk_size = fixed_array_dblk_disk_size(&self.ctx, &fa_header); - let fa_dblk_addr = self.allocator.allocate(dblk_size); - - // Update header with data block address - fa_header.data_blk_addr = fa_dblk_addr; - - // Write both. The data block content is finalized in `flush_dataset` - // once all chunk addresses are known; here we just reserve space and - // write the header so the file is structurally consistent. - let hdr_encoded = fa_header.encode(&self.ctx); - self.handle.write_at(fa_header_addr, &hdr_encoded)?; - let dblk_encoded = encode_fixed_array_dblk(&self.ctx, &fa_header, &fa_dblk); - debug_assert_eq!(dblk_encoded.len() as u64, dblk_size); - self.handle.write_at(fa_dblk_addr, &dblk_encoded)?; - - let dataspace = DataspaceMessage::simple(dims); - - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: None, - deleted: false, - fill_value: None, - chunked: None, - btree_v2: None, - fixed_array: Some(FixedArrayDatasetInfo { - chunk_dims: chunk_dims.to_vec(), - fa_header_addr, - fa_dblk_addr, - fa_header, - fa_dblk, - chunks_written: 0, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); - - Ok(idx) + self.create_fixed_array_dataset_with_max(name, datatype, dims, dims, chunk_dims, None) } - /// Define a fixed-shape (no unlimited dimension) compressed chunked dataset - /// indexed by a *filtered* Fixed Array. + /// Define a fixed-shape compressed chunked dataset indexed by a + /// *filtered* Fixed Array (`max_dims == dims`). /// /// Like `create_fixed_array_dataset`, but the FA header carries the filtered /// client id and a `chunk_size_len`-wide compressed-size field per chunk @@ -3699,88 +3883,124 @@ impl Hdf5Writer { chunk_dims: &[u64], pipeline: FilterPipeline, ) -> IoResult { - // Hold the create gate across the uniqueness check and the registry - // push so the two are atomic (see `create_lock`). - let _create = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; - let ndims = dims.len(); - if chunk_dims.len() != ndims { - return Err(crate::io::IoError::InvalidState(format!( - "chunk shape has {} dimensions but the dataspace has {}", - chunk_dims.len(), - ndims - ))); + self.create_fixed_array_dataset_with_max( + name, + datatype, + dims, + dims, + chunk_dims, + Some(pipeline), + ) + } + + /// Define a chunked dataset indexed by a fixed array, growable up to + /// `max_dims` (every maximum finite — libhdf5 picks this index exactly + /// when no dimension is unlimited). + /// + /// The array is sized for the chunk grid of the *maximum* extent, the + /// libhdf5 rule (`H5D__farray_idx_create` uses `max_nchunks`), so the + /// dataset can be extended to `max_dims` without re-indexing chunks. + pub fn create_fixed_array_dataset_with_max( + &self, + name: &str, + datatype: DatatypeMessage, + dims: &[u64], + max_dims: &[u64], + chunk_dims: &[u64], + pipeline: Option, + ) -> IoResult { + let create = self.begin_create(name)?; + validate_chunk_geometry(dims, max_dims, chunk_dims)?; + if max_dims.contains(&u64::MAX) { + return Err(crate::io::IoError::InvalidState( + "a fixed-array index requires a fixed maximum shape (no unlimited dimension)" + .into(), + )); } let mut num_chunks: u64 = 1; - for d in 0..ndims { - if chunk_dims[d] == 0 { - return Err(crate::io::IoError::InvalidState(format!( - "chunk dimension {d} is zero" - ))); - } - num_chunks = num_chunks - .checked_mul(dims[d].div_ceil(chunk_dims[d])) - .ok_or_else(|| { - crate::io::IoError::InvalidState("chunk count overflows u64".into()) - })?; + for g in crate::io::chunk_grid::index_grid(dims, Some(max_dims), chunk_dims)? { + num_chunks = num_chunks.checked_mul(g).ok_or_else(|| { + crate::io::IoError::InvalidState("chunk count overflows u64".into()) + })?; } - // chunk_size_len is sized from the uncompressed chunk byte count, the - // same way the filtered Extensible Array path computes it: the - // compressed size never exceeds the uncompressed size meaningfully, so - // this width always holds the stored value. - let element_size = datatype.element_size() as u64; - let chunk_bytes: u64 = chunk_dims.iter().product::() * element_size; - let chunk_size_len = compute_chunk_size_len(chunk_bytes); + let chunk_bytes: u64 = chunk_dims.iter().product::() * datatype.element_size() as u64; + let layout_version = self.chunk_layout_version(pipeline.is_some(), chunk_bytes); - // Create the filtered FA header. - let mut fa_header = - FixedArrayHeader::new_for_filtered_chunks(&self.ctx, num_chunks, chunk_size_len); + // Create the FA header. For a filtered FA, chunk_size_len is sized + // the same way the filtered Extensible Array path computes it: + // derived from the uncompressed chunk byte count under layout v4, + // the fixed `sizeof_size` under layout v5. + let mut fa_header = if pipeline.is_some() { + let chunk_size_len = self.chunk_size_len_for(layout_version, chunk_bytes); + FixedArrayHeader::new_for_filtered_chunks(&self.ctx, num_chunks, chunk_size_len) + } else { + FixedArrayHeader::new_for_chunks(&self.ctx, num_chunks) + }; let hdr_encoded = fa_header.encode(&self.ctx); let fa_header_addr = self.allocator.allocate(hdr_encoded.len() as u64); - // Create the filtered FA data block; both flat and paged layouts - // reserve space for `num_chunks` filtered entries up front. - let fa_dblk = FixedArrayDataBlock::new_filtered(fa_header_addr, num_chunks as usize); + // Create the FA data block. libhdf5 switches to a paged layout once + // num_elmts exceeds dblk_page_nelmts; both layouts allocate space + // for `num_chunks` entries up front, but the paged layout also + // reserves the page-init bitmap and a per-page checksum. + let fa_dblk = if pipeline.is_some() { + FixedArrayDataBlock::new_filtered(fa_header_addr, num_chunks as usize) + } else { + FixedArrayDataBlock::new_unfiltered(fa_header_addr, num_chunks as usize) + }; let dblk_size = fixed_array_dblk_disk_size(&self.ctx, &fa_header); let fa_dblk_addr = self.allocator.allocate(dblk_size); + // Update header with data block address fa_header.data_blk_addr = fa_dblk_addr; + // Write both. The data block content is finalized in `flush_dataset` + // once all chunk addresses are known; here we just reserve space and + // write the header so the file is structurally consistent. let hdr_encoded = fa_header.encode(&self.ctx); self.handle.write_at(fa_header_addr, &hdr_encoded)?; let dblk_encoded = encode_fixed_array_dblk(&self.ctx, &fa_header, &fa_dblk); debug_assert_eq!(dblk_encoded.len() as u64, dblk_size); self.handle.write_at(fa_dblk_addr, &dblk_encoded)?; - let dataspace = DataspaceMessage::simple(dims); + // The maximum is stored even when it equals the dims: it is what + // `extend_dataset` checks growth against, and the FA capacity above + // is exactly its chunk grid. + let dataspace = DataspaceMessage { + dims: dims.to_vec(), + max_dims: Some(max_dims.to_vec()), + }; - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: Some(pipeline), - deleted: false, - fill_value: None, - chunked: None, - btree_v2: None, - fixed_array: Some(FixedArrayDatasetInfo { - chunk_dims: chunk_dims.to_vec(), - fa_header_addr, - fa_dblk_addr, - fa_header, - fa_dblk, - chunks_written: 0, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr: UNDEF_ADDR, + data_size: 0, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: pipeline, + deleted: false, + fill_value: None, + layout_version, + chunked: None, + btree_v2: None, + fixed_array: Some(FixedArrayDatasetInfo { + chunk_dims: chunk_dims.to_vec(), + fa_header_addr, + fa_dblk_addr, + fa_header, + fa_dblk, + chunks_written: 0, + }), + append: None, + }, + ); Ok(idx) } @@ -3835,31 +4055,21 @@ impl Hdf5Writer { chunk_dims: &[u64], pipeline: Option, ) -> IoResult { - use crate::format::chunk_index::btree_v2::{ - compute_chunk_size_len, Bt2Header, BT2_NODE_SIZE, - }; + use crate::format::chunk_index::btree_v2::{Bt2Header, BT2_NODE_SIZE}; - // Hold the create gate across the uniqueness check and the registry - // push so the two are atomic (see `create_lock`). - let _create = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; + let create = self.begin_create(name)?; + validate_chunk_geometry(dims, max_dims, chunk_dims)?; let ndims = dims.len(); - if chunk_dims.len() != ndims { - return Err(crate::io::IoError::InvalidState(format!( - "chunk shape has {} dimensions but the dataspace has {}", - chunk_dims.len(), - ndims - ))); - } + let chunk_bytes: u64 = chunk_dims.iter().product::() * datatype.element_size() as u64; + let layout_version = self.chunk_layout_version(pipeline.is_some(), chunk_bytes); // The filtered record's size field is as wide as libhdf5 will - // recompute it from the uncompressed chunk size, exactly as the + // recompute it — from the uncompressed chunk size under layout v4, + // the fixed `sizeof_size` under layout v5 — exactly as the // extensible- and fixed-array filtered paths size theirs. let bt2_index = match pipeline { Some(_) => { - let chunk_bytes: u64 = - chunk_dims.iter().product::() * datatype.element_size() as u64; - let len = compute_chunk_size_len(chunk_bytes); + let len = self.chunk_size_len_for(layout_version, chunk_bytes); Bt2ChunkIndex::new_filtered(ndims, len) } None => Bt2ChunkIndex::new_unfiltered(ndims), @@ -3894,32 +4104,35 @@ impl Hdf5Writer { max_dims: Some(max_dims.to_vec()), }; - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: pipeline, - deleted: false, - fill_value: None, - chunked: None, - fixed_array: None, - btree_v2: Some(Bt2DatasetInfo { - chunk_dims: chunk_dims.to_vec(), - max_dims: max_dims.to_vec(), - bt2_header_addr, - node_addrs: Vec::new(), - index: bt2_index, - chunks_written: 0, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr: UNDEF_ADDR, + data_size: 0, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: pipeline, + deleted: false, + fill_value: None, + layout_version, + chunked: None, + fixed_array: None, + btree_v2: Some(Bt2DatasetInfo { + chunk_dims: chunk_dims.to_vec(), + max_dims: max_dims.to_vec(), + bt2_header_addr, + node_addrs: Vec::new(), + index: bt2_index, + chunks_written: 0, + }), + append: None, + }, + ); Ok(idx) } @@ -3937,9 +4150,13 @@ impl Hdf5Writer { chunk_dims: &[u64], compression_level: u32, ) -> IoResult { + let create = self.begin_create(name)?; + validate_chunk_geometry(dims, max_dims, chunk_dims)?; + ensure_unlimited_is_leading(max_dims)?; let element_size = datatype.element_size() as u64; let chunk_bytes: u64 = chunk_dims.iter().product::() * element_size; - let chunk_size_len = compute_chunk_size_len(chunk_bytes); + let layout_version = self.chunk_layout_version(true, chunk_bytes); + let chunk_size_len = self.chunk_size_len_for(layout_version, chunk_bytes); let earray_params = EarrayParams::default_params(); let ndblk_addrs = compute_ndblk_addrs(earray_params.sup_blk_min_data_ptrs)?; @@ -3991,37 +4208,40 @@ impl Hdf5Writer { nsblk_addrs, ); - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: Some(FilterPipeline::deflate(compression_level)), - deleted: false, - fill_value: None, - fixed_array: None, - btree_v2: None, - chunked: Some(ChunkedDatasetInfo { - chunk_dims: chunk_dims.to_vec(), - max_dims: max_dims.to_vec(), - earray_params, - ea_header_addr, - ea_iblk_addr, - ndblk_addrs, - ea_header, - ea_iblk, - chunks_written: 0, - filt_iblk: Some(filt_iblk), - chunk_size_len, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr: UNDEF_ADDR, + data_size: 0, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: Some(FilterPipeline::deflate(compression_level)), + deleted: false, + fill_value: None, + layout_version, + fixed_array: None, + btree_v2: None, + chunked: Some(ChunkedDatasetInfo { + chunk_dims: chunk_dims.to_vec(), + max_dims: max_dims.to_vec(), + earray_params, + ea_header_addr, + ea_iblk_addr, + ndblk_addrs, + ea_header, + ea_iblk, + chunks_written: 0, + filt_iblk: Some(filt_iblk), + chunk_size_len, + }), + append: None, + }, + ); Ok(idx) } @@ -4036,13 +4256,13 @@ impl Hdf5Writer { chunk_dims: &[u64], pipeline: FilterPipeline, ) -> IoResult { - // Hold the create gate across the uniqueness check and the registry - // push so the two are atomic (see `create_lock`). - let _create = self.create_lock.lock(); - self.ensure_unique_dataset_name(name)?; + let create = self.begin_create(name)?; + validate_chunk_geometry(dims, max_dims, chunk_dims)?; + ensure_unlimited_is_leading(max_dims)?; let element_size = datatype.element_size() as u64; let chunk_bytes: u64 = chunk_dims.iter().product::() * element_size; - let chunk_size_len = compute_chunk_size_len(chunk_bytes); + let layout_version = self.chunk_layout_version(true, chunk_bytes); + let chunk_size_len = self.chunk_size_len_for(layout_version, chunk_bytes); let earray_params = EarrayParams::default_params(); let ndblk_addrs = compute_ndblk_addrs(earray_params.sup_blk_min_data_ptrs)?; @@ -4089,37 +4309,40 @@ impl Hdf5Writer { nsblk_addrs, ); - let idx = self.push_dataset(DatasetInfo { - name: name.to_string(), - datatype, - dataspace, - obj_header_addr: 0, - data_addr: UNDEF_ADDR, - data_size: 0, - attributes: Vec::new(), - obj_header_written_addr: None, - obj_header_encoded_size: 0, - filter_pipeline: Some(pipeline), - deleted: false, - fill_value: None, - fixed_array: None, - btree_v2: None, - chunked: Some(ChunkedDatasetInfo { - chunk_dims: chunk_dims.to_vec(), - max_dims: max_dims.to_vec(), - earray_params, - ea_header_addr, - ea_iblk_addr, - ndblk_addrs, - ea_header, - ea_iblk, - chunks_written: 0, - filt_iblk: Some(filt_iblk), - chunk_size_len, - }), - append_buffer: Vec::new(), - append_buffered_frames: 0, - }); + let idx = self.push_dataset( + &create, + DatasetInfo { + name: name.to_string(), + datatype, + dataspace, + obj_header_addr: 0, + data_addr: UNDEF_ADDR, + data_size: 0, + attributes: Vec::new(), + obj_header_written_addr: None, + obj_header_encoded_size: 0, + filter_pipeline: Some(pipeline), + deleted: false, + fill_value: None, + layout_version, + fixed_array: None, + btree_v2: None, + chunked: Some(ChunkedDatasetInfo { + chunk_dims: chunk_dims.to_vec(), + max_dims: max_dims.to_vec(), + earray_params, + ea_header_addr, + ea_iblk_addr, + ndblk_addrs, + ea_header, + ea_iblk, + chunks_written: 0, + filt_iblk: Some(filt_iblk), + chunk_size_len, + }), + append: None, + }, + ); Ok(idx) } @@ -4133,6 +4356,19 @@ impl Hdf5Writer { index: usize, chunk_coords: &[u64], data: &[u8], + ) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_chunk_fixed_array_inner(index, chunk_coords, data) + } + + /// [`Self::write_chunk_fixed_array`] body; the caller holds the dataset's + /// op lock or the writer exclusively. + pub(crate) fn write_chunk_fixed_array_inner( + &self, + index: usize, + chunk_coords: &[u64], + data: &[u8], ) -> IoResult<()> { // Read what we need under one brief slot guard, then compress // OUTSIDE the lock: `record_fixed_array_chunk` re-locks the same slot, @@ -4187,6 +4423,20 @@ impl Hdf5Writer { chunk_coords: &[u64], data: &[u8], filter_mask: u32, + ) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_compressed_chunk_fixed_array_inner(index, chunk_coords, data, filter_mask) + } + + /// [`Self::write_compressed_chunk_fixed_array`] body; the caller holds + /// the dataset's op lock or the writer exclusively. + pub(crate) fn write_compressed_chunk_fixed_array_inner( + &self, + index: usize, + chunk_coords: &[u64], + data: &[u8], + filter_mask: u32, ) -> IoResult<()> { if self.ds(index).lock().filter_pipeline.is_none() { return Err(crate::io::IoError::InvalidState( @@ -4221,32 +4471,14 @@ impl Hdf5Writer { .as_ref() .ok_or_else(|| crate::io::IoError::InvalidState("not a fixed-array dataset".into()))?; - // Compute linear chunk index from multidimensional coordinates. - let dims = &m.dataspace.dims; - let chunk_dims = &fa.chunk_dims; - let ndims = dims.len(); - if chunk_coords.len() != ndims { - return Err(crate::io::IoError::InvalidState(format!( - "chunk_coords has {} entries but the dataset has {} dimensions", - chunk_coords.len(), - ndims - ))); - } - let mut linear_idx: u64 = 0; - let mut stride: u64 = 1; - for d in (0..ndims).rev() { - let n_chunks_in_dim = dims[d].div_ceil(chunk_dims[d]); - // Reject an out-of-grid coordinate: without this an inner - // dimension's overflow silently aliases a different chunk slot. - if chunk_coords[d] >= n_chunks_in_dim { - return Err(crate::io::IoError::InvalidState(format!( - "chunk coordinate {} in dimension {} is outside the chunk grid (0..{})", - chunk_coords[d], d, n_chunks_in_dim - ))); - } - linear_idx += chunk_coords[d] * stride; - stride *= n_chunks_in_dim; - } + // Linear chunk index in the maximum-extent grid — the slot the fixed + // array (sized from that grid at create) records the chunk under. + let linear_idx = crate::io::chunk_grid::linear_index( + &m.dataspace.dims, + m.dataspace.max_dims.as_deref(), + &fa.chunk_dims, + chunk_coords, + )?; // Update the fixed array data block. The slot is read before the bytes // are placed so a rewrite can stay where it is (see `place_chunk`). @@ -4256,11 +4488,6 @@ impl Hdf5Writer { // Filtered FA: store address + stored size + filter mask. A // non-zero mask bit means "filter i was skipped for this chunk". let stored_size = final_bytes.len(); - if stored_size > u32::MAX as usize { - return Err(crate::io::IoError::InvalidState(format!( - "compressed chunk size {stored_size} exceeds u32::MAX" - ))); - } // The stored size is encoded in the FA header's `chunk_size_len`-byte // field; libhdf5 errors if it does not fit (H5D_CHUNK_ENCODE_SIZE_CHECK) // rather than truncating silently. element_size = sizeof_addr + @@ -4280,14 +4507,12 @@ impl Hdf5Writer { } if lidx < fa.fa_dblk.filtered_elements.len() { let old = &fa.fa_dblk.filtered_elements[lidx]; - let chunk_addr = self.place_chunk( - Some((old.address, old.chunk_size as u64)), - stored_size as u64, - ); + let chunk_addr = + self.place_chunk(Some((old.address, old.chunk_size)), stored_size as u64); self.handle.write_at(chunk_addr, final_bytes)?; fa.fa_dblk.filtered_elements[lidx] = FixedArrayFilteredChunkElement { address: chunk_addr, - chunk_size: stored_size as u32, + chunk_size: stored_size as u64, filter_mask, }; fa.chunks_written += 1; @@ -4337,6 +4562,19 @@ impl Hdf5Writer { index: usize, chunk_coords: &[u64], data: &[u8], + ) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_chunk_btree_v2_inner(index, chunk_coords, data) + } + + /// [`Self::write_chunk_btree_v2`] body; the caller holds the dataset's op + /// lock or the writer exclusively. + pub(crate) fn write_chunk_btree_v2_inner( + &self, + index: usize, + chunk_coords: &[u64], + data: &[u8], ) -> IoResult<()> { // Read what the write needs under a brief guard, then compress OUTSIDE // the lock — filtering a chunk must not hold the dataset slot. @@ -4389,6 +4627,20 @@ impl Hdf5Writer { chunk_coords: &[u64], data: &[u8], filter_mask: u32, + ) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_compressed_chunk_btree_v2_inner(index, chunk_coords, data, filter_mask) + } + + /// [`Self::write_compressed_chunk_btree_v2`] body; the caller holds the + /// dataset's op lock or the writer exclusively. + pub(crate) fn write_compressed_chunk_btree_v2_inner( + &self, + index: usize, + chunk_coords: &[u64], + data: &[u8], + filter_mask: u32, ) -> IoResult<()> { if self.ds(index).lock().filter_pipeline.is_none() { return Err(crate::io::IoError::InvalidState( @@ -4444,7 +4696,7 @@ impl Hdf5Writer { let old = if bt2.index.filtered { bt2.index .lookup_filtered(chunk_coords) - .map(|r| (r.chunk_address, r.chunk_size as u64)) + .map(|r| (r.chunk_address, r.chunk_size)) } else { bt2.index .lookup(chunk_coords) @@ -4455,12 +4707,8 @@ impl Hdf5Writer { let bt2 = m.btree_v2.as_mut().unwrap(); if bt2.index.filtered { - bt2.index.insert_filtered( - chunk_coords.to_vec(), - chunk_addr, - stored_len as u32, - filter_mask, - ); + bt2.index + .insert_filtered(chunk_coords.to_vec(), chunk_addr, stored_len, filter_mask); } else { bt2.index.insert(chunk_coords.to_vec(), chunk_addr); } @@ -4473,6 +4721,18 @@ impl Hdf5Writer { /// /// `chunks` is a list of (chunk_idx, data) pairs for an EA-indexed dataset. pub fn write_chunks_batch(&self, ds_index: usize, chunks: &[(u64, &[u8])]) -> IoResult<()> { + let ds = self.ds(ds_index); + let _op = ds.op.lock(); + self.write_chunks_batch_inner(ds_index, chunks) + } + + /// [`Self::write_chunks_batch`] body; the caller holds the dataset's op + /// lock or the writer exclusively. + pub(crate) fn write_chunks_batch_inner( + &self, + ds_index: usize, + chunks: &[(u64, &[u8])], + ) -> IoResult<()> { #[cfg(feature = "parallel")] { // If filter pipeline is set, compress all chunks in parallel. @@ -4487,14 +4747,14 @@ impl Hdf5Writer { // compressed fully, so filter_mask = 0 is truthful. let compressed = filter::apply_filters_parallel(pipeline, &chunk_data)?; for ((idx, _), compressed_data) in chunks.iter().zip(compressed.iter()) { - self.write_compressed_chunk(ds_index, *idx, compressed_data, 0)?; + self.write_compressed_chunk_inner(ds_index, *idx, compressed_data, 0)?; } return Ok(()); } } // Fallback: sequential for (idx, data) in chunks { - self.write_chunk(ds_index, *idx, data)?; + self.write_chunk_inner(ds_index, *idx, data)?; } Ok(()) } @@ -4512,6 +4772,18 @@ impl Hdf5Writer { &self, ds_index: usize, chunks: &[(&[u64], &[u8])], + ) -> IoResult<()> { + let ds = self.ds(ds_index); + let _op = ds.op.lock(); + self.write_chunks_fixed_array_batch_inner(ds_index, chunks) + } + + /// [`Self::write_chunks_fixed_array_batch`] body; the caller holds the + /// dataset's op lock or the writer exclusively. + pub(crate) fn write_chunks_fixed_array_batch_inner( + &self, + ds_index: usize, + chunks: &[(&[u64], &[u8])], ) -> IoResult<()> { #[cfg(feature = "parallel")] { @@ -4531,9 +4803,10 @@ impl Hdf5Writer { return Ok(()); } } - // Fallback: sequential (write_chunk_fixed_array compresses per chunk). + // Fallback: sequential (write_chunk_fixed_array_inner compresses per + // chunk). for (coords, data) in chunks { - self.write_chunk_fixed_array(ds_index, coords, data)?; + self.write_chunk_fixed_array_inner(ds_index, coords, data)?; } Ok(()) } @@ -4556,6 +4829,20 @@ impl Hdf5Writer { chunk_idx: u64, compressed_data: &[u8], filter_mask: u32, + ) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.write_compressed_chunk_inner(index, chunk_idx, compressed_data, filter_mask) + } + + /// [`Self::write_compressed_chunk`] body; the caller holds the dataset's + /// op lock or the writer exclusively. + pub(crate) fn write_compressed_chunk_inner( + &self, + index: usize, + chunk_idx: u64, + compressed_data: &[u8], + filter_mask: u32, ) -> IoResult<()> { if self.ds(index).lock().filter_pipeline.is_none() { return Err(crate::io::IoError::InvalidState( @@ -4569,6 +4856,14 @@ impl Hdf5Writer { /// Extend the dimensions of a chunked dataset. pub fn extend_dataset(&self, index: usize, new_dims: &[u64]) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); + self.extend_dataset_inner(index, new_dims) + } + + /// [`Self::extend_dataset`] body; the caller holds the dataset's op lock + /// or the writer exclusively. + pub(crate) fn extend_dataset_inner(&self, index: usize, new_dims: &[u64]) -> IoResult<()> { let ds = self.ds(index); let mut m = ds.lock(); let is_unindexed = m.chunked.is_none() && m.fixed_array.is_none() && m.btree_v2.is_none(); @@ -4592,13 +4887,22 @@ impl Hdf5Writer { "extend_dataset cannot shrink dimension {d} from {cur} to {new}" ))); } - if let Some(ref max) = m.dataspace.max_dims { - if new > max[d] { + // An absent maximum shape means the shape is fixed (libhdf5 + // defaults maxdims to dims at creation), so any growth exceeds it. + match m.dataspace.max_dims { + Some(ref max) if new > max[d] => { return Err(crate::io::IoError::InvalidState(format!( "extend_dataset dimension {d} ({new}) exceeds the maximum {}", max[d] ))); } + None if new > cur => { + return Err(crate::io::IoError::InvalidState(format!( + "extend_dataset dimension {d} ({new}) exceeds the maximum {cur}: \ + a dataset without a stored maximum shape is fixed at its extent" + ))); + } + _ => {} } } m.dataspace.dims = new_dims.to_vec(); @@ -4616,6 +4920,7 @@ impl Hdf5Writer { /// corrected back to the true number of frames written. pub fn set_dataset_extent(&self, index: usize, new_dims: &[u64]) -> IoResult<()> { let ds = self.ds(index); + let _op = ds.op.lock(); let mut m = ds.lock(); let is_unindexed = m.chunked.is_none() && m.fixed_array.is_none() && m.btree_v2.is_none(); if is_unindexed { @@ -4630,22 +4935,35 @@ impl Hdf5Writer { new_dims.len() ))); } - // A pending append buffer is positioned relative to the current - // logical size; changing the extent underneath it would make - // `flush_append_buffers` write the chunk at the wrong index. - if m.append_buffered_frames > 0 { + // A shrink can cut into buffered rows, whose recorded base would + // then point past the extent; refuse rather than reconcile. + if m.append.is_some() { return Err(crate::io::IoError::InvalidState( "set_extent cannot run while the dataset has buffered appends; \ flush them first" .into(), )); } - if let Some(ref max) = m.dataspace.max_dims { - for (d, (&new, &mx)) in new_dims.iter().zip(max).enumerate() { - if new > mx { - return Err(crate::io::IoError::InvalidState(format!( - "set_extent dimension {d} ({new}) exceeds the maximum {mx}" - ))); + // An absent maximum shape means the shape is fixed (libhdf5 defaults + // maxdims to dims at creation), so growth is bounded by the extent. + match m.dataspace.max_dims { + Some(ref max) => { + for (d, (&new, &mx)) in new_dims.iter().zip(max).enumerate() { + if new > mx { + return Err(crate::io::IoError::InvalidState(format!( + "set_extent dimension {d} ({new}) exceeds the maximum {mx}" + ))); + } + } + } + None => { + for (d, (&new, &cur)) in new_dims.iter().zip(&m.dataspace.dims).enumerate() { + if new > cur { + return Err(crate::io::IoError::InvalidState(format!( + "set_extent dimension {d} ({new}) exceeds the maximum {cur}: \ + a dataset without a stored maximum shape is fixed at its extent" + ))); + } } } } @@ -4658,6 +4976,8 @@ impl Hdf5Writer { /// Writes the index blocks and issues an `fdatasync` so the data is /// durable — the guarantee SWMR readers and standalone callers rely on. pub fn flush_dataset(&self, index: usize) -> IoResult<()> { + let ds = self.ds(index); + let _op = ds.op.lock(); self.flush_dataset_synced(index, true) } @@ -4930,49 +5250,13 @@ impl Hdf5Writer { // Internal helpers // ------------------------------------------------------------------ - /// Flush any partial append buffers into the chunks they belong to, - /// through [`append_frames_into_chunk`](Self::append_frames_into_chunk): - /// frames already in the chunk survive, and the rest of it reads back as - /// the dataset's fill value (zeros when none is defined). + /// Flush every dataset's append buffer into the chunks it belongs to, + /// through [`flush_append_buffer`](Self::flush_append_buffer): frames + /// already in the chunk survive, and the rest of it reads back as the + /// dataset's fill value (zeros when none is defined). fn flush_append_buffers(&mut self) -> IoResult<()> { for i in 0..self.dataset_count() { - // Snapshot everything needed under one brief slot guard, then drop - // it: `new_chunk_buffer` and `write_chunk` below re-lock the slot. - let (buf, buffered_frames, chunk_dims, es, dims) = { - let ds = self.ds(i); - let mut m = ds.lock(); - if m.append_buffer.is_empty() { - continue; - } - let chunk_dims = if let Some(ref c) = m.chunked { - c.chunk_dims.clone() - } else if let Some(ref f) = m.fixed_array { - f.chunk_dims.clone() - } else if let Some(ref b) = m.btree_v2 { - b.chunk_dims.clone() - } else { - continue; - }; - let es = m.datatype.element_size() as usize; - let buffered_frames = m.append_buffered_frames as usize; - let dims = m.dataspace.dims.clone(); - let buf = std::mem::take(&mut m.append_buffer); - m.append_buffered_frames = 0; - (buf, buffered_frames, chunk_dims, es, dims) - }; - - let chunk_dim0 = chunk_dims[0] as usize; - let current_dim0 = dims[0] as usize; - let base_frame = current_dim0 - buffered_frames; - let chunk_idx = base_frame / chunk_dim0; - - let frame_bytes = if dims.len() > 1 { - dims[1..].iter().map(|&d| d as usize).product::() * es - } else { - es - }; - let offset_in_chunk = (base_frame % chunk_dim0) * frame_bytes; - self.append_frames_into_chunk(i, chunk_idx as u64, offset_in_chunk, &buf)?; + self.flush_append_buffer(i)?; } Ok(()) } @@ -5129,6 +5413,7 @@ impl Hdf5Writer { let mut layout_dims = chunked.chunk_dims.clone(); layout_dims.push(m.datatype.element_size() as u64); DataLayoutMessage::chunked_v4_earray( + m.layout_version, layout_dims, chunked.earray_params.clone(), chunked.ea_header_addr, @@ -5137,6 +5422,7 @@ impl Hdf5Writer { let mut layout_dims = fa.chunk_dims.clone(); layout_dims.push(m.datatype.element_size() as u64); DataLayoutMessage::chunked_v4_farray( + m.layout_version, layout_dims, FixedArrayParams::default_params(), fa.fa_header_addr, @@ -5144,7 +5430,11 @@ impl Hdf5Writer { } else if let Some(ref bt2) = m.btree_v2 { let mut layout_dims = bt2.chunk_dims.clone(); layout_dims.push(m.datatype.element_size() as u64); - DataLayoutMessage::chunked_v4_btree_v2(layout_dims, bt2.bt2_header_addr) + DataLayoutMessage::chunked_v4_btree_v2( + m.layout_version, + layout_dims, + bt2.bt2_header_addr, + ) } else { DataLayoutMessage::contiguous(m.data_addr, m.data_size) }; @@ -5350,24 +5640,92 @@ mod tests { )) } - /// A concurrent append publishes its buffered count and extends the dims - /// under two different lock acquisitions, so a slice update can observe a - /// count that exceeds the extent. That half-published state must come back - /// as an error, not wrap `dims[0] - buffered` around zero. + /// The charset rule is one owner shared by every vlen string writer: + /// appends into an ASCII-declared dataset reject non-ASCII strings the + /// same way the slice writer does, and a dataset whose elements are not + /// vlen references at all is refused instead of overwritten with them. #[test] - fn vlen_slice_reports_a_half_published_append_count() { - let path = temp_path("vlen_slice_half_published"); + fn append_vlen_strings_checks_the_datatype_and_charset() { + let path = temp_path("append_vlen_charset"); let writer = Hdf5Writer::create(&path).unwrap(); - let idx = writer.create_vlen_string_dataset("d", &["a", "b"]).unwrap(); - writer.ds(idx).lock().append_buffered_frames = 3; // dims[0] == 2 - let err = writer.write_vlen_strings_slice(idx, 0, &["x"]).unwrap_err(); + let idx = writer + .create_appendable_vlen_string_dataset("d", 4, None) + .unwrap(); + writer.ds(idx).lock().datatype = DatatypeMessage::vlen_string_ascii(); + let err = writer + .append_vlen_strings(idx, &["ok", "안녕"]) + .unwrap_err(); + assert!( + err.to_string().contains("is not ASCII"), + "unexpected error: {err}" + ); + writer.append_vlen_strings(idx, &["ok", "fine"]).unwrap(); + + let nums = writer + .create_chunked_dataset("n", DatatypeMessage::i32_type(), &[0], &[u64::MAX], &[4]) + .unwrap(); + let err = writer.append_vlen_strings(nums, &["x"]).unwrap_err(); assert!( - err.to_string().contains("append buffer holds 3 frames"), + err.to_string() + .contains("only for variable-length string datasets"), "unexpected error: {err}" ); - writer.ds(idx).lock().append_buffered_frames = 0; + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// Every creator must enter through `begin_create`; the four that used + /// to bypass it could push a second dataset under an existing name and + /// emit an invalid file with two same-named links. + #[test] + fn every_creator_rejects_an_existing_dataset_name() { + let path = temp_path("create_gate"); + + let writer = Hdf5Writer::create(&path).unwrap(); + writer + .create_dataset("d", DatatypeMessage::i32_type(), &[2]) + .unwrap(); + + let attempts: [(&str, IoResult); 4] = [ + ( + "vlen_string", + writer.create_vlen_string_dataset("d", &["x"]), + ), + ("vlen_bytes", writer.create_vlen_bytes_dataset("d", &[b"x"])), + ( + "vlen_string_compressed", + writer.create_vlen_string_dataset_compressed( + "d", + &["x"], + 1, + FilterPipeline::deflate(6), + ), + ), + ( + "chunked_compressed", + writer.create_chunked_dataset_compressed( + "d", + DatatypeMessage::i32_type(), + &[0], + &[u64::MAX], + &[4], + 6, + ), + ), + ]; + for (which, res) in attempts { + match res { + Ok(_) => panic!("{which} accepted a duplicate name"), + Err(e) => assert!( + e.to_string().contains("already exists"), + "{which}: unexpected error: {e}" + ), + } + } + + writer.close().unwrap(); std::fs::remove_file(&path).ok(); } @@ -5909,7 +6267,7 @@ mod tests { let mut paged_dblk = FixedArrayDataBlock::new_filtered(0x1000, 2500); for (i, e) in paged_dblk.filtered_elements.iter_mut().enumerate() { e.address = 0x10000 + (i as u64) * 0x100; - e.chunk_size = (i % 200) as u32; + e.chunk_size = (i % 200) as u64; } let encoded = encode_fixed_array_dblk(&ctx, &paged, &paged_dblk); assert_eq!( @@ -6358,8 +6716,14 @@ mod tests { std::fs::remove_file(&path).ok(); } + /// A chunk tile larger than the frame is geometry libhdf5 refuses to + /// create (`H5D__chunk_construct`: chunk must not exceed a fixed maximum + /// dimension), so no libhdf5-based writer — including the NDFileHDF5 + /// tiling controls this API mirrors — can produce such a file. Until + /// 0.4.1 we accepted it and zero-padded the frame up to the tile; now + /// the create is rejected like every other creator's. #[test] - fn swmr_writer_tiled_chunk_larger_than_frame() { + fn swmr_writer_tiled_chunk_larger_than_frame_is_rejected() { use crate::io::swmr::SwmrWriter; use std::sync::atomic::{AtomicU64, Ordering}; static COUNTER: AtomicU64 = AtomicU64::new(0); @@ -6370,34 +6734,15 @@ mod tests { n )); - // Chunk tile larger than the frame: a 1x1 chunk grid, but the frame - // must still be zero-padded up to the full chunk size. let mut swmr = SwmrWriter::create(&path).unwrap(); - let idx = swmr + let err = swmr .create_streaming_dataset_tiled("det", DatatypeMessage::u16_type(), &[3, 3], &[8, 8]) - .unwrap(); - swmr.start_swmr().unwrap(); - for frame in 0..2u16 { - let data: Vec = (0..9).map(|i| frame * 10 + i).collect(); - let raw: Vec = data.iter().flat_map(|v| v.to_le_bytes()).collect(); - swmr.append_frame(idx, &raw).unwrap(); - } - swmr.flush().unwrap(); + .unwrap_err(); + assert!( + err.to_string().contains("maximum dimension size"), + "unexpected error: {err}" + ); swmr.close().unwrap(); - - let mut reader = Hdf5Reader::open(&path).unwrap(); - assert_eq!(reader.dataset_shape("det").unwrap(), vec![2, 3, 3]); - let raw = reader.read_dataset_raw("det").unwrap(); - let values: Vec = raw - .chunks(2) - .map(|c| u16::from_le_bytes(c.try_into().unwrap())) - .collect(); - assert_eq!(values.len(), 18); - for frame in 0..2u16 { - for i in 0..9usize { - assert_eq!(values[frame as usize * 9 + i], frame * 10 + i as u16); - } - } std::fs::remove_file(&path).ok(); } @@ -6610,4 +6955,363 @@ mod tests { std::fs::remove_file(&path).ok(); } + + /// libhdf5 (`H5D__chunk_construct`) rejects a chunk dimension that + /// exceeds a fixed maximum dimension. Before this check, such a dataset + /// was created and appends landed rows at the chunk stride instead of + /// the row stride, reading back [1, 2, 0, 0] for [1, 2, 3, 4]. + #[test] + fn create_rejects_a_chunk_wider_than_a_fixed_max_dimension() { + let path = temp_path("chunk_wider_than_max"); + + let writer = Hdf5Writer::create(&path).unwrap(); + let err = writer + .create_chunked_dataset( + "data", + DatatypeMessage::f64_type(), + &[0, 2], + &[u64::MAX, 2], + &[2, 4], + ) + .unwrap_err(); + assert!( + err.to_string().contains("maximum dimension size"), + "unexpected error: {err}" + ); + + // The fixed-array creators derive the maximum from the fixed dims. + let err = writer + .create_fixed_array_dataset("fa", DatatypeMessage::f64_type(), &[3], &[5]) + .unwrap_err(); + assert!( + err.to_string().contains("maximum dimension size"), + "unexpected error: {err}" + ); + + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// libhdf5 exempts a dimension whose *current* size is zero from the + /// chunk-vs-maximum check (`curr_dims[u] &&` in `H5D__chunk_construct`), + /// and rejects a zero chunk dimension on every path. + #[test] + fn create_mirrors_the_libhdf5_chunk_geometry_exemptions() { + let path = temp_path("chunk_geometry_exemptions"); + + let writer = Hdf5Writer::create(&path).unwrap(); + // dims[1] == 0: chunk 4 > max 2 is allowed, as libhdf5 allows it. + writer + .create_chunked_dataset( + "exempt", + DatatypeMessage::f64_type(), + &[0, 0], + &[u64::MAX, 2], + &[2, 4], + ) + .unwrap(); + + let err = writer + .create_chunked_dataset("zero", DatatypeMessage::f64_type(), &[0], &[u64::MAX], &[0]) + .unwrap_err(); + assert!( + err.to_string().contains("chunk dimension 0 is zero"), + "unexpected error: {err}" + ); + + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// A file written by 0.4.0 can carry a chunk row wider than the frame + /// row — create now rejects that geometry, but reopened files keep it. + /// Appends must scatter frames at the chunk stride, not pack them at + /// the frame stride (which read back `[1, 2, 0, 0]` for `[1, 2, 3, 4]`). + /// The wide shape is simulated by widening the registered chunk dims + /// after create, which also lands in the layout message at close. + #[test] + fn append_scatters_into_a_legacy_wider_than_row_chunk() { + let path = temp_path("legacy_wide_chunk_append"); + + let writer = Hdf5Writer::create(&path).unwrap(); + let idx = writer + .create_chunked_dataset( + "data", + DatatypeMessage::i32_type(), + &[0, 2], + &[u64::MAX, 2], + &[2, 2], + ) + .unwrap(); + writer.ds(idx).lock().chunked.as_mut().unwrap().chunk_dims = vec![2, 4]; + + let frames: Vec = [1i32, 2, 3, 4] + .iter() + .flat_map(|v| v.to_le_bytes()) + .collect(); + writer.write_append_frames(idx, 0, 2, &frames).unwrap(); + writer.extend_dataset(idx, &[2, 2]).unwrap(); + writer.close().unwrap(); + + let mut reader = Hdf5Reader::open(&path).unwrap(); + assert_eq!(reader.dataset_shape("data").unwrap(), vec![2, 2]); + let raw = reader.read_dataset_raw("data").unwrap(); + let values: Vec = raw + .chunks(4) + .map(|c| i32::from_le_bytes(c.try_into().unwrap())) + .collect(); + assert_eq!(values, vec![1, 2, 3, 4]); + std::fs::remove_file(&path).ok(); + } + + /// The compressed vlen creator sizes its chunked layout from a + /// caller-supplied chunk size; it goes through the same geometry + /// validation as every other creator (empty inputs are exempt because + /// their current size is zero). + #[test] + #[cfg(feature = "deflate")] + fn compressed_vlen_create_validates_its_chunk_size() { + use crate::format::messages::filter::FilterPipeline; + let path = temp_path("vlen_compressed_chunk"); + + let writer = Hdf5Writer::create(&path).unwrap(); + let err = writer + .create_vlen_string_dataset_compressed( + "texts", + &["a", "b", "c"], + 100, + FilterPipeline::deflate(6), + ) + .unwrap_err(); + assert!( + err.to_string().contains("maximum dimension size"), + "unexpected error: {err}" + ); + + writer + .create_vlen_string_dataset_compressed("empty", &[], 16, FilterPipeline::deflate(6)) + .unwrap(); + + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + + /// `set_libver_latest` moves *filtered* chunked datasets to layout v5 with + /// fixed 8-byte chunk-size fields; unfiltered chunked and pre-opt-in + /// datasets keep v4 with the derived width, matching libhdf5's + /// `version_perf` rule (only the filtered index arms bump to 5). + #[cfg(feature = "deflate")] + #[test] + fn libver_latest_selects_v5_for_filtered_chunks_only() { + let path = temp_path("libver_v5_select"); + + let mut writer = Hdf5Writer::create(&path).unwrap(); + let before = writer + .create_chunked_dataset_compressed( + "d4", + DatatypeMessage::i32_type(), + &[0], + &[u64::MAX], + &[16], + 4, + ) + .unwrap(); + writer.set_libver_latest(true); + let ea5 = writer + .create_chunked_dataset_compressed( + "ea5", + DatatypeMessage::i32_type(), + &[0], + &[u64::MAX], + &[16], + 4, + ) + .unwrap(); + let plain = writer + .create_chunked_dataset( + "plain", + DatatypeMessage::i32_type(), + &[0], + &[u64::MAX], + &[16], + ) + .unwrap(); + let fa5 = writer + .create_fixed_array_dataset_with_pipeline( + "fa5", + DatatypeMessage::i32_type(), + &[4, 6], + &[2, 3], + FilterPipeline::deflate(6), + ) + .unwrap(); + let bt5 = writer + .create_btree_v2_dataset_with_pipeline( + "bt5", + DatatypeMessage::i32_type(), + &[0, 0], + &[u64::MAX, u64::MAX], + &[2, 3], + FilterPipeline::deflate(6), + ) + .unwrap(); + + { + let d4 = writer.ds(before); + let d4 = d4.lock(); + assert_eq!(d4.layout_version, 4); + assert_eq!( + d4.chunked.as_ref().unwrap().chunk_size_len, + compute_chunk_size_len(16 * 4) + ); + let e5 = writer.ds(ea5); + let e5 = e5.lock(); + assert_eq!(e5.layout_version, 5); + assert_eq!(e5.chunked.as_ref().unwrap().chunk_size_len, 8); + assert_eq!(writer.ds(plain).lock().layout_version, 4); + assert_eq!(writer.ds(fa5).lock().layout_version, 5); + assert_eq!(writer.ds(bt5).lock().layout_version, 5); + } + + // Write through the FA and BT2 v5 indexes so their 8-byte chunk-size + // fields are exercised end to end, not just selected. + for (coords, vals) in [ + ([0u64, 0], [0i32, 1, 2, 6, 7, 8]), + ([0, 1], [3, 4, 5, 9, 10, 11]), + ([1, 0], [12, 13, 14, 18, 19, 20]), + ([1, 1], [15, 16, 17, 21, 22, 23]), + ] { + let bytes: Vec = vals.iter().flat_map(|v| v.to_le_bytes()).collect(); + writer + .write_chunk_fixed_array(fa5, &coords, &bytes) + .unwrap(); + writer.write_chunk_btree_v2(bt5, &coords, &bytes).unwrap(); + } + writer.extend_dataset(bt5, &[4, 6]).unwrap(); + writer.close().unwrap(); + + let mut reader = Hdf5Reader::open(&path).unwrap(); + for name in ["fa5", "bt5"] { + let raw = reader.read_dataset_raw(name).unwrap(); + let values: Vec = raw + .chunks(4) + .map(|c| i32::from_le_bytes(c.try_into().unwrap())) + .collect(); + assert_eq!(values, (0..24).collect::>(), "dataset {name}"); + } + + std::fs::remove_file(&path).ok(); + } + + /// A v5 file reopened for append must stay v5: the decode → `DatasetInfo` + /// → finalize path carries the version through, so the re-encoded layout + /// message matches the 8-byte size fields the filtered index was built + /// with. A silent v4 downgrade here would make libhdf5 derive a narrower + /// field width than the index uses. + #[cfg(feature = "deflate")] + #[test] + fn v5_layout_survives_reopen_and_append() { + let path = temp_path("libver_v5_reopen"); + let chunk: usize = 8; + + let mut writer = Hdf5Writer::create(&path).unwrap(); + writer.set_libver_latest(true); + let idx = writer + .create_chunked_dataset_compressed( + "d", + DatatypeMessage::i32_type(), + &[0], + &[u64::MAX], + &[chunk as u64], + 4, + ) + .unwrap(); + for c in 0..2u64 { + let data: Vec = (0..chunk as i32) + .flat_map(|i| (c as i32 * chunk as i32 + i).to_le_bytes()) + .collect(); + writer.write_chunk(idx, c, &data).unwrap(); + } + writer.extend_dataset(idx, &[2 * chunk as u64]).unwrap(); + writer.close().unwrap(); + + // Reopen: the decoded layout version must be preserved, and appends + // must keep working against the 8-byte-size-field index. + let writer = Hdf5Writer::open_append(&path).unwrap(); + assert_eq!(writer.ds(0).lock().layout_version, 5); + for c in 2..4u64 { + let data: Vec = (0..chunk as i32) + .flat_map(|i| (c as i32 * chunk as i32 + i).to_le_bytes()) + .collect(); + writer.write_chunk(0, c, &data).unwrap(); + } + writer.extend_dataset(0, &[4 * chunk as u64]).unwrap(); + writer.close().unwrap(); + + // Still v5 after the second finalize, and fully readable. + let writer = Hdf5Writer::open_append(&path).unwrap(); + assert_eq!(writer.ds(0).lock().layout_version, 5); + writer.close().unwrap(); + + let mut reader = Hdf5Reader::open(&path).unwrap(); + let raw = reader.read_dataset_raw("d").unwrap(); + let values: Vec = raw + .chunks(4) + .map(|c| i32::from_le_bytes(c.try_into().unwrap())) + .collect(); + assert_eq!(values, (0..4 * chunk as i32).collect::>()); + + std::fs::remove_file(&path).ok(); + } + + /// A chunk strictly larger than `u32::MAX` bytes forces layout v5 with no + /// opt-in — v4's size field cannot represent it — while a chunk of exactly + /// `u32::MAX` bytes stays v4, matching libhdf5's `version_req` boundary + /// (`> 0xffffffff`, filtered or not). + #[test] + fn oversized_chunk_forces_v5_without_opt_in() { + let path = temp_path("libver_4gib_force"); + + let writer = Hdf5Writer::create(&path).unwrap(); + let at_limit = writer + .create_chunked_dataset_compressed( + "at_limit", + DatatypeMessage::u8_type(), + &[0], + &[u64::MAX], + &[u32::MAX as u64], + 4, + ) + .unwrap(); + let over = writer + .create_chunked_dataset_compressed( + "over", + DatatypeMessage::u8_type(), + &[0], + &[u64::MAX], + &[u32::MAX as u64 + 1], + 4, + ) + .unwrap(); + let over_unfiltered = writer + .create_chunked_dataset( + "over_plain", + DatatypeMessage::u8_type(), + &[0], + &[u64::MAX], + &[u32::MAX as u64 + 1], + ) + .unwrap(); + + assert_eq!(writer.ds(at_limit).lock().layout_version, 4); + { + let ds = writer.ds(over); + let ds = ds.lock(); + assert_eq!(ds.layout_version, 5); + assert_eq!(ds.chunked.as_ref().unwrap().chunk_size_len, 8); + } + assert_eq!(writer.ds(over_unfiltered).lock().layout_version, 5); + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } } diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index 4ca9430..eb8f5c8 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -1,8 +1,9 @@ //! Cross-validation against h5py / libhdf5. //! //! Each test writes a file with rust-hdf5's public API and reads it back with -//! h5py to confirm the bytes are standard-tool readable. The tests skip (pass) -//! when the pinned h5py interpreter is not present, so CI without h5py is green. +//! h5py to confirm the bytes are standard-tool readable. The interpreter comes +//! from `RUST_HDF5_TEST_PYTHON`, falling back to the pinned path; the tests +//! skip (pass) when neither is present, so CI without h5py is green. use rust_hdf5::types::VarLenUnicode; use rust_hdf5::H5File; @@ -10,12 +11,18 @@ use rust_hdf5::H5File; const TEST_PYTHON: &str = "/Users/stevek/mamba/envs/bs2026.1/bin/python"; fn python() -> Option<&'static str> { - if std::path::Path::new(TEST_PYTHON).exists() { - Some(TEST_PYTHON) - } else { - eprintln!("skipping h5py cross-check: {TEST_PYTHON} not present"); - None - } + static PY: std::sync::OnceLock> = std::sync::OnceLock::new(); + PY.get_or_init(|| { + let candidate = + std::env::var("RUST_HDF5_TEST_PYTHON").unwrap_or_else(|_| TEST_PYTHON.to_string()); + if std::path::Path::new(&candidate).exists() { + Some(candidate) + } else { + eprintln!("skipping h5py cross-check: {candidate} not present"); + None + } + }) + .as_deref() } fn tmp(name: &str) -> std::path::PathBuf { @@ -772,3 +779,115 @@ fn bt2_4_depth_two_tree_readable_by_h5py() { ); std::fs::remove_file(&path).ok(); } + +/// A fixed-array dataset with a finite maximum above its shape: the array is +/// sized from the maximum's chunk grid and slots are row-major in that grid +/// (libhdf5 `max_nchunks` / `max_down_chunks`), including a maximum that +/// grows a non-leading dimension — the case where the index grid and the +/// current-extent grid disagree. h5py must see the maxshape and read every +/// value written before and after the extends. +#[test] +fn fa_growable_max_shape_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("fa_growable_max"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 3]) + .chunk(&[2, 3]) + .max_shape(&[Some(6), Some(9)]) + .create("grid") + .unwrap(); + ds.write_raw(&(0..12).collect::>()).unwrap(); + ds.extend(&[6, 6]).unwrap(); + ds.write_slice(&[0, 3], &[6, 3], &(100..118).collect::>()) + .unwrap(); + ds.write_slice(&[4, 0], &[2, 3], &(200..206).collect::>()) + .unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['grid']\n\ + assert ds.shape == (6, 6), ds.shape\n\ + assert ds.maxshape == (6, 9), ds.maxshape\n\ + assert ds.chunks == (2, 3), ds.chunks\n\ + expect = np.zeros((6, 6), dtype=np.int32)\n\ + expect[:4, :3] = np.arange(12).reshape(4, 3)\n\ + expect[:, 3:6] = np.arange(100, 118).reshape(6, 3)\n\ + expect[4:6, :3] = np.arange(200, 206).reshape(2, 3)\n\ + assert np.array_equal(ds[...], expect), ds[...]\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// Issue #8: `set_libver_latest(true)` writes a version-5 data layout message +/// for filtered chunked datasets. Two identical files differing only in the +/// knob prove both directions: the default file stays h5py-readable (v4), and +/// hdf5 < 2.0 rejects the opt-in file — the rejection is the on-disk proof +/// that a genuine v5 message was written, not a v4 one with wider index +/// fields. Under hdf5 >= 2.0 the v5 file must instead read back exactly. +/// Either way, rust-hdf5's own reader must read the v5 file. +#[cfg(feature = "deflate")] +#[test] +fn libver_latest_v5_layout_write_and_hdf5_1x_rejection() { + let Some(py) = python() else { return }; + let data: Vec = (0..35).collect(); // 7 x 5 row-major + let path_v4 = tmp("layout_default_v4"); + let path_v5 = tmp("layout_optin_v5"); + for (path, latest) in [(&path_v4, false), (&path_v5, true)] { + let file = H5File::create(path).unwrap(); + file.set_libver_latest(latest).unwrap(); + let ds = file + .new_dataset::() + .shape([7, 5]) + .chunk(&[3, 2]) + .deflate(4) + .create("grid") + .unwrap(); + ds.write_raw(&data).unwrap(); + file.close().unwrap(); + } + + // rust-hdf5's reader handles both versions. + for path in [&path_v4, &path_v5] { + let file = H5File::open(path).unwrap(); + let got = file.dataset("grid").unwrap().read_raw::().unwrap(); + assert_eq!(got, data, "rust read-back of {}", path.display()); + } + + // Positive control: the default file is plain v4 and h5py-readable. + read_back_with_h5py( + py, + &path_v4, + "ds = f['grid']\n\ + assert ds.compression == 'gzip', ds.compression\n\ + assert np.array_equal(ds[...], np.arange(35).reshape(7, 5)), ds[...]\n", + ); + + // The v5 file: rejected below hdf5 2.0, readable at 2.0+. + let script = format!( + "import h5py, numpy as np, sys\n\ + v2 = h5py.version.hdf5_version_tuple >= (2, 0, 0)\n\ + try:\n\ + \x20 f = h5py.File(r'{}', 'r')\n\ + \x20 v = f['grid'][...]\n\ + except Exception:\n\ + \x20 assert not v2, 'hdf5 >= 2.0 must read a v5 layout'\n\ + \x20 sys.exit(0)\n\ + assert v2, 'hdf5 < 2.0 read the opt-in file, so it is not v5 on disk'\n\ + assert np.array_equal(v, np.arange(35).reshape(7, 5)), v\n", + path_v5.display() + ); + let status = std::process::Command::new(py) + .arg("-c") + .arg(&script) + .status() + .expect("failed to spawn python"); + assert!(status.success(), "v5 layout check failed for {path_v5:?}"); + + std::fs::remove_file(&path_v4).ok(); + std::fs::remove_file(&path_v5).ok(); +} diff --git a/tests/threadsafe_concurrent_create.rs b/tests/threadsafe_concurrent_create.rs index d3b51a9..28946f6 100644 --- a/tests/threadsafe_concurrent_create.rs +++ b/tests/threadsafe_concurrent_create.rs @@ -28,12 +28,18 @@ use rust_hdf5::H5File; const TEST_PYTHON: &str = "/Users/stevek/mamba/envs/bs2026.1/bin/python"; fn python() -> Option<&'static str> { - if std::path::Path::new(TEST_PYTHON).exists() { - Some(TEST_PYTHON) - } else { - eprintln!("skipping h5py cross-check: {TEST_PYTHON} not present"); - None - } + static PY: std::sync::OnceLock> = std::sync::OnceLock::new(); + PY.get_or_init(|| { + let candidate = + std::env::var("RUST_HDF5_TEST_PYTHON").unwrap_or_else(|_| TEST_PYTHON.to_string()); + if std::path::Path::new(&candidate).exists() { + Some(candidate) + } else { + eprintln!("skipping h5py cross-check: {candidate} not present"); + None + } + }) + .as_deref() } fn tmp(name: &str) -> std::path::PathBuf { diff --git a/tests/threadsafe_parallel_write.rs b/tests/threadsafe_parallel_write.rs index 805de79..f27c214 100644 --- a/tests/threadsafe_parallel_write.rs +++ b/tests/threadsafe_parallel_write.rs @@ -29,12 +29,18 @@ use rust_hdf5::H5File; const TEST_PYTHON: &str = "/Users/stevek/mamba/envs/bs2026.1/bin/python"; fn python() -> Option<&'static str> { - if std::path::Path::new(TEST_PYTHON).exists() { - Some(TEST_PYTHON) - } else { - eprintln!("skipping h5py cross-check: {TEST_PYTHON} not present"); - None - } + static PY: std::sync::OnceLock> = std::sync::OnceLock::new(); + PY.get_or_init(|| { + let candidate = + std::env::var("RUST_HDF5_TEST_PYTHON").unwrap_or_else(|_| TEST_PYTHON.to_string()); + if std::path::Path::new(&candidate).exists() { + Some(candidate) + } else { + eprintln!("skipping h5py cross-check: {candidate} not present"); + None + } + }) + .as_deref() } fn tmp(name: &str) -> std::path::PathBuf { diff --git a/tests/threadsafe_same_dataset_append.rs b/tests/threadsafe_same_dataset_append.rs new file mode 100644 index 0000000..2b71e72 --- /dev/null +++ b/tests/threadsafe_same_dataset_append.rs @@ -0,0 +1,194 @@ +//! Concurrent same-dataset appends under the `threadsafe` writer. +//! +//! An append is several dataset-slot acquisitions: take the buffered tail, +//! write the chunk-aligned rows, re-buffer the new tail, extend the extent. +//! The per-slot mutex serializes each acquisition but not the operation, so +//! without the per-dataset op lock two appends interleave between +//! acquisitions — two calls take the same base row, rows are lost or +//! overwritten. These tests drive that exact schedule: N threads append to +//! *one* dataset, with a frame count that never divides the chunk so the +//! buffered tail is always in play. +//! +//! Every append call must land as a contiguous, in-order block of rows +//! (atomicity), every appended frame must appear exactly once (no lost or +//! doubled rows), and no frame may be torn. Order *between* calls is the +//! scheduler's. + +#![cfg(feature = "threadsafe")] + +use rust_hdf5::H5File; + +fn tmp(name: &str) -> std::path::PathBuf { + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "rust_hdf5_same_ds_{}_{}_{}.h5", + name, + std::process::id(), + n + )) +} + +/// Tag of frame `f` of append call `j` by thread `k`. Distinct per +/// (k, j, f) so a lost, doubled, or split append shows up as a wrong +/// multiset or a broken run on read-back. +fn tag(k: usize, j: usize, f: usize) -> i32 { + (k * 1_000_000 + j * 100 + f) as i32 +} + +#[test] +fn concurrent_same_dataset_appends_serialize_wholly() { + const N: usize = 4; // threads + const M: usize = 40; // append calls per thread + const FRAMES: usize = 3; // frames per call — never a multiple of CHUNK0 + const W: usize = 8; // row width + const CHUNK0: usize = 4; // chunk rows: every call leaves a buffered tail + const ITERS: usize = 10; // repeat to shake out schedules + + for iter in 0..ITERS { + let path = tmp(&format!("iter{iter}")); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, W]) + .chunk(&[CHUNK0, W]) + .max_shape(&[None, Some(W)]) + .create("d") + .unwrap(); + + std::thread::scope(|s| { + for k in 0..N { + let ds = &ds; + s.spawn(move || { + for j in 0..M { + let data: Vec = (0..FRAMES) + .flat_map(|f| std::iter::repeat_n(tag(k, j, f), W)) + .collect(); + ds.append(&data) + .unwrap_or_else(|e| panic!("append k={k} j={j}: {e}")); + } + }); + } + }); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![N * M * FRAMES, W], "iter {iter}"); + let got = ds.read_raw::().unwrap(); + + // No torn frames: each row is uniform. + let rows: Vec = got + .chunks_exact(W) + .enumerate() + .map(|(r, row)| { + assert!( + row.iter().all(|&v| v == row[0]), + "iter {iter}: torn frame at row {r}: {row:?}" + ); + row[0] + }) + .collect(); + + // Atomicity: each call's frames form one contiguous in-order run. + // Walking runs also proves every frame appears exactly once — the + // walk consumes exactly FRAMES rows per (k, j) and the total length + // already matched. + let mut seen = std::collections::HashSet::new(); + let mut r = 0; + while r < rows.len() { + let first = rows[r]; + let (k, rem) = ((first / 1_000_000) as usize, first % 1_000_000); + let (j, f) = ((rem / 100) as usize, (rem % 100) as usize); + assert_eq!(f, 0, "iter {iter}: run at row {r} starts mid-call: {first}"); + for f in 0..FRAMES { + assert_eq!( + rows[r + f], + tag(k, j, f), + "iter {iter}: call (k={k}, j={j}) split at row {}", + r + f + ); + } + assert!( + seen.insert((k, j)), + "iter {iter}: call (k={k}, j={j}) appended twice" + ); + r += FRAMES; + } + assert_eq!(seen.len(), N * M, "iter {iter}"); + + std::fs::remove_file(&path).ok(); + } +} + +#[test] +fn concurrent_same_dataset_vlen_appends_serialize_wholly() { + const N: usize = 4; // threads + const M: usize = 30; // append calls per thread + const STRINGS: usize = 3; // strings per call + const CHUNK: usize = 4; // never a multiple of STRINGS: tail always buffered + const ITERS: usize = 10; + + for iter in 0..ITERS { + let path = tmp(&format!("vlen{iter}")); + { + let file = H5File::create(&path).unwrap(); + file.create_appendable_vlen_dataset("strs", CHUNK, None) + .unwrap(); + + std::thread::scope(|s| { + for k in 0..N { + let file = &file; + s.spawn(move || { + for j in 0..M { + let batch: Vec = + (0..STRINGS).map(|f| format!("{k}:{j}:{f}")).collect(); + let refs: Vec<&str> = batch.iter().map(String::as_str).collect(); + file.append_vlen_strings("strs", &refs) + .unwrap_or_else(|e| panic!("append k={k} j={j}: {e}")); + } + }); + } + }); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("strs").unwrap(); + let got = ds.read_vlen_strings().unwrap(); + assert_eq!(got.len(), N * M * STRINGS, "iter {iter}"); + + // Each call's strings must form one contiguous in-order run, and the + // runs must cover every (k, j) exactly once. + let mut seen = std::collections::HashSet::new(); + let mut r = 0; + while r < got.len() { + let parts: Vec = got[r].split(':').map(|p| p.parse().unwrap()).collect(); + let (k, j, f) = (parts[0], parts[1], parts[2]); + assert_eq!( + f, 0, + "iter {iter}: run at element {r} starts mid-call: {}", + got[r] + ); + for f in 0..STRINGS { + assert_eq!( + got[r + f], + format!("{k}:{j}:{f}"), + "iter {iter}: call (k={k}, j={j}) split at element {}", + r + f + ); + } + assert!( + seen.insert((k, j)), + "iter {iter}: call (k={k}, j={j}) appended twice" + ); + r += STRINGS; + } + assert_eq!(seen.len(), N * M, "iter {iter}"); + + std::fs::remove_file(&path).ok(); + } +}