Skip to content

Commit 93b5e28

Browse files
zhangfengcdtclaude
andauthored
[GH-25] Fix persistent split/merged flags causing data corruption (#41)
* Fix persistent split/merged flags causing data corruption The bool-typed fields `merged` and `split` in the ProllyNode struct were being persisted through serialization and never reset to false, causing severe data corruption during tree operations. This commit fixes the issue by: 1. Making split/merged flags transient using #[serde(skip)] 2. Ensuring flags are always reset to false when nodes are retrieved from storage 3. Adding comprehensive tests to verify the fix and prevent regressions The fix prevents catastrophic data corruption where: - Split nodes would have their edges incorrectly hoisted to parent on every operation - Merged nodes would have their siblings incorrectly dropped from parent 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Apply cargo fmt formatting 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent 3cb107e commit 93b5e28

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

src/node.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,9 @@ pub struct ProllyNode<const N: usize> {
143143
pub min_chunk_size: usize,
144144
pub max_chunk_size: usize,
145145
pub pattern: u64,
146+
#[serde(skip)]
146147
pub split: bool,
148+
#[serde(skip)]
147149
pub merged: bool,
148150
pub encode_types: Vec<EncodingType>,
149151
pub encode_values: Vec<Vec<u8>>,
@@ -1540,4 +1542,57 @@ mod tests {
15401542
assert!(node.find(&[i], &storage).is_some());
15411543
}
15421544
}
1545+
1546+
#[test]
1547+
fn test_flags_reset_after_operations() {
1548+
// Test that split/merged flags are reset after insert/delete operations
1549+
let mut storage = InMemoryNodeStorage::<32>::default();
1550+
let mut node: ProllyNode<32> = ProllyNode::builder()
1551+
.pattern(0b1)
1552+
.min_chunk_size(2)
1553+
.max_chunk_size(4)
1554+
.build();
1555+
1556+
// Insert enough items to trigger splits
1557+
for i in 0..6 {
1558+
node.insert(vec![i], vec![i], &mut storage, Vec::new());
1559+
storage.insert_node(node.get_hash(), node.clone());
1560+
// Flags should be reset after each operation
1561+
assert!(
1562+
!node.split,
1563+
"Split flag should be reset after insert operation {}",
1564+
i
1565+
);
1566+
assert!(
1567+
!node.merged,
1568+
"Merged flag should be reset after insert operation {}",
1569+
i
1570+
);
1571+
}
1572+
1573+
// Test deletion as well
1574+
assert!(node.delete(&[0], &mut storage, Vec::new()));
1575+
assert!(
1576+
!node.split,
1577+
"Split flag should be reset after delete operation"
1578+
);
1579+
assert!(
1580+
!node.merged,
1581+
"Merged flag should be reset after delete operation"
1582+
);
1583+
}
1584+
1585+
#[test]
1586+
fn test_flags_not_serialized() {
1587+
// Test that split/merged flags are not serialized
1588+
let mut node = ProllyNode::<32>::default();
1589+
node.split = true;
1590+
node.merged = true;
1591+
let bytes = bincode::serialize(&node).unwrap();
1592+
let de: ProllyNode<32> = bincode::deserialize(&bytes).unwrap();
1593+
assert!(
1594+
!de.split && !de.merged,
1595+
"Split/merged flags should not be serialized"
1596+
);
1597+
}
15431598
}

src/storage.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,11 @@ impl<const N: usize> InMemoryNodeStorage<N> {
8989

9090
impl<const N: usize> NodeStorage<N> for InMemoryNodeStorage<N> {
9191
fn get_node_by_hash(&self, hash: &ValueDigest<N>) -> Option<ProllyNode<N>> {
92-
self.map.get(hash).cloned()
92+
self.map.get(hash).cloned().map(|mut node| {
93+
node.split = false;
94+
node.merged = false;
95+
node
96+
})
9397
}
9498

9599
fn insert_node(&mut self, hash: ValueDigest<N>, node: ProllyNode<N>) -> Option<()> {
@@ -156,7 +160,10 @@ impl<const N: usize> NodeStorage<N> for FileNodeStorage<N> {
156160
let mut file = File::open(path).unwrap();
157161
let mut data = Vec::new();
158162
file.read_to_end(&mut data).unwrap();
159-
Some(bincode::deserialize(&data).unwrap())
163+
let mut node: ProllyNode<N> = bincode::deserialize(&data).unwrap();
164+
node.split = false;
165+
node.merged = false;
166+
Some(node)
160167
} else {
161168
None
162169
}

0 commit comments

Comments
 (0)