FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Make Milvus index and search params configurable by simonhearne · Pull Request #6917 · feast-dev/feast · GitHub

Repository navigation

feat: Make Milvus index and search params configurable - #6917

Merged
ntkathole merged 1 commit into
feast-dev:masterfrom
simonhearne:feat/milvus-index-search-params
Oct 1, 2026
Merged

ntkathole merged 1 commit into
feast-dev:masterfrom
simonhearne:feat/milvus-index-search-params

Conversation

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Milvus search params were hard-coded to {"nprobe": 10} and index params to {"nlist": nlist}.
index_type: AUTOINDEX (the only option on Zilliz Cloud / managed Milvus) failed with
only metric type can be passed when use AutoIndex, for two reasons: nlist was always sent, and
collection_name was passed to IndexParams.add_index, which treats unknown kwargs as index params.

  • New index_params and search_params dicts are passed through to Milvus.
  • When unset, the old defaults are kept, except that AUTOINDEX gets no params.
  • collection_name is no longer passed to add_index.
  • AUTOINDEX can be tuned with search_params: {level: N}.
online_store:
  type: milvus
  index_type: "AUTOINDEX"
  search_params:
    level: 2

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 client): default params unchanged for IVF_FLAT; AUTOINDEX sends only the metric;
    HNSW index_params/search_params pass through.

  • Unit (Milvus Lite): AUTOINDEX + level search round trip.

  • Server: AUTOINDEX collection is created and searchable with level: 2.

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, following #6882 and #6895.

simonhearne requested a review from a team as a code owner October 1, 2026 08:28

codecov-commenter commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.52%. Comparing base (fe27230) to head (c48b65d).
⚠️ Report is 3 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6917      +/-   ##
==========================================
+ Coverage   48.50%   48.52%   +0.01%     
==========================================
  Files         427      427              
  Lines       53755    53774      +19     
  Branches     7827     7832       +5     
==========================================
+ Hits        26076    26092      +16     
- Misses      25813    25816       +3     
  Partials     1866     1866              
Flag Coverage Δ *Carryforward flag
go-feature-server 30.58% <ø> (ø) Carriedforward from fe27230
python-unit 49.88% <100.00%> (+0.01%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
.../infra/online_stores/milvus_online_store/milvus.py 72.21% <100.00%> (+0.96%) ⬆️

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 6161766...c48b65d. 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.

ntkathole force-pushed the feat/milvus-index-search-params branch from fe65208 to 37bbcb9 Compare October 1, 2026 09:43
Search params were hard-coded to {"nprobe": 10} and index params to
{"nlist": nlist}. index_type: AUTOINDEX failed on Milvus servers and
Zilliz Cloud with "only metric type can be passed when use AutoIndex",
because nlist and the collection name were sent as index params.

Adds optional index_params and search_params config fields that are
passed through to Milvus. When unset, the previous defaults are kept,
except that AUTOINDEX gets no params. The collection name is no longer
passed as an index param. AUTOINDEX can now be tuned with the level
search param.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Simon Hearne <simon.hearne@gmail.com>
ntkathole force-pushed the feat/milvus-index-search-params branch from 37bbcb9 to c48b65d Compare October 1, 2026 12:34
ntkathole merged commit ef743ff into feast-dev:master Oct 1, 2026
20 of 25 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL