fixed backend live capture storage lag#27
Conversation
tigercraft4
left a comment
tigercraft4
left a comment
There was a problem hiding this comment.
The Swift changes are clean and ready to ship. The two Rust files have issues that should be addressed before merge — four inline comments below in priority order.
| } | ||
|
|
||
| fn configure_read_write_connection(conn: &Connection) -> GooseResult<()> { | ||
| conn.execute_batch( |
There was a problem hiding this comment.
WAL + iOS file protection — highest-severity issue.
journal_mode=WAL is a persistent DB property: once set, the database stays WAL across all future opens, including open_read_only connections. Under WAL, read-only opens still require write access to the -wal and -shm sidecar files.
This app runs with bluetooth-central + location background modes and an overnight guard that writes to SQLite while the device is locked. If the DB sits under default NSFileProtectionComplete, locked-device sidecar writes fail with SQLITE_IOERR or SQLITE_CANTOPEN — turning a performance fix into a write failure in production.
Fix: force NSFileProtectionCompleteUntilFirstUserAuthentication on the DB file (and ideally the sidecars after first WAL open), or confirm in the PR that the file-protection class is already set to a WAL-compatible level.
| } | ||
|
|
||
| pub fn open_existing_current(path: &Path) -> GooseResult<Self> { | ||
| let conn = Connection::open_with_flags(path, OpenFlags::SQLITE_OPEN_READ_WRITE)?; |
There was a problem hiding this comment.
open_existing_current schema-completeness gap.
This method gates on user_version == CURRENT_SCHEMA_VERSION, but ensure_*_columns() ALTERs run in migrate() after user_version is set. A DB stamped version 14 by an older binary that predates a later ensure_*_column addition passes this gate, then hits a runtime SQL error on the first column-dependent query — a failure the open-time fallback in open_bridge_store_hot cannot catch (the fallback only triggers on open errors, not later query errors).
Either run ensure_*_columns inside open_existing_current, or document that user_version is a heuristic and that the hot path assumes a prior full open() has already run.
| } | ||
|
|
||
| fn open_bridge_store_hot(database_path: &str) -> GooseResult<GooseStore> { | ||
| if database_path.trim().is_empty() { |
There was a problem hiding this comment.
No checkpoint policy — potential storage regression inside a storage-lag fix.
With WAL enabled across all 92+ open_bridge_store call sites (each opening a fresh Connection), WAL auto-checkpoint (default 1000 pages) fires on whichever connection crosses the threshold. Without an explicit checkpoint strategy the -wal file can grow unbounded during long capture or overnight sessions.
Consider adding to configure_read_write_connection:
PRAGMA wal_autocheckpoint = 100;or scheduling periodic PRAGMA wal_checkpoint(TRUNCATE) from the capture queue.
| } | ||
| let path = Path::new(database_path); | ||
| GooseStore::open_existing_current(path).or_else(|_| GooseStore::open(path)) | ||
| } |
There was a problem hiding this comment.
Silent fallback swallows the real error.
.or_else(|_| GooseStore::open(path)) discards why open_existing_current failed. If it fails for I/O, lock contention, or corruption (not just a stale schema), you run a full migration against a possibly damaged DB with no log. Recommend logging the discarded error:
.or_else(|err| {
log::warn!("open_existing_current failed ({err}), falling back to full open");
GooseStore::open(path)
})Also: the startup ordering assumption is currently implicit — the hot path works correctly only after a full open() (with migrate()) has run at least once. A comment here or in GooseAppModel documenting that dependency would prevent regressions if startup order ever changes.
There was a problem hiding this comment.
Thanks for catching the error, I believe it is ready now
2 participants
improves local sqlite performance for live capture by enabling wal/busy timeout pragmas, avoiding repeated migration work on hot capture bridge calls, adding capture-path indexes, and centralizing capture queue timing.