Skip to content

dialect/sql/schema: one process applies a directory at a time, and an edited file it ran is refused - #17

Merged
lesomnus merged 2 commits into
masterfrom
migrate-lock
Oct 21, 2026
Merged

lesomnus merged 2 commits into
masterfrom
migrate-lock

Conversation

@lesomnus

@lesomnus lesomnus commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

For lesomnus/payday#33's directory (#7), and for the planned one beside it. Both are applied by Migrations.Apply, on every start, and by every replica that starts.

What was wrong

  • Two processes could apply one directory at once. Nothing in Apply or in atlas's Executor locks; the Atlas CLI takes schema.Locker itself. On PostgreSQL:
    • Both processes create the history table. The second is refused with duplicate key value violates unique constraint "pg_type_typname_nsp_index", despite IF NOT EXISTS.
    • Both run a pending file's statements. The second to reach one that cannot run twice fails (relation "once" already exists), and its deferred revision write replaces the winner's record.
    • I reproduced that last case: the history then reads applied=1 total=2 error="… already exists". Every later Apply reruns the file, fails the same way, and the deployment cannot start until someone fixes the row by hand.
  • An applied file that was edited afterwards went unnoticed. Atlas compares a file's statements only while the file is half-applied, and atlas.sum is whatever Rehash last wrote. So an edit plus a rehash changed nothing in a database that had already run the file, and changed what ran in the next database migrated. Nothing reported the difference.

What changed

  • Apply holds a session advisory lock (pg_advisory_lock) from before it creates the history table until the files have run.
    • The key is an FNV-64 of the schema and the history table. The two directories of one database, or two schemas in one database, do not wait on each other.
    • A waiting process waits as long as ctx allows, then finds nothing pending.
    • A process that dies releases the lock with its session. If the unlock itself fails, the connection is discarded rather than returned to the pool still holding the lock.
    • SQLite takes no lock. Atlas's SQLite locker is a file in os.TempDir() keyed by name, which would serialize unrelated databases, and several processes migrating one SQLite file is not a supported setup.
  • ErrEdited. Pending, Apply and Current compare each fully applied revision's stored hash with the directory's hash for that file, and refuse a mismatch, naming the files.
    • Atlas's per-file hash covers the directory up to and including the file. So an edit, addition or removal before a file the database ran also changes that file's hash. All three rewrite history under a database that ran it, and atlas already refuses an older file added out of order.
    • Baselines, and files still half-applied (atlas checks those statement by statement), are skipped.
  • CI: the unit job gets a postgres:17 service. The schema migration step gets ENT_SCHEMA_POSTGRES; its tests create a database of their own and drop it.
  • dialect/sql/schema/integration requires lib/pq v1.10.7, the version entc/integration already pins.

And what the directory needs: files, not Schema.Create

The second commit records why Schema.Create keeps dropping undeclared foreign keys and checks. One option was to make its default skip match Check's, so it would leave them alone. I did not, because Create drops them on purpose: that is how it retires a constraint the schema stopped declaring.

  • An edge pointed at another table gets a foreign key under a new name (<table>_<ref>_<edge>). Left in place, the old key would refuse every row the new one allows.
  • Create cannot tell that stale key from one the extra directory made, so it drops both.
  • The extra directory's history still says the file ran, so nothing recreates them. On PostgreSQL 17 I checked that a CHECK and a foreign key added beside the schema are gone after the next Create.
  • On SQLite, most changes rebuild the table from the schema, which drops them as well.

So the package doc and Extra now say the directory is for a database migrated from files. An app that migrates with Create instead runs its DDL after every Create, with each statement safe to run again; rove's cli/pg.sql works this way.

