fix(pipeline): sweep orphaned/nested .db.stage files and stop the false invalid_existing_db route (#1839, #1864) - #2111
Open
DeusData wants to merge 5 commits into
Open
fix(pipeline): sweep orphaned/nested .db.stage files and stop the false invalid_existing_db route (#1839, #1864)#2111DeusData wants to merge 5 commits into
DeusData wants to merge 5 commits into
Conversation
The outer run rewrites the pipeline's db_path to its own stage, so the inner publication (gbuf dump and the delta clone) minted ITS stage from that stage: <db>.stage.A.stage.B, with -wal/-shm beside it under WAL. Every such copy is a full-size generation, so one project could occupy three times its database on disk (#1839). Mint from the stage root instead. The root is recognised only by the exact minted shape "<name>.stage.<6 alphanumerics>" in the basename, so a database whose own name merely contains ".stage." keeps its full name, and the marker is never matched across a path separator. The inner finalize still renames B over A; the unpredictable-name contract of the mkstemp helper is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A stage was recognisable only by its name: the mkstemp descriptor was closed at once and nothing marked who was writing it. A worker killed mid-run -- the daemon cancels with SIGTERM and SIGKILLs after one second of grace, which a gigabyte backup or delta clone never finishes inside -- left its full-size stage behind for good, and no later run could tell a dead stage from a live one, so none tried (#1839). Ownership is now an exclusive kernel lock on the sidecar "<stage>.lock", taken when the stage is minted and dropped by the same helpers that discard the stage or rename it into place. The kernel releases it on any death, so "can I take this lock?" is exactly "is this stage dead?" -- no pid, no mtime, no grace period. The lock lives on a sidecar rather than the stage file itself because on macOS an flock conflicts with SQLite's fcntl byte locks on the same file (verified: flock(LOCK_EX) then fcntl(F_WRLCK) on another descriptor fails with EAGAIN, both directions). The primitive is cbm_lockfile_open: POSIX open(O_CLOEXEC|O_NOFOLLOW) plus flock(LOCK_EX|LOCK_NB); Windows _wsopen with _SH_DENYRW and _O_NOINHERIT. Neither is inherited by spawned children. A backup that fails keeps the outer stage NAME owned: the rebuilt generation is renamed over it by the inner finalize and published from it, so its lock is held to the end. The sweep that uses this ownership follows separately. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
… run Nothing on the worker-death path ever cleaned a stage up: the supervisor reaps only an exit status, and a cancel is SIGTERM then SIGKILL after one second, which a gigabyte backup or clone never finishes inside. Every daemon-side cancel or OOM kill therefore left a full-size <db>.stage.X (plus -wal/-shm) behind for good, and with auto_index a large project filled the disk with copies of itself (#1839). Each run now sweeps before minting its own stage. Only names of the exact minted shape for THIS database are considered -- "<basename>.stage.<6 alphanumerics>" and that stage's -wal/-shm/-journal/.lock sidecars -- never the live database, a quarantined .corrupt, or another project's files. Liveness is the ownership lock and nothing else: a stage whose sidecar lock can be taken has no writer and is removed (logged as pipeline.stage action=orphan_removed bytes=N); one whose lock a live writer holds is kept (orphan_kept reason=live_writer). No pid, no mtime, no grace period, so the sweep is deterministic and never races a writer. A stage minted by an older binary carries no lock and is swept too; that writer's final rename then fails and it discards its stage. The live database is never named on either path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…sting_db The route probe opened whatever resolve_db_path returned, which inside the staged run is the run's own stage. On a first index nothing was copied into it, so the probe opened a 0-byte mkstemp placeholder, failed its integrity check, and warned "pipeline.route path=full reason=invalid_existing_db" -- on every first index of every project. Read back after an OOM kill it looked like a corrupted database and a daily "corrupt -> full rebuild -> OOM" loop (#1864); the loop is "no database -> full -> OOM", and the stale 0-byte stage never influenced the next run at all (each run mints a fresh name). cbm_pipeline_run now records whether the destination existed and was copied into the stage, and the route decision returns full early with reason=no_existing_db (or existing_db_backup_failed, after the backup_failed_full_rebuild warning) without probing a placeholder. "invalid_existing_db" stays reserved for a real copy that fails its integrity check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…lose #2111 create→register race) create_staging_path() minted the stage's main file with mkstemp and only then took its sidecar .lock via stage_owner_register(). A second, concurrent cbm_pipeline_run() against the same final_path (auto_index, or the stale-rendezvous-recovery retry path) could land its sweep_orphan_stages() in that window: the main file was visible but not yet locked, so the sweep treated it like a genuinely orphaned stage and deleted it out from under the live writer (POSIX), or collided on the not-yet-created sidecar and surfaced EACCES/errno=13 (Windows). That is the #2111 red on test-windows-guards and both test-windows CLANG64 x86_64 shards (index_format / simhash / index_resilience, with pipeline.stage action=lock_failed errno=13). Fix: mint the six-char alphanumeric suffix in create_staging_path() itself, create and hold the sidecar lock FIRST, then create the main file with O_EXCL. The stage's main file is therefore never present on disk without its lock already held, so a racing sweep can only ever find the stage lock-held: it reaches the stage through the .lock entry too (stage_entry_stage_length() matches it), probes the lock, sees a live holder, and keeps it. The create->register TOCTOU is closed by construction. #1839 is preserved: sweep_one_stage() still removes a stage whose sidecar is absent (ENOENT). With live stages now always carrying their sidecar, ENOENT is only ever a genuine pre-lock-era orphan minted by an older binary, which #1839 requires be swept. stage_owner_register() becomes stage_owner_adopt(), recording the already-held descriptor; re-taking the lock would self-conflict, since flock() and Windows _SH_DENYRW deny a second acquire of the same sidecar even from this process. A new after_stage_created test seam and pipeline_concurrent_sweep_must_not_- remove_inflight_stage drive a real concurrent sweep in that window; the test is RED without the lock-first ordering. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> 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
1 participant
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.
Fixes #1839 and #1864 — orphaned and nested
.db.stage.*files, and the misleadinginvalid_existing_dbroute on a first index.Root cause:
cbm_pipeline_runrewritesp->db_pathto its staging copy, so the inner publish/delta minted<db>.stage.A.stage.B; themkstempfd was closed immediately, so a SIGKILL'd or SIGTERM'd worker left a stage with no owner and nothing ever swept it; and the route probe integrity-checked the run's own 0-byte placeholder on every first index, printinginvalid_existing_dbwhere nothing was invalid.Four commits:
<db>.stage.B; the finalize rename is unchanged.<stage>.locksidecar (POSIXflock/ Windows_SH_DENYRW). A probe proved macOSflockconflicts with SQLite's ownfcntllocks on the same file, so ownership lives on a sidecar, not the stage fd.<own-basename>.stage.<6>(+-wal/-shm/-journal/.lock); a dead writer's stage is unlinked (orphan_removed bytes=N), a live one kept (orphan_kept reason=live_writer). Never touches the live.db,.corrupt, or another project.no_existing_db—invalid_existing_dbis reserved for a real full-size copy that fails integrity.Local verification (macOS):
pipeline_semantic_manifest_repro35,pipeline275,incremental163,store_*252,daemon_application53, all green;make lint-ciclean; five new tests each with RED→GREEN→RED-on-revert. Windows is compile-by-inspection; the CI Windows leg is the gate there. Release note: a new<db>.stage.*.locksidecar appears during an index, and anorphan_removedline prints once on the first run after upgrade.🤖 Generated with Claude Code