mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: dedicated goal DB (#23300)
## Why Thread goals are moving toward extension-owned runtime behavior, but their persisted state was still stored in the shared state database. This makes the goal store harder to isolate and keeps future storage splits tied to ad hoc runtime plumbing. This PR gives goals their own SQLite database while keeping the existing `StateRuntime` entry point. The goal is to make this the pattern for adding more dedicated runtime databases later. This also reduce load on existing DB and reduce contention ## Limitation Thread preview from goal is not supported anymore. I'm looking into this [EDIT]: solved ## What changed - Added a dedicated `goals_1.sqlite` database with its own `goals_migrations` directory. - Moved `thread_goals` creation into the goals DB migration set. - Dropped the old `thread_goals` table from the main state DB with a normal state migration. There is intentionally no backfill for existing goal rows. - Changed `GoalStore` to be backed only by the goals DB pool. - Removed the old goal-write side effect that filled empty `threads.preview` values from the goal objective. - Added shared runtime DB path metadata so startup, telemetry, `codex doctor`, and repair handling can include future DBs without bespoke path lists. - Updated Bazel compile data so the new goals migration directory is available to `sqlx::migrate!`. ## Verification - `cargo check --tests -p codex-state -p codex-cli -p codex-core -p codex-app-server` - `just fix -p codex-state` - `just fix -p codex-cli` - `just fix -p codex-app-server`
This commit is contained in:
committed by
GitHub
Unverified
parent
826b2182ed
commit
ba57aab13a
@@ -1943,13 +1943,11 @@ async fn state_check(config: &Config) -> DoctorCheck {
|
||||
path_readiness(&mut details, "CODEX_HOME", &config.codex_home);
|
||||
path_readiness(&mut details, "log dir", &config.log_dir);
|
||||
path_readiness(&mut details, "sqlite home", &config.sqlite_home);
|
||||
let state_db = codex_state::state_db_path(&config.sqlite_home);
|
||||
let log_db = codex_state::logs_db_path(&config.sqlite_home);
|
||||
path_readiness(&mut details, "state DB", &state_db);
|
||||
path_readiness(&mut details, "log DB", &log_db);
|
||||
let mut integrity_failures = Vec::new();
|
||||
sqlite_integrity_detail(&mut details, &mut integrity_failures, "state DB", &state_db).await;
|
||||
sqlite_integrity_detail(&mut details, &mut integrity_failures, "log DB", &log_db).await;
|
||||
for db in codex_state::runtime_db_paths(&config.sqlite_home) {
|
||||
path_readiness(&mut details, db.label, &db.path);
|
||||
sqlite_integrity_detail(&mut details, &mut integrity_failures, db.label, &db.path).await;
|
||||
}
|
||||
rollout_stats_details(&mut details, &config.codex_home);
|
||||
standalone_release_cache_details(&mut details);
|
||||
|
||||
|
||||
@@ -669,10 +669,14 @@ fn terminal_summary(check: &DoctorCheck) -> String {
|
||||
}
|
||||
|
||||
fn state_summary(check: &DoctorCheck) -> String {
|
||||
let state_ok =
|
||||
detail::detail_value(check, "state DB integrity").is_some_and(|value| value == "ok");
|
||||
let log_ok = detail::detail_value(check, "log DB integrity").is_some_and(|value| value == "ok");
|
||||
if state_ok && log_ok {
|
||||
let databases_ok = [
|
||||
"state DB integrity",
|
||||
"log DB integrity",
|
||||
"goals DB integrity",
|
||||
]
|
||||
.into_iter()
|
||||
.all(|label| detail::detail_value(check, label).is_some_and(|value| value == "ok"));
|
||||
if check.status == CheckStatus::Ok && databases_ok {
|
||||
"databases healthy".to_string()
|
||||
} else {
|
||||
check.summary.clone()
|
||||
|
||||
@@ -294,6 +294,7 @@ fn state_details(parsed: &[ParsedDetail]) -> Vec<HumanDetail> {
|
||||
push_row_if_present(&mut out, parsed, "sqlite home", "sqlite home");
|
||||
push_database_row(&mut out, parsed, "state DB");
|
||||
push_database_row(&mut out, parsed, "log DB");
|
||||
push_database_row(&mut out, parsed, "goals DB");
|
||||
|
||||
for (source, label) in [
|
||||
("active rollout files", "active rollouts"),
|
||||
@@ -317,8 +318,10 @@ fn state_details(parsed: &[ParsedDetail]) -> Vec<HumanDetail> {
|
||||
"sqlite home",
|
||||
"state DB",
|
||||
"log DB",
|
||||
"goals DB",
|
||||
"state DB integrity",
|
||||
"log DB integrity",
|
||||
"goals DB integrity",
|
||||
"active rollout files",
|
||||
"archived rollout files",
|
||||
],
|
||||
|
||||
@@ -48,10 +48,9 @@ pub(crate) async fn repair_files(
|
||||
Err(err) => return Err(err),
|
||||
}
|
||||
|
||||
let logs_db_path = codex_state::logs_db_path(sqlite_home);
|
||||
for path in sqlite_paths(state_db_path)
|
||||
for path in codex_state::runtime_db_paths(sqlite_home)
|
||||
.into_iter()
|
||||
.chain(sqlite_paths(logs_db_path.as_path()))
|
||||
.flat_map(|db| sqlite_paths(db.path.as_path()))
|
||||
{
|
||||
if tokio::fs::try_exists(path.as_path()).await? {
|
||||
backups.push(backup_path(path.as_path(), &repair_suffix).await?);
|
||||
@@ -137,19 +136,22 @@ mod tests {
|
||||
let temp_dir = TempDir::new()?;
|
||||
let state_path = codex_state::state_db_path(temp_dir.path());
|
||||
let logs_path = codex_state::logs_db_path(temp_dir.path());
|
||||
let goals_path = codex_state::goals_db_path(temp_dir.path());
|
||||
let state_sidecars = sqlite_paths(state_path.as_path());
|
||||
tokio::fs::write(state_path.as_path(), b"state").await?;
|
||||
tokio::fs::write(state_sidecars[1].as_path(), b"state-wal").await?;
|
||||
tokio::fs::write(logs_path.as_path(), b"logs").await?;
|
||||
tokio::fs::write(goals_path.as_path(), b"goals").await?;
|
||||
|
||||
let startup_error =
|
||||
LocalStateDbStartupError::new(state_path.clone(), "corrupt".to_string());
|
||||
let backups = repair_files(&startup_error).await?;
|
||||
|
||||
assert_eq!(backups.len(), 3);
|
||||
assert_eq!(backups.len(), 4);
|
||||
assert!(!tokio::fs::try_exists(state_path.as_path()).await?);
|
||||
assert!(!tokio::fs::try_exists(state_sidecars[1].as_path()).await?);
|
||||
assert!(!tokio::fs::try_exists(logs_path.as_path()).await?);
|
||||
assert!(!tokio::fs::try_exists(goals_path.as_path()).await?);
|
||||
for backup in backups {
|
||||
assert!(tokio::fs::try_exists(backup.as_path()).await?);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user