fix(schema): serialize CREATE SCHEMA so concurrent sessions cannot collide

[axellpadilla]

Closes #839

The bug

Four macros create a schema on demand, each with its own copy of this guard:

IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '<schema>')
BEGIN
  EXEC('CREATE SCHEMA [<schema>]')
END

That's check-then-act. With threads > 1, or two dbt runs against one database, sessions pass the check together and all but one fail with Msg 2714, There is already an object named '<schema>' in the database. The schema exists and is fine by the time you see the error, which is why a rerun works — and why CI that builds a schema per PR only hits it on a branch's first run.

sqlserver__get_test_sql is the one reported. The same guard is in sqlserver__get_unit_test_sql, sqlserver__create_schema and sqlserver__create_schema_with_authorization. All four are fixed.

8 sessions released from a barrier: 15 failures in 48 attempts, with the reporter's exact message.

The fix

All four now share one create_schema_if_not_exists macro that serializes the check and the create behind an sp_getapplock. Same race after the fix: 0 failures in 48 attempts.

Two decisions worth explaining:

The lock is session-scoped, not transaction-scoped. Giving the guard its own BEGIN TRAN would be tidier — a transaction-scoped lock frees itself on rollback, so no CATCH needed. But it doesn't survive dbt's transaction, which 1.12 opens by default: a nested BEGIN TRAN only bumps @@TRANCOUNT, so the lock is held until dbt commits. In get_test_sql that's after the test query runs, so data tests would queue behind each other — a second test waited 2.6s behind a 3s test query, against 0.0s as written.

The TRY/CATCH is a finally, not a swallow. A session-scoped lock lives until it's released or the connection closes, and XACT_ABORT ON aborts the batch on a failed create, skipping a trailing sp_releaseapplock. Left like that, one failed create strands the lock on a connection dbt reuses all run, and every other thread waits out the 30s timeout. The CATCH releases it and rethrows the original error unchanged. (Releasing works even with the transaction doomed.)

Also: sp_getapplock returns a return code, not a result set, so the caller's SELECT is still the first result set — which matters for get_test_sql. A lock request that times out falls through to the bare check, so the worst case is today's behaviour. And schema and authorization names are now escaped where they land inside string literals, so an apostrophe no longer breaks the statement.

Why not just swallow the error

Catching 2714 and carrying on gains nothing. XACT_ABORT ON (#718) dooms the transaction, so the run fails either way and the work is rolled back either way. All a swallow does is replace There is already an object named '<schema>' with Uncommittable transaction is detected at the end of the batch, which names nothing. SQL Server has no CREATE SCHEMA IF NOT EXISTS, so the only real option is to not race.

Correction: this section previously said a swallow would silently discard the transaction. It doesn't — dbt sees an error either way. The cost is the diagnosis, not silence.

Tests

tests/functional/adapter/mssql/test_concurrent_schema_creation.py, one class over one dbt project:

• test_racing_sessions_create_schema_once — 8 real connections race adapter.create_schema, three rounds, barrier-synced; all must succeed and the schema must exist exactly once.
• test_test_sql_serializes_schema_creation — racing dbt test deterministically isn't practical, so this asserts the statement get_test_sql emits is the serialized form.
• test_failed_create_releases_the_lock — fails a create via AUTHORIZATION to a missing principal, then checks from a second session that the lock came back and the original error was rethrown.

All three fail against the old macros and pass against these.

• tests/functional — 407 passed, 48 skipped, 2 xfailed (at -n 8, which exercises the concurrent path itself)
• tests/unit — 638 passed, 5 skipped

Note

Targets master (1.12), rebased onto d44ed4f. release/v1.11 has the same race in the same four macros — happy to backport to 1.11.2.

[Benjamin-Knight]

Seems very edge case and perhaps just CI related under normal conditions. See the comment, I'm concerned we could block.

[axellpadilla]

The lock is session-scoped, so it survives until the connection closes, and dbt keeps that connection for the whole run. So one failed create blocks that schema for every other thread indefinitely, not for 30 seconds. Confirmed on 2022, both in autocommit and inside dbt's transaction.

Fixed: the create now sits in a TRY/CATCH that releases the lock and rethrows the original error. It's a finally, not a swallow — the error stays exactly as loud as before. The bit I wasn't sure about was whether you can even release a lock once XACT_ABORT has doomed the transaction. You can.

I tried the neater option first — give the guard its own BEGIN TRAN and use @LockOwner = 'Transaction', which frees itself on rollback and needs no CATCH. It doesn't work inside dbt's transaction: a nested BEGIN TRAN only bumps @@TRANCOUNT, so the lock is held until dbt commits. In get_test_sql that's after the test query runs, so data tests would serialize — a second test waited 2.6s behind a 3s test query, against 0.0s as it now stands.

test_failed_create_releases_the_lock covers it: fails on the old macro, passes on this one.

Improved PR claims