From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 4EC251FF143 for ; Mon, 02 Feb 2026 12:10:59 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A79A82556; Mon, 2 Feb 2026 12:11:28 +0100 (CET) Message-ID: <74de1f48-e793-4146-9d1d-a958487fc568@proxmox.com> Date: Mon, 2 Feb 2026 12:11:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 proxmox-backup 06/16] datastore: test FixedIndexWriter To: Robert Obkircher , pbs-devel@lists.proxmox.com References: <20260130164552.281581-1-r.obkircher@proxmox.com> <20260130164552.281581-7-r.obkircher@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20260130164552.281581-7-r.obkircher@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770030610768 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.048 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: JUUWKSXBKRDQBS6ZJ4BW454LVSLNVRS7 X-Message-ID-Hash: JUUWKSXBKRDQBS6ZJ4BW454LVSLNVRS7 X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 1/30/26 5:45 PM, Robert Obkircher wrote: > Write fixed and dynamically sized fidx files to a temporary directory. > Compare the resulting files directly (ignoring uuid and ctime bytes) > and also read them back using the reader. > > The chunk hashes are just dummy values that don't actually exist in a > chunk store. > > Signed-off-by: Robert Obkircher > --- > pbs-datastore/src/fixed_index.rs | 170 +++++++++++++++++++++++++++++++ > pbs-datastore/src/lib.rs | 1 - > 2 files changed, 170 insertions(+), 1 deletion(-) > > diff --git a/pbs-datastore/src/fixed_index.rs b/pbs-datastore/src/fixed_index.rs > index bcb0cf94..0f87bf15 100644 > --- a/pbs-datastore/src/fixed_index.rs > +++ b/pbs-datastore/src/fixed_index.rs > @@ -534,3 +534,173 @@ impl FixedIndexWriter { > Ok(()) > } > } > + > +#[cfg(test)] > +mod tests { > + use std::fs; > + > + use super::*; > + use crate::temp_test_dir::TempTestDir; > + > + const CS: usize = 4096; > + > + #[test] > + fn test_empty() { > + let dir = TempTestDir::new(); > + let path = dir.join("test_empty"); > + let mut w = FixedIndexWriter::create(&path, None, CS).unwrap(); > + > + assert!(w.add_digest(0, &[1u8; 32]).is_err(), "out of bounds"); > + > + assert_eq!(0, w.size); > + assert_eq!(0, w.index_length(), "returns length, not capacity"); > + assert_eq!(FixedIndexWriter::INITIAL_CAPACITY, w.index_capacity); > + > + assert!(w.close().is_err(), "should refuse to create empty file"); > + > + drop(w); > + assert!(!fs::exists(path).unwrap()); > + } > + > + #[test] > + fn test_single_partial_chunk() { > + let dir = TempTestDir::new(); > + let path = dir.join("test_single_partial_chunk"); > + let mut w = FixedIndexWriter::create(&path, None, CS).unwrap(); > + > + let size = CS - 1; > + let expected = test_data(size); > + w.grow_to_size(size).unwrap(); > + expected[0].add_to(&mut w); > + > + w.close().unwrap(); > + drop(w); > + > + check_with_reader(&path, size, &expected); > + compare_to_known_size_writer(&path, size, &expected); > + } > + > + #[test] > + fn test_grow_to_multiples_of_chunk_size() { > + let dir = TempTestDir::new(); > + let path = dir.join("test_grow_to_multiples_of_chunk_size"); > + let mut w = FixedIndexWriter::create(&path, None, CS).unwrap(); > + > + let initial = FixedIndexWriter::INITIAL_CAPACITY; > + let steps = [1, 2, initial, initial + 1, 5 * initial, 10 * initial + 1]; > + let expected = test_data(steps.last().unwrap() * CS); > + > + let mut begin = 0; > + for chunk_count in steps { > + let last = &expected[chunk_count - 1]; > + w.grow_to_size(last.end).unwrap(); > + assert_eq!(last.index + 1, w.index_length()); > + assert!(w.add_digest(last.index + 1, &[1u8; 32]).is_err()); > + > + for c in expected[begin..chunk_count].iter().rev() { > + c.add_to(&mut w); > + } > + begin = chunk_count; > + } > + w.close().unwrap(); > + drop(w); > + > + let size = expected.len() * CS; > + check_with_reader(&path, size, &expected); > + compare_to_known_size_writer(&path, size, &expected); > + } > + > + #[test] > + fn test_grow_to_misaligned_size() { > + let dir = TempTestDir::new(); > + let path = dir.join("test_grow_to_misaligned_size"); > + let mut w = FixedIndexWriter::create(&path, None, CS).unwrap(); > + > + let size = (FixedIndexWriter::INITIAL_CAPACITY + 42) * CS - 1; // last is not full > + let expected = test_data(size); > + > + w.grow_to_size(size).unwrap(); > + assert!(w.grow_to_size(size + 1).is_err(), "size must be fixed now"); > + assert_eq!(expected.len(), w.index_length()); > + assert!(w.add_digest(expected.len(), &[1u8; 32]).is_err()); > + > + for c in expected.iter().rev() { > + c.add_to(&mut w); > + } > + > + w.close().unwrap(); > + drop(w); > + > + check_with_reader(&path, size, &expected); > + compare_to_known_size_writer(&path, size, &expected); > + } > + > + struct TestChunk { > + digest: [u8; 32], > + index: usize, > + size: usize, > + end: usize, > + } > + > + impl TestChunk { > + fn add_to(&self, w: &mut FixedIndexWriter) { > + assert_eq!( > + self.index, > + w.check_chunk_alignment(self.end, self.size).unwrap() > + ); > + w.add_digest(self.index, &self.digest).unwrap(); > + } > + } > + > + fn test_data(size: usize) -> Vec { > + (0..size.div_ceil(CS)) > + .map(|index| { > + let mut digest = [0u8; 32]; > + let i = &(index as u64).to_le_bytes(); > + for c in digest.chunks_mut(i.len()) { > + c.copy_from_slice(i); > + } > + let size = if ((index + 1) * CS) <= size { > + CS > + } else { > + size % CS > + }; > + TestChunk { > + digest, > + index, > + size, > + end: index * CS + size, > + } > + }) > + .collect() > + } > + > + fn check_with_reader(path: &Path, size: usize, chunks: &[TestChunk]) { > + let reader = FixedIndexReader::open(path).unwrap(); > + assert_eq!(size as u64, reader.index_bytes()); > + assert_eq!(chunks.len(), reader.index_count()); > + for c in chunks { > + assert_eq!(&c.digest, reader.index_digest(c.index).unwrap()); > + } > + } > + > + fn compare_to_known_size_writer(file: &Path, size: usize, chunks: &[TestChunk]) { > + let mut path = file.to_path_buf(); > + path.set_extension("reference"); > + let mut w = FixedIndexWriter::create(&path, Some(size), CS).unwrap(); > + for c in chunks { > + c.add_to(&mut w); > + } > + w.close().unwrap(); > + drop(w); > + > + let mut reference = fs::read(file).unwrap(); > + let mut tested = fs::read(path).unwrap(); > + > + // ignore uuid and ctime > + reference[8..32].fill(0); > + tested[8..32].fill(0); > + > + assert_eq!(reference, tested); > + } > +} > diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs > index b48ec93c..23a17d96 100644 > --- a/pbs-datastore/src/lib.rs > +++ b/pbs-datastore/src/lib.rs > @@ -236,4 +236,3 @@ pub use local_datastore_lru_cache::LocalDatastoreLruCache; > > #[cfg(test)] > mod temp_test_dir; > - nit: unrelated hunk introduced by patch 0003, which should be fixed up there. Apart from that, test look good to me. Reviewed-by: Christian Ebner