Repository navigation
Conversation
…erlap A branch worker held one commit window, so a second author's write closed the first author's window (identity_change) and split their change. Stage 2 of docs/design/concurrent-author-windows.md keeps a window per author of a GitTarget, side by side, while their footprints are proven disjoint. - commitWindows holds an ordered list of windows (open order = seq); the loop no longer embeds it, and tests read it through only()/bytes(). - footprint.go proves the set of files a write may change against a revision (resets, own commits, GitTarget generation). An object alone in its file, ungoverned by any kustomization, with no placement template, in a bootstrapped folder, is proven; anything else is unknown and serial. - window_admission.go: unknown footprints and other GitTargets close every other window (identity_change); an overlapping footprint closes the window that holds the file first (path_conflict). Before a write is admitted beside others, every open window is proven again (FootprintReproved). - A window once open beside another is checked before its first commit: every staged path must be inside its footprint, proven again on the tree the commit is planned on. A violation undoes the worktree, fails the write and its save, falls back to one window for the worker's life (SingleWindowFallback) and asks a resync of the GitTarget. - finalizeWindow arms the commit timer for the remaining windows instead of stopping it; attachWaitingCommitRequests skips a window an earlier attach closed. - Log lines carry the window ID and branch; path_conflict is a new close_reason. - The reference model grows to several windows, a kustomize root and a shared multi-document file, and checks footprints, every changed path and whole documents (yaml.v3) on every commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…age 3 configuration.md says which writes keep a window per author and which stay serial, and that commits land in the order their windows close. interpreting-metrics.md documents close_reason="path_conflict". The design page records what stage 2 built. Stage 2 ships without a flag, so TODO.md holds the next release for stage 3, the failure boundaries against the reference model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Local |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Stage 2 of concurrent-author-windows.md: a branch worker keeps a commit window per author of a GitTarget, side by side, while their writes are proven to change different files. Two people editing different objects in their own files get one commit each, instead of the second write closing the first person's window (
identity_change) and splitting their change. Stacked on #466.What stays serial
A window's footprint is the set of Git files its writes may change (
internal/git/footprint.go). Two windows sit side by side only while their footprints are proven disjoint; anything not proven is unknown, and unknown overlaps everything. A write is proven only when, on the checkout its commit is planned on:placement.default, nouseKustomize, and noplacement.byTypetemplate for the kind (a template can bundle objects into one file);spec.path, feeds noconfigMapGenerator, and holds no kustomization the writer does not model;resources:orpatches:); a create that placement routes into a kustomize root is unknown;Admission (
window_admission.go): an unknown footprint, a write of another GitTarget, or a worker that fell back closes every other window first (identity_change, as today). A proven write that overlaps another window's footprint closes that window first (path_conflict, a newclose_reason), so its older write commits before the newer one. Same object means same document in any one tree, so two windows never hold one object.Proof per revision
A proof is made against a revision: the count of the worker's resets and own commits, plus the GitTarget's generation. Before a write is admitted beside others, every open window whose proof is of another revision is proven again from all its writes (
FootprintReproved); windows whose new proofs overlap close first, oldest first, withpath_conflict. A window is proven only once it has company: a lone window stays "unproven", costs no scan, and its commit is not checked. Once proven, its footprint covers every write it collects.Check before the commit
A window that was ever open beside another carries its footprint onto its decided write. Right before the write's first commit, once the checkout holds every write before it, the footprint is proven again on that exact tree (
proveBeforeCommit): the windows its admission closed commit first and can move what it changes (a delete committed first turns its update into a create). Then every path the commit would change, read from the index against HEAD (notwriteBatch.touched), must be inside it. A violation is a footprint bug and is refused, not committed: the batch's worktree is undone, the write fails as terminal and its save fails with the paths in the message, the worker logsSingleWindowFallbackand holds one window at a time for the rest of its life, and a resync of the GitTarget is asked for. A proof of another tree (after a reset, or a GitTarget change) says nothing about this one, so that commit is not checked. Replays are never checked: their order is already fixed.Other changes
commitWindowsis an ordered list (open order =seq) and is no longer embedded in the loop; tests read it throughonly()/bytes()(and fake one withsetOnly).finalizeWindowarms the commit timer for the earliest deadline of the windows left open instead of stopping it;commitTimerAtstays accurate.attachWaitingCommitRequestsskips a window an earlier attach in the same pass closed (with several windows that leaked a request onto a closed window and its charge).window(the seq, reset per worker, never a metric label),branch, anddecision(WindowOpened,WindowClosed); joins logWriteJoinedWindowat V(1). These are diagnostic names, not a journal.pendingCarriesCommitRequestpushes at once; theholdPushes/cooldownHoldsSavesseam; a flush, a resync, buffer pressure and shutdown close every window, oldest first; the push-notice close skips windows carrying a save; the byte budget is shared; the withheld-field restore runs per object before admission.No flag: stage 3 before the release
By decision there is no startup flag; stage 2 is on once merged. So stage 3 (the failure boundaries against the reference model, with several windows open) must land before the next release;
docs/TODO.md"Before the next release" says so.Reference model
window_model_test.gonow models several windows with its own footprint rules, on a folder with three shapes:x,w,zalone in their files;kin a kustomize root, which also takes every object created anew; ands1,s2in one shared file. After every step it compares the open windows (seq, author, objects, save, deadline, charge, footprint), the armed timers, saves, the byte charge and the close reasons (with window numbers). On every commit it compares the full changed-path set, every changed file whole (documents parsed with yaml.v3, the kustomization'sresources:), write order per document, and assertschanged paths ⊆ footprintfor a window that had company. It fails on anySingleWindowFallback.finalizeWindow: caught by the 40 × 60 sweep (not the CI seeds) and byTestWindowAdmission_ClosingOneWindowArmsTheTimerForTheOthers;TestWindowAdmission_ACommitOutsideItsFootprintIsRefusedAndTheWorkerFallsBack(the model asserts⊆ footprintitself, so it never sees a refusal);proveBeforeCommit: not caught; the check then skips commits whose proof is stale, so it checks fewer commits but refuses none wrongly. An earlier version without it refused a correct commit in the sweep, which is why it exists.Found on the way and left alone: a ConfigMap named
yin a kustomize root is refused by the render oracle (kustomize reads a bareyas a boolean); the model has no objectyfor that reason.Validation
task fmt,task generate,task manifests,task vet,task lint,task testpass; local e2e queued; result will be posted as a PR comment (CI runs the e2e legs too).🤖 Generated with Claude Code