From 5878724d0e56ee8cd2d8081392a2515bcd50b905 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 24 Jun 2025 11:08:22 +0300 Subject: [PATCH 1/3] fix/btree: balance and seek after overwritten cell overflows --- core/storage/btree.rs | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index cec3766c6..057f89412 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -2128,12 +2128,22 @@ impl BTreeCursor { match cell { BTreeCell::TableLeafCell(tbl_leaf) => { if tbl_leaf._rowid == bkey.to_rowid() { - tracing::debug!("found exact match with cell_idx={cell_idx}, overwriting"); + tracing::debug!("TableLeafCell: found exact match with cell_idx={cell_idx}, overwriting"); self.overwrite_cell(page.clone(), cell_idx, record)?; - self.state + let write_info = self + .state .mut_write_info() - .expect("expected write info") - .state = WriteState::Finish; + .expect("expected write info"); + if page.get().get_contents().overflow_cells.is_empty() { + write_info.state = WriteState::Finish; + return Ok(CursorResult::Ok(())); + } else { + write_info.state = WriteState::BalanceStart; + // If we balance, we must save the cursor position and seek to it later. + // FIXME: we shouldn't have both DeleteState::SeekAfterBalancing and + // save_context()/restore/context(), they are practically the same thing. + self.save_context(CursorContext::TableRowId(bkey.to_rowid())); + } continue; } } @@ -2149,13 +2159,23 @@ impl BTreeCursor { &self.collations, ); if cmp == Ordering::Equal { - tracing::debug!("found exact match with cell_idx={cell_idx}, overwriting"); + tracing::debug!("IndexLeafCell: found exact match with cell_idx={cell_idx}, overwriting"); self.has_record.set(true); self.overwrite_cell(page.clone(), cell_idx, record)?; - self.state + let write_info = self + .state .mut_write_info() - .expect("expected write info") - .state = WriteState::Finish; + .expect("expected write info"); + if page.get().get_contents().overflow_cells.is_empty() { + write_info.state = WriteState::Finish; + return Ok(CursorResult::Ok(())); + } else { + write_info.state = WriteState::BalanceStart; + // If we balance, we must save the cursor position and seek to it later. + // FIXME: we shouldn't have both DeleteState::SeekAfterBalancing and + // save_context()/restore/context(), they are practically the same thing. + self.save_context(CursorContext::IndexKeyRowId((*record).clone())); + } continue; } } @@ -4023,7 +4043,10 @@ impl BTreeCursor { _rowid, _payload, .. }) = cell else { - unreachable!("unexpected page_type"); + unreachable!( + "BTreeCursor::rowid(): unexpected page_type: {:?}", + page_type + ); }; Ok(CursorResult::Ok(Some(_rowid))) } else { From bdfbb8fe541f752becbd4559178989eb43684c7e Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 24 Jun 2025 11:26:00 +0300 Subject: [PATCH 2/3] Fix erroneous early return --- core/storage/btree.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 057f89412..fc654ea0c 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -2136,7 +2136,6 @@ impl BTreeCursor { .expect("expected write info"); if page.get().get_contents().overflow_cells.is_empty() { write_info.state = WriteState::Finish; - return Ok(CursorResult::Ok(())); } else { write_info.state = WriteState::BalanceStart; // If we balance, we must save the cursor position and seek to it later. @@ -2168,7 +2167,6 @@ impl BTreeCursor { .expect("expected write info"); if page.get().get_contents().overflow_cells.is_empty() { write_info.state = WriteState::Finish; - return Ok(CursorResult::Ok(())); } else { write_info.state = WriteState::BalanceStart; // If we balance, we must save the cursor position and seek to it later. From 27b2c2530b4f1019208c9c2bf773d4b72b7358a1 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 24 Jun 2025 13:13:31 +0300 Subject: [PATCH 3/3] test/stress: only do integrity_check every 100 queries --- stress/main.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/stress/main.rs b/stress/main.rs index cb55c20d0..9721c1a0f 100644 --- a/stress/main.rs +++ b/stress/main.rs @@ -485,14 +485,17 @@ async fn main() -> Result<(), Box> { _ => panic!("Error executing query: {}", e), } } - let mut res = conn.query("PRAGMA integrity_check", ()).await.unwrap(); - if let Some(row) = res.next().await? { - let value = row.get_value(0).unwrap(); - if value != "ok".into() { - panic!("integrity check failed: {:?}", value); + const INTEGRITY_CHECK_INTERVAL: usize = 100; + if query_index % INTEGRITY_CHECK_INTERVAL == 0 { + let mut res = conn.query("PRAGMA integrity_check", ()).await.unwrap(); + if let Some(row) = res.next().await? { + let value = row.get_value(0).unwrap(); + if value != "ok".into() { + panic!("integrity check failed: {:?}", value); + } + } else { + panic!("integrity check failed: no rows"); } - } else { - panic!("integrity check failed: no rows"); } } Ok::<_, Box>(())