feat(storage): add SereneDB backend - #792
Open
lakhansamani wants to merge 3 commits into
Open
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/serenedbembedssql.Providerand overrides only those two — no existing backend changes behaviour.The two incompatibilities
Migration. SereneDB reports
varchar(n)astext, so GORM AutoMigrate re-issuesALTER COLUMN ... TYPEon every boot and SereneDB refuses it on an indexed column:First boot worked; every restart failed. Replaced with a create-only migrator (
CREATE TABLE/ADD COLUMN/CREATE INDEX) — the DDL SereneDB actually supports.sql.NewProviderWithMigrateis the new seam;sql.NewProviderdelegates to it with the existing AutoMigrate behaviour unchanged.Upserts with an explicit conflict target. SereneDB resolves
ON CONFLICT (cols)only against an inlineUNIQUEtable constraint, never aCREATE UNIQUE INDEX— which is what GORM emits foruniqueIndex, and whatALTER TABLE ADD CONSTRAINTcannot retrofit once the index exists:Targetless
DO UPDATEfails too.AddAuthenticatorandAddVerificationRequestnow 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_profilecalls for one user produced 17 failures, each leakingERROR: Conflict on tuple deletion! (SQLSTATE 40001)straight to the API caller.Fixed by moving the retry to a
gorm.ConnPoolwrapper, so every autocommit statement gets it. Two things that fix depends on:SkipDefaultTransactionis required. GORM otherwise wraps each singleCreate/Updatein a transaction, routing every write throughBeginTxand 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.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.TestDeleteUserCascadeStaysAtomicasserts 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 GraphQLmessage. 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(reportsPostgreSQL 18.3), pinned in the Makefile since the compat surface is young and these overrides depend on its specifics:TEST_DBS=serenedb, somake teststays SQLite-only.make test(SQLite),make test-mongodb(949 pass), Postgres storage +db/sqlsuites: all unchanged.make test-serenedbadded;test-all-db/test-docker-up/test-cleanupinclude it.Two notes for the reviewer:
make test-postgresrecipe is unchanged.make lintfails onmainalready — 3 govetreflect.Ptrand 1 gofmt inoauth_sso_verify_test.go, likely a newer local golangci-lint. Left alone; the packages here lint clean.