Skip to content

feat(storage): add SereneDB backend - #792

Open
lakhansamani wants to merge 3 commits into
mainfrom
feat/serenedb
Open

feat(storage): add SereneDB backend#792
lakhansamani wants to merge 3 commits into
mainfrom
feat/serenedb

Conversation

@lakhansamani

@lakhansamani lakhansamani commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

SereneDB speaks the PostgreSQL wire protocol, so the SQL provider drives it unchanged except in two places. Rather than branch shared code, internal/storage/db/serenedb embeds sql.Provider and overrides only those two — no existing backend changes behaviour.

The two incompatibilities

Migration. SereneDB reports varchar(n) as text, so GORM AutoMigrate re-issues ALTER COLUMN ... TYPE on every boot and SereneDB refuses it on an indexed column:

ALTER TABLE authorizer_verification_requests ALTER COLUMN identifier TYPE varchar(64)
  → ERROR: cannot alter type ... index "idx_email_identifier" depends on it (XX000)

First boot worked; every restart failed. Replaced with a create-only migrator (CREATE TABLE / ADD COLUMN / CREATE INDEX) — the DDL SereneDB actually supports. sql.NewProviderWithMigrate is the new seam; sql.NewProvider delegates to it with the existing AutoMigrate behaviour unchanged.

Upserts with an explicit conflict target. SereneDB resolves ON CONFLICT (cols) only against an inline UNIQUE table constraint, never a CREATE UNIQUE INDEX — which is what GORM emits for uniqueIndex, and what ALTER TABLE ADD CONSTRAINT cannot retrofit once the index exists:

CREATE UNIQUE INDEX ux ON t (a,b);
INSERT ... ON CONFLICT (a,b) DO UPDATE ...   -- ERROR: not referenced by a UNIQUE/PK CONSTRAINT or INDEX
CREATE TABLE t (..., UNIQUE(a,b));           -- inline constraint: works
ALTER TABLE t ADD CONSTRAINT ux UNIQUE(a,b)  -- ERROR: Dependency Error

Targetless DO UPDATE fails too. AddAuthenticator and AddVerificationRequest now insert bare and resolve the conflict off the 23505 the index still raises, keeping the race guarantees on MFA enrollment and repeat verification requests.

Write concurrency (found during review)

The first cut retried 40001 only inside the two overridden upserts. Driving the real binary showed that was not enough — 20 concurrent update_profile calls for one user produced 17 failures, each leaking ERROR: Conflict on tuple deletion! (SQLSTATE 40001) straight to the API caller.

Fixed by moving the retry to a gorm.ConnPool wrapper, so every autocommit statement gets it. Two things that fix depends on:

  • SkipDefaultTransaction is required. GORM otherwise wraps each single Create/Update in a transaction, routing every write through BeginTx and past the retry — the first attempt at the pool wrapper changed nothing for exactly this reason. These schemas carry no GORM associations, so one write is one statement and that implicit wrapper bought nothing here.
  • Explicit Transaction() blocks are deliberately not retried: replaying one statement inside an already-aborted transaction is wrong, and GORM offers no hook to replay the block. The four cascade-delete call sites (user/client/organization/webhook) still surface 40001 under sustained same-row contention. TestDeleteUserCascadeStaysAtomic asserts the cascade is still atomic under the new config.

After the fix: 20/20 succeed with zero conflicts in the server log; at 50 concurrent the app's own rate limiter trips first and still nothing leaks.

Separately and pre-existing on every backend: the service layer returns raw storage errors (return nil, nil, err), so driver text reaches the GraphQL message. Worth its own issue, not touched here.

Not auto-mapped as an FGA store: SereneDB is a search-OLAP engine, not an OpenFGA datastore, so it needs an explicit --fga-store. Guarded by a test alongside the cockroachdb case.

Verification

