sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

SqliteConnectOptions::new() doesn't set in_memory = true

Open hoxxep opened this issue 1 year ago • 1 comments

Bug Description

SqliteConnectOptions::filename() doesn't set in_memory = true, which caused me all sorts of connection pool deadlocking issues in a larger project. Pull request being filed shortly.

Minimal Reproduction

Due to issue https://github.com/launchbadge/sqlx/issues/2510 where in-memory SQLite databases appear to get wiped after being left idle for an extended period, I was doing a long-winded construction of the SqlitePool to use max_connections(1) as suggested.

let sqlite_opts = SqliteConnectOptions::new()
    .filename(":memory:")  // BUG: this does not set `in_memory = true`
    .create_if_missing(false);

// max_connections = 1 to prevent the DB from being wiped randomly
// issue: https://github.com/launchbadge/sqlx/issues/2510
let pool = SqlitePoolOptions::new()
    .max_connections(1)
    .connect_with(sqlite_opts)
    .await?;

Switching to this instantiation started giving me all sorts of weird deadlocking issues with a larger project, which changed the point of failure on a specific test depending on which order a single-threaded tokio::test runtime executed. I sadly have not been able to reproduce the deadlocking and pool timeouts on a smaller example or diagnose where the deadlock occurs, but I can reliably fix them by using SqliteConnectOptions::from_str(":memory:") instead.

Info

  • SQLx version: 0.7.4
  • SQLx features enabled: ["macros", "sqlite", "uuid", "runtime-tokio", "chrono"]
  • Database server and version: SQLite (in-memory)
  • Operating system: MacOS Sonoma 14.4
  • rustc --version: rustc 1.76.0 (07dca489a 2024-02-04)

hoxxep avatar Mar 20 '24 23:03 hoxxep

On second glance my pool timeout issue with .max_connections(1) still exists, but I still believe the PR could be worth merging for reasons outlined below.

The parsing code applies this logic for :memory: SQLite databases, which does not happen with a SqliteConnectOptions::new().filename(":memory:") instantiation and leads to issues where max_connections > 1.

if database == ":memory:" {
    options.in_memory = true;
    options.shared_cache = true;
    let seqno = IN_MEMORY_DB_SEQ.fetch_add(1, Ordering::Relaxed);
    options.filename = Cow::Owned(PathBuf::from(format!("file:sqlx-in-memory-{seqno}")));
} // ...

I can update the PR to include this logic in full, to make the instantiations equivalent.

hoxxep avatar Mar 20 '24 23:03 hoxxep