fix: make FTS metadata loading retry-safe by Xuanwo · Pull Request #7838 · lance-format/lance

Xuanwo · 2026-07-18T05:23:13Z

Distributed FTS global-stats planning needs one immutable set of index segments and parsed tokenizer parameters per table snapshot. Concurrent cold callers currently race while reading index metadata, and InvertedIndex::load_params treats every metadata.lance open failure as a legacy index. A transient object-store error can therefore become a successful default-tokenizer fallback and be retained by an upper-layer cache.

Make index-metadata cache misses use the existing fallible singleflight path and include the object-store identity in the cache key. Restrict the legacy tokens fallback to typed not-found errors, preserving that variant through shared-reader error cloning so timeouts and permission failures remain retryable errors.

This is the Lance-side prerequisite for the Sophon QN-owned FTS index snapshot cache in lancedb/sophon#6759.

coderabbitai · 2026-07-18T05:23:20Z

📝 Walkthrough

Walkthrough

The changes improve cache error propagation and concurrent loader coalescing, add not-found detection across wrapped errors, namespace index metadata caches by object-store identity, preserve inverted-index metadata errors during legacy fallback, and strengthen related tests.

Changes

Cache and index behavior

Layer / File(s)	Summary
Error classification and cache error coalescing `rust/lance-core/src/error.rs`, `rust/lance-core/src/cache/*`, `rust/lance/src/dataset/fragment.rs`	Wrapped errors now participate in source traversal, `is_not_found()` recognizes nested missing-object errors, cloned errors preserve categories, and moka cache failures are propagated through deduplicated loader calls.
Object-store-aware index cache keys `rust/lance/src/session/index_caches.rs`, `rust/lance/src/index.rs`, `rust/lance/src/dataset.rs`, `rust/lance/src/io/commit.rs`	`IndexMetadataKey` includes object-store identity, and index loading and commit caching use the updated key with concurrent singleflight coverage.
Legacy inverted-index fallback errors `rust/lance-index/src/scalar/inverted/index.rs`	Legacy token-file fallback preserves the original metadata access error when token-file access also fails, with tests for both fallback outcomes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

lance-format/lance#7816: Both changes update inverted-index legacy metadata and token-file fallback error handling.

Suggested reviewers: wjones127, jackye1995, bubblecal

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: making FTS metadata loading retry-safe.
Description check	✅ Passed	The description matches the changeset by describing singleflight cache misses, store identity keying, and typed not-found fallback.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch agent/fts-index-snapshot-support

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-07-18T06:15:53Z

Codecov Report

❌ Patch coverage is 84.87395% with 36 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance-index/src/scalar/inverted/index.rs	57.14%	29 Missing and 1 partial ⚠️
rust/lance-core/src/error.rs	92.20%	1 Missing and 5 partials ⚠️

📢 Thoughts on this report? Let us know!

BubbleCal · 2026-07-18T12:09:36Z

                Ok(serde_json::from_str::<InvertedIndexParams>(params)?)
            }
