Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions deploy/consolidated/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,9 @@ applies the numbered `.sql` files in
[`one_d4/migrations/`](../../domains/games/apis/one_d4/migrations/) and
exits. Both `one_d4` and `one_d4_worker` gate on it with
`service_completed_successfully` — the worker so it no longer waits for the
Java service to boot, the service so its own boot-time migration (which
still runs, until #1426 demotes it to a verifier) is serialized behind this
one rather than racing it.
Java service to boot, the service so the schema exists by the time its boot
check runs. This one-shot is the only thing that writes that schema; one_d4
verifies at boot and crash-loops rather than repairing what it finds.

Two things about `shared_postgres` are load-bearing and easy to undo by
accident:
Expand Down
20 changes: 9 additions & 11 deletions deploy/consolidated/compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,11 @@ services:

# The schema step (#1419): applies one_d4's migrations/ .sql files and
# exits, before anything that needs the tables. Idempotent — safe on every
# deploy, like golf_hub_db_init above. The Java service still runs the
# same migrations at boot, after this (#1426 is the demotion to a
# verifier); what the step already buys is one_d4_worker starting without
# waiting for the Java service (#1418 measured that coupling as an
# error-loop until one_d4 came up).
# deploy, like golf_hub_db_init above. The only writer of that schema:
# one_d4 checks its work at boot rather than repeating it (#1426), and
# one_d4_worker creates nothing, so it starts without waiting for the Java
# service (#1418 measured that coupling as an error-loop until one_d4
# came up).
one_d4_migrate:
image: ghcr.io/muchq/one_d4_migrate:${ONE_D4_MIGRATE_SHA:-${DEPLOY_SHA:-latest}}
labels:
Expand Down Expand Up @@ -454,12 +454,10 @@ services:
- one-d4
depends_on:
# The migrate gate is a strict superset of the old shared_postgres one
# (the one-shot itself waits for Postgres), and it serializes the two
# migration runners: this service still applies the same files at boot
# (#1426 is the demotion to a verifier), and released together the two
# would race — CREATE TABLE/INDEX IF NOT EXISTS is idempotent but not
# concurrency-safe, and the one-shot losing that race gates the worker
# off until the next deploy.
# (the one-shot itself waits for Postgres). This service verifies at
# boot that the migrations were applied and refuses to serve otherwise,
# so releasing the two together without the gate crash-loops it against
# a schema the one-shot has not finished writing.
one_d4_migrate:
condition: service_completed_successfully
deploy:
Expand Down
16 changes: 7 additions & 9 deletions deploy/consolidated/deploy_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1393,18 +1393,16 @@ func TestTheWorkerGatesOnTheMigrateStepNotTheJavaService(t *testing.T) {
}
}

// The Java service gates on the same one-shot — not for the schema (it still
// applies the migrations at boot until #1426), but for serialization: released
// together, the two runners execute identical DDL concurrently, and
// CREATE TABLE/INDEX IF NOT EXISTS is idempotent yet not concurrency-safe on
// Postgres. The loser of that race under restart:"no" is one_d4_migrate, and
// its failure gates one_d4_worker off until the next deploy.
// The Java service gates on the same one-shot, and now needs to: since #1426 its
// boot checks the migrations were applied rather than applying them, and refuses
// to serve otherwise. Released together without the gate it crash-loops against a
// schema the one-shot has not finished writing.
func TestTheJavaServiceAlsoGatesOnTheMigrateStep(t *testing.T) {
if !gatesOnCompletedMigrate(t, "one_d4") {
t.Errorf("one_d4 does not gate on one_d4_migrate with "+
"service_completed_successfully (depends_on: %v) — its boot-time migration then "+
"runs concurrently with the one-shot's, and the one-shot losing that race blocks "+
"the worker.", dependsOn(t, "one_d4"))
"service_completed_successfully (depends_on: %v) — its boot-time schema check then "+
"races the one-shot, and fails against the tables it has not created yet.",
dependsOn(t, "one_d4"))
}
}

