mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
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
This commit is contained in:
1 parent
d3fc1950a9
commit
18110b810f
2 files changed
+67
No files matched your search
@@ -69,6 +69,27 @@ pub(crate) async fn repair_legacy_recency_migration_version(
|
|||||||
return Ok(());
|
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(
|
sqlx::query(
|
||||||
r#"
|
r#"
|
||||||
UPDATE _sqlx_migrations
|
UPDATE _sqlx_migrations
|
||||||
|
|||||||
@@ -1,9 +1,15 @@
|
|||||||
use std::borrow::Cow;
|
use std::borrow::Cow;
|
||||||
|
use std::time::Duration;
|
||||||
|
|
||||||
|
use sqlx::Connection;
|
||||||
use sqlx::Row;
|
use sqlx::Row;
|
||||||
|
use sqlx::SqliteConnection;
|
||||||
use sqlx::migrate::Migration;
|
use sqlx::migrate::Migration;
|
||||||
use sqlx::migrate::Migrator;
|
use sqlx::migrate::Migrator;
|
||||||
|
use sqlx::sqlite::SqliteConnectOptions;
|
||||||
|
use sqlx::sqlite::SqliteJournalMode;
|
||||||
use sqlx::sqlite::SqlitePoolOptions;
|
use sqlx::sqlite::SqlitePoolOptions;
|
||||||
|
use uuid::Uuid;
|
||||||
|
|
||||||
use super::STATE_MIGRATOR;
|
use super::STATE_MIGRATOR;
|
||||||
use super::repair_legacy_recency_migration_version;
|
use super::repair_legacy_recency_migration_version;
|
||||||
@@ -196,3 +202,43 @@ async fn repairs_recency_migration_that_was_applied_as_version_38() {
|
|||||||
.collect::<Vec<_>>();
|
.collect::<Vec<_>>();
|
||||||
assert_eq!(applied, expected);
|
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");
|
||||||
|
}
|
||||||
Reference in new issue
Block a user