DuckLake: detect commit conflicts against concurrent writers, retry only when safe - #15
Open
fuziontech wants to merge 2 commits into
Open
DuckLake: detect commit conflicts against concurrent writers, retry only when safe#15fuziontech wants to merge 2 commits into
fuziontech wants to merge 2 commits into
Conversation
A DuckLake catalog orders every change on one chain of snapshots, so a commit has to claim the snapshot after the newest one. With a DuckDB ingest pipeline writing to the same catalog, Trino loses that race often. The connector already replayed the action when the database reported the race, but it never asked whether replaying was allowed: it re-based onto the newer snapshot whatever the other writer had done. An insert whose target table was altered underneath it registered data files that no longer described the table's columns. Apply the DuckLake conflict rules before the snapshot is written. The commit compares the changes it is about to record against the changes of every snapshot committed since the one the statement read, and refuses to land when the other writer touched an object the result depends on: inserted into T vs T dropped, altered, or deleted from deleted from T vs T dropped, altered, inserted into, or compacted altered T vs T dropped or altered dropped T vs T dropped These are DuckDB's rules for the same catalog, so a statement is accepted or rejected here exactly as it would be there. Two writers inserting into one table is not a conflict, which is the case that matters for an ingest pipeline running beside a transform. Changes recorded by name -- creating a schema, table, or view -- carry no rule, because the replay re-resolves the name against the newer catalog and reports the ordinary "already exists" or "not found" error there. A refusal now carries its own error code, DUCKLAKE_COMMIT_CONFLICT, so that a client such as SQLMesh can tell a lost race apart from a broken catalog. The retry bound and backoff become catalog properties, ducklake.commit.max-retries and ducklake.commit.retry-backoff. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
Parsing a foreign snapshot's changes threw IllegalArgumentException. That
is neither JdbiException nor ConcurrentModificationFailure, so it escaped
the commit retry loop and reached Trino as an unclassified internal error.
It is reachable without corruption: a newer DuckLake writer commits a
change type this connector does not know, and every commit whose read
snapshot precedes that row hits it.
Throw a TrinoException instead, and separate the two cases, which mean
different things to whoever reads the error:
DUCKLAKE_UNSUPPORTED_CHANGE_TYPE a newer DuckLake wrote a change type
this connector cannot interpret, so it
cannot tell whether committing on top
is safe. The message names the token
and says to upgrade the connector.
DUCKLAKE_INVALID_METADATA the row is not shaped like a change at
all, which is corruption.
Failing stays the right direction: ignoring an unknown change could drop
the other writer's work. Neither failure is retried, because reading the
row again reaches the same conclusion.
The malformed-entry path had the same defect and is fixed with it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
fuziontech
force-pushed
the
james/ducklake-commit-retry
branch
from
September 3, 2026 23:39
23cf907 to
29508b5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
40001/40P01/23505), but it re-based unconditionally: it never checked whether a snapshot committed after the statement's read snapshot conflicts with what this commit writes. AnINSERTwhose table was concurrentlyALTERed committed data files that no longer matched the columns.DuckLakeSnapshotChanges: parsesducklake_snapshot_changesfor every snapshot newer than the statement's pinned read snapshot and applies DuckLake's conflict rules (fromducklake_transaction_state.cppCheckForConflicts). Same-table insert vs insert and any change on other objects re-base and retry; insert/delete vs drop/alter/compact of the same table, and alter vs alter/drop, fail with a newDUCKLAKE_COMMIT_CONFLICTand are not retried (the statement must be replanned).DUCKLAKE_UNSUPPORTED_CHANGE_TYPEnaming the token and the remedy (upgrade the connector); corrupt rows fail withDUCKLAKE_INVALID_METADATA. Neither is retried.ducklake.commit.max-retries(default 10, DuckDB'sducklake_max_retry_count) andducklake.commit.retry-backoff(default 20 ms, doubled per attempt, capped at 32×).Design notes
INSERT INTO mart SELECT FROM rawwhile an ingest pipeline writesraw, which is the production workload.verifyDataFilesUnchanged/verifyDeleteFilesUnchanged, matching DuckDB.altered_table:<viewId>written for view comments.created_*) need no rule: the replay re-resolves names inside the fresh transaction and fails with the ordinaryALREADY_EXISTS/NOT_FOUND.Tests
TestDuckLakeConcurrentCommits(4): a foreign DuckDB snapshot is committed from inside the connector's commit action so attempt 1 deterministically loses the race — re-based onto an unrelated table (2 attempts, one data-file row, DuckDB reads it); re-based onto a concurrent insert into the same table (both rows land);DUCKLAKE_COMMIT_CONFLICTwith zero data-file rows when the target table was altered;DUCKLAKE_UNSUPPORTED_CHANGE_TYPEwhen a foreign snapshot carriesteleported_table:99.TestDuckLakeSnapshotChanges(11): the matrix, token parsing, unknown and corrupt entries.-P cichecks clean.Docs
ducklake.mdgains a "Concurrent writers" subsection and the two properties. (A separate PR rewrites the Limitations / SQL support sections; the two are meant to merge cleanly.)🤖 Generated with Claude Code
https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe