Skip to content

fix: Load Milvus collections once instead of on every query - #6882

Merged
ntkathole merged 2 commits into
feast-dev:masterfrom
simonhearne:fix/milvus-load-once
Sep 29, 2026
Merged

ntkathole merged 2 commits into
feast-dev:masterfrom
simonhearne:fix/milvus-load-once

Conversation

@simonhearne

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

online_read and retrieve_online_documents_v2 called load_collection on every request.
That call also hid a bug: collections were created, then indexed with a separate create_index,
which leaves them unloaded on Milvus servers.

  • Collections are created with their index_params, so Milvus indexes and loads them at once.
  • For existing collections, the load state is checked on first access and load_collection is
    only called if the collection isn't loaded. The result is cached per store instance.
  • The per-query load_collection calls are removed.

Depends on #6881: every vector field must be indexed for the collection to load when it's created. Until #6881 merges this PR also shows its commit; only the last commit (fix: Load Milvus collections once instead of on every query) is new here.

Which issue(s) this PR fixes:

N/A

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests

  • Integration tests

  • Manual tests

  • Testing is not required for this change

  • Unit (mocked): an existing collection is loaded 0 times if already loaded and once if not,
    across 5 reads; new collections are created with index params and no separate load.

  • Unit (Milvus Lite): spy confirms load_collection isn't called by reads or searches.

  • Server: no load_collection per query; a released collection is loaded exactly once by a new store.

Unit tests run on Milvus Lite 3.2.1 (pymilvus 3.0.2). Server tests in sdk/python/tests/integration/online_store/test_milvus_remote.py are marked integration and skip unless ZILLIZ_URI and ZILLIZ_TOKEN are set; they passed against a local Milvus 2.6.0 server and against Zilliz Cloud. The existing Milvus unit and universal integration tests pass unchanged.

Misc

Part of a series of Milvus online store improvements for Zilliz Cloud and production Milvus.

🤖 Generated with Claude Code

@simonhearne
simonhearne requested a review from a team as a code owner September 28, 2026 13:26
@codecov-commenter

codecov-commenter commented Sep 28, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 47.87%. Comparing base (213a130) to head (e88bb51).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
.../infra/online_stores/milvus_online_store/milvus.py 94.11% 0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6882      +/-   ##
==========================================
+ Coverage   47.86%   47.87%   +0.01%     
==========================================
  Files         422      422              
  Lines       52408    52414       +6     
  Branches     7607     7608       +1     
==========================================
+ Hits        25084    25095      +11     
+ Misses      25512    25509       -3     
+ Partials     1812     1810       -2     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 49.23% <94.11%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
.../infra/online_stores/milvus_online_store/milvus.py 70.33% <94.11%> (+1.54%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 213a130...e88bb51. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

simonhearne and others added 2 commits September 29, 2026 14:40
Feature views without a vector field get a placeholder vector because
Milvus requires one per collection. The placeholder was 1-dimensional,
filled with NaN and left unindexed. Milvus Lite accepts all three, but
Milvus servers and Zilliz Cloud reject dim < 2 and non-finite values,
and refuse to load a collection with an unindexed vector field, so
these feature views could not be created or read.

The placeholder is now 2-dimensional, zero-filled and FLAT-indexed.
Vector fields without vector_index=True also get a FLAT index so the
collection can be loaded. Existing collections keep working because
the placeholder is filled to the dimension recorded in the collection.

Also fixes the Milvus docs page title and example config.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Simon Hearne <simon.hearne@gmail.com>
online_read and retrieve_online_documents_v2 called load_collection on
every request, adding a round-trip to each read. The call also hid the
fact that newly created collections were never loaded on Milvus
servers, because indexes were created after the collection.

Collections are now created with their index params, which makes
Milvus load them straight away. Existing collections are loaded only
if their load state isn't Loaded, and only the first time a store
accesses them. The per-query load_collection calls are removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Simon Hearne <simon.hearne@gmail.com>
@ntkathole
ntkathole merged commit f9e91fa into feast-dev:master Sep 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants