diff --git a/CHANGELOG.md b/CHANGELOG.md index a7272ba..9d94b66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -305,6 +305,11 @@ with libhdf5's defaults: a few thousand long link names, or ~20 000 short ones) could not be listed: child indirect blocks were given the wrong number of rows, so every link stored in one was unreachable. + - v2 B-trees of depth 3 or more (a dense group of ~22 000+ links) were + misparsed: internal-node child pointers were read with widths from an + estimate instead of libhdf5's per-depth record capacities, and the + listing failed. The same B-tree code indexes dense attributes, shared + messages and chunks. - `clawhdf5-format` writer — **files libhdf5 rejects or reads wrong:** - Extensible Array (one unlimited dimension): chunks from index 244 on were written but never indexed and read as 0, by libhdf5 and by us. diff --git a/crates/clawhdf5-format/src/btree_v2.rs b/crates/clawhdf5-format/src/btree_v2.rs index 43fd7b0..0b70981 100644 --- a/crates/clawhdf5-format/src/btree_v2.rs +++ b/crates/clawhdf5-format/src/btree_v2.rs @@ -323,39 +323,21 @@ fn collect_internal_records( let records_start = pos; pos += records_total; - // Compute sizes for child pointers - // max_records at child depth - for variable-width nrec encoding + // Child pointer layout, as libhdf5 computes it (H5B2__hdr_init): the + // child's record count is always encoded in the width needed for a + // *leaf's* maximum, and — below the first internal level — the child + // subtree's total record count in the width needed for the most records + // a subtree of that depth can hold. let child_depth = depth - 1; - let max_nrec_child = if child_depth == 0 { - max_leaf_nrec - } else { - // For internal nodes at child_depth, the true max_nrec depends on the - // node size, record size, and the recursive width of child pointer - // entries (which themselves depend on max_nrec at deeper levels). - // Computing the exact value requires iterating from the leaf level - // upward, as described in the HDF5 spec (III.A.2 "Computing the Size - // of B-tree Nodes"). - // - // We use `max_leaf_nrec * 2` as a conservative upper bound. This - // over-estimates the nrec encoding width, which means we may read - // slightly more bytes per child pointer than strictly necessary, but - // never fewer. The over-read bytes are harmless because we only - // decode `num_records` entries (the actual count from the node header). - // - // Known limitation: for very deep trees (depth > 3) with small record - // sizes, the true max could exceed this estimate, causing us to - // under-allocate the nrec encoding width and misparse child pointers. - // In practice, HDF5 B-tree v2 depths rarely exceed 2-3. - max_leaf_nrec * 2 - }; - let nrec_width = bytes_for_max_records(max_nrec_child); - - // Total records in subtree width (only if depth > 1) + let nrec_width = bytes_for_max_records(max_leaf_nrec); let total_nrec_width = if depth > 1 { - // Width to hold total records in a subtree - // We compute max possible total records at this subtree depth - let max_total = header_max_total_records(max_leaf_nrec, depth - 1); - bytes_for_max_records(max_total) + bytes_for_max_records(cum_max_records( + node_size, + record_size, + offset_size, + max_leaf_nrec, + child_depth, + )) } else { 0 }; @@ -435,14 +417,36 @@ fn collect_internal_records( Ok(()) } -/// Estimate maximum total records at a given depth (for variable-width encoding). -fn header_max_total_records(max_leaf_nrec: u64, depth: u16) -> u64 { - // Conservative: branching factor * max_leaf at each level - let mut total = max_leaf_nrec; - for _ in 0..depth { - total = total.saturating_mul(max_leaf_nrec.max(2)); +/// Most records a subtree whose root is at `depth` can hold (libhdf5's +/// `cum_max_nrec`): a leaf holds `max_leaf_nrec`; an internal node at depth +/// `d` holds `max_nrec(d)` records and `max_nrec(d) + 1` subtrees of depth +/// `d - 1`, where `max_nrec(d)` is what fits in a node once each record is +/// paired with a child pointer of the width depth `d` needs. +fn cum_max_records( + node_size: u32, + record_size: u16, + offset_size: u8, + max_leaf_nrec: u64, + depth: u16, +) -> u64 { + // Internal node overhead: signature(4) + version(1) + type(1) + checksum(4). + const PREFIX: u64 = 10; + let nrec_width = bytes_for_max_records(max_leaf_nrec) as u64; + let mut cum = max_leaf_nrec; + let mut cum_width = 0u64; + for d in 1..=depth { + let ptr = u64::from(offset_size) + nrec_width + if d > 1 { cum_width } else { 0 }; + let max_nrec = u64::from(node_size) + .saturating_sub(PREFIX) + .saturating_sub(ptr) + / (u64::from(record_size) + ptr).max(1); + cum = max_nrec + .saturating_add(1) + .saturating_mul(cum) + .saturating_add(max_nrec); + cum_width = bytes_for_max_records(cum) as u64; } - total + cum } #[cfg(test)] @@ -512,9 +516,15 @@ mod tests { child_nrec: u64, ) -> Vec { let max_leaf = max_records_leaf(node_size, record_size); - let nrec_width = bytes_for_max_records(if depth == 1 { max_leaf } else { max_leaf * 2 }); + let nrec_width = bytes_for_max_records(max_leaf); let total_width = if depth > 1 { - bytes_for_max_records(header_max_total_records(max_leaf, depth - 1)) + bytes_for_max_records(cum_max_records( + node_size, + record_size, + 8, + max_leaf, + depth - 1, + )) } else { 0 }; @@ -673,4 +683,18 @@ mod tests { let records = collect_btree_v2_records(&header, &hdr, 8, 8).unwrap(); assert!(records.is_empty()); } + + #[test] + fn subtree_capacity_matches_libhdf5() { + // A link-name index (11-byte records, 512-byte nodes, 8-byte + // addresses): libhdf5's H5B2__hdr_init gives 45 records per leaf, + // then cum_max_nrec 1 149 at depth 1 and 26 449 at depth 2 — two + // bytes of subtree count in a depth-3 root's child pointers, where + // leaf_max^3 = 91 125 would need three. + let leaf = max_records_leaf(512, 11); + assert_eq!(leaf, 45); + assert_eq!(cum_max_records(512, 11, 8, leaf, 0), 45); + assert_eq!(cum_max_records(512, 11, 8, leaf, 1), 1_149); + assert_eq!(cum_max_records(512, 11, 8, leaf, 2), 26_449); + } } diff --git a/crates/clawhdf5/tests/dense_storage_interop.rs b/crates/clawhdf5/tests/dense_storage_interop.rs index e808d85..ecfff7a 100644 --- a/crates/clawhdf5/tests/dense_storage_interop.rs +++ b/crates/clawhdf5/tests/dense_storage_interop.rs @@ -141,3 +141,25 @@ fn dense_group_whose_heap_outgrows_the_root_direct_rows() { let last = format!("g/n02499_{}", "x".repeat(240)); assert_eq!(f.dataset(&last).unwrap().read_f64().unwrap(), vec![1.0]); } + +#[test] +fn dense_group_with_a_three_level_name_index() { + skip_if_no_python!(); + // 24 000 links give the link-name v2 B-tree a depth of 3. Internal-node + // child pointers carry the subtree's total record count in a width that + // depends on the most records a subtree can hold; the reader estimated + // that as leaf_max^depth, read the root's pointers 3 bytes wide instead + // of 2, and decoded garbage heap IDs. + let (_dir, path) = h5py_file( + "t = f.create_dataset('t', data=[1.0])\n\ + g = f.create_group('g')\n\ + for i in range(24000):\n\ + \x20 g['l%06d' % i] = t\n", + ); + assert_same_listing(&path, "g"); + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("g/l023999").unwrap().read_f64().unwrap(), + vec![1.0] + ); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 59064cb..39cdcf2 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -77,8 +77,9 @@ the VDS item, which is marked. - **Groups and links:** - Groups with a user-defined link type (e.g. 187) cannot be listed. - Dense groups with more than about 22 000 links cannot be listed. - *Partly fixed 2026-09-25:* a link heap past its root block's direct rows - (child indirect blocks) is now read. + **Fixed 2026-09-25:** two bugs — fractal-heap child indirect blocks had + the wrong row count, and v2 B-tree internal nodes at depth 3+ were read + with the wrong pointer widths. - Soft links are left out of `datasets()`. - **Dense attributes:** a large attribute stored as a fractal-heap "huge" object makes every attribute on the object fail. This affects real NetCDF