Skip to content

Clean up transactions after metadata rollback failures#1092

Open
bill-ph wants to merge 1 commit into
duckdb:mainfrom
bill-ph:fix/metadata-rollback-connection-cleanup
Open

Clean up transactions after metadata rollback failures#1092
bill-ph wants to merge 1 commit into
duckdb:mainfrom
bill-ph:fix/metadata-rollback-connection-cleanup

Conversation

@bill-ph
Copy link
Copy Markdown

@bill-ph bill-ph commented Apr 29, 2026

Summary

Relevant: #1023

Fix DuckLake rollback cleanup when rolling back the metadata catalog connection fails.

Before this change, failures during metadata rollback could interrupt cleanup and leave transaction state behind. This was especially visible with Postgres metadata catalogs when the metadata backend connection was terminated: DuckLake could fail while trying to rollback, skip connection reset/local file cleanup, and leave the transaction registered in the transaction manager.

This change:

  • factors metadata rollback into RollbackAndResetConnection()
  • always resets the metadata connection after a rollback attempt
  • preserves rollback errors and reports them to the caller
  • ensures local file cleanup and local change cleanup still run before reporting rollback failure
  • ensures DuckLakeTransactionManager::RollbackTransaction() erases the transaction before rethrowing rollback errors

Tests

Added Postgres metadata catalog sqllogictests for:

  • commit failure where metadata rollback also fails, asserting both the commit error and rollback error are surfaced
  • explicit ROLLBACK after terminating the metadata backend, asserting the rollback error is surfaced and the attachment remains usable afterward
  • cleanup of locally written parquet files after rollback failure

Ran:

  • cmake --build build/release --target unittest
  • ./build/release/test/unittest --test-config test/configs/postgres.json --test-dir ./ test/sql/transaction/postgres_metadata_rollback_error.test
  • ./build/release/test/unittest --test-config test/configs/postgres.json --test-dir ./ test/sql/transaction/postgres_metadata_explicit_rollback_error.test
  • ./build/release/test/unittest --test-dir ./ test/sql/transaction/transaction_conflict_cleanup.test

@bill-ph bill-ph changed the title Handle metadata rollback connection failures Report and reset failed metadata rollback connections Apr 29, 2026
@bill-ph bill-ph marked this pull request as draft April 29, 2026 20:31
@bill-ph bill-ph force-pushed the fix/metadata-rollback-connection-cleanup branch 2 times, most recently from 6913bd8 to f0fbbe3 Compare April 30, 2026 01:01
@bill-ph bill-ph force-pushed the fix/metadata-rollback-connection-cleanup branch from f0fbbe3 to 1621ae0 Compare April 30, 2026 02:18
@bill-ph bill-ph marked this pull request as ready for review April 30, 2026 02:56
@bill-ph bill-ph changed the title Report and reset failed metadata rollback connections Clean up transactions after metadata rollback failures Apr 30, 2026
@bill-ph
Copy link
Copy Markdown
Author

bill-ph commented Apr 30, 2026

The CI failure seems unrelated, these are existing tests, and are passing in other archs, can you rerun the failed CI job, @pdet ?

Sign up for free to 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.

1 participant