feat(transaction): implement the remaining drafted actions - #8646
Draft
wjones127 wants to merge 9 commits into
Draft
feat(transaction): implement the remaining drafted actions#8646wjones127 wants to merge 9 commits into
wjones127 wants to merge 9 commits into
Conversation
Appends overlay files to a fragment, supplying new values for a subset of its (row offset, field) cells without rewriting its base data files. Each overlay's `committed_version` is stamped with the version the commit produces, so a retry against a newer manifest re-stamps rather than backdates. Overlays are appended, never replaced, so the action writes no coordinate of its own -- two concurrent overlays over the same cells both land and the newer version wins. It does record that the fragment must still be there, which is a new kind of entry in the footprint: a dependency rather than a write.
Restamps the per-row `last_updated_at_version` sequence of fragments whose columns were rewritten in place, which is what a legacy Merge does implicitly. Nothing else in an operation restates when those rows last changed, because rewriting columns in place leaves the rows where they are. `created_at_version` is left alone: the rows are the same rows, and a row this operation mints gets both stamps from the AddFragment that minted it. Naming a fragment on a dataset without stable row ids is rejected rather than fabricating sequences that have nowhere to live.
Records which MemWAL SSTables have been compacted into the base table, in the MemWAL system index. Per shard the highest generation wins, so replaying an older commit over a newer one cannot walk the progress backwards. The rows were already readable through the WAL, so this is bookkeeping about where they live rather than a change to them. The drafted `update_compacted_sstables` oneof field is renamed to `update_compacted_ss_tables` so the generated variant name matches the message name, which is what the action vocabulary keys the wire encoding off. The tag is unchanged.
A precondition rather than a delta: the keys this operation inserts must not collide with keys a concurrent commit inserted. The key columns are an unenforced primary key, so nothing in the manifest records which keys exist -- the filter of inserted key hashes has to travel with the operation because it cannot be recovered from any post-image. This is the first thing two footprints compare that is not a coordinate, so the footprint grows a row-insertion marker (set by an AddFragment that is a data change) and the assertions themselves. Two sets are compatible when both say which keys they insert, over the same columns, and the filters provably do not intersect; an unqualified insert, different key columns, or filters built with incomparable parameters all leave the assertion unverifiable, which counts as a conflict. With this the implemented vocabulary covers the whole draft, so the "drafted but not implemented" rejection has nothing left to reject and is replaced by one for an action written by a newer Lance -- which protobuf decodes as no variant at all.
…ctions Both lower onto a single new action each, so the parity tests build the same manifest twice -- once down the legacy path, once through the actions -- and assert they agree. They agree except in one place: the legacy UpdateMemWalState arm never carries the read version's fragments into the manifest it builds, so it empties the table. The translated path leaves the data alone, which is what the operation means. The test asserts both, so the difference is recorded rather than hidden.
Four commits through the real commit path: appending a fragment and overlaying an existing one in the same version, restamping row versions for a fragment whose column was rewritten in place, recording MemWAL compaction progress, and carrying a key assertion alongside the insert it guards.
`Operation::UpdateMemWalState` now carries `require_index_catchup`. Requiring catch-up is a one-way feature-flag migration with no action of its own, so an operation asking for it does not translate; ordinary progress updates translate as before. `update_mem_wal_index_compacted_sstables` also stopped creating the index and stopped tolerating a stale generation, so `UpdateCompactedSsTables` now inherits both rejections. Its docs and tests say so, and the tests seed the index the way a real table would have it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment claimed rows are not a coordinate because a row a concurrent writer inserts has no id anyone could name. Both halves are wrong: rows do have ids, and a new data file can carry rows that already existed. The actual reason is that two writers inserting the same user-supplied key write it into fragments of their own, so their coordinates stay disjoint however badly the keys collide. Also records that the flag over-approximates -- the rows a merge insert updates arrive in a new fragment too -- and why `Update` has no translation yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An update that moves rows -- out of the fragments they were in, by deletion file or by the fragment going away, and into fragments it mints -- is a `Delete` and an `Append` in one step, plus the inserted-key assertion and the SSTable progress it carries. It now translates, reusing the recipes those two operations already have. Its other forms are rejected, for the same reason `Merge` and `Project` are: they turn on what the read version holds, which a conversion taking only the operation cannot see. A row rewrite decides which indices still cover the rows it moved by reading the current indices, schema and overlays; a column rewrite tombstones overlaid fields by reading the overlays a fragment carries now; in-place field modification would have to diff against the read version to tell the data files it wrote from the ones already there; and a partial restamp has no action, since RefreshRowVersionMetadata restamps a whole fragment. The conflict resolver gets this for free: a legacy vertical update concurrent with an action set is now compared by footprint instead of being conservatively rejected, so a deletion and a column rewrite of one fragment both commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Stacked on the index-actions PR. Implements the four actions the Transaction V2 draft still listed as unwritten —
AddOverlays,RefreshRowVersionMetadata,UpdateCompactedSsTables, andAssertUniqueKeys— plus the lowering of the three legacy operations that map onto them:DataOverlay,UpdateMemWalState, and the vertical form ofUpdate. With this the implemented vocabulary covers the whole draft. No format change: the wire shapes land in #7954.Supersedes #8632, whose head was on a fork and so could not be part of a GitHub stack.
Three of the four actions are ordinary deltas. The fourth,
AssertUniqueKeys, is the first thing in the vocabulary that is a precondition rather than a change: it carries the filter of key hashes a merge insert is about to insert, so that two concurrent inserts can be shown not to collide. Nothing in the manifest records which keys exist — the key columns are an unenforced primary key — so the filter has to travel with the operation, because it cannot be recovered from any post-image.Example
Appending rows and asserting their keys are new, in one commit. If a concurrent commit inserted rows without saying which keys they carry, this one is rejected, because there is nothing to compare against:
Conflict rules
Two of the new actions needed something the footprint could not previously express.
An overlay writes no coordinate at all. Overlays are appended, never replaced, and when two cover the same cell the newer
committed_versionwins — so two concurrent overlays must not collide with each other. What an overlay does need is for its fragment to still be there, which is a dependency rather than a write, and is now tracked as one.A key assertion is not a coordinate either. The keys are user-supplied, so two writers can insert the very same key while writing it into fragments of their own — their coordinates stay disjoint however badly the keys collide. Uniqueness is a claim about values, not about structure. The footprint gained a row-insertion marker and the assertions themselves. Two sets are compatible when both say which keys they insert, over the same columns, and the filters provably do not intersect; an unqualified insert, different key columns, or filters built with incomparable parameters all leave the assertion unverifiable, which counts as a conflict. The marker is set by any
AddFragmentthat is a data change, which over-approximates: the rows a merge insert updates arrive in a new fragment too and carry no new key, so two writers who only ever touched existing keys can still conflict.Translating
UpdateUpdatetranslates in its vertical form — rows leaving the fragments they were in and arriving in new ones, which is aDeleteand anAppendin one step, plus the key assertion and the SSTable progress it carries. ItsRewriteRowsandRewriteColumnsforms are rejected, for the same reasonMergeandProjectare: what they mean depends on what the read version holds, and the translation only sees the operation. Rejection falls back to the conservative always-retry, so nothing is approximated.Behaviour differences from the legacy path
The legacy
UpdateMemWalStatearm of the manifest build never carries the read version's fragments into the manifest it produces, so committing it against a non-empty table empties it. The action path leaves the data alone, which is what the operation means. The parity test asserts both, so the difference is recorded rather than hidden; the legacy bug is untouched here.A key assertion is compared symmetrically, so a plain append concurrent with a merge insert conflicts in both directions. The legacy check only runs from the retrying transaction's side, so an append that lands after a merge insert is currently allowed through and can introduce a duplicate key.
An update and a column rewrite of the same fragment no longer conflict. Which rows are gone and what a column holds are separate facts about a fragment, so writing a deletion file and rebinding a field's data both land; the legacy operation pairing had to reject this.
Not included
MergeandProjectremain untranslatable from the operation alone, as do the two rewrite forms ofUpdate, for the reason above.