Skip to content

test(db): migrate OpenMemory's schema once per process and copy it - #2983

Merged
vavallee merged 2 commits into
vavallee:mainfrom
francisrath:test/openmemory-template
Oct 6, 2026
Merged

vavallee merged 2 commits into
vavallee:mainfrom
francisrath:test/openmemory-template

Conversation

@francisrath

@francisrath francisrath commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

db.OpenMemory() ran all 95 migrations for every test. Under -race, modernc's transpiled SQLite made that ~2.5s of setup per test, which is why the race shards were at 840–847s of a 900s budget on CI and timed out outright on Apple Silicon. OpenMemory now migrates a template once per process, serializes it, and deserializes a copy into each new single-connection :memory: database. Production Open is unchanged.

Closes #2982

Implementation notes

  • memoryTemplate() runs inside a sync.Once: open, migrate, Serialize(). Any error is cached and returned to every caller, the same as a migration failure today.
  • openMemoryConn() is the old open path (DSN pragmas, SetMaxOpenConns(1), setPragmas) pulled out so the template and the copies are opened the same way.
  • Serialize/Deserialize are reached via sql.Conn.Raw through a two-method interface. No new dependency, and go.mod is untouched.
  • Connection-level pragmas (foreign_keys, busy_timeout, …) belong to the connection, not the database image, so they survive the deserialize. The new test checks foreign_keys.

Scope decisions

  • Isolation is the same as before: each call returns its own database, pinned to one connection, with exactly the state migrate produces (schema, seed rows, schema_migrations).
  • Migration tests didn't need a separate path. They delete a schema_migrations row and call migrate() on the OpenMemory database, which still runs the real code. No test swaps migrationsFS or any migration input.
  • No changelog fragment: nothing user-visible changed.

How it was verified

The change is about speed, so the evidence is timings rather than a test that fails on main. All runs use GOTOOLCHAIN=go1.26.8 on an Apple Silicon Mac:

Run main This branch
go test -race -run '^TestDownloadClient' ./internal/api 157s 6.6s
make test-race, internal/api ^Test[A-B] >15m (timeout) 61s
make test-race, internal/api ^Test[C-K] >15m 57s
make test-race, internal/api ^Test[L-Q] >15m 48s
make test-race, internal/api rest >15m 30s
make test-race, internal/db >15m 78s

On CI, the validate (Go race) shards of this PR's run compared with a recent successful CI run on another PR (37187253010):

Shard Recent PR run This PR
api-a-b 934s 116s
api-c-k 857s 135s
api-l-q 1305s 103s
api-rest 660s 105s
db 1403s 95s
non-api 1757s 169s

The non-api shard drops too, because other packages also call OpenMemory. codecov/patch reports 54%: the uncovered lines are the error branches (template migration, serialize/deserialize, and a driver conn without Serialize), which would need a fake driver to reach.

TestOpenMemory_CopiesAreIndependent checks the copy semantics: a row written to one OpenMemory database doesn't appear in the next, foreign_keys is still on, and every migration version is recorded.

Checklist

  • Every commit carries a Signed-off-by that matches its author (git commit -s)
  • Changelog fragment: N/A, test-only change
  • Tests added or updated
  • docs/DEPLOYMENT.md: N/A
  • Wiki pages: N/A
  • No new dependency

Test plan

  • make test
  • make test-race (all six shards green, no races reported)
  • go vet ./internal/db

🤖 Generated with Claude Code

Every OpenMemory call ran all 95 migrations through modernc's
transpiled SQLite, which the race detector instruments heavily: a
trivial internal/api test took 2.85s under -race, ~69% of it in
migrate. The race shards ran at 840-847s against a 900s budget on CI
and timed out outright on Apple Silicon.

The first call now migrates a template database and serializes it;
each call after deserializes a copy onto a fresh single-connection
:memory: database. Per-test isolation is unchanged: every caller still
gets its own database, pinned to one connection, holding exactly the
state migrate produces. Production Open is untouched, and tests that
re-run migrate on an OpenMemory database still exercise the real path.

go test -race -run '^TestDownloadClient' ./internal/api: 157s -> 6.6s.
make test-race shards: >15m timeout -> 30-78s each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Francis Rath <fr@ncis.no>
@github-actions github-actions Bot added the bindery-notified Discord notification already sent for this PR label Oct 4, 2026
@codecov

codecov Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.37838% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/db/db.go 78.37% 4 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@francisrath
francisrath marked this pull request as ready for review October 4, 2026 13:22
A failed template build and a template that cannot be loaded both fail
the call, a closed database fails withRawConn, and a driver connection
that cannot serialize is an error rather than a panic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Francis Rath <fr@ncis.no>

@vavallee vavallee left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Francis, nice find. The race shards timing out had been a pain for a while and this is a small, tidy fix.

I merged current main into it (migrations 099 and 100 included) and checked a few things. A freshly migrated database and an OpenMemory copy have identical schema and rows, 32 parallel copies under race stay isolated, and 20 opens under race went from 259s to about 16s on my loaded box. Full suite passes.

One small nit, not blocking: synchronous and cache_size are per schema, so they reset on deserialize (they come back as 2 and -2000). Harmless for in memory tests, but the doc comment says the pragmas carry over. Happy to take a follow up that reruns those two after Deserialize or softens the comment.

Merging this one now, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindery-notified Discord notification already sent for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Race-detector test shards are bound by per-test migrations in db.OpenMemory

2 participants