mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Detect SQLite corruption during startup and preserve recovery backups (#49701)
## Why A database can open successfully while containing corruption that startup previously missed. Detecting it before runtime initialization completes allows recoverable databases to be rebuilt while preserving the damaged files. ## What changed - Run `PRAGMA quick_check(1)` when opening writable databases, with a 100 ms scan budget. Share attempts across cloned `SqliteConfig` instances by file identity so repeated opens skip validation and replacement files are checked again. - Back up and rebuild runtime databases that support recovery. Report corruption without rebuilding thread history or other databases without an automatic recovery policy. Treat interrupted or locked checks as incomplete rather than confirmed corruption. - Collect startup backups across recovery retries and include their locations in the app server warning. Explain that saved conversations can restore the thread list and history, while some database-only metadata may be unavailable. - Record confirmed corruption in `codex.sqlite.corruption.count`. ## Testing Add tests for recovery policies, backup preservation, validation caching and concurrency, cancelled and interrupted checks, and locked databases. Add app server coverage for recovery warnings and saved conversation restoration, a TUI warning snapshot, and message board post preservation. GitOrigin-RevId: 43e26328888e0301def642d69bd324acee867566
This commit is contained in:
committed by
copyberry
parent
f2b2e5b2a9
commit
3620b2caf8
+133
-51
@@ -25,6 +25,12 @@ use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
mod validation;
|
||||
|
||||
use validation::SqliteQuickCheckManager;
|
||||
|
||||
const DEFAULT_BUSY_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
|
||||
const LOGS_DB_FILENAME: &str = "logs_2.sqlite";
|
||||
const GOALS_DB_FILENAME: &str = "goals_1.sqlite";
|
||||
const MEMORIES_DB_FILENAME: &str = "memories_1.sqlite";
|
||||
@@ -32,6 +38,12 @@ const QUEUE_DB_FILENAME: &str = "queue_1.sqlite";
|
||||
const STATE_DB_FILENAME: &str = "state_5.sqlite";
|
||||
const THREAD_HISTORY_DB_FILENAME: &str = "thread_history_1.sqlite";
|
||||
|
||||
#[derive(Clone, Copy, Eq, PartialEq)]
|
||||
enum RecoveryMode {
|
||||
BackupAndRebuild,
|
||||
Unavailable,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct RuntimeDbSpec {
|
||||
label: &'static str,
|
||||
@@ -42,6 +54,7 @@ struct RuntimeDbSpec {
|
||||
/// Opt in only after auditing writers for deferred read-to-write upgrades:
|
||||
/// an intervening reclamation commit can make those fail with SQLITE_BUSY_SNAPSHOT.
|
||||
background_reclamation: bool,
|
||||
recovery: RecoveryMode,
|
||||
}
|
||||
|
||||
impl RuntimeDbSpec {
|
||||
@@ -57,6 +70,7 @@ const STATE_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
open_phase: "open_state",
|
||||
migrate_phase: "migrate_state",
|
||||
background_reclamation: false,
|
||||
recovery: RecoveryMode::BackupAndRebuild,
|
||||
};
|
||||
|
||||
const LOGS_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
@@ -67,6 +81,7 @@ const LOGS_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
migrate_phase: "migrate_logs",
|
||||
// Log transactions write before reading, so they already hold the writer lock.
|
||||
background_reclamation: true,
|
||||
recovery: RecoveryMode::BackupAndRebuild,
|
||||
};
|
||||
|
||||
const GOALS_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
@@ -76,6 +91,7 @@ const GOALS_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
open_phase: "open_goals",
|
||||
migrate_phase: "migrate_goals",
|
||||
background_reclamation: false,
|
||||
recovery: RecoveryMode::BackupAndRebuild,
|
||||
};
|
||||
|
||||
const MEMORIES_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
@@ -85,6 +101,7 @@ const MEMORIES_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
open_phase: "open_memories",
|
||||
migrate_phase: "migrate_memories",
|
||||
background_reclamation: false,
|
||||
recovery: RecoveryMode::BackupAndRebuild,
|
||||
};
|
||||
|
||||
const MEMORIES_V2_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
@@ -101,6 +118,7 @@ const QUEUE_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
open_phase: "open_queue",
|
||||
migrate_phase: "migrate_queue",
|
||||
background_reclamation: false,
|
||||
recovery: RecoveryMode::BackupAndRebuild,
|
||||
};
|
||||
|
||||
const THREAD_HISTORY_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
@@ -110,6 +128,7 @@ const THREAD_HISTORY_DB: RuntimeDbSpec = RuntimeDbSpec {
|
||||
open_phase: "open_thread_history",
|
||||
migrate_phase: "migrate_thread_history",
|
||||
background_reclamation: false,
|
||||
recovery: RecoveryMode::Unavailable,
|
||||
};
|
||||
|
||||
const RUNTIME_DBS: [RuntimeDbSpec; 7] = [
|
||||
@@ -130,14 +149,28 @@ pub struct RuntimeDbPath {
|
||||
}
|
||||
|
||||
/// Resolved configuration shared by all Codex SQLite connections.
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
/// Clones share quick-check attempts; a newly constructed config checks independently.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct SqliteConfig {
|
||||
sqlite_home: AbsolutePathBuf,
|
||||
quick_check_manager: SqliteQuickCheckManager,
|
||||
}
|
||||
|
||||
// Validation history is operational state, not part of configuration equality.
|
||||
impl PartialEq for SqliteConfig {
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
self.sqlite_home == other.sqlite_home
|
||||
}
|
||||
}
|
||||
|
||||
impl Eq for SqliteConfig {}
|
||||
|
||||
impl SqliteConfig {
|
||||
pub fn from_sqlite_home(sqlite_home: AbsolutePathBuf) -> Self {
|
||||
Self { sqlite_home }
|
||||
Self {
|
||||
sqlite_home,
|
||||
quick_check_manager: SqliteQuickCheckManager::new(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn new_for_testing(sqlite_home: AbsolutePathBuf) -> Self {
|
||||
@@ -269,9 +302,8 @@ impl SqliteConfig {
|
||||
let path = spec.path(self.home());
|
||||
let started = Instant::now();
|
||||
let pool_result = self
|
||||
.open_read_write_pool(&path)
|
||||
.await
|
||||
.map_err(anyhow::Error::from);
|
||||
.open_read_write_pool_with_spec(path.as_path(), Some(spec), telemetry_override)
|
||||
.await;
|
||||
telemetry::record_init_result(
|
||||
telemetry_override,
|
||||
spec.kind,
|
||||
@@ -307,61 +339,111 @@ impl SqliteConfig {
|
||||
}
|
||||
|
||||
/// Open a writable Codex SQLite database, creating it if necessary.
|
||||
pub async fn open_read_write_pool(&self, path: &Path) -> Result<SqlitePool, Error> {
|
||||
pub async fn open_read_write_pool(&self, path: &Path) -> anyhow::Result<SqlitePool> {
|
||||
self.open_read_write_pool_with_spec(
|
||||
path, /*recover_spec*/ None, /*telemetry_override*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn open_read_write_pool_with_spec(
|
||||
&self,
|
||||
path: &Path,
|
||||
recover_spec: Option<RuntimeDbSpec>,
|
||||
telemetry_override: Option<&dyn DbTelemetry>,
|
||||
) -> anyhow::Result<SqlitePool> {
|
||||
let recovery = recover_spec.map_or(RecoveryMode::Unavailable, |spec| spec.recovery);
|
||||
let options = SqliteConnectOptions::new()
|
||||
.filename(path)
|
||||
.create_if_missing(true)
|
||||
.synchronous(SqliteSynchronous::Normal)
|
||||
.busy_timeout(Duration::from_secs(5))
|
||||
.busy_timeout(DEFAULT_BUSY_TIMEOUT)
|
||||
.log_statements(LevelFilter::Off);
|
||||
// SQLx retries after_connect errors, eventually replacing them with PoolTimedOut.
|
||||
// Return the first initialization error directly while opening this pool.
|
||||
let (init_error_tx, mut init_error_rx) = tokio::sync::mpsc::channel(/*buffer*/ 1);
|
||||
let pool_options = SqlitePoolOptions::new().max_connections(5).after_connect(
|
||||
move |connection, _metadata| {
|
||||
let init_error_tx = init_error_tx.clone();
|
||||
Box::pin(async move {
|
||||
let result = async {
|
||||
let mode: i64 = sqlx::query_scalar("PRAGMA auto_vacuum")
|
||||
.fetch_one(&mut *connection)
|
||||
.await?;
|
||||
// The setter takes the writer lock even when the mode is unchanged.
|
||||
// Initialize before WAL creates the first database page; preserve
|
||||
// existing modes, including FULL, without taking the writer lock.
|
||||
let empty = if mode == 0 {
|
||||
sqlx::query_scalar::<_, bool>(
|
||||
"SELECT NOT EXISTS (SELECT 1 FROM sqlite_schema)",
|
||||
)
|
||||
.fetch_one(&mut *connection)
|
||||
.await?
|
||||
} else {
|
||||
false
|
||||
};
|
||||
if empty {
|
||||
sqlx::query("PRAGMA auto_vacuum = INCREMENTAL")
|
||||
.execute(&mut *connection)
|
||||
let connect = || async {
|
||||
// SQLx retries after_connect errors, eventually replacing them with PoolTimedOut.
|
||||
// Return the first initialization error directly while opening this pool.
|
||||
let (init_error_tx, mut init_error_rx) = tokio::sync::mpsc::channel(/*buffer*/ 1);
|
||||
let pool_options = SqlitePoolOptions::new().max_connections(5).after_connect(
|
||||
move |connection, _metadata| {
|
||||
let init_error_tx = init_error_tx.clone();
|
||||
Box::pin(async move {
|
||||
let result = async {
|
||||
let mode: i64 = sqlx::query_scalar("PRAGMA auto_vacuum")
|
||||
.fetch_one(&mut *connection)
|
||||
.await?;
|
||||
// The setter takes the writer lock even when the mode is unchanged.
|
||||
// Initialize before WAL creates the first database page; preserve
|
||||
// existing modes, including FULL, without taking the writer lock.
|
||||
let empty = if mode == 0 {
|
||||
sqlx::query_scalar::<_, bool>(
|
||||
"SELECT NOT EXISTS (SELECT 1 FROM sqlite_schema)",
|
||||
)
|
||||
.fetch_one(&mut *connection)
|
||||
.await?
|
||||
} else {
|
||||
false
|
||||
};
|
||||
if empty {
|
||||
sqlx::query("PRAGMA auto_vacuum = INCREMENTAL")
|
||||
.execute(&mut *connection)
|
||||
.await?;
|
||||
}
|
||||
sqlx::query("PRAGMA journal_mode = WAL")
|
||||
.execute(connection)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
sqlx::query("PRAGMA journal_mode = WAL")
|
||||
.execute(connection)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
.await;
|
||||
result.map_err(|error| match init_error_tx.try_send(error) {
|
||||
// The opener owns the original error and cancels connection retries.
|
||||
Ok(()) => Error::PoolClosed,
|
||||
// After opening, lazy connections retain SQLx's normal error handling.
|
||||
Err(error) => error.into_inner(),
|
||||
.await;
|
||||
result.map_err(|error| match init_error_tx.try_send(error) {
|
||||
// The opener owns the original error and cancels connection retries.
|
||||
Ok(()) => Error::PoolClosed,
|
||||
// After opening, lazy connections retain SQLx's normal error handling.
|
||||
Err(error) => error.into_inner(),
|
||||
})
|
||||
})
|
||||
})
|
||||
},
|
||||
);
|
||||
tokio::select! {
|
||||
biased;
|
||||
Some(error) = init_error_rx.recv() => Err(error),
|
||||
result = pool_options.connect_with(options) => result,
|
||||
},
|
||||
);
|
||||
tokio::select! {
|
||||
biased;
|
||||
Some(error) = init_error_rx.recv() => Err(error),
|
||||
result = pool_options.connect_with(options.clone()) => result,
|
||||
}
|
||||
};
|
||||
let pool = connect().await?;
|
||||
let validation_result = self
|
||||
.quick_check_manager
|
||||
.quick_check_once(
|
||||
&pool,
|
||||
path,
|
||||
// Limit startup validation to 100 ms.
|
||||
Duration::from_millis(/*millis*/ 100),
|
||||
)
|
||||
.await;
|
||||
|
||||
let Ok(result) = validation_result else {
|
||||
pool.close().await;
|
||||
return validation_result.map(|_| pool);
|
||||
};
|
||||
|
||||
if result == validation::QuickCheckOutcome::CorruptedNeedsFixed {
|
||||
tracing::error!(database = %path.display(), "SQLite quick check detected corruption");
|
||||
telemetry::record_corruption(telemetry_override, recover_spec.map(|spec| spec.kind));
|
||||
if recovery == RecoveryMode::BackupAndRebuild {
|
||||
pool.close().await;
|
||||
let backups = crate::backup_runtime_db_for_fresh_start(path).await?;
|
||||
for backup in backups {
|
||||
tracing::warn!(
|
||||
database = %backup.original_path.display(),
|
||||
backup = %backup.backup_path.display(),
|
||||
"Preserved corrupt SQLite database before rebuilding"
|
||||
);
|
||||
}
|
||||
|
||||
return connect().await.map_err(anyhow::Error::from);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(pool)
|
||||
}
|
||||
|
||||
/// Open an existing Codex SQLite database without creating or modifying it.
|
||||
|
||||
Reference in New Issue
Block a user