From 2ae3b3004edc686e0b938c31c84af939f0ad7503 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Fri, 4 Jul 2025 12:13:18 +0200 Subject: [PATCH 1/2] ignore wal frames after bad checksum SQLite basically ignores bad frames instead of panicking, let's try to do the same. --- core/storage/sqlite3_ondisk.rs | 164 ++++++++++-------- .../query_processing/test_write_path.rs | 33 ++-- 2 files changed, 107 insertions(+), 90 deletions(-) diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index f4b43df41..a22cc1e6e 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -1401,15 +1401,22 @@ pub fn read_entire_wal_dumb(file: &Arc) -> Result) -> Result 0; + if is_commit_record { + wfs_data.max_frame.store(frame_idx, Ordering::SeqCst); + wfs_data.last_checksum = cumulative_checksum; + } + + frame_idx += 1; + current_offset += WAL_FRAME_HEADER_SIZE + page_size; } - - cumulative_checksum = calculated_frame_checksum; - - wfs_data - .frame_cache - .lock() - .entry(frame_h_page_number as u64) - .or_default() - .push(frame_idx); - wfs_data - .pages_in_frames - .lock() - .push(frame_h_page_number as u64); - - let is_commit_record = frame_h_db_size > 0; - if is_commit_record { - wfs_data.max_frame.store(frame_idx, Ordering::SeqCst); - } - - frame_idx += 1; - current_offset += WAL_FRAME_HEADER_SIZE + page_size; } wfs_data.last_checksum = cumulative_checksum; + wfs_data.nbackfills.store(0, Ordering::SeqCst); wfs_data.loaded.store(true, Ordering::SeqCst); }); let c = Completion::new_read(buf_for_pread, complete); diff --git a/tests/integration/query_processing/test_write_path.rs b/tests/integration/query_processing/test_write_path.rs index d11a54ff1..4e63d3782 100644 --- a/tests/integration/query_processing/test_write_path.rs +++ b/tests/integration/query_processing/test_write_path.rs @@ -3,7 +3,7 @@ use crate::common::{compare_string, do_flush, TempDatabase}; use log::debug; use std::io::{Read, Seek, Write}; use std::sync::Arc; -use turso_core::{Connection, Database, Row, Statement, StepResult, Value}; +use turso_core::{Connection, Database, LimboError, Row, Statement, StepResult, Value}; const WAL_HEADER_SIZE: usize = 32; const WAL_FRAME_HEADER_SIZE: usize = 24; @@ -696,7 +696,7 @@ fn test_wal_bad_frame() -> anyhow::Result<()> { db_path }; { - let result = std::panic::catch_unwind(|| { + let result = { let io: Arc = Arc::new(turso_core::PlatformIO::new().unwrap()); let db = Database::open_file_with_flags( io.clone(), @@ -716,19 +716,21 @@ fn test_wal_bad_frame() -> anyhow::Result<()> { let x = row.get::(0).unwrap(); assert_eq!(x, 0); }) - }); + }; match result { - Err(panic_info) => { - let panic_msg = panic_info - .downcast_ref::() - .map(|s| s.as_str()) - .or_else(|| panic_info.downcast_ref::<&str>().copied()) - .unwrap_or("Unknown panic message"); + Err(error) => { + dbg!(&error); + let panic_msg = error.downcast_ref::().unwrap(); + let msg = match panic_msg { + LimboError::ParseError(message) => message, + _ => panic!("Unexpected panic message: {}", panic_msg), + }; assert!( - panic_msg.contains("WAL frame checksum mismatch."), - "Expected panic message not found. Got: {panic_msg}" + msg.contains("Table t2 not found"), + "Expected panic message not found. Got: {}", + msg ); } Ok(_) => panic!("Expected query to panic, but it succeeded"), @@ -784,8 +786,8 @@ pub fn run_query_core( query: &str, mut on_row: Option, ) -> anyhow::Result<()> { - match conn.query(query) { - Ok(Some(ref mut rows)) => loop { + match conn.query(query)? { + Some(ref mut rows) => loop { match rows.step()? { StepResult::IO => { rows.run_once()?; @@ -800,10 +802,7 @@ pub fn run_query_core( _ => unreachable!(), } }, - Ok(None) => {} - Err(err) => { - eprintln!("{err}"); - } + None => {} }; Ok(()) } From 8150a725505a9bfb6c4114af403e6f320afefc36 Mon Sep 17 00:00:00 2001 From: Pere Diaz Bou Date: Fri, 4 Jul 2025 12:17:58 +0200 Subject: [PATCH 2/2] check frame number is not 0 clippy fmt fix after rebase clippy --- core/storage/sqlite3_ondisk.rs | 19 +++++++++++++------ .../query_processing/test_write_path.rs | 14 ++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index a22cc1e6e..674b31183 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -1453,6 +1453,13 @@ pub fn read_entire_wal_dumb(file: &Arc) -> Result) -> Result anyhow::Result<()> { let panic_msg = error.downcast_ref::().unwrap(); let msg = match panic_msg { LimboError::ParseError(message) => message, - _ => panic!("Unexpected panic message: {}", panic_msg), + _ => panic!("Unexpected panic message: {panic_msg}"), }; assert!( - msg.contains("Table t2 not found"), - "Expected panic message not found. Got: {}", - msg + msg.contains("no such table: t2"), + "Expected panic message not found. Got: {msg}" ); } Ok(_) => panic!("Expected query to panic, but it succeeded"), @@ -786,8 +785,8 @@ pub fn run_query_core( query: &str, mut on_row: Option, ) -> anyhow::Result<()> { - match conn.query(query)? { - Some(ref mut rows) => loop { + if let Some(ref mut rows) = conn.query(query)? { + loop { match rows.step()? { StepResult::IO => { rows.run_once()?; @@ -801,8 +800,7 @@ pub fn run_query_core( } _ => unreachable!(), } - }, - None => {} + } }; Ok(()) }