Skip to content

search: the portable search service (LanceDB on R2, S3 or a disk) - #777

Merged
futurepaul merged 7 commits into
masterfrom
search/service
Oct 31, 2026
Merged

futurepaul merged 7 commits into
masterfrom
search/service

Conversation

@futurepaul

@futurepaul futurepaul commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

PR 3a of the search stack, on #772. It adds the portable search service: one Rust binary and one image, LanceDB OSS on R2 (Cloudflare), any S3-compatible store, or a disk (self-hosted). The cell's capability (3b) comes next and calls it. This PR changes nothing at runtime.

What it is

search/: its own Cargo workspace and Docker image, plus a CI workflow (.github/workflows/search.yml: clippy and tests). The API, storage and self-host recipe are in search/README.md.

  • A table a scope. A scope is a name the platform makes from a fragment's identity, incarnation and read scope; to the service it is only a name.
  • Upserts are atomic per source. merge_insert writes a source's new chunks and deletes its old ones in one commit, so a search sees its old revision or its new, never part.
    • The same (source, revision) again writes nothing.
    • replaces is an optional compare-and-set. A stale replay gets 409 conflict with current.
  • One space a table. The table's first write pins {model, dimensions, version} (or null, words alone). A write or search in another space is 409 space_changed, so vectors are never mixed.
    • Chunks may omit their vector when the table already holds one for that text's hash (known). A grown conversation embeds only its new turns.
  • Search.
    • The lexical leg is BM25F over title, heading, context and body with the brain's weights: LanceDB's CombinedFields, which the benchmark preferred over its per-column match.
    • The semantic leg is cosine similarity over the vectors.
    • Each leg brings 100 chunks; sections are ranked by their best chunk and fused by RRF (k = 60).
    • Hits carry a snippet (≤ 300 bytes around the words) and signals.
    • Optional field filters and a time range; modes hybrid, lexical, semantic.
    • New rows are searched before they're indexed.
  • Deleted bytes leave storage. A write that removed rows starts maintenance. Each pass, holding the table's writes, does three things: it rewrites every file holding a deleted row (threshold 0, not Lance's 10%); after a removal it rebuilds the text indexes from the rows left; and it prunes every version older than the newest made more than a minute ago. Passes repeat until one version is left and it deletes no row. A failed pass is retried, and after a restart every table is swept and finished. Deleted text leaves the store's files within about three minutes. Tests check the files after a low-fraction delete, a delete between passes, a restart and a failed pass.
  • Revisions and initialization. Each source has its own row holding its revision, so sections: [] keeps it. A table is opened only with its text indexes, so an interrupted first write is finished before it is served.
  • An ended scope. POST /v1/{scope}/end tombstones a scope and drops its table; every write after is 410 ended.
  • Limits and auth.
    • Every limit is checked before any read or write (src/api.rs).
    • Bodies are read to 64 MiB and no further.
    • A bearer token of at least 32 bytes is compared in constant time; only /healthz is open.
    • Writes are serialized per table, and the platform will route each scope to one instance.
  • Storage is configuration.
    • SEARCH_STORAGE is a directory or s3://bucket/prefix.
    • Each SEARCH_STORAGE_<OPTION> becomes a LanceDB storage option.
    • On S3 or R2, Lance commits by conditional put (If-None-Match), so no lock service is needed.

Tests

  • Unit:
    • limits and refusals;
    • scope names;
    • vector decoding (wrong length, NaN);
    • section fusion and its best chunk;
    • snippets at character boundaries;
    • filter quoting;
    • query decoding;
    • constant-time token comparison.
  • Integration, on a directory:
    • search by words and by meaning, with filters;
    • a replay writes nothing (the version stays the same);
    • replaces refuses a stale write;
    • a source is replaced whole;
    • vectors are reused by hash, a missing one is refused, and another space or version is refused;
    • a words-only table refuses a later space;
    • a restart reads what was written;
    • sources pages in order;
    • delete is immediate, and after purge the deleted word is in no file;
    • drop;
    • the HTTP API refuses a missing or wrong token, a bad scope and unknown fields.
  • on_an_object_store (ignored; needs a store): the same flow on S3. It passed here against SeaweedFS's S3 gateway; MinIO's images are no longer freely pullable. R2 comes in 3b, since it's a paid store.
  • The image builds (472 MB), and a container run here on a volume wrote, restarted, and searched what it had written.
  • Repo: cargo xtask check passes (search/ is placed in xtask/src/eval_sinks.rs's RUNTIME).

Notes

  • lancedb is pinned to 0.40.0 by the two-week cooldown. It compiles only with its remote feature (an upstream bug); aws provides the S3 store.
  • The binary is large (~250 MB, LanceDB's tree), which is why it lives in its own workspace and image.

🤖 Generated with Claude Code

@futurepaul
futurepaul requested a review from alexlwn123 October 31, 2026 00:35
@futurepaul
futurepaul added this pull request to stack #773 October 31, 2026 00:35

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 956d111df1afe379400e9bf20cc9ae7bc07d5097, reviewed against #772 at 71951b4568ecd8e7a076c61d113190337550f5ee.

The separate service/workspace is appropriate for this layer. I found three failures in its storage and retry contracts.

Blocking:

  1. Deleted bytes are not reliably purged (search/src/store.rs:467–507). compact() uses OptimizeAction::All, which in pinned lancedb 0.40.0 uses default Lance 13.0.0 compaction options: deletions must exceed 10% before a fragment is rewritten on its own. For example, deleting one source from a single compacted file holding 20 equal-sized sources leaves its bytes in the current file; pruning old versions cannot remove that file. Separately, a deletion between the maintenance task's compaction and prune is ignored by maintaining, with no subsequent compaction scheduled. Restarting before the task completes loses it entirely; opening a table schedules no recovery, and maintenance errors only log. These are ordinary deletes/restarts that break the stated physical-erasure guarantee. Force materialization of every deletion and make cleanup recoverable, retried, and aware of writes arriving during a pass. Cover low-fraction deletes, a delete between phases, restart, and transient failure with checks of the stored bytes, not just search results.

  2. Empty upserts lose their replay/CAS revision (search/src/store.rs:268–284, 531–534). Write source a at r1, then upsert {source:"a", revision:"r2", replaces:"r1", sections:[]}. The first call succeeds, but deletes the only rows holding the revision. If its response is lost, the identical retry sees current: None and returns 409 instead of the promised no-op success. A subsequent update replacing r2 also conflicts. Preserve the source's revision independently of its chunks (including zero-chunk sources), atomically with replacement, and test empty-write replay/CAS across restart and an initially empty source.

  3. Interrupted first-write initialization leaves an unrecoverable lexical table (search/src/store.rs:327–329). The data table is committed before its FTS indexes. A restart or storage error before the first index commits leaves a valid table with the source revision but no FTS index. On retry, entry() opens it and the same-revision path returns success without completing initialization. Pinned Lance's combined_fields planner explicitly refuses a table with no inverted index on any requested column, so lexical/hybrid searches now return 500; the other write/maintenance paths do not create missing FTS indexes. Make initialization resumable before acknowledging a replay or serving the table, and cover restart/failure between table creation and index creation.

Validation: source traces include the pinned Lance compaction planner and CombinedFields planner. cargo test --manifest-path search/Cargo.toml --locked could not reach tests because this machine lacks protoc (lance-encoding build failure). CI check passed; search check and the e2e shards were still pending. The externally referenced engineering-style file was unavailable on this machine.

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 108d4eea652dd8cad52f1bdf70f0a7d86ee918d0, against #772 at 71951b4568ecd8e7a076c61d113190337550f5ee.

None of the three blockers in my previous review is fixed. I checked the new file-based configuration/timing changes and rechecked the affected storage paths; search/src/store.rs and its pinned dependencies are unchanged.

Blocking:

  1. Physical erasure remains unreliable (search/src/store.rs:467–507). Default compaction leaves low-fraction deletions in the current data file, which pruning cannot remove. Deletes arriving between compaction and prune receive no subsequent pass; restart loses scheduled cleanup, and failures only log. Force deletion materialization and provide recoverable, retried cleanup that accounts for intervening writes. Test stored bytes after low-fraction deletes, intervening deletes, restart and transient failure.
  2. Empty upserts still lose revision/replay state (search/src/store.rs:268–284, 531–534). After r1, an empty upsert of r2 replacing r1 succeeds but deletes its revision. Repeating that request after a lost response returns 409, as does a later write replacing r2. Preserve revision state atomically for zero-chunk sources; cover replay and CAS across restart, including an initially empty source.
  3. First-write initialization is still not resumable (search/src/store.rs:327–329). Failure or restart between table creation and its first FTS index leaves a table whose same-revision retry succeeds without creating indexes. Lance's CombinedFields planner then rejects lexical/hybrid searches. Resume index initialization when reopening/retrying, and test interruption between these commits.

The separate service workspace and small configuration helper remain appropriate in complexity for this layer.

Non-blocking: search/README.md promises _FILE for every setting, but SEARCH_LISTEN still uses std::env::var (search/src/main.rs:44), silently ignoring SEARCH_LISTEN_FILE. Route it through env::value or narrow the documented support.

Validation: rechecked the pinned Lance compaction defaults and CombinedFields planner against this head. Local service tests were not rerun: protoc remains unavailable (the earlier build stopped there). Current CI search/check/e2e jobs are pending. The external engineering-style file is also unavailable.

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 2fdfc1ae9377a4456c63c417d4924dc54a8c5955, against #772 at 71951b4568ecd8e7a076c61d113190337550f5ee.

None of the three blockers in my previous review is fixed. This head only adds a directory cleanup guard to the tests; the storage implementation and dependencies are unchanged. That small cleanup is appropriate in scope and complexity.

Blocking:

  1. Physical erasure remains unreliable (search/src/store.rs:467–507). Default compaction can retain low-fraction deletions in the current data file (Lance's threshold is 10%), which pruning cannot remove. A delete between compaction and prune gets no subsequent pass; restart loses scheduled cleanup, and failures only log. Force deletion materialization and make cleanup recoverable, retried, and aware of intervening writes. Verify stored bytes after low-fraction deletes, intervening deletes, restart, and transient failure.
  2. Empty upserts lose replay/CAS state (search/src/store.rs:268–284, 531–534). An empty r2 replacing r1 succeeds but removes the rows holding its revision. Retrying after a lost response returns 409, and a later write replacing r2 also conflicts. Preserve revision state atomically for zero-chunk sources; cover replay and CAS across restart and an initially empty source.
  3. First-write initialization is not resumable (search/src/store.rs:327–329). A failure or restart after table creation but before the first FTS index leaves a table whose same-revision retry succeeds without creating indexes. Lance's CombinedFields planner then refuses lexical/hybrid searches. Resume initialization on reopen/retry and test interruption between these commits.

Validation: reviewed the new cleanup guard and rechecked the storage paths and pinned Lance compaction/CombinedFields code. Local tests were not rerun because protoc remains unavailable. CI search/check/e2e jobs are pending. The external engineering-style file is unavailable on this machine.

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 416f658c07a3bed636325a3dc8eb5df54e956415, against #772 at 98c1081af2b65f444f14ce2aa79d676e056c6272.

None of the three blockers in my previous review is fixed. This is a rebase: search/, its workflow, and the eval-sink change are identical to that reviewed head. The separate service workspace remains appropriate in scope and complexity.

Blocking:

  1. Physical erasure remains unreliable (search/src/store.rs:467–507). Default compaction can retain low-fraction deletions in the current file (the pinned Lance threshold is 10%), which pruning cannot remove. Deletes between compaction and prune get no subsequent pass; restart loses scheduled cleanup, and failures only log. Force deletion materialization and make cleanup recoverable, retried, and aware of intervening writes. Verify stored bytes after low-fraction deletes, intervening deletes, restart, and transient failure.
  2. Empty upserts lose replay/CAS state (search/src/store.rs:268–284, 531–534). An empty r2 replacing r1 succeeds but removes the rows holding its revision. Retrying after a lost response returns 409; a later write replacing r2 also conflicts. Preserve revision state atomically for zero-chunk sources; cover replay and CAS across restart and an initially empty source.
  3. First-write initialization is not resumable (search/src/store.rs:327–329). Failure or restart after table creation but before the first FTS index leaves a table whose same-revision retry succeeds without creating indexes. CombinedFields then refuses lexical/hybrid searches. Resume initialization on reopen/retry and cover interruption between these commits.

Validation: checked out this exact head, compared the service with the previous reviewed head, and rechecked its storage paths and pinned compaction implementation. Local tests were not rerun: protoc is still unavailable. CI search/check/e2e jobs are pending. The external engineering-style file is unavailable on this machine.

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 3ca27eb8805c6e669dc86f95646dbefcffa70a95, against #772 at 63058fdad1976412425f2c05f22990b6fc5eb580.

The earlier empty-source revision/CAS and interrupted-initialization blockers are addressed: marker rows preserve revisions atomically, and opening a table completes its text indexes. SEARCH_LISTEN_FILE is fixed too. Cleanup now forces deletion materialization, rebuilds indexes, serializes passes with writes, and retries failed passes. Two failure paths still leave the physical-erasure blocker unresolved.

Blocking:

  1. A committed replacement can escape cleanup after index optimization fails (search/src/store.rs:400–409, replay at 380–381). On the 32nd write since optimization, merge.execute commits the replacement before optimize(Index).await?. A transient index read/write failure returns before setting removed or starting maintenance. With no maintenance already pending (e.g. initial source inserts followed by this replacement), retrying the same revision succeeds through the early replay return and never schedules cleanup. The old text remains in storage until another replacement/delete or a restart. The pinned LanceDB Index action propagates errors and does not roll back the preceding merge. Register cleanup immediately after the committed replacement, before fallible optional optimization; cover an optimization failure followed by a successful same-revision replay and no further writes, checking that the old bytes eventually disappear. This ordering hole also existed in the earlier implementation and my earlier reviews missed it.

  2. Restart recovery abandons cleanup after three failed table opens (search/src/store.rs:307–321). If a restart interrupts deletion cleanup and that table's reads fail for the sweep's three attempts, the last error is only logged; the sweep advances and eventually marks itself done. When storage recovers, nothing retries that table unless another request opens it. A quiet scope can therefore retain deleted text indefinitely after an ordinary transient storage outage. Keep failed scopes scheduled for retry with bounded backoff (without preventing other scopes from progressing). Cover failed opens through the current retry limit, then recovery without a client request, and assert physical erasure.

The added maintenance state is justified by the durability contract; the service remains appropriately isolated from the platform. The remaining fix boundary is ensuring cleanup cannot be forgotten either between a committed write and its scheduling or during restart discovery.

Validation: source review of the changes and pinned LanceDB optimization path; the new head merges the embeddings base with no search-service changes from d96de7ba. Local cargo test --manifest-path search/Cargo.toml --locked was started but stopped during dependency compilation after confirming protoc remains unavailable; no tests completed. Search/check/e2e CI jobs are pending. The external engineering-style file is unavailable on this machine.

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at b40838865cb9bfc550a79a8b3fb52f5ba66f745e, against #772 at 63058fdad1976412425f2c05f22990b6fc5eb580.

Both blockers from my previous review remain. The earlier empty-source revision and interrupted-index initialization fixes remain intact. This head adds permanent scope termination, with an additional interrupted-operation hole.

Blocking:

  1. Committed replacements can still escape cleanup after index optimization fails (search/src/store.rs:410–419). On the optimization write, the merge commits before fallible index optimization, which can return before marking removal or scheduling maintenance. A same-revision retry returns success without scheduling it. With no existing maintenance, old text remains until another removal or restart. Register cleanup immediately after the committed replacement, before optional optimization. Cover optimization failure followed by replay and no further writes, asserting eventual physical erasure.

  2. Restart recovery still abandons a table after three failed opens (search/src/store.rs:312–325). After an ordinary storage outage spanning those attempts, the sweep logs the last failure and advances permanently. A quiet scope whose cleanup was interrupted can retain deleted bytes indefinitely after storage recovers. Keep failed scopes scheduled with bounded backoff while allowing others to progress; test recovery after the retry limit without a client request.

  3. The termination tombstone is ignored when the data table survives (search/src/store.rs:579–586, entry/upsert at 343–388). Restart after the tombstone commits but before the table drops, or let that drop fail before deleting it. On restart/the next request, entry opens the surviving table without checking ended; upsert then writes it normally. Only create checks the tombstone, and the recovery sweep explicitly skips tombstones, so it never completes this interrupted end. Enforce the durable tombstone when opening/admitting writes to an existing table, and recover/retry the outstanding drop. Cover restart between these commits and a failed drop, with a delayed upsert refused and the old table eventually removed without another end request.

The tombstone approach is appropriate in scope and complexity; the missing boundary is making persisted termination authoritative even while its cleanup is incomplete.

Validation: checked out the exact head, reviewed its delta and affected recovery/write paths, and inspected CI (search/check/e2e still running). Local tests were not run because protoc is unavailable. The external engineering-style file is also unavailable on this machine.

@futurepaul

Copy link
Copy Markdown
Collaborator Author

Thanks, Alex. All three are fixed at the root in d96de7b. b408388 adds one more service change, for #783's lifecycle blocker. Each test below failed before its fix.

1. Physical erasure.

A pass of maintenance runs holding the table's writes:

  • Compaction with materialize_deletions_threshold: 0.0, so every fragment holding any deleted row is rewritten. At Lance's 10% default, the loop below never reaches clean; I checked by reverting the threshold.
  • Index rebuild after a removal. The text indexes are rebuilt from the remaining rows, and the vector index too if it exists, rather than trusting remap. A merge that empties a whole fragment is never remapped at all.
  • Prune of the versions older than keep, with every file only they held.

Passes repeat until the table is clean: one version and no deletion file.

  • A failed pass is retried with doubling backoff (at most 15 minutes), keeping its removal flag.
  • A write takes the same lock. It lands before a pass (which then materializes it) or after one. If after, it sets the flag, and the still-pending loop runs again, or a new loop starts. The decision to stop is taken under the lock, so no write can slip between "clean" and "stopped".
  • Writes now only fold rows into indexes (OptimizeAction::Index). Only maintenance compacts.

Recovery needs no separate marker: the store's own state (more than one version, or a deletion file) says what is left. After a restart, sweep opens every table (paged table_names), and an unclean one gets a pass with a conservative rebuild.

Tests check the store's files:

  • a_low_fraction_delete_leaves_the_store_files: one of 20 sources in one compacted file (5%), then a replaced revision.
  • a_delete_while_maintenance_runs_gets_a_pass_of_its_own: a second delete after pass 1, which keep left pending.
  • maintenance_a_restart_cut_off_is_finished_after_it: the store stops before its pass; the next store's sweep alone finishes it.
  • a_failed_pass_is_retried: data/ is read-only until a pass has failed, then writable again; the table ends clean.

One limit of the byte check: index files hold words encoded. I checked that the data files hold them as plain bytes and the FTS files don't. For indexes, the guarantee is by construction (rebuilt from the remaining rows after any removal), not by grep.

Maintenance tasks now end with their Store (a JoinSet), so a dropped store behaves as a stopped service in the restart tests.

2. Empty upserts keep their revision.

Every source has its own row holding its revision and at. It has section '' (section ids are never empty), no text, a null vector and no fields. That row is written in the same merge_insert as the source's chunks, so sections: [] replaces the chunks and keeps the revision atomically.

  • sources counts chunks and takes the revision and at from that row.
  • status counts each kind of row.
  • The vector leg filters the row out.

an_empty_write_keeps_its_revision covers:

  • your sequence;
  • its replay, after a restart too;
  • CAS on the empty revision;
  • a source empty from its first write, both in an existing table and as a table's first write.

On 2fdfc1a it failed exactly as you described: Conflict { current: None }.

3. First-write initialization is resumable.

A table is opened only with its four FTS indexes: index_text makes any that are missing before the table is cached or served. create acknowledges nothing before indexing.

an_interrupted_first_write_is_finished_before_it_is_served makes _indices read-only for the first write: the table commits, the index fails, and an error is returned. Then:

  • the retry succeeds, and lexical search works;
  • separately, after a restart, lexical search works before any write.

On 2fdfc1a it failed with "combined_fields requires an inverted index".

Non-blocking. SEARCH_LISTEN now goes through env::value, so _FILE works for it too.

For #783. POST /v1/{scope}/end makes the scope's tombstone (an empty table, <scope>.ended, a name no scope can have), then drops the table. Every write after it is 410 ended, however late it lands and across restarts, so a write sent before a life's end can't recreate its index. A write that waited on a table dropped under it starts over. DELETE /v1/{scope} stays a plain drop, for a new space. Test: an_ended_scope_is_never_written_again.

@futurepaul
futurepaul requested a review from alexlwn123 October 31, 2026 02:54

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 3214c72a2c25bd3200f4b8c6834eff72007dc798, against #772 at 5b643e9d714b768f30ed6ef1fd5b89f46e2b53a6.

All three blockers from my previous review remain in unchanged code. The new pruning cutoff addresses the separate reader-grace problem; I checked its strict timestamp comparison in pinned Lance. The earlier empty-source revision and interrupted-index initialization fixes remain intact.

Blocking:

  1. Committed replacements can escape cleanup after index optimization fails (search/src/store.rs:410–419). On the optimization write, the merge commits before fallible index optimization, which can return before marking removal or scheduling maintenance. A same-revision retry succeeds without scheduling it. With no existing maintenance, old text remains until another removal or restart. Register cleanup immediately after the committed replacement, before optional optimization. Cover optimization failure followed by replay and no further writes, asserting physical erasure.

  2. Restart recovery abandons a table after three failed opens (search/src/store.rs:312–325). A storage outage spanning those attempts makes the sweep log and advance permanently. A quiet scope whose cleanup was interrupted can retain deleted bytes indefinitely after storage recovers. Keep failed scopes scheduled with bounded backoff while allowing others to progress; test recovery beyond the retry limit without a client request.

  3. A surviving data table bypasses its termination tombstone (search/src/store.rs:579–586, admission at 343–388). Restart after the tombstone commits but before the drop, or let the drop fail before removing the table. entry opens the surviving table without checking ended, and upsert writes it. Only create checks the tombstone; the sweep skips tombstones and never completes the interrupted end. Enforce persisted termination on existing-table admission and recover/retry its outstanding drop. Cover interruption and failed drop, with delayed writes refused and the old table eventually removed without another end request.

The separate service and this small pruning correction remain appropriate in scope and complexity. The outstanding fix boundary is durable cleanup scheduling and authoritative termination across ordinary failures.

Validation: reviewed the exact head and its delta, rechecked the affected storage paths and pinned pruning implementation. Local tests were not run because protoc remains unavailable. CI check passed; search and e2e jobs are pending. The external engineering-style file is unavailable on this machine.

Base automatically changed from search/embeddings to master October 31, 2026 04:07
futurepaul and others added 6 commits October 30, 2026 23:07
fragment-search, its own workspace and image (search/README.md): one
LanceDB table a scope (a name the platform makes from a fragment's
identity, incarnation and read scope), its rows a source's sections'
chunks, so a section's vectors are its chunks'. Upsert replaces a source
whole in one commit (merge_insert, its old chunks deleted in it); the
same revision again writes nothing; `replaces` is a compare-and-set; a
table's space is pinned by its first write and another is refused;
vectors are reused by their text's hash. Search is BM25F over title,
heading, context and text, and cosine over the vectors, each leg's
sections ranked by their best chunk and fused by reciprocal rank, with
snippets and signals. What a write removes leaves the store's files
within about three minutes (compaction, then a prune). Bearer-token
auth; a body read to its limit and no further.

The same image runs in Cloudflare Containers on R2 (its S3 API) and
beside celld on S3 or a disk: only its storage URI and keys change.
Tests: limits, fusion, snippets; then on a directory, valid, replay,
stale, reuse, spaces, restart, sources paging, purge (no deleted word in
any file) and drop, and the HTTP API's token; and, by hand, the same on
an S3-compatible store (passed on SeaweedFS's gateway).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every setting the service reads may be a file's instead (SEARCH_TOKEN_FILE,
SEARCH_STORAGE_<OPTION>_FILE): a secret mounted in, never on a command
line. The object-store test reads its keys so too, and says how long each
step took (first write, a cold search after a reopen, a warm one, a
replace, a purge): the latencies R2 is measured by.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nterrupted tables

Physical erasure: a pass of maintenance, holding the table's writes,
rewrites every file holding a deleted row (threshold 0: Lance's 10% kept
low-fraction deletes for good), rebuilds the text indexes after a
removal, and prunes; passes repeat until one version deleting no row is
left, a failed pass is retried with backoff, and a write meanwhile gets a
pass after it. The store's own state says what is left, so the service
sweeps every table after a restart and finishes it. Writes fold rows into
indexes only; maintenance alone compacts.

Revisions: every source has its own row holding its revision, written in
the same merge_insert, so sections: [] keeps it (replay is a no-op, CAS
holds, across restarts, from a source's first write).

Initialization: a table is opened only with its text indexes, made when
missing, so a first write cut off between table and index is finished by
its retry or the next open before it is served.

Tests check the store's files after a delete of a twentieth of a file, a
delete between passes, a restart and a failed pass, and inject the
interrupted first write with a read-only index directory. SEARCH_LISTEN
reads _FILE too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
POST /v1/{scope}/end makes the scope's tombstone, then drops its table;
a write after it is refused 410 ended, however late it comes (one sent
before the end), across restarts. A scope is one life of one fragment, so
it is never needed again. A write that waited on a table dropped under it
starts over (made again after a drop, refused after an end).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lance prunes by when a version was made, so a pass pruned the version it
had just superseded (made long before) from under a search still reading
it: CI's debug run caught one read failing on a rebuilt index's pruned
files. Prune every version older than the newest made more than keep
ago (lancedb's absolute cutoff). Tests keep a second, as the service keeps
a minute.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail for now at 080727c3666ad2ae4c43150c704736bea418d7a1.

All three blockers from my previous review remain. This head rebases onto master; search/, its workflow, and the eval-sink change are identical to the previously reviewed head. The earlier empty-source revision and interrupted-index initialization fixes remain intact.

Blocking:

  1. Committed replacements can escape cleanup after index optimization fails (search/src/store.rs:410–419). The merge commits before fallible index optimization, which can return before marking removal or scheduling maintenance. A same-revision retry returns success at lines 390–391 without scheduling it. With no existing maintenance, old text remains until another removal or restart. Register cleanup immediately after the committed replacement, before optional optimization. Cover optimization failure followed by replay and no further writes, asserting physical erasure.

  2. Restart recovery abandons a table after three failed opens (search/src/store.rs:312–325). An ordinary storage outage spanning those attempts makes the sweep log and advance permanently. A quiet scope whose cleanup was interrupted can retain deleted bytes indefinitely after storage recovers. Keep failed scopes scheduled with bounded backoff while allowing others to progress; test recovery beyond the retry limit without a client request.

  3. A surviving data table bypasses its termination tombstone (search/src/store.rs:579–586, admission at 341–388). Restart after the tombstone commits but before the drop, or let the drop fail before removing the table. entry opens the surviving table without checking ended, and upsert writes it. Only create checks the tombstone; the sweep skips tombstones and never completes the interrupted end. Enforce persisted termination on existing-table admission and recover/retry its outstanding drop. Cover interruption and failed drop, with delayed writes refused and the old table eventually removed without another end request.

The separate service remains appropriate in scope and complexity. The outstanding fix boundary is durable cleanup scheduling and authoritative termination across ordinary failures.

Validation: checked out the exact head, compared the service with the previous reviewed head, and rechecked the affected storage and test paths. Local tests were not rerun because protoc remains unavailable. CI search/check/e2e jobs are pending. The external engineering-style file is unavailable on this machine.

…nds that finish

- A replacement registers its cleanup as soon as its merge commits, before
  the optional index fold, which no longer fails the write: a fold error
  left a committed removal unscheduled, and its replay never scheduled it.
- The sweep after a restart keeps a table it cannot open, trying it again
  with backoff (at most 15 minutes) while the others go on; it gave up
  after three tries.
- An ended scope's tombstone is authoritative: a table that survived its
  drop (a restart between tombstone and drop, a failed drop) is never
  opened, and its drop is finished by entry, by the sweep (tombstones are
  swept), and by a retry loop after a failed drop.

Tests, each failing on 080727c: a replacement whose fold fails then
replays with no write after; a table unreadable past four sweep tries;
an end cut off before its drop; a failed drop with a late write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@alexlwn123 alexlwn123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass at d5b9a4dd61ab903ef574311c245afce4394a465b.

All three blockers from my previous review are addressed: committed replacements schedule cleanup before fallible index optimization; restart recovery retains failed scopes and retries with bounded backoff; and persisted termination is checked before serving an opened table, with interrupted and failed drops retried. The earlier empty-source revision and interrupted-index initialization fixes remain intact.

I reviewed the delta and surrounding write, admission, maintenance and termination paths, including the new failure/restart tests. The recovery machinery is appropriate to the durability contract, and I see no remaining blocking regression.

Validation: exact head checked out; diff whitespace check passed. CI search/check/e2e jobs are still pending. Local tests were not run because protoc is unavailable; the referenced external engineering-style file is also unavailable on this machine.

@futurepaul

Copy link
Copy Markdown
Collaborator Author

Thanks, Alex. All three are fixed at the root in d5b9a4d, a fast-forward on 080727c (no rebase). I'm sorry: I posted my last round's reply before your 03:15 review arrived, so these three went unanswered until now.

1. Cleanup is scheduled at the commit (blocking).

A replacement now sets removed and starts maintenance right after its merge_insert commits, before anything else can fail.

The every-32-writes index fold is now optional. A failure is logged and since_optimize is left as it is, so the next write or a maintenance pass folds the rows. Unfolded rows are still searched as the unindexed tail. A committed write is no longer reported as failed, and nothing after the commit can skip its cleanup.

Test a_replacement_whose_index_fold_fails_is_still_erased follows your sequence:

  1. _indices is made read-only for the 32nd write, a replacement, so its fold fails.
  2. The replay is a no-op.
  3. No further writes.
  4. Maintenance settles, and the old revision's word is gone from every file.

On 080727c it failed at the last assertion: no maintenance had been scheduled, and the old text stayed.

2. The sweep never gives up on a table (blocking).

sweep lists every table, then works through them. A table it fails to open (or an end it fails to finish) is kept, and tried again later with doubling backoff (at most 15 minutes) until it opens, while the others go on. swept() resolves only when every table has been seen. The new sweeping() watch counts failures.

Test the_sweep_keeps_trying_a_table_it_cannot_open:

  1. A delete's maintenance is cut off by a stop.
  2. The table's _versions is made unreadable (mode 000). This is a real permission error, not a not-found.
  3. A new store's sweep fails more than 3 times, which is past the old limit, and is not done.
  4. Read access is restored.
  5. The sweep opens the table without any request, and its maintenance erases the deleted word from the files.

This test needs the new sweeping() watch, so it can't build on 080727c. There, the third failure ended the sweep with the table abandoned.

3. The tombstone is authoritative (blocking).

  • Admission. entry checks ended(scope) before admitting any existing table. A table that survived its end is dropped there, never served or written, and every caller sees no table. A late upsert then goes to create, which refuses it with 410.
  • Recovery. The sweep now visits tombstones and finishes their drop (finish_end, a retried drop_table; lancedb's drop is a remove_dir_all, so it also finishes a half-deleted directory). Surviving data tables are dropped through entry as well.
  • A drop that fails inside end is tried again by a retry loop with doubling backoff, until it succeeds. ends_finished() waits for that loop.

Tests:

  • an_end_cut_off_before_its_drop_is_finished: a tombstone is committed with its data table still in place (a restart between the two). The next store's sweep removes the table with no request, and a late write is refused with Ended. On 080727c it failed: the sweep opened the surviving table and left it there.
  • a_failed_drop_of_an_ended_scope_is_tried_again: data/ is read-only during end, so the drop fails, and a delayed write landing then is refused. Once data/ is writable again, the table directory is gone and the deleted text with it, with no second end request; later writes are Ended. This test also needs a new API, ends_finished.

Results: the service suite is 17/17, three times in debug (as CI runs it) and in release, with clippy clean. The capability's e2e search section passes 13/13 on #783 with this service.

@futurepaul
futurepaul requested a review from alexlwn123 October 31, 2026 06:01
@futurepaul
futurepaul added this pull request to the merge queue Oct 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 31, 2026
@futurepaul
futurepaul added this pull request to the merge queue Oct 31, 2026
Merged via the queue into master with commit 7628bc4 Oct 31, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants