fix(watcher): reindex dirty repos once per distinct state, not per poll#1045
Merged
Conversation
A persistently dirty or untracked worktree re-triggered a full reindex (and its DB/artifact rewrite) on every poll cycle - git_is_dirty() answered 'is the tree dirty' where the watcher needs 'did the tree CHANGE'. Idle repos with one uncommitted file produced sustained disk write amplification (reported at 1 TB/day across several repos). The watcher now tracks a dirty-state signature: FNV-1a over the entries of git status --porcelain -uall -z plus each listed path's (size, mtime_ns). Same signature -> no reindex; editing a dirty file, adding/removing one, or reverting the tree to clean each produce a new signature and trigger exactly one reindex. Submodule status is folded in on POSIX, mirroring the old submodule dirty probe. 0 is reserved for a clean tree; a dirty-at-baseline tree still reindexes once (the watcher cannot know whether that state reached the DB, e.g. after a server restart with a stale artifact). Baselines (HEAD and signature) are now committed only after a SUCCESSFUL reindex: check_changes stages observations in pending_* fields, and busy-skips (index_fn > 0, e.g. pipeline lock held) or failed runs leave the baselines untouched so the change is retried on the next poll instead of being recorded as seen and silently lost - the old code stored the new HEAD at check time, dropping any commit that arrived while the pipeline was busy. Closes #937 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.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.
Closes #937
Bug
check_changes()answered is the tree dirty where the watcher needs did the tree change: any dirty/untracked worktree returned true on every poll, so an idle repo with one uncommitted file re-triggered a full reindex — and its DB/artifact rewrite — every 5–60s cycle. Multiplied across several watched repos this produced the reported sustained >1 TB/day of disk writes.A second, adjacent lost-update flaw:
check_changescommitted the new HEAD at check time, so a commit observed while the pipeline lock was busy (watcher.skip pipeline_busyreturns success) was recorded as seen and never indexed.Fix
Dirty-state signature. The watcher now hashes the dirty state — FNV-1a over the entries of
git status --porcelain -uall -zplus each listed path's (size, mtime_ns) — and reindexes only when the signature changes:-ualllists files inside untracked directories individually, so nested additions aren't hidden behind an unchanged?? dir/entry;-zgives unquoted, stable paths that also stat cleanlygit_is_dirty()is fully subsumed and removedCommit baselines only on success.
check_changesno longer mutateslast_head; observations are staged inpending_*fields and committed bypoll_projectonly after a successful reindex. Thecbm_index_fncontract gains a positive "skipped, retry" return used by the busy/shutdown paths inmain.c— a busy-skip or failed run now retries on the next poll instead of silently losing the change.Reproduce-first
Both RED on the old code, GREEN with the fix (tests/test_watcher.c):
watcher_dirty_state_reindexes_once_issue937— dirty once → 1 reindex; 3 idle polls on the same state → still 1 (was 4); edit again → 2; revert to clean → 3; stable clean → 3.watcher_failed_reindex_retries_issue937— HEAD moves, first reindex attempt fails → retried and succeeds on the next poll (was: silently lost).watcher_continued_dirtypreviously asserted one reindex per poll while dirty — that assertion was the Watcher can repeatedly re-index dirty repos and cause large disk write amplification #937 amplification; updated to the corrected contract.Verification
git status+ the existing POSIX submodule probe per poll, plus onestat()per dirty file)