Against serenedb/serenedb:26.08.2 (reports PostgreSQL 18.3), pinned in the Makefile since the compat surface is young and these overrides depend on its specifics:

  • Full module suite against SereneDB: 946 pass, 0 fail — on a fresh database and on a second run against the existing schema (the restart path).
  • The real binary boots on SereneDB and serves the full auth flow end to end: signup -> login -> profile -> userinfo. 20 concurrent signups (distinct rows): 20/20.
  • New tests: migration is repeatable, verification-request upsert replaces rather than errors, 8-way concurrent enrollment leaves exactly one row. Gated on TEST_DBS=serenedb, so make test stays SQLite-only.
  • make test (SQLite), make test-mongodb (949 pass), Postgres storage + db/sql suites: all unchanged.
  • make test-serenedb added; test-all-db / test-docker-up / test-cleanup include it.

Two notes for the reviewer:

  • Postgres was verified on a spare port (5455) because 5434 is held by an unrelated container on my machine. The make test-postgres recipe is unchanged.
  • make lint fails on main already — 3 govet reflect.Ptr and 1 gofmt in oauth_sso_verify_test.go, likely a newer local golangci-lint. Left alone; the packages here lint clean.

SereneDB speaks the PostgreSQL wire protocol, so the SQL provider drives
it unchanged except in two places. Rather than branch shared code, the
new provider embeds sql.Provider and overrides only those two:

- Migration. SereneDB reports varchar(n) as text, so GORM AutoMigrate
  re-issues ALTER COLUMN ... TYPE on every boot and SereneDB refuses it
  on an indexed column — the second startup failed. Replaced with a
  create-only migrator (CREATE TABLE / ADD COLUMN / CREATE INDEX), the
  DDL SereneDB actually supports.
- Upserts with an explicit conflict target. SereneDB resolves ON
  CONFLICT (cols) only against an inline UNIQUE constraint, never a
  CREATE UNIQUE INDEX — which is what GORM emits for `uniqueIndex` and
  what ALTER TABLE cannot retrofit. AddAuthenticator and
  AddVerificationRequest now insert bare and resolve the conflict off
  the 23505 the index still raises, so MFA enrollment and repeat
  verification requests keep their race guarantees.

Its MVCC is optimistic: contended writers abort with 40001 instead of
blocking, so both upserts retry rather than surfacing the conflict.

Not auto-mapped as an FGA store — it is a search-OLAP engine, not an
OpenFGA datastore, and needs an explicit --fga-store.

Verified against serenedb/serenedb 26.08.2: full storage suite passes on
a fresh database and on a second run against the existing schema.
Postgres, SQLite and MongoDB suites unchanged.

Claude-Session: https://claude.ai/code/session_01JGxmhT63jvFwuTeujfbC45
AddAuthenticator's conflict fallback depends on the (user_id, method)
index raising 23505. Without a direct assertion, a migrator that stopped
creating it would make the concurrency test flaky rather than red.

Claude-Session: https://claude.ai/code/session_01JGxmhT63jvFwuTeujfbC45
Reviewing the backend against the real binary found the documented
ceiling was not theoretical: 20 concurrent update_profile calls for one
user produced 17 failures, each leaking `ERROR: Conflict on tuple
deletion! (SQLSTATE 40001)` to the API caller. The per-method retry
covered only the two overridden upserts; every other write path had none.

Moved the retry to a gorm.ConnPool wrapper so every autocommit statement
gets it. Two consequences worth stating:

- SkipDefaultTransaction is required. GORM otherwise wraps each single
  Create/Update in a transaction, routing all writes through BeginTx and
  past the retry — which is why the first attempt at this changed
  nothing. These schemas carry no GORM associations, so one write is one
  statement and that wrapper bought nothing here anyway.
- Explicit Transaction() blocks are deliberately not retried: replaying
  one statement inside an aborted transaction is wrong, and GORM offers
  no hook to replay the whole block. The four cascade-delete call sites
  still surface 40001 under sustained same-row contention.

NewProviderWithMigrate becomes NewProviderWithOptions; both hooks are nil
for every other database type, so their path is unchanged.

After: 20/20 succeed, 0 conflicts in the server log; at 50 concurrent the
app's own rate limiter trips first and still nothing leaks. New test
asserts the cascade delete stays atomic under SkipDefaultTransaction.

Verified: full suite on SereneDB (946 pass), make test (SQLite), Postgres
storage suite, 5x flakiness loop.

Claude-Session: https://claude.ai/code/session_01JGxmhT63jvFwuTeujfbC45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant