Skip to content

feat(transaction): implement the index management actions - #8645

Draft
wjones127 wants to merge 8 commits into
will/transaction-v2-actionsfrom
will/transaction-v2-index-actions
Draft

feat(transaction): implement the index management actions#8645
wjones127 wants to merge 8 commits into
will/transaction-v2-actionsfrom
will/transaction-v2-index-actions

Conversation

@wjones127

Copy link
Copy Markdown
Contributor

Stacked on the action vocabulary PR. Implements the three index-management actions the Transaction V2 draft left unwritten — AddIndexSegment, RemoveIndexSegment, and AdjustIndexCoverage — the lowering of the legacy CreateIndex operation onto them, and the conflict rule that lets two writers extend one index at once. No format change: the wire shapes these need land in #7954.

Supersedes #8630 and #8641, which are folded together here and whose heads were on a fork, so they could not be part of a GitHub stack.

The format has no first-class "index" separate from its segments — a logical index is the set of segments sharing a name — so the actions operate on segments. Creating an index and extending one are the same action; dropping an index is one removal per segment. Because a segment's fields, coverage, and base path are all Refs, one commit can now append data and index what it just appended, which previously took two versions.

Two writers can extend one index

Conflict detection previously treated a logical index as a single thing only one writer could touch: any two commits adding a segment to the same index collided. That was a faithful port of the legacy CreateIndex rule, which conflicts on index name alone.

But the query path unions the segments, so two writers indexing different fragments are both right and both should land. The name-level rule is replaced with a claim on the index — the same kind of pairwise footprint entry key assertions use. Two claims on one index conflict when they cover overlapping committed fragments, when either does not state what it covers (the system indices), or when they disagree about what the index is: its fields, its config, or its version. Fragments the operation mints itself are left out of the comparison, since they have no id yet and no concurrent writer can be covering one.

RemoveIndexSegment and AdjustIndexCoverage carry a name. The coverage adjustment needs it to say which index it is widening; both use it when applying, to reject an action whose segment turns out to belong to a different index than the one it was planned against.

Example

Appending a fragment and covering it with a new index segment, in one atomic commit — the index names the fragment by the token it was minted under, since the fragment has no id until the commit lands:

CompositeOperation::new(vec![UserAction::new("append and index", vec![
    Action::AddFragment(AddFragment { local: 0, physical_rows: 5, .. }),
    Action::AddDataFile(AddDataFile { fragment: Ref::Local(0), file, .. }),
    Action::AddIndexSegment(AddIndexSegment {
        name: "by_a".into(),
        covered_fragments: Some(vec![Ref::Committed(0), Ref::Local(0)]),
        ..
    }),
])])

Two writers each doing this against different fragments now both commit, and the index is the union of their segments.

Not included

AdjustIndexCoverage keeps the shape the draft gave it, including the note that coverage representation is still an open design area. It rejects a segment that records no coverage rather than treating "unknown" as an empty set to add to.

The remaining drafted actions (AddOverlays, RefreshRowVersionMetadata, UpdateCompactedSsTables, AssertUniqueKeys) are still unimplemented and still rejected on load.

Behavior differences from the legacy path

The CreateIndex lowering is verified by building the same manifest both ways and asserting the resulting index metadata is identical, but two edges the legacy path silently tolerates are rejected here: removing a segment the manifest does not have, and adding one whose uuid an existing segment already uses. Either means the operation was planned against a different set of segments than it is landing on.

wjones127 and others added 8 commits August 19, 2026 15:22
The index actions edit the index list as they are reached, so the list has
to be part of the state actions are applied against rather than an argument
the manifest assembly receives separately. ResetTable clears it where it
stands instead of setting a flag the assembly reads back.
The format has no first-class index apart from its segments, so one action
covers both creating an index and extending one: a logical index is the set
of segments sharing a name. Its fields, coverage, and base path are Refs, so
a segment can index what the same operation just wrote.

Three format changes fall out of implementing it:

- `covered_fragments` becomes an optional wrapper message. A bare `repeated`
  cannot tell "no coverage recorded" -- what the system indices carry, and
  what the query path treats as "serve this segment" -- from "covers no
  fragment", which it treats as "skip it".
- Added `base`, without which a segment imported from another dataset cannot
  be expressed.
- Added `created_at` and `dataset_version`, both describing the build rather
  than where it lands. `dataset_version` in particular is a correctness gate
  (an overlay committed at or before it counts as folded into the index) and
  a merged segment reflects only as much as its oldest input, so it is
  genuinely below the read version and cannot be derived. It defaults to the
  read version and may not exceed it.
Dropping a logical index is one of these per segment carrying its name,
since the format knows only segments. Removing a segment the dataset does
not have is rejected rather than treated as a no-op: it means the operation
was planned against a different set of segments.

Segments are named by uuid, which the writer picks, so the footprint
coordinate is the segment itself -- a concurrent writer extending the same
logical index adds a segment of its own and does not collide.
Moves fragments in and out of a segment's coverage without rewriting the
segment, which is what lets an append and the coverage extension over what
it appended commit as one operation.

A segment recording no coverage is rejected rather than treated as an empty
set to add to: "unknown coverage" is what the query path serves everything
for, so turning it into a concrete set would silently narrow the segment.
The legacy operation already carries its removals and additions as two
lists, so the recipe is one RemoveIndexSegment per removal followed by one
AddIndexSegment per addition. Parity tests build the same manifest down both
paths and assert the resulting index metadata is identical.

Two edges the legacy path tolerates are rejected here: removing a segment
the manifest does not have, and adding one whose uuid an existing segment
already uses. Either means the operation was planned against a different set
of segments than it is landing on.
Covers the three index actions through the real commit path: one commit
that appends a fragment and adds a segment covering it by local token, one
that swaps a segment out, and one that moves coverage around.
…itions

`AddIndexSegment`'s DeepSizeOf skipped `index_details` on the grounds that
it is opaque. It is only a type url and a byte string, so both are now
measured.

`AdjustIndexCoverage` did not say when adding a fragment to a segment's
coverage is legitimate. It is one case -- a rewrite moved rows the segment
already covered into a new fragment, which the segment reaches through the
fragment-reuse remapping. Adding a fragment of new rows is a writer error
that nothing here can detect, so it is called out.
Conflict detection treated a logical index as a single coordinate, so two
writers adding segments to the same index always collided. That was a port of
the legacy `CreateIndex` rule, which conflicts on index name alone. Since the
query path unions the segments of an index, two segments over disjoint
fragments are both valid and should both commit.

Replaces `Coordinate::IndexName` with an index claim, a pairwise footprint entry
rather than a coordinate. Two claims on one index conflict when they describe
overlapping committed fragments, when either does not state its reach, or when
they disagree about what the index is (fields, details, version). Fragments
minted in the same operation are left out of the comparison: they have no id a
concurrent writer could be covering.

`RemoveIndexSegment` and `AdjustIndexCoverage` use the `name` the wire format
carries. The coverage adjustment needs it to make its claim; both use it at
apply to reject an action whose segment belongs to a different index.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant