fix(lancedb): reclaim stale versions via write-locked prune - #379
fix(lancedb): reclaim stale versions via write-locked prune#379gloryfromca wants to merge 12 commits into
Conversation
The storage soak (48h, sustained churn + fuzz) proved the bundled lock-free `optimize(cleanup_older_than=...)` loses its commit-conflict race against concurrent writes — cleanup ran only 16 of ~250 times, so old dataset versions / FTS orphans piled up and the index dir grew to the 40G disk guardrail and never reclaimed under load. main still had that bundled call. Split the maintenance path: - `LanceRepoBase.optimize()` is now compact-only and lock-free (a commit conflict here is benign — the next beat retries, so it must not stall writers). - `LanceRepoBase.prune(older_than)` runs `cleanup_older_than + delete_unverified=True` **under the per-table write lock**, so no writer is in flight: the Rewrite has the manifest to itself (cleanup completes every beat) and aggressive deletion is safe. It also removes the empty `_indices/<uuid>/` husks cleanup leaves behind (soak: 13061 dirs, 98% empty), offloaded to a thread. - The cascade worker's heavy beat calls `prune()`; the light beat calls `optimize()`. A benign light-beat commit conflict is logged at debug and does not count toward the failure streak or trigger a rebuild. - Prune's retention window (`cleanup_older_than`) is decoupled from the prune cadence and defaulted short (60s). It runs under the write lock, so the window only needs to outlive an in-flight read; keeping it = cadence (300s) left ~2 cadences of superseded full-table copies on disk between beats (soak: transient ~15G/table peaks). 60s reclaims all but the last minute each beat — same live floor (~625MB/table), far lower transient footprint. Result on the re-run soak: disk sawtooths and reclaims to live-data size (~1.3G total) under active load — vs run1 stuck at 40G until writes stopped — with 0 crashes / 0 OOM / 0 stuck cleanups over 48h. Cascade projection health is now observable: - `CascadeOrchestrator.health()` -> `CascadeHealth`, combining the worker's in-memory signals (drain-loop failures, unrecoverable count, optimize streak, prune staleness) with the SQLite queue summary. - `GET /health` gains a typed `cascade` readiness block. `healthy` reflects operational health only (drain / optimize / prune); `failed_permanent` (files awaiting `cascade fix`) is a data-quality backlog reported as an informational count that does NOT flip `healthy` — otherwise the signal would sit red permanently. The scanner-side retry cap and the `_MAX_TOTAL_RETRIES` budget already on main handle re-enqueue storms, so no duplicate is added here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous open-ended `>=0.13.0` let any environment float to an untested release, including 0.32-0.34 which carry a compaction offset-overflow regression (lance-format/lance#7653) that stalls version cleanup and grows the index dir without bound. - Floor 0.34.0: the current resolved version; runs safely thanks to the with_position=False FTS workaround shipped in #336. Verified that data written by lancedb 0.32.0 (lance v6) reads correctly under 0.34.0 (lance v8), so existing deployments upgrade cleanly. Never widen the floor below 0.34 -- older lance cannot read v8-format data. - Ceiling <0.35.0: 0.35 embeds lance-rust v9 (large encoding jump, not yet stable-released); it must pass the soak harness before we allow it. Resolved version is unchanged (still 0.34.0); this only tightens the declared constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 94f9aa6)
The agent AGENTIC path (`agent_case` / `agent_skill`) fed recall
candidates straight into `aagentic_retrieve`, whose `_format_docs`
(LLM sufficiency / multi-query prompt) reads `metadata["episode"]` as a
`{subject, content}` dict plus a ms-epoch `timestamp`. Agent-kind rows
carry their body in the recaller's `text_field` and time as a datetime,
so `_format_docs` raised `TypeError: Candidate ... has no episode dict`
and `POST /api/v*/memory/search` returned 500 for any
`owner_type=agent` + `method=agentic` request.
Mirror the episode path's bridge: reshape agent candidate metadata into
the everalgo doc contract before `aagentic_retrieve`, and revert it
before DTO shaping so the agent shapers still see a datetime timestamp.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit bb9a3a5)
The committed `search_seed` fixture and several search e2e tests were written for the pre-1.5 memcell fact-linkage model. Current extraction links atomic_facts to episodes via `parent_id == episode.entry_id` (parent_type="episode"), and user_memory clusters store episode entry_id members — so the stale fixture made VECTOR/AGENTIC recall and the cluster-narrowing path find nothing, and stale assertions checked an old error code. - Regenerate `search_seed/*` from a fresh corpus in the current entry_id format; facts now bridge across multiple episodes (richer agentic / hierarchical-eviction coverage). - Fix `_dump_search_seed.py` sampling: pick episodes that host facts first and keep facts by episode entry_id, so re-dumps stay coherent. - Migrate e2e tests to the entry_id model (hierarchical-eviction, session/timestamp filters, cluster seeding helper) and update the filter-error assertion to the current `INVALID_INPUT` code. - Provision `ome.toml` in the full-app pipeline fixture (the OME config reloader requires it; strategies are code-registered so the packaged default suffices), unblocking corpus regeneration. Full search e2e suite now green (49/49). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 44cd9ce)
verify_business_schemas only compared column names, so a column whose on-disk Arrow type had drifted (name unchanged) slipped through and detonated later inside merge_insert as an opaque LanceError(IO) "Spill has sent an error" (#337). Now compare each shared column's Arrow type against schema.to_arrow_schema() — the exact schema get_table builds tables from, so a healthy table never false-positives. Reproduced #337 byte-identically: an episode.subject_vector column left as string or null by an older build, plus a real 1024-d vector on upsert, yields the exact crash. No lancedb version (0.13-0.34) renders Optional[Vector] as a non-vector type, so the startup guard is what should catch it — not the runtime. Add `everos cascade rebuild` as the safe recovery: it drops the business LanceDB tables and re-indexes from markdown, skipping the verify guard (which the drift would otherwise trip on startup). Unlike removing only .index/lancedb it re-populates already-done entries (reset_all clears the cascade queue); unlike removing all of .index it preserves unprocessed_buffer (messages not yet extracted). Fixes #337.
Add the `everos cascade rebuild` command to the runbook, CLI, and how-memory-works docs. Correct the old recovery guidance: a bare `rm -rf .index/lancedb` leaves md_change_state marked `done`, so the scanner skips those files and the index comes back empty — the runbook previously claimed a full repopulation that does not happen. `cascade rebuild` is the safe path (re-populates done entries, preserves unprocessed_buffer). Also document that verify now checks column types.
CascadeOrchestrator dropped the embedder param when embedding became a soft dependency (main); fold the schema-drift integration test onto it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both prune-staleness health tests fabricated `_started_at = time.monotonic() - (ALERT + 100)`, assuming monotonic() is a large value. On a fresh CI runner monotonic() is only ~100-180s, so the subtraction went negative, the source clamped the baseline to 0, and staleness read back as ~130s < 900s — failing on CI while passing on long-lived dev boxes where monotonic() is huge. Freeze the monotonic clock via monkeypatch so staleness is deterministic regardless of the runner's boot uptime. Source logic is unchanged; only the tests are made hermetic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`_wait_path_done` reached a terminal status, slept a 0.1s settle window,
then asserted the status was still terminal — which contradicted its own
docstring ("absorb any last-second re-enqueue"). A rename's delete event
or an atomic-replace echo can flip a done row back to `processing` inside
that window, so on a slow CI runner the assert fired
("flipped back to processing after reaching done"), failing
test_rename_cross_owner_keeps_frontmatter_owner intermittently (seen on
the 3.13 job). `make integration` runs without `--reruns`, so a single
flake fails the whole job.
Wait for a terminal state that *survives* the settle window instead:
absorb a transient re-enqueue by waiting for terminal again, still bounded
by `deadline` so a row that never settles surfaces as a timeout. Pre-
existing flake on main, unrelated to the prune change; the scenario's real
assertions (row counts, frontmatter owner) are untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
af3025e to
176883b
Compare
…ne safety, /health resilience Review of #379 found two P0s plus P1/P2s, all verified against the code: - P0-1: cascade backfill still called the removed optimize(cleanup_older_than=…) kwarg → TypeError swallowed by the best-effort try/except → backfill silently skipped all compaction + reclaim (the exact bloat this PR fixes). CI stayed green because the test double kept the stale signature. Fix: call optimize() + prune(0) at the call site; make the fake mirror the real signature so the drift can't hide again; pin prune in the backfill tests. - P0-2: prune ran delete_unverified=True guarded only by an in-process asyncio lock, but the runbook promises `cascade sync` is safe alongside a live server — and the CLI's first optimize beat does prune, in a separate process. It could delete files the daemon is mid-commit on. Fix: switch prune to delete_unverified=False. Measured to reclaim identically on churned tables (both collapse superseded versions ~97%); True only additionally deletes in-flight/dangling files — exactly the corruption vector. No cross-process lock needed; the write-lock/commit fix (the real reclaim win) is unchanged. - P1-3: /health called orch.health() (6 SQLite aggregates) with no guard → a locked/full/migrating DB would 500 the liveness probe and restart the container. Wrap it: unhealthy readiness + reason, HTTP stays 200. - P1-4: rebuild drops + recreates tables; a live daemon holds cached handles pointing at the dropped dataset. Runbook now says stop the server first — the one cascade command unsafe alongside a live server. - P2: narrow _is_benign_commit_conflict to the "commit conflict" phrase (a bare "retryable" swallowed unrelated recoverable errors); add a timeout around the prune cleanup so a hung lance call can't wedge the write lock; correct two stale docstrings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review of #379 found two P0s plus P1/P2s, all verified against the code: - P0-1: cascade backfill still called the removed optimize(cleanup_older_than=…) kwarg → TypeError swallowed by the best-effort try/except → backfill silently skipped all compaction + reclaim (the exact bloat this PR fixes). CI stayed green because the test double kept the stale signature. Fix: call optimize() + prune(0) at the call site; make the fake mirror the real signature so the drift can't hide again; pin prune in the backfill tests. - P0-2: prune ran delete_unverified=True guarded only by an in-process asyncio lock, but the runbook promises `cascade sync` is safe alongside a live server — and the CLI's first optimize beat does prune, in a separate process. It could delete files the daemon is mid-commit on. Fix: switch prune to delete_unverified=False. Measured to reclaim identically on churned tables (both collapse superseded versions ~97%); True only additionally deletes in-flight/dangling files — exactly the corruption vector. No cross-process lock needed; the write-lock/commit fix (the real reclaim win) is unchanged. - P1-3: /health called orch.health() (6 SQLite aggregates) with no guard → a locked/full/migrating DB would 500 the liveness probe and restart the container. Wrap it: unhealthy readiness + reason, HTTP stays 200. - P1-4: rebuild drops + recreates tables; a live daemon holds cached handles pointing at the dropped dataset. Runbook now says stop the server first — the one cascade command unsafe alongside a live server. - P2: narrow _is_benign_commit_conflict to the "commit conflict" phrase (a bare "retryable" swallowed unrelated recoverable errors); add a timeout around the prune cleanup so a hung lance call can't wedge the write lock; correct two stale docstrings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ef10380 to
df142c0
Compare
Follow-up to the review; two doc issues neither the review nor the fix commit caught: - The schema-drift startup error's docstrings (verify_business_schemas and LanceDBLifespanProvider) still described the recovery as `rm -rf ~/.everos/.index/lancedb` — which the runbook explicitly calls the WRONG recovery (it leaves the cascade queue `done`, so nothing re-indexes and the index comes back empty). The raised error already points to `everos cascade rebuild`; align the docstrings to match. - 15 dangling references to an internal numbered design-doc set (12_/13_/16_/17_*.md) that was never shipped to this repo. Point the schema-recovery ones at docs/cascade_runbook.md; drop the rest (pure provenance in table/component docstrings) while keeping the substance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough review — every finding was verified against the code and addressed. Resolution summary: P0P0-1 — backfill silently lost compaction/cleanup (masked by a stale test double) P0-2 — P1P1-3 — P1-4 — P2
Follow-up ( Validation — run5 (targeted, on
|
Consolidated LanceDB hardening PR. Folds in #354 / #355 / #357 (now closed) plus the prune-under-lock storage fix + cascade health, all rebased on latest
main. This mirrors the code the endurance soak actually exercised.Commits
fix(deps): pin lancedb to>=0.34.0,<0.35.0(was fix(deps): pin lancedb to >=0.34.0,<0.35.0 #355) — 0.35 embeds lance-rust v9; pin the version the soak validated.fix(search): agent-agentic 500 + regenerate stale e2e seed/tests (was fix(search): agent-agentic 500 + regenerate stale e2e seed/tests #357).fix(lancedb): detect schema type drift +cascade rebuildrecovery (was fix(lancedb): detect schema type drift and add cascade rebuild recovery #354).fix(lancedb): reclaim stale versions via write-locked prune— the main change (below).chore(rebase): adapt fix(lancedb): detect schema type drift and add cascade rebuild recovery #354's integration test to main's soft-embeddingCascadeOrchestrator.The prune-under-lock fix (why)
A storage-endurance soak (~100h across three runs, sustained churn + fault injection) reproduced the disk-bloat root cause: the bundled lock-free
optimize(cleanup_older_than=...)loses its commit-conflict race against concurrent writes, so cleanup ran only 16 of ~250 times and superseded dataset versions piled up until the index dir hit the 40G disk guardrail and never reclaimed under load.Split the maintenance path:
LanceRepoBase.optimize()— compact-only, lock-free (aRetryable commit conflicthere is benign; the next beat retries, so it must not stall writers).LanceRepoBase.prune(older_than)— new;cleanup_older_than + delete_unverified=Trueunder the per-table write lock (no writer in flight → the Rewrite owns the manifest → cleanup completes every beat; aggressive deletion is safe). Also removes empty_indices/<uuid>/husks (soak: 13061 dirs, 98% empty), threaded.prune(), light beatoptimize(). Benign light-beat conflict → debug, doesn't count toward the failure streak or trigger a fallback rebuild.Plus cascade observability:
CascadeOrchestrator.health()→CascadeHealth, surfaced as a typedcascadeblock onGET /health.healthy= operational only (drain / optimize / prune);failed_permanentis an informational count that does not fliphealthy.Soak result
Full write-up (code + conditions + results, with figures): DlabDev wiki → "EverOS LanceDB 存储浸泡测试 — 最终报告".
Tests
make lint+ import-linter +dump_openapi.py --check+ full unit suite (1882 passed).Follow-up (not in this PR)
Embedding/vector errors during cascade indexing are currently classified permanent (
retryable=False); the soak's fuzz showed a transient embedder blip permanently fails a file. Candidate: reclassify asRecoverableError.Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code