oxidecomputer / oxidecomputer/omicron
populate() submits dbinit.sql as one batch, putting SET CLUSTER SETTING inside an implicit transaction — the comment above it asserts the opposite
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
Description
schema/crdb/dbinit.sql opens with a cluster setting and a comment explaining
why it is placed where it is:
/*
* Reduce the index backfill batch size from the default of 50,000 to 5,000.
* ...
* This must be outside the transaction because SET CLUSTER SETTING cannot be
* used inside a transaction.
*/
SET CLUSTER SETTING bulkio.index_backfill.batch_size = 5000;
BEGIN;
The statement is indeed outside the explicit BEGIN. But
test-utils/src/dev/db.rs:938 submits the whole file in a single call:
pub async fn populate(
client: &tokio_postgres::Client,
) -> Result<(), anyhow::Error> {
let sql = include_str!("../../../schema/crdb/dbinit.sql");
client.batch_execute(sql).await.context("populating Omicron database")
}
batch_execute sends the file as one simple query. A multi-statement simple
query runs as an implicit transaction. So the statement is outside the
explicit transaction and inside an implicit one, and the comment's guarantee
does not hold — it holds only because the CockroachDB version currently pinned
(v22.1.22) tolerates it.
On CockroachDB 22.2 the same file fails:
failed to populate database: populate
Caused by:
0: populating Omicron database
1: db error
2: ERROR: SET CLUSTER SETTING cannot be used inside a multi-statement transaction
I hit this while running the simulated stack against 22.2, but the reason to
report it is not my configuration — it is that the safety property the comment
claims is not actually provided by the code, and the next CockroachDB major
version bump will surface that. The comment is load-bearing documentation for
anyone editing dbinit.sql; someone adding a second SET CLUSTER SETTING
alongside the first would reasonably trust it.
Steps to reproduce
- Point omicron at a CockroachDB 22.2 binary (update
tools/cockroachdb_version
so the startup version check passes). cargo xtask omicron-dev run-all
Expected result
SET CLUSTER SETTING executes outside any transaction, as the comment states,
and schema population proceeds.
Actual result
Population aborts on the first statement of the file. CockroachDB 22.1 accepts
it; 22.2 does not.
Suggested fix
Execute statements that cannot run in a transaction on their own, rather than
relying on their position within the file. For example, in populate():
let mut rest = String::with_capacity(sql.len());
for line in sql.lines() {
if line.trim_start().to_uppercase().starts_with("SET CLUSTER SETTING") {
client.batch_execute(line).await.with_context(|| {
format!("applying cluster setting: {line}")
})?;
} else {
rest.push_str(line);
rest.push('\n');
}
}
client.batch_execute(&rest).await.context("populating Omicron database")
A tidier variant would be a dedicated list of pre-transaction statements, or a
sentinel comment in the SQL marking the split point, so the behaviour is
explicit rather than inferred from a prefix match.
At minimum, the comment in dbinit.sql should be corrected to say that the
statement's placement is safe only because batch_execute's implicit
transaction currently accepts it on the pinned version.
The same consideration applies to wipe() (dbwipe.sql), which uses the same
batch_execute pattern.
Environment
omicron df990b0578fbee4afcf805423a20d85e23544e0d (2026-09-04)
cockroachdb v22.2.19 (pinned version is v22.1.22-64-g86fdbfca06)
platform aarch64-unknown-linux-gnu, Ubuntu 24.04
mode simulated (cargo xtask omicron-dev run-all)
Not architecture-specific: the same failure occurs on any platform running
CockroachDB 22.2.
Disclosure: this issue was investigated and written up with AI assistance
(Claude). Everything in it was measured rather than inferred — the timings,
error output, version numbers and reproduction steps are all from real runs on
real hardware, and where a fix is suggested it is one I am actually running. I
have read it through before filing. Happy to clarify anything or test a patch.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with populate() at test-utils/src/dev/db.rs:938 and the transaction structure in schema/crdb/dbinit.sql; reproduce with CockroachDB 22.2 using cargo xtask omicron-dev run-all. Check the related wipe() path and dbwipe.sql as well. Done means schema population succeeds with SET CLUSTER SETTING outside a transaction and the documented safety guarantee is accurate.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100