phase-8: run it on Postgres, and find out what that was hiding
Six phases claimed the product runs on SQLite and on Postgres. Nothing had ever run it on Postgres. TEST_DATABASE_URL now points the whole suite at a real server and CI runs both arms. The first run found a bug that would have shipped. better-auth's banned flag is integer 0/1 on SQLite and a real boolean on Postgres, and the code read it as `banned === 1`, so on Postgres an account someone asked us to freeze went on receiving email. Both of those flags are now typed for either dialect and read through isFlagSet. It also found the migration advisory lock being taken on a pool. An advisory lock belongs to the session that took it, so a lock on one pooled connection and an unlock on another leaves it held. Two replicas migrating at once is the ordinary case in k8s and is precisely what it was there to protect. New for the scaled mode: k8s manifests with migrations as an initContainer, one poller in its own worker Deployment rather than one per API replica, and an Ingress that exposes the web app only. The storage drivers finally have tests, S3 included, since scaled mode requires it and it had never been exercised. Two acceptance tests, both checked against a deliberately broken build first: two workers claiming a hundred jobs report 188 claims with SKIP LOCKED removed, and the in-flight request is cut off with the drain wait removed. 340 tests on SQLite, 341 on Postgres, 70 Playwright, rules coverage 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
8c8463924f
commit
37370dd079
@@ -752,3 +752,78 @@ either has its subject or is a 404, and there is no third case. The crossfade fr
|
||||
to content is applied where the skeleton is a separate early return; the three list screens
|
||||
render their skeleton inside the same tree as their heading, where fading the whole tree
|
||||
would also fade a heading that never changed.
|
||||
|
||||
## Phase 8
|
||||
|
||||
### The dual-dialect claim is now run, not asserted
|
||||
Six phases said the product runs on SQLite and on Postgres. Nothing had ever run it on
|
||||
Postgres. `TEST_DATABASE_URL` now points the whole suite at a real server, and CI runs both
|
||||
arms of the matrix.
|
||||
|
||||
The first run found three things, one of them a bug that would have shipped:
|
||||
|
||||
**A frozen account went on being notified on Postgres.** `banned` is integer 0/1 on SQLite
|
||||
and a real boolean on Postgres, and the code read it as `recipient.banned === 1`. On
|
||||
Postgres the driver hands back `true`, the comparison is false, and an account someone
|
||||
asked us to freeze keeps getting email. The two better-auth flags are now typed for both
|
||||
dialects and read through `isFlagSet`, and a test asserts the behaviour on whichever
|
||||
dialect the suite is pointed at.
|
||||
|
||||
**The migration advisory lock was taken on a pool.** `pg_advisory_lock` belongs to the
|
||||
session that took it. Issued on one pooled connection and released on another, the unlock
|
||||
is a no-op with a warning and the lock stays held until that connection happens to close.
|
||||
Two replicas migrating at once, which is the ordinary case in k8s, is exactly what it was
|
||||
supposed to protect. It now runs on one pinned connection.
|
||||
|
||||
**Writes survived by accident.** `set banned = 1` and `where banned = 0` work on Postgres
|
||||
only because its boolean input syntax accepts the strings '1' and '0'. That is luck rather
|
||||
than design, and it is why the read path was the one that broke.
|
||||
|
||||
### Each Postgres harness gets a database, not a schema
|
||||
A schema per harness was the obvious choice and it does not work. Kysely's migrator asks
|
||||
the introspector whether its bookkeeping tables exist, and with no schema configured a
|
||||
table of that name in any schema counts, so twenty parallel schemas each holding a
|
||||
`kysely_migration` convince each other the work is already done. A database each has no
|
||||
such ambiguity. It is slower, which is what the longer timeout under `TEST_DATABASE_URL`
|
||||
is for: building a real database is honest work, not a hang.
|
||||
|
||||
### DATABASE_POOL_MAX
|
||||
Connections are the resource that runs out first when replicas multiply. It was hardcoded
|
||||
at ten. It is now a variable, documented next to the arithmetic that matters: pool size
|
||||
times (api replicas plus worker replicas) has to stay under the server's max_connections.
|
||||
|
||||
### Both new tests were checked against a broken implementation
|
||||
A green test that cannot fail proves nothing, so both were run against a deliberately
|
||||
broken version first:
|
||||
|
||||
- Remove `FOR UPDATE SKIP LOCKED` from the Postgres claim and the two-worker test reports
|
||||
188 claims for 100 jobs.
|
||||
- Remove the drain wait from the shutdown path and the in-flight request is cut off.
|
||||
|
||||
The shutdown test spawns a real process and holds a request open by sending its body in two
|
||||
pieces, so the server is genuinely inside the handler when SIGTERM arrives. Asserting on
|
||||
the app object would have proved nothing about a socket.
|
||||
|
||||
### The manifests are checked for the thing that actually rots
|
||||
They cannot be applied to a cluster from here, so `deploy.test.ts` checks the part that
|
||||
drifts: every variable set in a ConfigMap, Secret or `env:` block has to be one the env
|
||||
schema knows, the grace period has to outlast the 25 second drain, and the API has to be
|
||||
the one that does not poll. A variable renamed in `src/lib/env.ts` and forgotten in a
|
||||
ConfigMap is a pod that starts and then refuses to boot, and no generic schema validator
|
||||
would catch it.
|
||||
|
||||
### The e2e job stays on SQLite
|
||||
SPEC.md section 13 puts the golden paths against the compose stack, which is combined mode:
|
||||
SQLite and local files. It is also the only shape `e2e/db.ts` can read, since those helpers
|
||||
open the database file to check what a flow actually wrote. S3 is covered where it belongs,
|
||||
in the unit suite against MinIO, and Postgres by the dialect matrix.
|
||||
|
||||
### The storage drivers had no tests at all
|
||||
S3 is required in scaled mode and had never been exercised. Both drivers are now held to
|
||||
the same round trip, with the S3 half gated on `TEST_S3_ENDPOINT`. That half has not run on
|
||||
this machine: there is no object store here and no Docker to start one. CI runs it.
|
||||
|
||||
### Still not run here
|
||||
The images, the compose stack and the manifests have never been built or applied on this
|
||||
machine. No Docker daemon it can reach, no cluster. The README says so where someone about
|
||||
to deploy will read it, rather than only here.
|
||||
|
||||
Reference in New Issue
Block a user