Skip to content

Commit 02ff474

Browse files
committed
Treat trailing empty export slots as value-equal in Network::value_equal
1 parent f934aed commit 02ff474

1 file changed

Lines changed: 65 additions & 5 deletions

File tree

document/graph-storage/src/model.rs

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,19 @@ pub struct Network {
9494
impl Network {
9595
/// True if both networks agree on every value-bearing field, ignoring slot/attribute timestamps.
9696
pub fn value_equal(&self, other: &Self) -> bool {
97-
if self.exports.len() != other.exports.len() {
98-
return false;
99-
}
100-
if !self.exports.iter().zip(&other.exports).all(|(a, b)| a.target == b.target) {
101-
return false;
97+
// Compare slot targets index-by-index, treating out-of-range slots as `None`. A `SetExport(None)`
98+
// truncation leaves a trailing empty slot (a tombstone in the CRDT state) that is value-equal to
99+
// the slot being absent, so trailing `None`s must not count as drift. Mirrors `compute_deltas`
100+
// (emits nothing for them) and `to_runtime` (drops them).
101+
let max_len = self.exports.len().max(other.exports.len());
102+
for slot_idx in 0..max_len {
103+
let self_target = self.exports.get(slot_idx).and_then(|slot| slot.target.as_ref());
104+
let other_target = other.exports.get(slot_idx).and_then(|slot| slot.target.as_ref());
105+
if self_target != other_target {
106+
return false;
107+
}
102108
}
109+
103110
attributes_value_equal(&self.attributes, &other.attributes)
104111
}
105112
}
@@ -122,3 +129,56 @@ pub struct ProtoNode {
122129
pub wasm: Option<Vec<u8>>,
123130
pub attributes: Attributes,
124131
}
132+
133+
#[cfg(test)]
134+
mod tests {
135+
use super::*;
136+
use crate::TimeStamp;
137+
138+
fn target_slot(node_id: u64) -> ExportSlot {
139+
ExportSlot {
140+
target: Some(NodeInput::Node { node_id, output_index: 0 }),
141+
timestamp: TimeStamp::ORIGIN,
142+
}
143+
}
144+
145+
fn empty_slot() -> ExportSlot {
146+
ExportSlot {
147+
target: None,
148+
timestamp: TimeStamp { counter: 5, peer: crate::PeerId(1) },
149+
}
150+
}
151+
152+
/// A `SetExport(None)` truncation leaves a trailing empty slot. Such a network is value-equal to
153+
/// the same network without that slot, so the soak oracle doesn't false-report drift.
154+
#[test]
155+
fn trailing_empty_export_slot_is_value_equal() {
156+
let compact = Network {
157+
exports: vec![target_slot(1), target_slot(2)],
158+
..Default::default()
159+
};
160+
let with_trailing_empty = Network {
161+
exports: vec![target_slot(1), target_slot(2), empty_slot()],
162+
..Default::default()
163+
};
164+
165+
assert!(compact.value_equal(&with_trailing_empty));
166+
assert!(with_trailing_empty.value_equal(&compact));
167+
}
168+
169+
/// A `None` slot *between* live targets is a real value difference (a hole), not a trailing
170+
/// tombstone, so it must still count as drift.
171+
#[test]
172+
fn interior_empty_export_slot_is_not_value_equal() {
173+
let dense = Network {
174+
exports: vec![target_slot(1), target_slot(2)],
175+
..Default::default()
176+
};
177+
let with_hole = Network {
178+
exports: vec![target_slot(1), empty_slot(), target_slot(2)],
179+
..Default::default()
180+
};
181+
182+
assert!(!dense.value_equal(&with_hole));
183+
}
184+
}

0 commit comments

Comments
 (0)