Hardening for clickhouse non-transactional DDL

[ringerc]

Problem

Clickhouse doesn't support transactional, multi-statement DDL, so atomic updates of the schema version along with a DDL mutating the schema are not presently possible.

See these diagrams for sequence diagram illustrations of the problem: https://gist.github.com/ringerc/b97904e8f1611c17ab9ebff1874643a0

No truly robust fix for this is possible, but goose can do more to defend against failures and detect them than is presently the case.

Ideally clickhouse itself would support some kind of durable, crash-safe DDL status tracking and reporting, but it doesn't (see "alternatives" at end), so workarounds client-side are required.

Note

This isn't LLM output, I've just tried to set out the problem and proposal clearly so there's a fair bit here. I did use an LLM for the mermaid diagram and to help with some research before prepping this proposal.

I'm happy to attempt a PR for this if the proposal sounds acceptable.

Feature proposal

I propose an intent-log for DDL execution on non-transactional DDL systems, a migration flag to identify idempotent migrations that can be safely retried blind, and a cli tool to let humans tell goose what to do next for non-retryable migrations.

Specifically:

• An intent-log table for migrations that's written to before each DDL statement is run, so goose can detect if there was a crash or interruption between when goose sends the DDL and gets the server's response;
• Support for an IDEMPOTENT flag in migration files for migrations identifying them as safe to re-run even if their prior state is unknown;
• Logic to check the intent log for interrupted DDL, re-run DDL if known/marked safe to retry, or bail with an error indicating that human intervention is required
• CLI sub-command to report current migration progress/state;
• CLI sub-command to force an interrupted migration in an indeterminate state into a resumable, determinate state after human examination of the db state determines which is correct; and
• Docs warning that idempotent DDL is vital for CH, and that non-idempotent DDL can leave the system in a state that requires manual intervention

A future enhancement might be to also check the clickhouse system.query_log to discover if a DDL was received and executed by CH but goose never got the success confirmation. But that'd be hard to make robust, and I'd ticket that separately anyway.

Why

Today, in no-tx mode, statements can partially apply before failure, but goose only records final version state.

That makes incident response and manual recovery harder than needed because tools and users have no structured,reliable way to tell the difference between "migration never started" and "migration ran a statement successfully but was interrupted before recording confirmation in the db". Goose will therefore blindly re-run all migrations but it has no way to know whether this is safe for any given migration, or to even detect a prior failed migration attempt.

This proposal does not claim perfect resumability; it improves visibility and post-failure diagnosis. To support perfectly recovery without human intervention, clickhouse itself would need to provide a reliable means of asking the database whether or not a given specific DDL statement executed or not. This doesn't currently appear to be possible.

Proposed behavior

• Feature flag (off by default), e.g. --progress-tracking.
• New metadata table, e.g. goose_migration_runs with fields:
  • migration_version
  • direction (up/down)
  • status (running / failed / retry)
  • last_completed_stmt_index
  • started_at, updated_at, completed_at
  • error_text (nullable)

Execution flow in no-tx mode with progress tracking enabled (see flow-diagram below for pictorial form):

1. connect and check for the presence of a goose_migration_runs row for the current migration version;
   1. if found, check the migration for an IDEMPOTENT flag;
      1. if idempotent flag found, continue migration run by re-running statement after last_completed_stmt_index;
      2. otherwise, if status == resume, the user has manually intervened to tell goose the statement at last_completed_stmt_index+1 never ran, so it's safe to retry it; do so;
      3. otherwise abort with an error demanding human intervention and pointing to docs/cli options
   2. if no status row found, insert run row (status = running, last_completed_stmt_index = -1)
2. execute statement at last_completed_stmt_index+1
3. on success, update last_completed_stmt_index; on error, set failed + error_text (if connection state allows) then terminate;
4. loop until no statements remain in the migration
5. in one txn delete the progress tracking row and update the main schema version table

A new goose cli subcommand is added to print out the migration that was interrupted, with a marker for the DDL statement that's indeterminate. So the user can see what exactly was pending, and translate the statement index to a specific position in the migration file.

Another new goose cli subcommand is added to let the user recover from non-idempotent migrations that are interrupted mid-execution by telling Goose they either confirmed the statement did not run, it ran successfully, or did re-ran it themselves. It should update the run state/progress table to mark the indeterminate-state command as successful by incrementing last_completed_stmt_index or marking the whole migration as successful depending on what the user claims on cli input. The command must warn that it _must only ever be run once the DB's state matches what the migration script expected it to be.

Compatibility

• Backward-compatible by default (feature disabled).

References

• Existing extension TODO in store layer:
  • https://github.com/pressly/goose/blob/master/database/store.go
  • references: Possibility to save migration file name #422, feature: add additional columns to goose table #288
• Partial-apply error model already exists (but has no way to represent "unknown" as an outcome):
  • https://github.com/pressly/goose/blob/master/provider_errors.go
• No-transaction behavior docs:
  • https://github.com/pressly/goose/blob/master/README.md
• Current no-tx execution path:
  • https://github.com/pressly/goose/blob/master/migration_sql.go
• Parser currently returns statements without line metadata:
  • https://github.com/pressly/goose/blob/master/internal/sqlparser/parse.go
• Related status-model direction:
  • fix: include DB-only applied versions in Status as untracked #1096
