Repository navigation
Conversation
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
a50585d to
da55fd1
Compare
e5eb0b7 to
93ae432
Compare
|
Reviewed slice 5. The attempt record is the right idea and I want it. There is a collision to resolve first, and the decision does not go your way on one field — so let me be specific about which part and why. The collision#1561 also adds a top-level
Whichever merged second would overwrite the other, and a client parsing Maintainer decision: #1561's object stands, and this PR drops its Keep
That is exactly right, and it is why the record matters more than the verdict word. Two things from your design that should survive into the merged shape"A dirty tree is never treated as evidence in either direction." Reporting Read-only Git probing — The rest of the sliceJudging worker liveness by process identity rather than pid alone, so a recycled pid cannot keep a dead attempt alive, is the same instinct as #1724's handle-based watchdog and #1874's Preserving the previous response shape for projects with no attempt record is the correct default. StatusPlease drop the Sorry to ask you to remove working code. The field genuinely can only exist once. |
DeusData
added
enhancement
94119b3 to
579336c
Compare
9e753e1 to
f7b50aa
Compare
|
Thank you for separating the attempt record from the freshness verdict and keeping generation snapshots inside last_index_attempt. That addresses the earlier field-collision request. This remains behind the stack’s maintainer follow-up; sorry for the delay, and I’ll follow up soon. |
An index that fits in memory and finishes in time can still fill the disk. Publication needs room for the staging artifacts and the final database at the same time, and running out of space during publication is the one failure that can cost a working index. Add index_cache_max_mb and index_min_free_disk_mb, measured before staging and again before publication, together with internal ceilings on the final database, the staging artifacts and the task temporary directory. Those three have no public keys because they are only meaningful as part of one composed decision. Crossing any of them fails the attempt before the old database is touched, so atomic publication is unchanged and the previous index keeps serving. Staging cleanup is scoped by a private per-task token, so a worker removes only the artifacts it created and never a concurrent run's. An old database is treated as replaceable only after an integrity verdict distinguishes real corruption from a transient busy error. A probe that cannot complete fails closed. Both public keys default to off. The shell fixture that stands in for the supervisor names them too, so the worker still recognises the policy it is handed. Signed-off-by: 刘冲 <[email protected]>
The extra indent is the only clang-format failure on the storage slice. Signed-off-by: 刘冲 <[email protected]>
The storage start path takes the per-request clock. This probe is not about duration, so the origin stays unset. Signed-off-by: 刘冲 <[email protected]>
Six independent keys are an honest interface and a poor default. An operator who only wants indexing to stay within reason has to learn all six, choose a value for each, and keep them consistent as the machine changes. Add index_resource_profile, accepting off, balanced or strict. Balanced guards the host: it lets an index use what the machine can spare and stops a runaway worker before the host is exhausted. Strict holds indexing to the daemon's own budget, so a repository that needs more fails fast and attributed instead of finishing at the host's expense. Any individual key, including an explicit off, replaces that dimension of the selected profile, so a profile is a baseline rather than a lock. The balanced worker ceiling is derived from detected host memory instead of being tabled. Large-repository indexing peaks in the tens of gigabytes, so any round number low enough to feel safe would reject repositories that index successfully today. When host memory cannot be read, balanced falls back to a fixed floor. Profiles also bound directory count, entry count, traversal depth and discovery time. These have no individual keys for the same reason as the storage internals: they are only meaningful inside a composed decision. The resolved profile travels through the same trusted worker contract as the individual limits, so every entry point enforces the same decision. That contract now also carries the resolved profile and the override mask, so the shell fixture standing in for the supervisor sends both. The coverage that proves a forged policy cannot fork a second job asserts that an explicit request joins the auto-index job already running for the same root, and that assertion found the join broken on Windows. A project root reaches the daemon through the platform canonicalizer, which answers in backslash form there, while every tool handler normalizes the separators of the path it canonicalizes. One directory therefore had two spellings, and they met in comparisons that are exact: whether an index request may join the running job, and whether a watch is still live for that root. Both were wrong on Windows, in opposite directions, so aligning either one alone moved the failure rather than removing it. The session context now stores the normalized spelling, and the daemon canonicalizes project roots through a single function that normalizes on the way out. Only the storage side is directly assertable off Windows, since separators are folded on every platform; the watch-liveness side is held by the existing watcher-ownership tests, which is where the second half of this surfaced. Signed-off-by: 刘冲 <[email protected]>
f7b50aa to
c6e31f3
Compare
Strict is defined against the slice main budgets for one worker. An explicit charged-memory limit above 80% of host memory is lowered, and that tightening is logged. Signed-off-by: 刘冲 <[email protected]>
A failed index is currently a log line. A watcher-triggered rebuild that dies on a resource limit leaves the previously published graph in service and nothing durable behind it, so index_status reports a healthy generation and the operator has no way to learn that it is no longer the tree. Record one attempt per project under the cache directory, updated under a file lock across queued, running and terminal states, carrying the origin, timestamps, the effective profile source and the resource failure when there is one. index_status returns it as last_index_attempt, and delete_project removes a matching record when no database was published. Freshness is reported alongside it and is deliberately conservative. A generation is comparable only when matching clean Git snapshots were observed before and after indexing, and it is fresh only while the worktree is still clean at that HEAD. A different clean HEAD is stale. Non-Git roots, dirty trees, a tree that changed during indexing, a failed probe and a corrupt record are all unknown, because a dirty tree is evidence of neither freshness nor staleness. Daemon startup fails attempts whose worker no longer exists. Liveness is judged by process identity rather than the pid alone, so a recycled pid cannot keep a dead attempt alive, and an unavailable start time falls back to plain liveness rather than declaring the worker gone. Projects with no record keep the previous response shape. Signed-off-by: 刘冲 <[email protected]>
index_status keeps last_index_attempt. The freshness name belongs to the checkout-evidence object, so this slice no longer publishes a competing string. Signed-off-by: 刘冲 <[email protected]>
c6e31f3 to
3b3adf2
Compare
Related to #1347.
Problem
Everything the previous PRs add can fail an index attempt, and a failed attempt is currently a log line. A watcher-triggered rebuild that dies on a resource limit leaves the previously published graph in service and nothing durable behind it.
index_statusreports a healthy generation, and the operator has no durable record that the last rebuild failed.That gap is what makes the limits usable or dangerous: a limit whose enforcement is invisible converts a loud failure into a quiet wrong answer.
What this changes
One attempt record per project under the cache directory, updated under a file lock across
queued,runningand terminal states, carrying the origin, timestamps, the effective profile source and the resource failure when there is one.index_statusreturns it aslast_index_attempt, anddelete_projectremoves a matching record when no database was published.index_statusdoes not add afreshnessstring. #1561 owns afreshnessobject, and a second string under the same name would clash with it. Generation snapshots stay insidelast_index_attempt. A dirty tree is never treated as evidence of freshness or staleness.index_statusdoes not probe Git just to emit the dropped field.Daemon startup fails attempts whose worker no longer exists. Liveness is judged by process identity rather than the pid alone, so a recycled pid cannot keep a dead attempt alive; when a start time cannot be read the code falls back to plain liveness rather than declaring the worker gone.
Projects with no attempt record keep the previous response shape.
Testing
make -f Makefile.cbm testandmake -f Makefile.cbm lint-cion macOS. New coverage: record lifecycle and locked transitions, resource-failure metadata inindex_status, the absence of a freshness string, generation snapshots kept inside the attempt record, abandoned-attempt recovery at daemon startup, and record removal ondelete_project.Stack