Tests

  • TestApplyIsOneAtATime (PostgreSQL; skipped without ENT_SCHEMA_POSTGRES): two pools apply the same directory concurrently.
    • First, a file that also creates the history table.
    • Then a file of SELECT pg_sleep(0.5); CREATE TABLE once ….
    • Both calls must succeed, and the history must say applied = total with no error.
  • TestEdited (SQLite):
    • a file added after the ones applied is not an edit;
    • an applied file edited and rehashed is refused by Pending, Apply and Current, and the error names it;
    • a fresh database runs the files as they are.
  • Mutations:
    • with the lock skipped, both phases of TestApplyIsOneAtATime fail, three runs out of three (the catalog duplicate, then relation "once" already exists);
    • with the check skipped, TestEdited fails.
  • go test -race ./... passes in dialect and, with PostgreSQL 17, in dialect/sql/schema/integration. .github/lint.sh is clean on the changed files.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KxtnpRW533bdErnW8QPhsk

lesomnus and others added 2 commits October 22, 2026 05:47
… edited file it ran is refused

A deployment applies its migrations on every start, and so does each
replica of it, and nothing kept two from applying one directory at once.
On PostgreSQL both made the history table, and the second was refused for
a duplicate catalog row; both ran a pending file's statements, and the
second to reach one that cannot run twice failed -- and atlas wrote its
failure over the record of the one that ran the file, so the history said
the file stopped halfway and every start after ran it again, failed again,
and did not start. `Apply` now holds a session advisory lock, keyed by the
schema and the history table, from before the history table is made until
the files have run. The other process waits, and then finds nothing to do.
SQLite is not locked: a file several processes migrate is not what this is
for.

And a file a database ran is not run again, so one edited afterwards, its
sum written again with `Rehash`, changed nothing where it had run and
everything where it ran next, with nothing to say the two differ. Atlas
compares a file's statements only while it is halfway, and `atlas.sum` is
rewritten from the files as they are. The history keeps the hash each file
had when it ran, so `Pending`, `Apply` and `Current` now refuse a directory
that no longer agrees with it, with `ErrEdited`.

The unit job gets a PostgreSQL service for the test that needs several
sessions; the schema migration tests make a database of their own on it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KxtnpRW533bdErnW8QPhsk
… not on Create

`Schema.Create` drops a foreign key or a check the schema does not
declare: that is how it retires one the schema stopped declaring, since an
edge pointed at another table is a foreign key of a new name and the old
one, left behind, would refuse every row the new one allows. It cannot tell
those from one the extra directory made, so a database kept in step by
Create loses that directory's foreign keys and checks the next time Create
runs, and the directory's history says the file ran, so nothing makes them
again. (Checked on PostgreSQL 17: a CHECK and a foreign key added beside the
schema are gone after the next Create.)

Leaving them alone in Create, as `Check` does, would keep the stale one too,
so the package says what the directory needs instead: a database migrated
from files. An app that migrates with Create states what the schema cannot
in statements it runs after every Create, each safe to run again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KxtnpRW533bdErnW8QPhsk
@lesomnus
lesomnus merged commit ff0f6e3 into master Oct 21, 2026
8 checks passed
@lesomnus
lesomnus deleted the migrate-lock branch October 21, 2026 21:08
lesomnus added a commit to lesomnus/payday that referenced this pull request Oct 23, 2026
…it calls, and a test links it to say so

payday's own sandbox was 63 MB, and 24 MB of it was methods nothing
calls. The generated client's runtime.go makes the ent schema's fields
at init, for their defaults and value scanners, and every field it made
listed its Go type's methods with reflect.Type.Method, by an index the
compiler cannot see. In a program that can reach such a call the linker
has to assume that any exported method of any type converted to an
interface may be called by name, so it keeps all of them, in every
package. The page worked, and no list of packages showed it: the call
was in one the sandbox has to link.

