From 064b794c047a2993944f0408c2ae14d3413f616d Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 26 Aug 2025 11:07:41 +0300 Subject: [PATCH 1/3] libsql: Make sync_db_if_needed() work in offline mode In preparation for erroring database open if boostrap fails, make sure sync_db_if_needed() is robust in offline mode and does not probe the server if we already have a database and metadata files. --- libsql/src/sync.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/libsql/src/sync.rs b/libsql/src/sync.rs index c13fe7055b..b52d9473ce 100644 --- a/libsql/src/sync.rs +++ b/libsql/src/sync.rs @@ -621,7 +621,14 @@ impl SyncContext { Ok(info) } - async fn sync_db_if_needed(&mut self, generation: u32) -> Result<()> { + async fn sync_db_if_needed(&mut self) -> Result<()> { + let db_file_exists = check_if_file_exists(&self.db_path)?; + let metadata_exists = check_if_file_exists(&format!("{}-info", self.db_path))?; + if db_file_exists && metadata_exists { + return Ok(()); + } + let info = self.get_remote_info().await?; + let generation = info.current_generation; // somehow we are ahead of the remote in generations. following should not happen because // we checkpoint only if the remote server tells us to do so. if self.durable_generation > generation { @@ -641,8 +648,6 @@ impl SyncContext { // then local db is in an incorrect state. we stop and return with an error // 3. if the db file exists and the metadata file exists, then we don't need to do the // sync - let metadata_exists = check_if_file_exists(&format!("{}-info", self.db_path))?; - let db_file_exists = check_if_file_exists(&self.db_path)?; match (metadata_exists, db_file_exists) { (false, false) => { // neither the db file nor the metadata file exists, lets bootstrap from remote @@ -675,8 +680,8 @@ impl SyncContext { .into()) } (true, true) => { - // both files exists, no need to sync - Ok(()) + // We already handled this case earlier in the function. + unreachable!(); } } } @@ -820,11 +825,7 @@ pub async fn bootstrap_db(sync_ctx: &mut SyncContext) -> Result<()> { // we need to do this when we notice a large gap in generations, when bootstrapping is cheaper // than pulling each frame if !sync_ctx.initial_server_sync { - // sync is being called first time. so we will call remote, get the generation information - // if we are lagging behind, then we will call the export API and get to the latest - // generation directly. - let info = sync_ctx.get_remote_info().await?; - sync_ctx.sync_db_if_needed(info.current_generation).await?; + sync_ctx.sync_db_if_needed().await?; // when sync_ctx is initialised, we set durable_generation to 0. however, once // sync_db is called, it should be > 0. assert!(sync_ctx.durable_generation > 0, "generation should be > 0"); From 70c9bbc5b4bcd0fbb9882cb9a98d4e0eb6730fca Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 26 Aug 2025 11:02:55 +0300 Subject: [PATCH 2/3] libsql: Return error if boostrap fails on database open If boostrap fails in database open, we need to return an error; otherwise we'll have the local database in inconsistent state. --- libsql/src/database.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libsql/src/database.rs b/libsql/src/database.rs index 322913eefc..5bacda9bf2 100644 --- a/libsql/src/database.rs +++ b/libsql/src/database.rs @@ -691,17 +691,16 @@ impl Database { }; use tokio::sync::Mutex; - let _ = tokio::task::block_in_place(move || { + tokio::task::block_in_place(move || { let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() .unwrap(); rt.block_on(async { - // we will ignore if any errors occurred during the bootstrapping the db, - // because the client could be offline when trying to connect. - let _ = db.bootstrap_db().await; + db.bootstrap_db().await?; + Ok::<(), crate::Error>(()) }) - }); + })?; let local = db.connect()?; From 6c86667ff1378d80853a07cf7a3cf525cdc3d4ef Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 26 Aug 2025 11:09:30 +0300 Subject: [PATCH 3/3] libsql: Fail sync_db_if_needed() if database exists but metadata is missing This is an inconsistent state because there is no way for us to know what generation the database file is tracking. For all we know, it could be a totally unrelated database file than the one we have remotely. Therefore, return an error. --- libsql/src/sync.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/libsql/src/sync.rs b/libsql/src/sync.rs index b52d9473ce..5a0536eb9d 100644 --- a/libsql/src/sync.rs +++ b/libsql/src/sync.rs @@ -658,16 +658,14 @@ impl SyncContext { self.sync_db(generation).await } (false, true) => { - // kinda inconsistent state: DB exists but metadata missing - // however, this generally not an issue. For a fresh db, a user might do writes - // locally and then try to do sync later. So in this case, we will not - // bootstrap the db file and let the user proceed. If it is not a fresh db, the - // push will fail anyways later. - // if metadata file does not exist, then generation should be zero - assert_eq!(self.durable_generation, 0); - // lets initialise it to first generation - self.durable_generation = 1; - Ok(()) + // inconsistent state: DB exists but metadata missing + tracing::error!( + "local state is incorrect, db file exists but metadata file does not" + ); + Err(SyncError::InvalidLocalState( + "db file exists but metadata file does not".to_string(), + ) + .into()) } (true, false) => { // inconsistent state: Metadata exists but DB missing