mirror of
https://github.com/aljazceru/turso.git
synced 2025-12-18 17:14:20 +01:00
Merge 'Pager: clear overflow cells when freeing page' from Jussi Saurio
## Background The `balance_non_root` procedure can end up freeing a page if the pages to be balanced can fit the required combined number of cells in less pages, even if the page that triggered balancing is overfull. This can then free the originally overfull pages, leaving a non-zero `overflow_cells` on the in-mem representation of the page. ```rust balance_non_root: page=305, overflow_cells=0 balance_non_root: page=304, overflow_cells=0 balance_non_root: page=302, overflow_cells=1 pre_edit_page(page=304, page_idx=0, new_cells=4, old_cells=1, cells_per_page_old=[1, 3, 9, 0, 0], cells_per_page_new=[4, 9, 9, 0, 0], cell_array_count=9) edit_page start_old_cells=0 start_new_cells=0 number_new_cells=4 cell_array=9 end_old_cells=1 end_new_cells=4 pre_edit_page(page=305, page_idx=1, new_cells=4, old_cells=1, cells_per_page_old=[1, 3, 9, 0, 0], cells_per_page_new=[4, 9, 9, 0, 0], cell_array_count=9) edit_page start_old_cells=2 start_new_cells=5 number_new_cells=4 cell_array=9 end_old_cells=3 end_new_cells=9 balance_non_root: sibling_count_new=2, sibling_count=3 // Custom assertion to demonstrate this: thread 'main' panicked at core/storage/pager.rs:1127:29: Pager::free_page: In memory page with id 302 has overflow cells ``` ## Why is this a problem Right now this is not an immediate problem, because we always allocate brand new pages. However, in #2233 we begin to reuse pages from the freelist for page allocation to improve performance and reduce database size bloat. In that PR, the `balance_non_root` procedure will calculate cell counts incorrectly in `edit_page()` and panic if: 1. a new allocated page is taken from the freelist, 2. the page is still in memory, and 3. and it still contains `overflow_cells`. ## Solution Clear `page_contents.overflow_cells` when an in-memory page is freed. Reviewed-by: Pere Diaz Bou <pere-altea@homail.com> Closes #2238
This commit is contained in:
@@ -1140,7 +1140,17 @@ impl Pager {
|
||||
|
||||
let page = match page.clone() {
|
||||
Some(page) => {
|
||||
assert_eq!(page.get().id, page_id, "Page id mismatch");
|
||||
assert_eq!(
|
||||
page.get().id,
|
||||
page_id,
|
||||
"Pager::free_page: Page id mismatch: expected {} but got {}",
|
||||
page_id,
|
||||
page.get().id
|
||||
);
|
||||
if page.is_loaded() {
|
||||
let page_contents = page.get_contents();
|
||||
page_contents.overflow_cells.clear();
|
||||
}
|
||||
page
|
||||
}
|
||||
None => self.read_page(page_id)?,
|
||||
|
||||
Reference in New Issue
Block a user