dbt-labs / dbt-labs/dbt-adapters

[dbt-redshift] Opt-in config to run persist_docs in its own transaction (reduce lock contention on wide tables)

Open
#2,022 1 comment 0 reactions 0 assignees View on GitHub
feature:performance feature:transactions pkg:dbt-redshift type:enhancement
Dominant language
Python
Stars
233
Forks
362
Avg merge
3d 22h
Merged PRs (30d)
9

Description

### Summary

On dbt-redshift, `persist_docs` emits its `COMMENT ON TABLE` / `COMMENT ON COLUMN` statements **inside the model build transaction**, right before `adapter.commit()`. On very wide tables the comment phase is long, and the locks those statements hold aren't released until the build commits. Under high concurrency this serializes parallel models: their `ALTER … RENAME` can't commit until the documenting transaction releases its locks.

This issue proposes a new **opt-in** config for dbt-redshift that runs `persist_docs` in its **own dedicated transaction**, after the build transaction commits — keeping the build swap atomic while taking the comment locks out of the critical path.

### Background / where this happens

`persist_docs()` is called inside the build transaction in every materialization:

- `table` — `dbt-redshift/.../materializations/table.sql` (Redshift override), `persist_docs` immediately before `adapter.commit()`
- `view` — `dbt-redshift/.../materializations/view.sql`
- `incremental` — base `dbt-adapters/.../materializations/models/incremental/incremental.sql` (Redshift does not override the materialization, only the strategies)
- `snapshot` — base `dbt-adapters/.../materializations/snapshots/snapshot.sql`

`redshift__persist_docs` already batches column comments into a single `run_query`, so batching is not the lever — the **transaction boundary** is.

### Impact (reported by a user)

Two parallel models on wide tables (large column counts):

- `persist_docs` **off**: ~130s / ~130s
- `persist_docs` **on**: ~154s / ~296s — clear serialization from lock contention

Across a 12-thread overnight run this contention compounded and contributed to a redline. The user has had to disable `persist_docs` as a stop-gap, losing table/column descriptions in Redshift and downstream tools.

### Existing workaround and why it's not enough

`redshift_skip_autocommit_transaction_statements: True` (default `False`) drops the explicit `BEGIN`/`COMMIT` so every statement auto-commits and locks release immediately. It resolves the contention, but it removes atomicity for the **entire** materialization (the rename/swap is no longer transactional), which is a heavier trade-off than the problem requires.

### Proposal

Add a new opt-in behavior flag for dbt-redshift — e.g. `redshift_persist_docs_own_transaction` (default `False` for backwards compatibility). When enabled:

1. The build transaction commits as normal (swap stays atomic).
2. `persist_docs` then runs in its **own** transaction: `BEGIN` → relation comment → column comments → `COMMIT`.

This keeps the docs atomic *as a group* (no half-documented relations within the persist step) while removing the comment locks from the build transaction, so parallel models' renames are no longer blocked.

### Scope / open questions

- Cover all four materializations consistently (`table`, `view`, `incremental`, `snapshot`). `table`/`view` are already Redshift-overridden; `incremental`/`snapshot` would need Redshift overrides or another mechanism — partial coverage would be confusing.
- **Atomicity change (by design, opt-in):** if a comment fails, the relation is already committed, so it can exist briefly without (some) docs while the run still reports failure. Acceptable for an opt-in flag; should be documented.
- **`autocommit: false` profiles:** behavior must be validated — running `persist_docs` in its own explicit `BEGIN`/`COMMIT` should make this well-defined regardless of the connection's autocommit setting.
- Lock relief is on **duration**, not elimination — the comment statements still take short, isolated locks.

### Alternatives considered

- **Unconditionally** moving `persist_docs` after commit (no flag): simpler, but changes default behavior for everyone (docs no longer atomic with the build). Preferring an opt-in flag.
- Recommending `redshift_skip_autocommit_transaction_statements`: usable today, but broader semantic change as noted above.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.