-            Err(_) => {
+            Err(error) if error.is_not_found() => {


Legacy FTS indexes can still fail to open on S3 readers that have s3:GetObject but not s3:ListBucket. These indexes do not contain metadata.lance, and S3 returns 403 rather than 404 for a missing object when ListBucket is unavailable, so is_not_found() is false even though tokens.lance is readable. Could we probe the legacy tokens file when opening metadata fails, use the fallback only if that succeeds, and otherwise preserve the original metadata error? https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html

BubbleCal · 2026-07-18T12:09:37Z

-            None => {
+        let mut indices = self
+            .index_cache
+            .get_or_insert_with_key(metadata_key, || async {


get_or_insert_with_key does not preserve loader errors for coalesced waiters. Moka runs one initializer, but each caller owns a separate oneshot; when the loader fails, only the initializer receives the original timeout or permission error and the other callers return Internal("Failed to retrieve error from cache loader"). A five-caller repro produced one Timeout and four Internal errors. Could we fan out the same fallible result and add a concurrent failure test before using this path here?

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

rust/lance-core/src/error.rs (1)
931-970: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a short comment on DisplayError's purpose.

The clone logic itself is correct: wrapping not-found causes in DisplayError before boxing (rather than boxing the Error directly) is what keeps format!("{:?}", ...) on the cloned error human-readable (DisplayError's Debug forwards to Display), which downstream consumers rely on (e.g. fragment.rs tests assert message.contains("miss.lance") on format!("{err:?}")). This is non-obvious — a future refactor could "simplify" away the DisplayError wrapper and silently break Debug-format matching in callers. A one-line comment on the struct would prevent that.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance-core/src/error.rs` around lines 931 - 970, Add a concise comment
above DisplayError documenting that its Debug implementation forwards to Display
so wrapped not-found errors remain human-readable in debug output; preserve the
existing CloneableError cloning logic and behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/lance-core/src/error.rs`:
- Around line 523-536: Update Error::is_not_found to explicitly handle the
DatasetNotFound variant by inspecting its boxed source with
error_source_is_not_found, matching the existing IO and Wrapped source
classification. Preserve the current handling for NotFound, IO, Wrapped, and all
other variants.

---

Nitpick comments:
In `@rust/lance-core/src/error.rs`:
- Around line 931-970: Add a concise comment above DisplayError documenting that
its Debug implementation forwards to Display so wrapped not-found errors remain
human-readable in debug output; preserve the existing CloneableError cloning
logic and behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 35e41ea4-97a6-4854-9789-133ff803b157

📥 Commits

Reviewing files that changed from the base of the PR and between e934cc2 and 74568a1.

📒 Files selected for processing (9)

rust/lance-core/src/cache/mod.rs
rust/lance-core/src/cache/moka.rs
rust/lance-core/src/error.rs
rust/lance-index/src/scalar/inverted/index.rs
rust/lance/src/dataset.rs
rust/lance/src/dataset/fragment.rs
rust/lance/src/index.rs
rust/lance/src/io/commit.rs
rust/lance/src/session/index_caches.rs

Distributed FTS global-stats planning needs one immutable set of index segments and parsed tokenizer parameters per table snapshot. Concurrent cold callers currently race while reading index metadata, and `InvertedIndex::load_params` treats every `metadata.lance` open failure as a legacy index. A transient object-store error can therefore become a successful default-tokenizer fallback and be retained by an upper-layer cache. Make index-metadata cache misses use the existing fallible singleflight path and include the object-store identity in the cache key. Restrict the legacy tokens fallback to typed not-found errors, preserving that variant through shared-reader error cloning so timeouts and permission failures remain retryable errors. This is the Lance-side prerequisite for the Sophon QN-owned FTS index snapshot cache in [lancedb/sophon#6759](lancedb/sophon#6759). (cherry picked from commit 654f0d4)

fix: make FTS metadata loading retry-safe

539395c

github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Jul 18, 2026

fix: expose typed not-found detection

ce64c6d

BubbleCal reviewed Jul 18, 2026

View reviewed changes

Xuanwo added 2 commits July 19, 2026 03:11

fix: preserve index loader error compatibility

df0e099

fix: preserve not-found across repeated clones

95de72f

Xuanwo marked this pull request as ready for review July 20, 2026 05:29

Merge branch 'main' into agent/fts-index-snapshot-support

74568a1

coderabbitai Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread rust/lance-core/src/error.rs

BubbleCal approved these changes Jul 20, 2026

View reviewed changes

Xuanwo merged commit 654f0d4 into main Jul 20, 2026
30 of 32 checks passed

Xuanwo deleted the agent/fts-index-snapshot-support branch July 20, 2026 06:26

This was referenced Jul 21, 2026

perf(cache)!: use fixed-size cache keys #7878

Open

fix(mem-wal): expose typed writer fence detection #7896

Open

feat(index): per-query index cache hit/miss stats #7862

Open

Xuanwo mentioned this pull request Jul 22, 2026

Errors with index bitmap like bitmap_page_lookup.lance not found #5454

Closed

coderabbitai Bot mentioned this pull request Jul 23, 2026

fix(index): rebind vector readers to current object store #7944

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: make FTS metadata loading retry-safe#7838

fix: make FTS metadata loading retry-safe#7838
Xuanwo merged 5 commits into
mainfrom
agent/fts-index-snapshot-support

Xuanwo commented Jul 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

codecov Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

BubbleCal Jul 18, 2026

Uh oh!

BubbleCal Jul 18, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Xuanwo commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codecov Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BubbleCal Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

BubbleCal Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Xuanwo commented Jul 18, 2026 •

edited

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

codecov Bot commented Jul 18, 2026 •

edited

Loading