Reset pooled test databases with DELETE instead of TRUNCATE - #473
Closed
devhawk wants to merge 1 commit into
Closed
Conversation
CockroachDB implements TRUNCATE as a schema change, so it prices like CREATE INDEX however few rows a table holds. Measured against this schema it costs 1.16s where the equivalent deletes cost 0.05s — 23x — and `acquire` resets a pooled container on every `new PgContainer()`, which 46 test classes do in @beforeeach. That is roughly 900 resets a run. CI's own test-results artifacts say this is where the CockroachDB leg goes. Test bodies cost the same on both backends — 1397s on CockroachDB against 1435s on PostgreSQL — while per-class fixture time is +1308s on CockroachDB and ~0 on PostgreSQL. Roughly 1000s of that is these truncates. TRUNCATE would win only once a table is big enough for row count to dominate, which no test fixture is. The table list now comes from the catalogue rather than a hard-coded list, so a migration that adds a table cannot silently leave it uncleaned. Deleting from all of them in one statement batch is safe in any order: emptying everything cannot strand a foreign key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
Abandoned. |
devhawk
added a commit
that referenced
this pull request
Sep 3, 2026
Two costs, both CockroachDB's, both paid on every pooled test database.
**Starting one** ran the whole migration corpus — about a minute of
online
schema changes. The pool now starts from images whose DBOS schema is
already
migrated: `ghcr.io/dbos-inc/dbos-test-{postgres,cockroach}`, published
from
dbos-ctl and pinned here to a migration version, so a migration landing
there
cannot change what this suite runs against without a commit here saying
so.
**Resetting one** ran `TRUNCATE`, which CockroachDB also implements as a
schema
change, so it prices like `CREATE INDEX` however few rows a table holds
—
measured at 1.16s against 0.05s for the equivalent deletes, roughly 900
times a
run. The reset is now `DELETE`, over a table list read from the
catalogue rather
than hard-coded, so a migration that adds a table cannot silently leave
it
uncleaned. That half is taken from #473, which was abandoned rather than
rejected.
## Measured on CI
[This
branch](https://github.com/dbos-inc/dbos-transact-java/actions/runs/33779764382)
against [the last PR run on
main](https://github.com/dbos-inc/dbos-transact-java/actions/runs/33563766739),
both green:
| job | before | after | change |
|---|---|---|---|
| **test-crdb** | **1403s** (23m23s) | **779s** (12m59s) | **−624s,
−44%** |
| test (17, temurin) | 672s | 671s | −1s |
| test (17, oracle) | 661s | 646s | −15s |
| test (21, temurin) | 596s | 632s | +36s |
| test (21, oracle) | 566s | 565s | −1s |
| test (25, temurin) | 609s | 581s | −28s |
| test (25, oracle) | 562s | 558s | −4s |
**Whole-run wall clock: 23m → 13m**, since `test-crdb` is the critical
path.
PostgreSQL is a wash, and that is the expected result rather than a
disappointing one: its `TRUNCATE` was never expensive and its migration
is under
a second, so there was nothing there to win. The spread of −28s to +36s
across
six jobs is run-to-run noise. What matters is that it did not regress.
## The pool is kept deliberately
An earlier attempt (#479) gave every test its own container instead. CI
rejected
it: `test-crdb` went from 23m23s to 30m *while running at four times the
parallelism*, because container creation cost more than the reset it
replaced.
Containers are worth pooling. What was worth removing is what the pool
did to
them.
## Parallelism is left alone
Still `CrdbParallelExecutionConfigurationStrategy` from `main`, which
gives
CockroachDB 2 on an 8-core machine and 1 below that — so **1** on a
4-core CI
runner. The 779s above is what one CockroachDB test at a time now costs.
Raising it is the obvious next lever and is now much more affordable,
since
concurrent containers no longer each migrate. It is left out so this
change is
one variable and the numbers above are readable — and because
parallelism is
what made the from-empty migration tests time out when tried alongside
per-test
containers, which is the same pressure that required the timeout in the
third
commit here.
## Tests that must start empty are untouched
`createFresh()`, `getPG()` and `getCRDB()` still hand out stock images
with no
DBOS schema in them — `MigrationManagerTest`, `CustomSchemaTest`,
`JdbcStepFactoryInitTest` and `CockroachMigrationTest`. They exist to
watch a
schema being built, and an image arriving with one built would test
nothing.
`CockroachMigrationTest` gains a five-minute class timeout. Every test
in it
migrates a stock CockroachDB container from empty, which is tens of
seconds even
idle, and the suite-wide two-minute default was a coin toss against
that. It
became marginal *because* of this change: pooled containers no longer
migrate,
so the suite runs denser and the tests still doing the slow thing get
less of the
runner. Raised rather than removed, so a genuine hang still fails.
## Two testcontainers traps, and one leak
PostgreSQL's default wait strategy looks for "ready to accept
connections"
*twice*, because a container that initialises itself starts the server
once for
initdb and again to serve. A prebaked image logs it once, so every
container
would wait out its startup timeout while being ready the whole time.
CockroachDB has to be told where its store is — a prebaked image keeps
it
outside the default location so a container that forgets to ask starts
an empty
node rather than appearing to work. Setting a password would also make
`CockroachContainer.configure()` replace the command and silently drop
both the
store path and `--insecure`.
The second commit fixes a leak that predates this branch: `acquire()`
started a
container and threw without closing it if preparing the database failed.
The
exception escapes the constructor, so the field is never assigned,
`@AutoClose`
never registers, and the container stays up unreachable while the next
test
starts another. One broken image therefore cost one container *per test*
— a
prebaked image that refused connections produced 181 orphans before the
run was
killed, and the machine rather than the assertion is what reported it.
## Requires
dbos-ctl#23, merged and force-republished. Before that these images
refused
every TCP connection.
## Known flake, untouched
`DynamicQueuesTest.testLimiter()` failed once on the CockroachDB leg
(`Diff: 1.452` against a hard-coded `waveTolerance = 1.0`) and passed on
the
re-run. A one-second budget for a scheduling assertion is tight on a
loaded
four-core runner. Loosening a timing test to make an unrelated change
green
should be its own decision, so it is left alone here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
1 participant
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.
CockroachDB implements TRUNCATE as a schema change, so it prices like CREATE INDEX however few rows a table holds. Measured against this schema it costs 1.16s where the equivalent deletes cost 0.05s — 23x — and
acquireresets a pooled container on everynew PgContainer(), which 46 test classes do in @beforeeach. That is roughly 900 resets a run.CI's own test-results artifacts say this is where the CockroachDB leg goes. Test bodies cost the same on both backends — 1397s on CockroachDB against 1435s on PostgreSQL — while per-class fixture time is +1308s on CockroachDB and ~0 on PostgreSQL. Roughly 1000s of that is these truncates.
TRUNCATE would win only once a table is big enough for row count to dominate, which no test fixture is.
The table list now comes from the catalogue rather than a hard-coded list, so a migration that adds a table cannot silently leave it uncleaned. Deleting from all of them in one statement batch is safe in any order: emptying everything cannot strand a foreign key.