Expand Down
16 changes: 6 additions & 10 deletions domains/games/apis/one_d4/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ java_library(

# The schema itself (#1419): numbered idempotent .sql files, manifest-ordered.
# One copy of the DDL, three readers — Migration at boot (both engines), the
# one_d4_migrate deploy step, and one_d4_worker's schema_contract_test via
# one_d4_migrate deploy step, and one_d4_worker's Postgres suites via
# :migrations_sql. Each file is named exactly once, in one of the three
# groups below; the shipping and test targets compose the groups, so a new
# step is a line in manifest.txt plus a line in one group.
Expand Down Expand Up @@ -129,9 +129,10 @@ java_library(
visibility = ["//visibility:public"],
)

# Every migration file plus the manifest, as plain files: data for
# one_d4_worker's schema_contract_test, which scrapes the Postgres DDL and
# checks that no .sql file is unreachable from the manifest.
# Every migration file plus the manifest, as plain files: what
# one_d4_worker's Postgres suites run to build the schema they test against,
# and what schema_contract_test walks to check that no .sql file is
# unreachable from the manifest.
filegroup(
name = "migrations_sql",
testonly = True,
Expand Down Expand Up @@ -378,12 +379,6 @@ filegroup(
visibility = ["//domains/games:__subpackages__"],
)

# The DDL itself lives in :migrations_sql (#1419); what schema_contract_test
# still scrapes from Java source is the attempt budget, and nothing else. The
# lease and retention windows reach it as data, from :retention_policy_json
# (#1424), so RetentionPolicy.java does not belong here — listing a source
# nothing opens would re-run the contract test on every edit to it.

# The retention windows, as data rather than as a constant in either language (#1424).
#
# Both readers load this at startup — the C++ worker out of its image's runfiles, Java off its
Expand Down Expand Up @@ -649,6 +644,7 @@ java_test_suite(
"src/test/java/com/muchq/games/one_d4/db/PostgresAggregateCompatTest.java",
"src/test/java/com/muchq/games/one_d4/db/PostgresConcurrentWriteTest.java",
"src/test/java/com/muchq/games/one_d4/db/PostgresMigrationRunnerTest.java",
"src/test/java/com/muchq/games/one_d4/db/PostgresMigrationVerifyTest.java",
"src/test/java/com/muchq/games/one_d4/db/PostgresPlayerIndexTest.java",
"src/test/java/com/muchq/games/one_d4/db/PostgresReadTimeoutTest.java",
"src/test/java/com/muchq/games/one_d4/db/PostgresRetentionIndexTest.java",
Expand Down
20 changes: 14 additions & 6 deletions domains/games/apis/one_d4/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,28 @@ disappears with the process.
docker run -d --name one_d4_dev -p 5432:5432 \
-e POSTGRES_USER=indexer -e POSTGRES_PASSWORD=indexer -e POSTGRES_DB=indexer postgres:18

INDEXER_DB_URL="jdbc:postgresql://localhost:5432/indexer" \
INDEXER_DB_USERNAME=indexer \
INDEXER_DB_PASSWORD=indexer \
bazel run //domains/games/apis/one_d4:one_d4_migrate

INDEXER_DB_URL="jdbc:postgresql://localhost:5432/indexer" \
INDEXER_DB_USERNAME=indexer \
INDEXER_DB_PASSWORD=indexer \
bazel run //domains/games/apis/one_d4:one_d4
```

`postgres:18` is the image the deploy runs (`shared_postgres` in `compose.yaml`), so local
dev and production speak the same dialect. Migrations run at startup against an empty
database, so nothing else is needed to bring one up.
dev and production speak the same dialect.

The migrate step is not optional: the service creates no schema of its own. It checks at
boot that the migrations were applied and refuses to serve otherwise (#1426), naming what
is missing. Re-run it after pulling a new `V<NNN>` step.

The schema itself is the numbered `.sql` files in [`migrations/`](migrations/) (#1419)
`Migration` applies them at boot, and the deploy runs the same files first as the
`one_d4_migrate` one-shot (`compose.yaml`), which is what lets `one_d4_worker` start
without waiting for this service. `migrations/README.md` has the authoring rules.
The schema itself is the numbered `.sql` files in [`migrations/`](migrations/) (#1419).
`one_d4_migrate` is the same one-shot the deploy runs before the services start
(`compose.yaml`), which is what lets `one_d4_worker` start without waiting for this
service. `migrations/README.md` has the authoring rules.

Credentials in the URL still work, but only if the password survives URL decoding —
pgjdbc decodes query values, so `+` becomes a space, `&` truncates the rest, and a bare
Expand Down
56 changes: 29 additions & 27 deletions domains/games/apis/one_d4/migrations/README.md
Comment thread
aaylward marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# one_d4 schema migrations

The schema as numbered, idempotent SQL files (#1419). This directory is the
one copy of the DDL: the Java service applies it at boot, the
`one_d4_migrate` deploy step applies it before the services start, and
`one_d4_worker`'s `schema_contract_test` reads it to hold the C++ fixtures
to it.
one copy of the DDL: the `one_d4_migrate` deploy step applies it before the
services start, the Java service verifies at boot that it did, and
`one_d4_worker`'s Postgres suites apply it to build the schema they test
against.

## Layout

Expand All @@ -25,32 +25,34 @@ notes in `V009__dedupe_key.sql` for why that is load-bearing).
- **Idempotent, always.** Every statement must be safe to re-run:
`IF NOT EXISTS`, `IF EXISTS`, `DO $$ ... EXCEPTION` blocks, guarded
UPDATEs. There is no tracking table; re-running everything *is* the
mechanism, and the Java service re-runs it on every boot (#1426 tracks
demoting that boot-time run to a verifier — and it is where the tracking
table question reopens, if boot time ever grows with the step count or a
step arrives that cannot be written idempotently).
mechanism, and it is what lets the deploy step run on every deploy. What
would end that: boot/deploy time growing with the step count, or a step
that cannot be written idempotently (a backfill too expensive to guard).
Neither exists yet.

Idempotence is also what `Migration.verify()` leans on — it applies the
steps to an empty scratch schema and compares, so a step that only works
against a populated database breaks boot verification, not just re-runs.
- **Append, don't edit.** A schema change is a new `V<NNN>` step: the next
number, a line in `manifest.txt`, and the file named in *each* applicable
`BUILD.bazel` list — `:migrations` (pg + shared, ships with the service),
`:h2_migrations` (h2, test-only), `:migrations_sql` (all of them, the C++
contract test's data). A file unlisted in BUILD neither ships nor runs,
and no test can see it. Editing an old step is for comments only.
- **Plain SQL, executed one statement at a time.** Each `pg/` and shared
file also works under `psql -f`; nothing here depends on the runner (the
`h2/` files are H2 syntax and are not psql-compatible). Statements are
split on top-level semicolons — dollar-quoting, `''` escapes and comments
respected — and executed individually, exactly as the old Java constants
were: whole-file execute would trade that for the driver's own
multi-statement semantics and lose which step failed. The splitter does
not model double-quoted identifiers or `E''` strings, so don't use them
(none of the schema needs either).
`:h2_migrations` (h2, test-only), `:migrations_sql` (all of them, what
the C++ suites run and walk). A file unlisted in BUILD neither ships nor
runs, and no test can see it. Editing an old step is for comments only.
- **Plain SQL, and a whole file has to work as one script.** Each `pg/` and
shared file also works under `psql -f`; nothing here depends on the runner
(the `h2/` files are H2 syntax and are not psql-compatible). Java splits
on top-level semicolons — dollar-quoting, `''` escapes and comments
respected — and executes them individually, so a failure names its step;
the splitter does not model double-quoted identifiers or `E''` strings, so
don't use them (none of the schema needs either). `one_d4_worker`'s
Postgres suites send each `pg/` or shared file whole through libpq
instead (`migration_files`), which puts its statements in one implicit
transaction — so no step may depend on an earlier statement in the same
file having committed.
- **Forked steps stay in step.** Both engines run the same step list in the
same order; only a step's SQL may differ. If you add a partial index on
Postgres, add the H2 stand-in with the same name (see `V016`, `V017`).
- **The C++ fixtures are held to `pg/` + shared.**
`schema_contract_test` scrapes `CREATE TABLE` bodies and
`ALTER TABLE ... ADD COLUMN IF NOT EXISTS` lines, so keep one column per
line in CREATE bodies and each ALTER on a single line.

## Running them by hand

Expand All @@ -64,6 +66,6 @@ done
The deploy step (`//domains/games/apis/one_d4:one_d4_migrate`, a one-shot
compose service) does the same through the Java `Migration` class, so the
statements production runs are the ones the tests ran. Both `one_d4` and
`one_d4_worker` gate on it — the service so the two migration runners are
serialized rather than concurrent, the worker so it can start without the
Java service at all.
`one_d4_worker` gate on it — the service because its boot check would
otherwise run against a schema the one-shot has not finished writing, the
worker because it can then start without the Java service at all.
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,15 @@ static String resolveJdbcUrl(@Nullable String configuredUrl) {
return configuredUrl == null || configuredUrl.isBlank() ? readJdbcUrl() : configuredUrl.strip();
}

/**
* The schema is {@code one_d4_migrate}'s to write (#1426); boot's job is to refuse to serve
* against one that step did not finish. The H2 test path has no such step and applies the
* migrations itself.
*/
@Context
public Migration migration(DataSource dataSource, SqlDialect dialect) {
Migration migration = new Migration(dataSource, dialect);
migration.run();
migration.atBoot();
return migration;
}

Expand Down
Loading
Loading