Skip to content

direct: Fix WAL corruption after two consecutive failed deploys - #5598

Closed
denik wants to merge 4 commits into
mainfrom
denik/wal-bug
Closed

direct: Fix WAL corruption after two consecutive failed deploys#5598
denik wants to merge 4 commits into
mainfrom
denik/wal-bug

Conversation

@denik

@denikdenik commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Changes

Two failed deploys in a row left the direct-engine state WAL with a serial
ahead of the committed state, after which every bundle command failed WAL
recovery (WAL serial (N) is ahead of expected) until the WAL was deleted by
hand.

  • Don't open the WAL for write when planning already failed, so a failed plan
    no longer leaves a header-only WAL behind.
  • Recovering a header-only WAL no longer advances the serial, so a crash
    between UpgradeToWrite and Finalize can't wedge later deploys either.

Why

Previously error in plan or deploy could leave bundle locally undeployable until .wal is manually removed.

Tests

  • bundle/deploy/wal/two-failed-deploys: two plan failures (injected fault) no
    longer leave a WAL; the next deploy succeeds.
  • bundle/deploy/wal/two-crashed-deploys: two deploys killed mid-apply recover
    without wedging.
  • Unit TestHeaderOnlyWALRecoveryDoesNotAdvanceSerial. Each test was confirmed
    to fail when its corresponding fix is reverted.

This pull request and its description were written by Isaac.

@github-actions

github-actionsBot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

10 files changed
Suggested: @janniklasrose
Also eligible: @pietern, @shreyas-goenka, @lennartkats-db, @andrewnester, @anton-107

/bundle/ - needs approval

Files: bundle/direct/dstate/state.go, bundle/direct/dstate/state_test.go, bundle/phases/deploy.go
Suggested: @janniklasrose
Also eligible: @pietern, @shreyas-goenka, @lennartkats-db, @andrewnester, @anton-107

General files (require maintainer)

Files: NEXT_CHANGELOG.md
Based on git history:

  • @pietern -- recent work in ./, bundle/direct/dstate/, bundle/phases/

Any maintainer (@andrewnester, @anton-107, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db) can approve all areas.
See OWNERS for ownership rules.

@denik
deniktemporarily deployed to test-trigger-is June 14, 2026 22:40 — with GitHub Actions Inactive
@denik
deniktemporarily deployed to test-trigger-is June 14, 2026 22:40 — with GitHub Actions Inactive
@denik
deniktemporarily deployed to test-trigger-is June 14, 2026 22:48 — with GitHub Actions Inactive
@denik
deniktemporarily deployed to test-trigger-is June 14, 2026 22:48 — with GitHub Actions Inactive
@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 9d377c6

Run: 27514518440

Env💚​RECOVERED🙈​SKIP✅​pass🙈​skipTime
💚​aws linux7152649797:48
💚​aws windows71526697711:59
💚​aws-ucws linux7153608937:18
💚​aws-ucws windows71536289112:13
💚​azure linux1172679778:08
💚​azure windows11726997511:12
💚​azure-ucws linux1173658897:06
💚​azure-ucws windows11736788712:27
💚​gcp linux1172639807:40
💚​gcp windows11726597813:20
22 interesting tests: 15 SKIP, 7 RECOVERED
Test Nameaws linuxaws windowsaws-ucws linuxaws-ucws windowsazure linuxazure windowsazure-ucws linuxazure-ucws windowsgcp linuxgcp windows
💚​TestAccept💚​R💚​R💚​R💚​R💚​R💚​R💚​R💚​R💚​R💚​R
🙈​TestAccept/bundle/invariant/no_drift🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/permissions🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
💚​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions💚​R💚​R💚​R💚​R🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
💚​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct💚​R💚​R💚​R💚​R
💚​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform💚​R💚​R💚​R💚​R
💚​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions💚​R💚​R💚​R💚​R🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
💚​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct💚​R💚​R💚​R💚​R
💚​TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform💚​R💚​R💚​R💚​R
🙈​TestAccept/bundle/resources/postgres_branches/basic🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_branches/recreate🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_branches/replace_existing🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_branches/update_protected🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_branches/without_branch_id🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_endpoints/basic🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_endpoints/recreate🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/postgres_projects/update_display_name🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/synced_database_tables/basic🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/vector_search_indexes/basic🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/bundle/resources/vector_search_indexes/grants/select🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
🙈​TestAccept/ssh/connection🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S🙈​S
Top 30 slowest tests (at least 2 minutes):
durationenvtestname
6:16aws windowsTestAccept
6:07azure windowsTestAccept
6:05gcp windowsTestAccept
5:58aws-ucws windowsTestAccept
5:57azure-ucws windowsTestAccept
4:48gcp linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:17gcp linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
4:07gcp windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:55gcp windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:22aws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:20azure windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:15aws-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:12azure-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:12azure linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:11aws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:59azure windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:58aws linuxTestAccept
2:54azure linuxTestAccept
2:51gcp linuxTestAccept
2:48aws-ucws linuxTestAccept
2:46azure-ucws linuxTestAccept
2:43azure-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:43azure-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:43azure linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:42aws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:38aws-ucws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:28aws-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:28aws windowsTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:25aws-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:23azure-ucws linuxTestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct

denik added 4 commits June 15, 2026 06:32
Two consecutive failed deploys left the local state WAL with a serial
ahead of the committed state, after which every bundle command failed
WAL recovery until the WAL was deleted by hand.
- Don't open the WAL for write when planning already failed, so a failed
plan no longer leaves a header-only WAL behind.
- Don't advance the serial when recovering a header-only WAL, so a crash
between UpgradeToWrite and Finalize can't wedge later deploys.
Co-authored-by: Isaac
InitForApply receives ctx and could log a diagnostic without returning an
error, so the call site cannot prove it never will. Re-check logdiag before
deploying. UpgradeToWrite takes no ctx and thus cannot log, so the earlier
check alone is enough to guard opening the WAL.
Co-authored-by: Isaac
Drop the hand-written resources.json.tmpl so the test no longer depends on
the internal state-file format. Deploy the job normally, then inject a fault
on the plan-stage refresh GET so the next two deploys fail while planning and
the last one recovers.
Co-authored-by: Isaac
@denik

Copy link
Copy Markdown
ContributorAuthor

Superseded by two focused PRs: #5606 (dstate: header-only WAL recovery no longer advances the serial — the fix that closes #5557) and #5607 (deploy phase: don't open the WAL when planning fails).

@denikdenik closed this Jun 15, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@denik@eng-dev-ecosystem-bot