From 3950ab1e52c8d1a83aedbe0518dada8c8f651b3a Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Mon, 7 Apr 2025 22:16:50 +0200 Subject: [PATCH 1/6] account for divider cell size in page size --- core/storage/btree.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index c505a3fee..06a7e65fd 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1642,6 +1642,10 @@ impl BTreeCursor { let size = new_page_sizes.last_mut().unwrap(); // 2 to account of pointer *size += 2 + overflow.payload.len() as u16; + if !leaf && i < balance_info.sibling_count - 1 { + // Account for divider cell which is included in this page. + let size = new_page_sizes.last_mut().unwrap(); + *size += cell_array.cells[cell_array.cell_count(i)].len() as i64; } } From 8e88b0cd147539eb5c9a3afe2d99f94bfa3f5795 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Mon, 7 Apr 2025 22:17:11 +0200 Subject: [PATCH 2/6] new_page_sizes as Vec --- core/storage/btree.rs | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 06a7e65fd..36a3a5ff0 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1624,7 +1624,7 @@ impl BTreeCursor { } } // calculate how many pages to allocate - let mut new_page_sizes = Vec::new(); + let mut new_page_sizes: Vec = Vec::new(); let leaf_correction = if leaf { 4 } else { 0 }; // number of bytes beyond header, different from global usableSapce which includes // header @@ -1637,11 +1637,12 @@ impl BTreeCursor { let page_contents = page.get_contents(); let free_space = compute_free_space(page_contents, self.usable_space() as u16); - new_page_sizes.push(usable_space as u16 - free_space); + new_page_sizes.push(usable_space as i64 - free_space as i64); for overflow in &page_contents.overflow_cells { let size = new_page_sizes.last_mut().unwrap(); // 2 to account of pointer - *size += 2 + overflow.payload.len() as u16; + *size += 2 + overflow.payload.len() as i64; + } if !leaf && i < balance_info.sibling_count - 1 { // Account for divider cell which is included in this page. let size = new_page_sizes.last_mut().unwrap(); @@ -1654,7 +1655,7 @@ impl BTreeCursor { let mut i = 0; while i < sibling_count_new { // First try to move cells to the right if they do not fit - while new_page_sizes[i] > usable_space as u16 { + while new_page_sizes[i] > usable_space as i64 { let needs_new_page = i + 1 >= sibling_count_new; if needs_new_page { sibling_count_new += 1; @@ -1668,7 +1669,9 @@ impl BTreeCursor { ); } let size_of_cell_to_remove_from_left = - 2 + cell_array.cells[cell_array.cell_count(i) - 1].len() as u16; + 2 + cell_array.cells[cell_array.cell_count(i) - 1].len() as i64; + // removing a page from the right might include removing from a page that is not directly adjacent, therefore, it could be possible we set page+1 + // to a negative number until we move the cell to the right page again. new_page_sizes[i] -= size_of_cell_to_remove_from_left; let size_of_cell_to_move_right = if !leaf_data { if cell_array.number_of_cells_per_page[i] @@ -1676,23 +1679,23 @@ impl BTreeCursor { { // This means we move to the right page the divider cell and we // promote left cell to divider - 2 + cell_array.cells[cell_array.cell_count(i)].len() as u16 + 2 + cell_array.cells[cell_array.cell_count(i)].len() as i64 } else { 0 } } else { size_of_cell_to_remove_from_left }; - new_page_sizes[i + 1] += size_of_cell_to_move_right; + new_page_sizes[i + 1] += size_of_cell_to_move_right as i64; cell_array.number_of_cells_per_page[i] -= 1; } // Now try to take from the right if we didn't have enough while cell_array.number_of_cells_per_page[i] < cell_array.cells.len() as u16 { let size_of_cell_to_remove_from_right = - 2 + cell_array.cells[cell_array.cell_count(i)].len() as u16; + 2 + cell_array.cells[cell_array.cell_count(i)].len() as i64; let can_take = new_page_sizes[i] + size_of_cell_to_remove_from_right - > usable_space as u16; + > usable_space as i64; if can_take { break; } @@ -1703,7 +1706,7 @@ impl BTreeCursor { if cell_array.number_of_cells_per_page[i] < cell_array.cells.len() as u16 { - 2 + cell_array.cells[cell_array.cell_count(i)].len() as u16 + 2 + cell_array.cells[cell_array.cell_count(i)].len() as i64 } else { 0 } @@ -1749,8 +1752,8 @@ impl BTreeCursor { // the same we add to right (we don't add divider to right). let mut cell_right = cell_left + 1 - leaf_data as u16; loop { - let cell_left_size = cell_array.cell_size(cell_left as usize); - let cell_right_size = cell_array.cell_size(cell_right as usize); + let cell_left_size = cell_array.cell_size(cell_left as usize) as i64; + let cell_right_size = cell_array.cell_size(cell_right as usize) as i64; // TODO: add assert nMaxCells let pointer_size = if i == sibling_count_new - 1 { 0 } else { 2 }; @@ -4739,7 +4742,7 @@ mod tests { let (pager, root_page) = empty_btree(); let mut cursor = BTreeCursor::new(None, pager.clone(), root_page); let mut keys = Vec::new(); - let seed = rng.next_u64(); + let seed = 3206743363843416341; tracing::info!("seed: {}", seed); let mut rng = ChaCha8Rng::seed_from_u64(seed); for insert_id in 0..inserts { @@ -4879,25 +4882,21 @@ mod tests { } #[test] - #[ignore] pub fn btree_insert_fuzz_run_random() { btree_insert_fuzz_run(128, 16, |rng| (rng.next_u32() % 4096) as usize); } #[test] - #[ignore] pub fn btree_insert_fuzz_run_small() { btree_insert_fuzz_run(1, 100, |rng| (rng.next_u32() % 128) as usize); } #[test] - #[ignore] pub fn btree_insert_fuzz_run_big() { btree_insert_fuzz_run(64, 32, |rng| 3 * 1024 + (rng.next_u32() % 1024) as usize); } #[test] - #[ignore] pub fn btree_insert_fuzz_run_overflow() { btree_insert_fuzz_run(64, 32, |rng| (rng.next_u32() % 32 * 1024) as usize); } From 40f8bbe1320ce2f0af85f5837ee2eec1dbbdbdcf Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Tue, 8 Apr 2025 11:05:40 +0200 Subject: [PATCH 3/6] clippy --- core/storage/btree.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 36a3a5ff0..349d75855 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -4742,9 +4742,7 @@ mod tests { let (pager, root_page) = empty_btree(); let mut cursor = BTreeCursor::new(None, pager.clone(), root_page); let mut keys = Vec::new(); - let seed = 3206743363843416341; tracing::info!("seed: {}", seed); - let mut rng = ChaCha8Rng::seed_from_u64(seed); for insert_id in 0..inserts { let size = size(&mut rng); let key = { From 8c4003908f30288f81e7002a6d62134c7967808e Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Tue, 8 Apr 2025 14:04:51 +0200 Subject: [PATCH 4/6] bring back usize, it shouldn't underflow --- core/storage/btree.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 349d75855..e5576cf28 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1624,7 +1624,7 @@ impl BTreeCursor { } } // calculate how many pages to allocate - let mut new_page_sizes: Vec = Vec::new(); + let mut new_page_sizes: Vec = Vec::new(); let leaf_correction = if leaf { 4 } else { 0 }; // number of bytes beyond header, different from global usableSapce which includes // header @@ -1637,16 +1637,16 @@ impl BTreeCursor { let page_contents = page.get_contents(); let free_space = compute_free_space(page_contents, self.usable_space() as u16); - new_page_sizes.push(usable_space as i64 - free_space as i64); + new_page_sizes.push(usable_space as usize - free_space as usize); for overflow in &page_contents.overflow_cells { let size = new_page_sizes.last_mut().unwrap(); // 2 to account of pointer - *size += 2 + overflow.payload.len() as i64; + *size += 2 + overflow.payload.len() as usize; } if !leaf && i < balance_info.sibling_count - 1 { // Account for divider cell which is included in this page. let size = new_page_sizes.last_mut().unwrap(); - *size += cell_array.cells[cell_array.cell_count(i)].len() as i64; + *size += cell_array.cells[cell_array.cell_count(i)].len() as usize; } } @@ -1655,7 +1655,7 @@ impl BTreeCursor { let mut i = 0; while i < sibling_count_new { // First try to move cells to the right if they do not fit - while new_page_sizes[i] > usable_space as i64 { + while new_page_sizes[i] > usable_space as usize { let needs_new_page = i + 1 >= sibling_count_new; if needs_new_page { sibling_count_new += 1; @@ -1669,7 +1669,7 @@ impl BTreeCursor { ); } let size_of_cell_to_remove_from_left = - 2 + cell_array.cells[cell_array.cell_count(i) - 1].len() as i64; + 2 + cell_array.cells[cell_array.cell_count(i) - 1].len() as usize; // removing a page from the right might include removing from a page that is not directly adjacent, therefore, it could be possible we set page+1 // to a negative number until we move the cell to the right page again. new_page_sizes[i] -= size_of_cell_to_remove_from_left; @@ -1679,23 +1679,23 @@ impl BTreeCursor { { // This means we move to the right page the divider cell and we // promote left cell to divider - 2 + cell_array.cells[cell_array.cell_count(i)].len() as i64 + 2 + cell_array.cells[cell_array.cell_count(i)].len() as usize } else { 0 } } else { size_of_cell_to_remove_from_left }; - new_page_sizes[i + 1] += size_of_cell_to_move_right as i64; + new_page_sizes[i + 1] += size_of_cell_to_move_right as usize; cell_array.number_of_cells_per_page[i] -= 1; } // Now try to take from the right if we didn't have enough while cell_array.number_of_cells_per_page[i] < cell_array.cells.len() as u16 { let size_of_cell_to_remove_from_right = - 2 + cell_array.cells[cell_array.cell_count(i)].len() as i64; + 2 + cell_array.cells[cell_array.cell_count(i)].len() as usize; let can_take = new_page_sizes[i] + size_of_cell_to_remove_from_right - > usable_space as i64; + > usable_space as usize; if can_take { break; } @@ -1706,7 +1706,7 @@ impl BTreeCursor { if cell_array.number_of_cells_per_page[i] < cell_array.cells.len() as u16 { - 2 + cell_array.cells[cell_array.cell_count(i)].len() as i64 + 2 + cell_array.cells[cell_array.cell_count(i)].len() as usize } else { 0 } @@ -1752,8 +1752,8 @@ impl BTreeCursor { // the same we add to right (we don't add divider to right). let mut cell_right = cell_left + 1 - leaf_data as u16; loop { - let cell_left_size = cell_array.cell_size(cell_left as usize) as i64; - let cell_right_size = cell_array.cell_size(cell_right as usize) as i64; + let cell_left_size = cell_array.cell_size(cell_left as usize) as usize; + let cell_right_size = cell_array.cell_size(cell_right as usize) as usize; // TODO: add assert nMaxCells let pointer_size = if i == sibling_count_new - 1 { 0 } else { 2 }; @@ -4896,7 +4896,7 @@ mod tests { #[test] pub fn btree_insert_fuzz_run_overflow() { - btree_insert_fuzz_run(64, 32, |rng| (rng.next_u32() % 32 * 1024) as usize); + btree_insert_fuzz_run(64, 10000, |rng| (rng.next_u32() % 32 * 1024) as usize); } #[allow(clippy::arc_with_non_send_sync)] From c0c66bf8af20b1c839d1c0b868c392a490287907 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Tue, 8 Apr 2025 14:06:48 +0200 Subject: [PATCH 5/6] remove wrong comment --- core/storage/btree.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index e5576cf28..87da725d7 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1670,8 +1670,6 @@ impl BTreeCursor { } let size_of_cell_to_remove_from_left = 2 + cell_array.cells[cell_array.cell_count(i) - 1].len() as usize; - // removing a page from the right might include removing from a page that is not directly adjacent, therefore, it could be possible we set page+1 - // to a negative number until we move the cell to the right page again. new_page_sizes[i] -= size_of_cell_to_remove_from_left; let size_of_cell_to_move_right = if !leaf_data { if cell_array.number_of_cells_per_page[i] From fded6ccaf3b06d31f454fb082de0d7469dad2c3a Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Tue, 8 Apr 2025 14:09:17 +0200 Subject: [PATCH 6/6] rever iterations fuzz test --- core/storage/btree.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 87da725d7..e29a9f8f7 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -4894,7 +4894,7 @@ mod tests { #[test] pub fn btree_insert_fuzz_run_overflow() { - btree_insert_fuzz_run(64, 10000, |rng| (rng.next_u32() % 32 * 1024) as usize); + btree_insert_fuzz_run(64, 32, |rng| (rng.next_u32() % 32 * 1024) as usize); } #[allow(clippy::arc_with_non_send_sync)]