From 18110b810f0a328147f6cd85e6f1ab6414927366 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 16 Jul 2026 20:49:16 +0000 Subject: [PATCH] Avoid unnecessary writes during migration repair (#33687) ## Why Opening a database with current migration history should not require SQLite's writer slot. The legacy recency repair previously issued an `UPDATE` even when there was nothing to repair, so it could fail while another connection held the writer slot. ## What changed Check `_sqlx_migrations` for the legacy version and checksum before issuing the repair update, and return early when the current recency migration is already recorded. ## Testing Add a WAL-mode regression test that holds the writer slot on a second connection and verifies that current migration history needs no write access. GitOrigin-RevId: 76649213912b510df42a836cf8ed7f84a7c70eb5 --- codex-rs/state/src/migrations.rs | 21 ++++++++++++ codex-rs/state/src/migrations_tests.rs | 46 ++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/codex-rs/state/src/migrations.rs b/codex-rs/state/src/migrations.rs index f5851378db..995ea4b642 100644 --- a/codex-rs/state/src/migrations.rs +++ b/codex-rs/state/src/migrations.rs @@ -69,6 +69,27 @@ pub(crate) async fn repair_legacy_recency_migration_version( return Ok(()); } + let legacy_recency_needs_repair = sqlx::query_scalar::<_, i64>( + r#" +SELECT 1 +FROM _sqlx_migrations +WHERE version = ? + AND checksum = ? + AND NOT EXISTS ( + SELECT 1 FROM _sqlx_migrations WHERE version = ? + ) + "#, + ) + .bind(38_i64) + .bind(recency_migration.checksum.as_ref()) + .bind(recency_migration.version) + .fetch_optional(pool) + .await? + .is_some(); + if !legacy_recency_needs_repair { + return Ok(()); + } + sqlx::query( r#" UPDATE _sqlx_migrations diff --git a/codex-rs/state/src/migrations_tests.rs b/codex-rs/state/src/migrations_tests.rs index d11cc13ea1..7cb4813605 100644 --- a/codex-rs/state/src/migrations_tests.rs +++ b/codex-rs/state/src/migrations_tests.rs @@ -1,9 +1,15 @@ use std::borrow::Cow; +use std::time::Duration; +use sqlx::Connection; use sqlx::Row; +use sqlx::SqliteConnection; use sqlx::migrate::Migration; use sqlx::migrate::Migrator; +use sqlx::sqlite::SqliteConnectOptions; +use sqlx::sqlite::SqliteJournalMode; use sqlx::sqlite::SqlitePoolOptions; +use uuid::Uuid; use super::STATE_MIGRATOR; use super::repair_legacy_recency_migration_version; @@ -196,3 +202,43 @@ async fn repairs_recency_migration_that_was_applied_as_version_38() { .collect::>(); assert_eq!(applied, expected); } + +#[tokio::test] +async fn repair_recency_migration_succeeds_while_another_connection_holds_writer_slot() { + let database_path = std::env::temp_dir().join(format!( + "codex-state-migrations-test-{}.sqlite", + Uuid::new_v4() + )); + let options = SqliteConnectOptions::new() + .filename(&database_path) + .create_if_missing(true) + .journal_mode(SqliteJournalMode::Wal) + .busy_timeout(Duration::from_millis(100)); + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect_with(options.clone()) + .await + .expect("database should open"); + STATE_MIGRATOR + .run(&pool) + .await + .expect("current migrations should apply"); + let mut write_connection = SqliteConnection::connect_with(&options) + .await + .expect("write connection should open"); + let write_transaction = write_connection + .begin_with("BEGIN IMMEDIATE") + .await + .expect("write transaction should acquire the writer slot"); + + let repair_result = repair_legacy_recency_migration_version(&pool, &STATE_MIGRATOR).await; + + write_transaction + .rollback() + .await + .expect("write transaction should roll back"); + drop(write_connection); + pool.close().await; + let _ = tokio::fs::remove_file(database_path).await; + repair_result.expect("current migration history should not need the writer slot"); +}