mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
sqlite: no more destructive version bumps (#21847)
## Why We'd like SQLite state to become required and load-bearing. As a first step, let's remove the mechanism that allows us to blow away the SQLite DB on a version bump, and instead rely on graceful migrations. The original motivation ([PR](https://github.com/openai/codex/pull/10623)) behind this mechanism was to care less about backwards compatibility while SQLite was being landed, but I'd say it's quite important now to keep the data in it. ## What changed - Make `STATE_DB_FILENAME` and `LOGS_DB_FILENAME` the full canonical filenames: `state_5.sqlite` and `logs_2.sqlite`. - Remove `STATE_DB_VERSION` / `LOGS_DB_VERSION` and the helper that constructed filenames from versions. - Stop `StateRuntime::init` from scanning for or deleting older SQLite DB filenames at startup. - Delete the tests that encoded legacy state/logs DB deletion behavior. ## Verification - `cargo test -p codex-state`
This commit is contained in:
committed by
GitHub
Unverified
parent
bd42660cb4
commit
95ca276373
@@ -122,88 +122,10 @@ ON CONFLICT(id) DO NOTHING
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::StateRuntime;
|
||||
use super::state_db_filename;
|
||||
use super::test_support::unique_temp_dir;
|
||||
use crate::STATE_DB_FILENAME;
|
||||
use crate::STATE_DB_VERSION;
|
||||
use chrono::Utc;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn init_removes_legacy_state_db_files() {
|
||||
let codex_home = unique_temp_dir();
|
||||
tokio::fs::create_dir_all(&codex_home)
|
||||
.await
|
||||
.expect("create codex_home");
|
||||
|
||||
let current_name = state_db_filename();
|
||||
let previous_version = STATE_DB_VERSION.saturating_sub(1);
|
||||
let unversioned_name = format!("{STATE_DB_FILENAME}.sqlite");
|
||||
for suffix in ["", "-wal", "-shm", "-journal"] {
|
||||
let path = codex_home.join(format!("{unversioned_name}{suffix}"));
|
||||
tokio::fs::write(path, b"legacy")
|
||||
.await
|
||||
.expect("write legacy");
|
||||
let old_version_path = codex_home.join(format!(
|
||||
"{STATE_DB_FILENAME}_{previous_version}.sqlite{suffix}"
|
||||
));
|
||||
tokio::fs::write(old_version_path, b"old_version")
|
||||
.await
|
||||
.expect("write old version");
|
||||
}
|
||||
let unrelated_path = codex_home.join("state.sqlite_backup");
|
||||
tokio::fs::write(&unrelated_path, b"keep")
|
||||
.await
|
||||
.expect("write unrelated");
|
||||
let numeric_path = codex_home.join("123");
|
||||
tokio::fs::write(&numeric_path, b"keep")
|
||||
.await
|
||||
.expect("write numeric");
|
||||
|
||||
let _runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string())
|
||||
.await
|
||||
.expect("initialize runtime");
|
||||
|
||||
for suffix in ["", "-wal", "-shm", "-journal"] {
|
||||
let legacy_path = codex_home.join(format!("{unversioned_name}{suffix}"));
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(&legacy_path)
|
||||
.await
|
||||
.expect("check legacy path"),
|
||||
false
|
||||
);
|
||||
let old_version_path = codex_home.join(format!(
|
||||
"{STATE_DB_FILENAME}_{previous_version}.sqlite{suffix}"
|
||||
));
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(&old_version_path)
|
||||
.await
|
||||
.expect("check old version path"),
|
||||
false
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(codex_home.join(current_name))
|
||||
.await
|
||||
.expect("check new db path"),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(&unrelated_path)
|
||||
.await
|
||||
.expect("check unrelated path"),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
tokio::fs::try_exists(&numeric_path)
|
||||
.await
|
||||
.expect("check numeric path"),
|
||||
true
|
||||
);
|
||||
|
||||
let _ = tokio::fs::remove_dir_all(codex_home).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn backfill_state_persists_progress_and_completion() {
|
||||
let codex_home = unique_temp_dir();
|
||||
|
||||
@@ -698,67 +698,6 @@ mod tests {
|
||||
let _ = tokio::fs::remove_dir_all(codex_home).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn init_recreates_legacy_logs_db_when_log_version_changes() {
|
||||
let codex_home = unique_temp_dir();
|
||||
tokio::fs::create_dir_all(&codex_home)
|
||||
.await
|
||||
.expect("create codex home");
|
||||
let legacy_logs_path = codex_home.join("logs_1.sqlite");
|
||||
let pool = SqlitePool::connect_with(
|
||||
SqliteConnectOptions::new()
|
||||
.filename(&legacy_logs_path)
|
||||
.create_if_missing(true),
|
||||
)
|
||||
.await
|
||||
.expect("open legacy logs db");
|
||||
LOGS_MIGRATOR
|
||||
.run(&pool)
|
||||
.await
|
||||
.expect("apply legacy logs schema");
|
||||
sqlx::query(
|
||||
"INSERT INTO logs (ts, ts_nanos, level, target, feedback_log_body, module_path, file, line, thread_id, process_uuid, estimated_bytes) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
|
||||
)
|
||||
.bind(1_i64)
|
||||
.bind(0_i64)
|
||||
.bind("INFO")
|
||||
.bind("cli")
|
||||
.bind("legacy-log-row")
|
||||
.bind("mod")
|
||||
.bind("main.rs")
|
||||
.bind(7_i64)
|
||||
.bind("thread-1")
|
||||
.bind("proc-1")
|
||||
.bind(16_i64)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("insert legacy log row");
|
||||
pool.close().await;
|
||||
drop(pool);
|
||||
|
||||
let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string())
|
||||
.await
|
||||
.expect("initialize runtime");
|
||||
|
||||
assert!(
|
||||
!legacy_logs_path.exists(),
|
||||
"legacy logs db should be removed when the version changes"
|
||||
);
|
||||
assert!(
|
||||
logs_db_path(codex_home.as_path()).exists(),
|
||||
"current logs db should be recreated during init"
|
||||
);
|
||||
assert!(
|
||||
runtime
|
||||
.query_logs(&LogQuery::default())
|
||||
.await
|
||||
.expect("query recreated logs db")
|
||||
.is_empty()
|
||||
);
|
||||
|
||||
let _ = tokio::fs::remove_dir_all(codex_home).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn init_configures_logs_db_with_incremental_auto_vacuum() {
|
||||
let codex_home = unique_temp_dir();
|
||||
|
||||
Reference in New Issue
Block a user