An app with no default of its own has paid it since #87.
protoc-gen-orm-ent began writing a declared default as the column's
(`.Default(v)`, protobuf-orm/protoc-gen-orm-ent#8), and payday's Audit
declares one on `domain`, so since then every app's runtime.go reads the
schema at init -- for Access's `domain` as well, since #112. khala's
sandbox measured 25.8 MB on payday 6bb6d9c and 46.9 MB on c0f5e7d. This
app paid it longer, for its Holder's value scanner.

protobuf-orm/ent#20 (4d86cf3) lists the methods in its loader, which
only codegen runs, and takes RType.Implements out of the exported API:
it has reflect.Type's name and signature, which the linker keeps on any
type converted to an interface, and the same call in its loop. What ent
generates is the same but for its stamp of which ent made it
(`internal/ent/runtime/runtime.go`), the one file apptest's regeneration
moves; an app takes the fix with payday's go.mod, and regenerates for
the stamp. The bump brings protobuf-orm/ent#17 too -- one process
applies a migration directory at a time, and an edited file it ran is
refused -- which payday does not call.

TestTheSandboxLetsTheLinkerPruneMethods links this app's sandbox as it
ships, under GOOS=js with grpcnotrace, with the linker's dump
(-dumpdep), and fails on any function in it marked for calling a method
by index, with the chain the linker took to it. On the ent before this
it fails at field.methods, reached from runtime.go's init through
schema.Access.Fields, field.Json and goType.

Measured, `GOOS=js GOARCH=wasm go build -tags grpcnotrace ./wasm`:

- this app: 62,994,314 B before, 39,013,431 B after (gzip -9: 10.99 MB,
  7.61 MB). Without runtime.go's init at all it is 38,628,342 B.
- roster c4ae61c, with this payday replaced in: 89,958,712 B before,
  56,631,026 B after (gzip -9: 15.47 MB, 11.03 MB).

docs: client.md says what a method called by index costs a sandbox,
runtime.md's table of what enforces what has the test, and migrating.md
says an app regenerates and what it gets back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KxtnpRW533bdErnW8QPhsk
lesomnus added a commit to lesomnus/payday that referenced this pull request Oct 23, 2026
…it calls, and a test links it to say so

payday's own sandbox was 63 MB, and 24 MB of it was methods nothing
calls. The generated client's runtime.go makes the ent schema's fields
at init, for their defaults and value scanners, and every field it made
listed its Go type's methods with reflect.Type.Method, by an index the
compiler cannot see. In a program that can reach such a call the linker
has to assume that any exported method of any type converted to an
interface may be called by name, so it keeps all of them, in every
package. The page worked, and no list of packages showed it: the call
was in one the sandbox has to link.

An app with no default of its own has paid it since #87.
protoc-gen-orm-ent began writing a declared default as the column's
(`.Default(v)`, protobuf-orm/protoc-gen-orm-ent#8), and payday's Audit
declares one on `domain`, so since then every app's runtime.go reads the
schema at init -- for Access's `domain` as well, since #112. khala's
sandbox measured 25.8 MB on payday 6bb6d9c and 46.9 MB on c0f5e7d. This
app paid it longer, for its Holder's value scanner.

protobuf-orm/ent#20 (4d86cf3) lists the methods in its loader, which
only codegen runs, and takes RType.Implements out of the exported API:
it has reflect.Type's name and signature, which the linker keeps on any
type converted to an interface, and the same call in its loop. What ent
generates is the same but for its stamp of which ent made it
(`internal/ent/runtime/runtime.go`), the one file apptest's regeneration
moves; an app takes the fix with payday's go.mod, and regenerates for
the stamp. The bump brings protobuf-orm/ent#17 too -- one process
applies a migration directory at a time, and an edited file it ran is
refused -- which payday does not call.

TestTheSandboxLetsTheLinkerPruneMethods links this app's sandbox as it
ships, under GOOS=js with grpcnotrace, with the linker's dump
(-dumpdep), and fails on any function in it marked for calling a method
by index, with the chain the linker took to it. On the ent before this
it fails at field.methods, reached from runtime.go's init through
schema.Access.Fields, field.Json and goType.

Measured, `GOOS=js GOARCH=wasm go build -tags grpcnotrace ./wasm`:

- this app: 62,994,314 B before, 39,013,431 B after (gzip -9: 10.99 MB,
  7.61 MB). Without runtime.go's init at all it is 38,628,342 B.
- roster c4ae61c, with this payday replaced in: 89,958,712 B before,
  56,631,026 B after (gzip -9: 15.47 MB, 11.03 MB).

docs: client.md says what a method called by index costs a sandbox,
runtime.md's table of what enforces what has the test, and migrating.md
says an app regenerates and what it gets back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KxtnpRW533bdErnW8QPhsk
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