diff --git a/Cargo.toml b/Cargo.toml index 17e90cfd..6de183fe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["codegen", "examples", "performance_measurement", "performance_measur [package] name = "worktable" -version = "1.0.0-beta.14" +version = "1.0.0-beta.15" edition = "2024" authors = ["Handy-caT"] license = "MIT" @@ -41,14 +41,14 @@ crc32fast = "1.5.0" # Already in the dependency graph transitively (indexset's concurrent # structures); used directly for read-side grace periods. crossbeam-epoch = "0.9.18" -data_bucket = "=0.5.4" +data_bucket = "=0.5.5" # data_bucket = { git = "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/pathscale/DataBucket", branch = "page_cdc_correction", version = "0.2.7" } # data_bucket = { path = "../DataBucket", version = "0.3.14" } derive_more = { version = "2.0.1", features = ["from", "error", "display", "debug", "into"] } eyre = "0.6.12" fastrand = "2.3.0" futures = "0.3.30" -indexset = { package = "WorkTablesIndex", version = "=0.0.8", default-features = false, features = ["concurrent", "cdc", "multimap"] } +indexset = { package = "WorkTablesIndex", version = "=0.0.9", default-features = false, features = ["concurrent", "cdc", "multimap"] } vanilla_indexset = { package = "indexset", version = "=0.15.0", features = ["concurrent", "cdc", "multimap"] } # indexset = { path = "../indexset", version = "0.15.0", features = ["concurrent", "cdc", "multimap"] } # indexset = { package = "wt-indexset", version = "=0.12.12", features = ["concurrent", "cdc", "multimap"] } diff --git a/src/index/multipair.rs b/src/index/multipair.rs index 38e4503a..f9de1381 100644 --- a/src/index/multipair.rs +++ b/src/index/multipair.rs @@ -1,25 +1,21 @@ use indexset::core::multipair::MultiPair; use indexset::core::pair::Pair; +/// Rebuild a multimap entry from a persisted pair. +/// +/// A `MultiPair` is identified by its `(key, value)` pair, both of which the snapshot +/// stores, so there is nothing to recreate beyond moving them across. The discriminator +/// these methods used to take existed only for the random representation, which could not +/// locate an entry by its value and had to invent an identity instead. pub trait MultiPairRecreate { - fn with_last_discriminator(self, discriminator: u64) -> MultiPair; - fn with_discriminator(self, discriminator: u64) -> MultiPair; + fn recreate(self) -> MultiPair; } impl MultiPairRecreate for Pair { - fn with_last_discriminator(self, discriminator: u64) -> MultiPair { + fn recreate(self) -> MultiPair { MultiPair { key: self.key, value: self.value, - discriminator: fastrand::u64(discriminator..), - } - } - - fn with_discriminator(self, discriminator: u64) -> MultiPair { - MultiPair { - key: self.key, - value: self.value, - discriminator, } } } diff --git a/src/persistence/space/index/reconstruct.rs b/src/persistence/space/index/reconstruct.rs index 0104f019..dab116ca 100644 --- a/src/persistence/space/index/reconstruct.rs +++ b/src/persistence/space/index/reconstruct.rs @@ -43,7 +43,7 @@ pub type PersistedMultiNode = (Pair, Vec>); /// several nodes can share one maximum key (a key's duplicates crossing /// leaf boundaries), and the persisted `(key, link)` id alone cannot /// recover their relative order — links are row locations, uncorrelated -/// with the lost discriminators. In a valid snapshot at most one of those +/// with the lost identities. In a valid snapshot at most one of those /// nodes also carries smaller keys (a run starts only once) and must come /// first; the duplicate-only rest are mutually order-free and get a /// deterministic link tiebreak. @@ -92,16 +92,21 @@ where prepared.push((node_id, entries)); } + // Nodes partition an ordered space, so they are ordered by their minimum, which is + // their first entry. Ordering by the node id instead sorts by each node's maximum, + // which puts a node that merely ends late ahead of one that starts earlier: with + // entries identified by `(key, value)` that produces overlapping node ranges. prepared.sort_by(|(a_id, a_entries), (b_id, b_entries)| { - a_id.key - .cmp(&b_id.key) - .then_with(|| a_entries[0].key.cmp(&b_entries[0].key)) + a_entries[0] + .key + .cmp(&b_entries[0].key) + .then_with(|| a_entries[0].value.cmp(&b_entries[0].value)) + .then_with(|| a_id.key.cmp(&b_id.key)) .then_with(|| a_id.value.cmp(&b_id.value)) }); let mut nodes = Vec::with_capacity(prepared.len()); let mut prev_key: Option = None; - let mut next_discriminator = 1u64; for (node_id, entries) in prepared { let mut node = Vec::with_capacity(entries.len()); for p in entries { @@ -119,14 +124,16 @@ where ); } prev_key = Some(p.key.clone()); - next_discriminator = 1; } + // A `MultiPair` is identified by its `(key, value)` pair, both of which the + // snapshot stores, so reconstruction has nothing left to synthesise. The + // discriminator counter this replaces existed only to separate entries the + // random representation could not tell apart, and it could collide node + // maxima on a damaged file. node.push(MultiPair { key: p.key, value: p.value, - discriminator: next_discriminator.min(u64::MAX - 1), }); - next_discriminator = next_discriminator.saturating_add(1); } nodes.push(node); } @@ -175,17 +182,19 @@ mod tests { } } - /// The review counterexample: a mixed boundary page (tail of key 1, start - /// of key 2's run) whose node-id link sorts AFTER a duplicate-only page - /// of key 2, with equal duplicate counts. Ordering pages by - /// `(node_id key, node_id link)` alone would reconstruct B before A, - /// strand the key-1 entries behind a (2, _) maximum, and give both nodes - /// the maximum (2, discriminator 2) — Equal maxima, one node replacing - /// the other in the outer index. + /// A mixed boundary page (tail of key 1, start of key 2's run) beside a + /// page holding the rest of key 2. Entries are identified by `(key, + /// value)`, so the pages partition an ordered space and reconstruction + /// must place them by that order however the input is shuffled. + /// + /// The counterexample this replaces had one page's range sitting inside + /// another's, which the random representation allowed because order + /// within a key came from a discriminator. A `(key, value)` index cannot + /// produce that: a page's maximum and its minimum agree on the ordering. #[test] fn mixed_boundary_page_with_adversarial_link_order() { - let page_a = (pair(2, 20), vec![pair(1, 11), pair(2, 21), pair(2, 20)]); - let page_b = (pair(2, 10), vec![pair(2, 12), pair(2, 10)]); + let page_a = (pair(2, 20), vec![pair(1, 11), pair(2, 10), pair(2, 20)]); + let page_b = (pair(2, 31), vec![pair(2, 30), pair(2, 31)]); // Input order must not matter; test both. for pages in [ @@ -200,16 +209,16 @@ mod tests { // Every persisted entry survives exactly once. let mut all = flatten(&nodes); all.sort_unstable(); - assert_eq!(all, vec![(1, 11), (2, 10), (2, 12), (2, 20), (2, 21)]); + assert_eq!(all, vec![(1, 11), (2, 10), (2, 20), (2, 30), (2, 31)]); // The mixed boundary node comes first (it carries key 1)... assert_eq!(nodes[0].first().unwrap().key, 1); // ...each node keeps its logical entry order verbatim... - assert_eq!(flatten(&nodes[..1]), vec![(1, 11), (2, 21), (2, 20)]); - assert_eq!(flatten(&nodes[1..]), vec![(2, 12), (2, 10)]); + assert_eq!(flatten(&nodes[..1]), vec![(1, 11), (2, 10), (2, 20)]); + assert_eq!(flatten(&nodes[1..]), vec![(2, 30), (2, 31)]); // ...and each node still ends with its persisted node id entry. assert_eq!((nodes[0].last().unwrap().key, nodes[0].last().unwrap().value), (2, 20)); - assert_eq!((nodes[1].last().unwrap().key, nodes[1].last().unwrap().value), (2, 10)); + assert_eq!((nodes[1].last().unwrap().key, nodes[1].last().unwrap().value), (2, 31)); } } @@ -219,10 +228,10 @@ mod tests { #[test] fn multi_node_straddle_chain() { let pages = vec![ - (pair(3, 5), vec![pair(2, 70), pair(3, 90), pair(3, 5)]), - (pair(2, 40), vec![pair(2, 41), pair(2, 40)]), - (pair(2, 30), vec![pair(2, 31), pair(2, 30)]), - (pair(2, 60), vec![pair(1, 2), pair(1, 1), pair(2, 61), pair(2, 60)]), + (pair(3, 90), vec![pair(2, 61), pair(2, 70), pair(3, 5), pair(3, 90)]), + (pair(2, 60), vec![pair(2, 41), pair(2, 60)]), + (pair(2, 40), vec![pair(2, 31), pair(2, 40)]), + (pair(2, 30), vec![pair(1, 1), pair(1, 2), pair(2, 30)]), ]; let expected_len: usize = pages.iter().map(|(_, e)| e.len()).sum(); @@ -235,16 +244,12 @@ mod tests { assert_eq!(nodes[0].first().unwrap().key, 1); assert_eq!(nodes[3].last().unwrap().key, 3); // Within every node the logical entry order is preserved verbatim. - assert_eq!(flatten(&nodes[..1]), vec![(1, 2), (1, 1), (2, 61), (2, 60)]); - // Key 2's discriminators grow across all four nodes. - let discs: Vec = nodes - .iter() - .flatten() - .filter(|p| p.key == 2) - .map(|p| p.discriminator) - .collect(); - for w in discs.windows(2) { - assert!(w[0] < w[1], "key 2 discriminators not strictly increasing: {discs:?}"); + assert_eq!(flatten(&nodes[..1]), vec![(1, 1), (1, 2), (2, 30)]); + // Every stored entry is distinct. That is what the discriminator counter was + // for; identity is now the `(key, value)` pair itself. + let mut seen = std::collections::BTreeSet::new(); + for p in nodes.iter().flatten() { + assert!(seen.insert((p.key, p.value)), "duplicate entry {:?}", (p.key, p.value)); } } diff --git a/tests/data/space_index_unsized/indexset/process_insert_at_big_amount.wt.idx b/tests/data/space_index_unsized/indexset/process_insert_at_big_amount.wt.idx index 94bd08f0..bd2fc259 100644 Binary files a/tests/data/space_index_unsized/indexset/process_insert_at_big_amount.wt.idx and b/tests/data/space_index_unsized/indexset/process_insert_at_big_amount.wt.idx differ diff --git a/tests/data/space_index_unsized/process_insert_at_big_amount.wt.idx b/tests/data/space_index_unsized/process_insert_at_big_amount.wt.idx index 6f7689b1..810c917c 100644 Binary files a/tests/data/space_index_unsized/process_insert_at_big_amount.wt.idx and b/tests/data/space_index_unsized/process_insert_at_big_amount.wt.idx differ diff --git a/tests/data/space_index_unsized/process_remove_at_node_id.wt.idx b/tests/data/space_index_unsized/process_remove_at_node_id.wt.idx index 1464eec5..271afb51 100644 Binary files a/tests/data/space_index_unsized/process_remove_at_node_id.wt.idx and b/tests/data/space_index_unsized/process_remove_at_node_id.wt.idx differ diff --git a/tests/data/space_index_unsized/process_split_node.wt.idx b/tests/data/space_index_unsized/process_split_node.wt.idx index 77b2488a..b01ebf72 100644 Binary files a/tests/data/space_index_unsized/process_split_node.wt.idx and b/tests/data/space_index_unsized/process_split_node.wt.idx differ