From ebe51f8e9798e539c26ced33df63c972d1e70654 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 17:26:57 -0500 Subject: [PATCH] clawhdf5-remote tests: open + list and a dataset read counted apart The per-file report now separates a tree view (open, every group's entries, every dataset's shape and type) from reading the largest dataset under 64 MiB, and checks the budget the design's testing section asks for: listing the IMERG file (file A of docs/design/range-reads.md section 2) takes at most 3 requests when CLAWHDF5_REMOTE_CORPUS includes it. The test server now counts a response's bytes before sending it: a client could read a body and reset the counters before the server thread had added it, so the counts of the next file were occasionally too high. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-remote/tests/common/mod.rs | 44 +++++++++----- crates/clawhdf5-remote/tests/common/server.rs | 4 +- crates/clawhdf5-remote/tests/http.rs | 58 ++++++++++++------- 3 files changed, 69 insertions(+), 37 deletions(-) diff --git a/crates/clawhdf5-remote/tests/common/mod.rs b/crates/clawhdf5-remote/tests/common/mod.rs index 498b603..9aeb1bc 100644 --- a/crates/clawhdf5-remote/tests/common/mod.rs +++ b/crates/clawhdf5-remote/tests/common/mod.rs @@ -147,11 +147,13 @@ fn dataset(out: &mut String, path: &str, ds: &clawhdf5::Dataset<'_>) { } } -/// Open, list every group, and read the first dataset found whose data is -/// at most `MAX_DATA_BYTES` (a tree view plus one plot). -pub fn list_and_read_one(file: &File) { +/// List the file as a tree view does — every group's entries, every +/// dataset's shape and type — and return the largest dataset whose data +/// is at most `MAX_DATA_BYTES` (address, bytes), the one a viewer would +/// plot. +pub fn list(file: &File) -> Option<(u64, u64)> { let mut seen = HashSet::new(); - let mut read_one = false; + let mut largest: Option<(u64, u64)> = None; let mut queue = VecDeque::from([file.superblock().root_group_address]); while let Some(addr) = queue.pop_front() { if seen.len() >= MAX_OBJECTS || !seen.insert(addr) { @@ -160,22 +162,36 @@ pub fn list_and_read_one(file: &File) { let group = file.group_at(addr); if let Ok(ds) = file.dataset_at(addr) { let _ = (ds.shape(), ds.dtype()); - if !read_one { - let small = ds.shape().ok().and_then(|s| { - let n = s.iter().try_fold(1u64, |a, &d| a.checked_mul(d))?; - let size = u64::from(ds.raw_datatype().ok()?.type_size()); - n.checked_mul(size).filter(|&b| b <= MAX_DATA_BYTES) - }); - if small.is_some() { - let _ = ds.read_selection(&Selection::All); - read_one = true; - } + let bytes = ds.shape().ok().and_then(|s| { + let n = s.iter().try_fold(1u64, |a, &d| a.checked_mul(d))?; + let size = u64::from(ds.raw_datatype().ok()?.type_size()); + n.checked_mul(size).filter(|&b| b <= MAX_DATA_BYTES) + }); + if let Some(b) = bytes + && largest.is_none_or(|(_, l)| b > l) + { + largest = Some((addr, b)); } } if let Ok(entries) = group.entries() { queue.extend(entries.into_iter().map(|(_, a)| a)); } } + largest +} + +/// Read the dataset at `addr` whole. +pub fn read_one(file: &File, addr: u64) { + if let Ok(ds) = file.dataset_at(addr) { + let _ = ds.read_selection(&Selection::All); + } +} + +/// [`list`], then [`read_one`] of the dataset it picks. +pub fn list_and_read_one(file: &File) { + if let Some((addr, _)) = list(file) { + read_one(file, addr); + } } /// External virtual-dataset sources read from `dir`, as `File::open` finds diff --git a/crates/clawhdf5-remote/tests/common/server.rs b/crates/clawhdf5-remote/tests/common/server.rs index e556d7f..4749f50 100644 --- a/crates/clawhdf5-remote/tests/common/server.rs +++ b/crates/clawhdf5-remote/tests/common/server.rs @@ -320,9 +320,11 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> { }; let mut response = head.into_bytes(); response.extend_from_slice(body); + // Counted before the client can have the bytes, so a test that + // resets the counters after a read never sees them arrive late. + s.bytes.fetch_add(body.len() as u64, Ordering::SeqCst); out.write_all(&response)?; out.flush()?; - s.bytes.fetch_add(body.len() as u64, Ordering::SeqCst); if truncate || close { let _ = out.shutdown(std::net::Shutdown::Both); return Ok(()); diff --git a/crates/clawhdf5-remote/tests/http.rs b/crates/clawhdf5-remote/tests/http.rs index 197e268..b0a0e34 100644 --- a/crates/clawhdf5-remote/tests/http.rs +++ b/crates/clawhdf5-remote/tests/http.rs @@ -53,7 +53,7 @@ fn compare(files: &[std::path::PathBuf], report: bool) -> (usize, usize) { .collect(); let server = Server::start(served.clone()); let (mut same, mut refused) = (0, 0); - let mut totals = [0u64; 5]; + let mut totals = [0u64; 7]; for (i, p) in files.iter().enumerate() { let Some((url_path, bytes)) = served .iter() @@ -104,45 +104,59 @@ fn compare(files: &[std::path::PathBuf], report: bool) -> (usize, usize) { assert!(storage.stats().reads > 0, "read through the cache"); same += 1; - // Cost of "list + read one dataset": with the block cache, and - // with none (every read a request). - let with = storage_for_url(&url, &quick()).unwrap(); + // Cost of "open + list" (a tree view) and of then reading the + // largest dataset (a plot): with the block cache, and with none + // (every read a request). Requests and bytes as the server saw + // them, the open included. server.reset(); - if let Ok(f) = File::open_storage(with.clone()) { - list_and_read_one(&f); + let with = storage_for_url(&url, &quick()).unwrap(); + let f = File::open_storage(with.clone()).unwrap(); + let pick = common::list(&f); + let (list_requests, list_bytes) = (server.requests(), server.bytes()); + if let Some((addr, _)) = pick { + common::read_one(&f, addr); } let (cached_requests, cached_bytes) = (server.requests(), server.bytes()); - let (bare, _) = HttpStorage::open(&url, quick().http).unwrap(); - let bare = Arc::new(bare); server.reset(); - if let Ok(f) = File::open_storage(bare.clone()) { + let (bare, _) = HttpStorage::open(&url, quick().http).unwrap(); + if let Ok(f) = File::open_storage(Arc::new(bare)) { list_and_read_one(&f); } let uncached_requests = server.requests(); - totals[0] += 1 + cached_requests; // + the open probe - totals[1] += cached_bytes + with.config().block_size.min(bytes.len() as u64); - totals[2] += 1 + uncached_requests; - totals[3] += bytes.len() as u64; - totals[4] += 1; + for (t, v) in totals.iter_mut().zip([ + list_requests, + list_bytes, + cached_requests, + cached_bytes, + uncached_requests, + bytes.len() as u64, + 1, + ]) { + *t += v; + } if report { eprintln!( - "requests cached {:>6} uncached {:>8} bytes {:>12} of {:>12} {}", - 1 + cached_requests, - 1 + uncached_requests, - cached_bytes + with.config().block_size.min(bytes.len() as u64), + "open+list {list_requests:>4} req {list_bytes:>10} B | +read {:>5} req \ + {cached_bytes:>10} B | uncached {uncached_requests:>7} req | file {:>10} B {}", + cached_requests, bytes.len(), p.display() ); } // Files within one block: opening fetched everything. if bytes.len() as u64 <= with.config().block_size { - assert_eq!(cached_requests, 0, "{}", p.display()); + assert_eq!(cached_requests, 1, "{}", p.display()); + } + // The budget of docs/design/range-reads.md (Testing): listing the + // IMERG file (file A of section 2) takes at most 3 requests. + if p.ends_with("xarray-data/imerghh_730.hdf5") { + assert!(list_requests <= 3, "{}: {list_requests}", p.display()); } } eprintln!( - "list + read one dataset over {} files: {} requests with the 1 MiB block cache \ - ({} bytes transferred, files {} bytes), {} requests without a cache", - totals[4], totals[0], totals[1], totals[3], totals[2] + "{} files ({} bytes): open + list {} requests, {} bytes; + read the largest \ + dataset {} requests, {} bytes (1 MiB block cache); without a cache {} requests", + totals[6], totals[5], totals[0], totals[1], totals[2], totals[3], totals[4] ); (same, refused) }