Repository navigation
feat(format): specify expiring pins for ongoing work - #10428
brendanclement wants to merge 3 commits into
Conversation
2ebb935 to
52fca11
Compare
|
Important Format specification voteThis PR modifies the Lance format specification, so it requires 3 binding +1 votes from PMC members (excluding the proposer), at least one of them on the latest commit, and a minimum 72-hour voting period, weekends excluded, before it can merge. Vote by approving this PR (+1) or requesting changes (−1, a veto). See the voting process. Approvals carry over across pushes, so a rebase or a typo fix does not send everyone back to re-vote. Whoever approves the latest commit is vouching that nothing substantive has changed since the earlier approvals; if something has, ask for fresh votes. Status: ❌ Blocked — 0 of 3 required approvals
Updated automatically by the format-spec vote gate, which re-checks every 15 minutes — just voted? Re-check now (press Run workflow; leave the input blank to re-check every open format PR). A PMC member may apply the |
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The clarified retention rules fit both FRI representations: tagged histories follow fragment lineage, retaining downstream dependencies and whole legacy records. The contract also retains mapping files added after registration.
#10440 reverted #9769’s runtime history protection on main. That does not block this specification while bit 19 remains unsupported. Activation must implement the specified retention checks in maintenance clients; the documented limits around destructive work already in flight still apply.
westonpace
left a comment
There was a problem hiding this comment.
Haven't finished. Some initial questions
| A version-1 history is called **tagged**. If it also contains legacy versions, | ||
| it is called **mixed**. |
There was a problem hiding this comment.
Are we trying to move away from the "tagged" terminology (in preference of "version 1")? I don't actually know. But yeah, Claude seems to like "tagged" (maybe we have a bunch of comments referring to it?)
| Once all dependent indices have caught up, the corresponding history can be | ||
| trimmed. Cleanup must retain intermediate transitions still needed to translate | ||
| old addresses. External mapping files can be deleted only when no retained | ||
| dataset version references them. |
There was a problem hiding this comment.
Can we fold this paragraph into the new section and just title it Cleanup?
| that union at the final commit target of every operation that trims or replaces | ||
| history, including work planned before registration. Restore and Overwrite | ||
| apply this check to the resulting current history; they may discard translation | ||
| paths while preserving current pins and their source snapshots. |
There was a problem hiding this comment.
This is very LLM-ish. It reads like a dump of dense facts. Can we provide a bit of motivation first?
Something like...
An FRI can be removed (cleaned up) once all artifacts referencing the source fragments have been removed. For example, an index that maps search results to fragments A and B will prevent an FRI from getting cleaned up when A and B are source fragments in the FRI. This is because the FRI is still needed to map the results of index searches.
Sometimes we also want to keep an FRI alive even when there are no artifacts referencing source fragments because those artifacts are currently being created by an in-progress task. For example, if an index is being trained against fragments A and B, this process may take several hours. We do not want the an FRI that maps A or B to be cleaned up while this training takes place.
To solve this problem we utilize ongoing-work pins...
| a monotonically increasing version number, and an optional reference to the index section that describes a list of index metadata. | ||
|
|
||
| [Ongoing work pins](ongoing_work_pins.md) record unfinished jobs' source snapshots | ||
| and FRI retention requirements in the current manifest. See that specification |
There was a problem hiding this comment.
Can we prefer "in-progress" over "unfinished"? When I see the latter term I think "failed" or "abandoned"
|
|
||
| [Ongoing work pins](ongoing_work_pins.md) record unfinished jobs' source snapshots | ||
| and FRI retention requirements in the current manifest. See that specification | ||
| for their lifecycle and the existing maintenance concurrency limits. |
There was a problem hiding this comment.
What does "existing maintenance concurrency limits" mean?
| inclusive source-version boundary. Tagged and mixed histories retain records | ||
| reachable from pinned source fragments, including all downstream dependencies |
There was a problem hiding this comment.
How is "pinned source fragments" determined? Do we read the pin's version and load all fragments at that version?
| /* UUID of the source manifest's committing transaction. | ||
| * Absent or empty is invalid; compare exact UTF-8 bytes. | ||
| */ | ||
| optional string source_transaction_uuid = 1; |
Compaction should be able to run while long-running jobs are in progress. The Fragment Reuse Index (FRI) records row movements from compaction so those jobs can account for changes to the table’s layout. Cleanup also needs to know which source snapshots and FRI history those jobs still depend on.
This proposal introduces expiring pins to record those retention requirements. Async index builds are the first consumer, but the mechanism is independent of the job type. Other long-running work, such as refresh-column jobs, can use the same pins to retain the resources it still needs.
Design discussion: #10395.
The format adds a current-manifest pin list and four transactions: registration, renewal, release, and expiration through ordinary cleanup. Pins retain source snapshots and the applicable FRI history. Compaction and history trimming check current pins before committing. Restore carries current pins forward, and branch deletion rejects pins it observes, including on the force path.
Force deletion also fails if the current manifest cannot be read or validated: Lance cannot establish that no pins exist. A corrupt head requires repair or administrator recovery outside ordinary branch deletion. This proposal adds no override for that check.
This PR contains the specification, protobuf definitions, and the minimum Rust changes needed to compile safely. The writer capability remains unsupported, and the four operations decode to
Operation::Unknownuntil implementation lands. Pin APIs, maintenance behavior, and job integration follow in separate PRs.Existing races involving cleanup, restore, and branch recovery remain outside this proposal. Registration does not cancel physical deletion already in flight.
This proposal uses writer bit 19 because #10352, #10358, #10384, and #10164 also propose bit 18. Bit 18 remains explicitly unsupported in this build until its separate feature is defined and implemented.