Skip to content

test(blob): cover secondary base store reuse - #8693

Open
lance-gatefixer[bot] wants to merge 1 commit into
mainfrom
gatekeeper/fix-8692-1
Open

test(blob): cover secondary base store reuse#8693
lance-gatefixer[bot] wants to merge 1 commit into
mainfrom
gatekeeper/fix-8692-1

Conversation

@lance-gatefixer

Copy link
Copy Markdown
Contributor

Summary

  • exercise repeated Blob V2 reads from an additional base across two fragments
  • assert the session registry resolves that base only once, protecting the dataset-scoped object store and its connection pool from per-call teardown
  • retain coverage for inline, packed, and dedicated blobs on non-default bases

Root cause

The object store registry intentionally keeps only weak references. Before the dataset-scoped additional-base cache added in #8530, the blob read context's per-call Arc<ObjectStore> was the only strong owner, so dropping each read also dropped the HTTP client and connection pool. Current main contains that ownership fix; this PR adds direct regression coverage for the reported Blob V2 path so later changes cannot silently restore per-read store construction.

Validation

  • cargo test -p lance test_take_blob_v2_from_non_default_base_inline_reuses_store
  • cargo test -p lance test_take_blob_v2_from_non_default_base
  • cargo fmt --all
  • cargo clippy --all --tests --benches -- -D warnings

Fixes #8692

@github-actions github-actions Bot added the chore label Aug 21, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gate recommendation: approve.

This adds a focused regression guard for Blob V2 reads from an additional base. The repeated-read registry assertion protects dataset-scoped store ownership, while the two-fragment fixture retains inline, packed, and dedicated coverage.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 21, 2026
@lance-gatefixer

Copy link
Copy Markdown
Contributor Author

Blocked: The current-head Rust windows-build Check is failing on an unrelated nondeterministic ANN assertion, and GitHub will not permit its rerun while the parent workflow is still running.

Remote head 2d400da3671400dd4182802042b305fa9598ec98 contains the current main tip 3128c0024427cb5bf8c04d492893ae45e78b0511. In windows-build job 96799343464, the changed test_take_blob_v2_from_non_default_base_inline_reuses_store passed, while unchanged test_ann_with_deletion::data_storage_version_2_LanceFileVersion__Stable::stable_row_ids_2_true returned {0,1,2,3,6} instead of {0,1,2,3,4}; all four parameterizations pass locally and the repair diff does not touch scanner/index code. A targeted rerun was attempted, but GitHub reports that the job cannot be rerun while downstream linux-build keeps run 32491301654 active. Once that run completes, rerun failed job 96799343464; alternatively rerun the Rust workflow for this head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connection reuse is broken for reading external blobs on secondary base

0 participants