• Related non-transactional DDL context:
  • fix: StarRocks version table initializing — fallback to NoTx #1100

---

Execution flow diagram

Main no-tx flow with --progress-tracking enabled:

flowchart TD
    Start([Start migration run]) --> Connect[Connect to DB]
    Connect --> CheckRow{Row exists in<br/>goose_migration_runs<br/>for this version?}

    CheckRow -- No --> Insert[Insert run row<br/>status=running<br/>last_completed_stmt_index=-1]
    Insert --> Exec

    CheckRow -- Yes --> Idem{Migration has<br/>IDEMPOTENT flag?}
    Idem -- Yes --> Resume[Resume: re-run statement at<br/>last_completed_stmt_index + 1]
    Resume --> Exec

    Idem -- No --> StatusResume{status == resume?<br/>human intervened}
    StatusResume -- Yes --> RetryOne[Retry statement at<br/>last_completed_stmt_index + 1<br/>user attests it never ran]
    RetryOne --> Exec
    StatusResume -- No --> Abort[Abort with error:<br/>human intervention required<br/>point to docs/CLI]
    Abort --> End([Terminate])

    Exec[Execute statement at<br/>last_completed_stmt_index + 1] --> Result{Statement<br/>result?}
    Result -- Success --> Bump[Update last_completed_stmt_index]
    Bump --> More{More statements<br/>in migration?}
    More -- Yes --> Exec
    More -- No --> Finalize[In one txn:<br/>delete progress row +<br/>update schema version table]
    Finalize --> Done([Migration complete])

    Result -- Error --> Fail[Set status=failed<br/>record error_text<br/>if conn state allows]
    Fail --> End

Loading

CLI-assisted recovery for non-idempotent, interrupted migrations:

flowchart LR
    Interrupted[Interrupted migration<br/>in indeterminate state] --> Report[goose CLI: report<br/>show pending migration &<br/>indeterminate DDL stmt]
    Report --> Human[Human inspects DB state]
    Human --> Recover[goose CLI: recover<br/>user tells goose:<br/>- stmt did not run<br/>- stmt ran successfully<br/>- user re-ran it]
    Recover --> Update[Update goose_migration_runs:<br/>bump last_completed_stmt_index<br/>or mark migration complete<br/>or set status=resume]
    Update --> Resumable([Resumable determinate state])

Loading

Alternatives

Clickhouse does have a weak mechanism of determining the outcome of a DDL that's dispatched by a client that then loses its connection. But the method is not officially supported, fragile, limited on clustered clickhouse, and ineffective if clickhouse itself crashes/restarts.

The workaround is to:

• generate a magic cookie for this specific ddl execution and store it somewhere durable (in another table in CH, on local disk, etc)
• put the magic cookie into a log_comment session variable when running the DDL, e.g. SET log_comment = 'my-magic-cookie' or protocol-level equivalent
• if execution is interrupted, on resume when a pending-ddl with unresolved magic-cookie state is discovered, force a flush of the system.query_log then check the system.query_log for a query with matching cookie
• if not found, assume the query never actually reached clickhouse for execution or was lost in a crash, and retry;
• if found, check the type field and use that to determine the query's outcome

but this is broken, because it's not safe to assume that the absence of the query in system.query_log means it never ran or at least never completed. Clickhouse can run the DDL, commit, and crash/restart before persisting the system.query_log record for the query. The query log could also be pruned/purged or disabled.

So type = 'QueryFinish' definitely confirms an orphaned DDL ran, but there's no way to be sure it didn't run for the other states.

Supporting LLM research output: clickhouse-ddl-tag-feasibility.md

Until/unless Clickhouse upstream provides a better solution, this ticket describes a workaround.

[mfridman]

Thanks for putting this together. There's a bit to digest here, so I'll need time to properly sit down and comb through it.

There's certainly a lot of room for improvement when it comes to ClickHouse. When it was first added, it solved one use case, but since then more and more requests and issues have come in. But no substantial work has been done yet. My main concern is a game of whack-a-mole where each ClickHouse request/issue addresses a narrow problem without tackling improvements holistically.

E.g., #869 and a few other random PRs.

[ringerc]

@mfridman Thanks for taking a look.

The alternative to whack-a-mole is the mega-PR-of-doom approach which can be difficult to review and understand. Sometimes incremental change does need to be guided by a higher-level vision, sure, but sometimes whack-a-mole gets us there.

I'm using a custom dialect that adds bare-bones clickhouse cluster support (creating the schema version table as a clustered table) at the moment so that issue is of interest to me too. Another one would be support for waiting for system.mutations for async work to complete, and waiting for zookeeper to confirm that cluster-replicated DDL has applied across the whole cluster. But I figured I'd start with the reliable DDL issue.

(I'm also looking at switching from the expensive update based schema version change to an insert based approach that's much more clickhouse friendly)

BTW, the intent-log approach I suggest here would continue to be beneficial even if Clickhouse improves their DDL handling upstream by adding a way for apps to retrieve the status of DDL after sending it. The app is most likely going to need to persist an identifier to associate with the specific DDL execution request, which will look at lot like this proposed intent log.

[ringerc]

See also these sequence diagrams for a visual explanation of the underlying clickhouse problem that might help clarify things. https://gist.github.com/ringerc/b97904e8f1611c17ab9ebff1874643a0

This clickhouse issue covers the upstream issue ClickHouse/ClickHouse#95963