mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 10:15:43 +00:00
aea5d701bd
issue: #51104 companion SDK PR (merge first): milvus-io/pymilvus#3670 ## Summary `add_function_field` previously accepted a vector output field without any index meta; bump-schema-version compaction then stalled forever (no segment index task can be created for a field with no index defined, and `GetQueryVChanPositions` never hands off unindexed compacted-to segments — the fail-closed chain is silent). This PR makes the index meta a mandatory, atomically-bound part of the DDL: - **Mandatory at every layer**: SDK requires `index_params`; proxy and rootcoord reject vector function-output fields without an explicit `index_type` strictly before the broadcast (no silent AUTOINDEX). - **create_index-aligned materialization**: proxy normalizes the params with the same logic as `create_index` (name rules via `validateIndexName`, checker existence, `CheckTrain` incl. dimension filling, function-type defaults such as `bm25_k1/bm25_b/bm25_avgdl`) and writes them back into the request; rootcoord prepare allocates index id/name, rejects name conflicts and unknown index types, and serializes a complete `indexpb.FieldIndex` into the new `AlterCollectionMessageUpdates.bound_field_indexes` WAL field. - **Atomic apply**: the alter-collection ack callback commits the schema and then applies the bound index by dispatching a synthetic `CreateIndexMessage` to datacoord's existing `createIndexV2AckCallback` (same pattern as `cascadeDropFieldIndexesInline`). The callback is replayed until success across crashes and is fully idempotent (all ids come from the message body), so `DDL success ⇒ index meta exists` holds with no partial terminal state. - **Reuse promotion**: `ValidateIndexParams`/`CheckDuplidateKey` moved from datacoord to `internal/util/indexparamcheck`; shared `ExpandIndexParams` / `FillFunctionOutputIndexParams` / `PrepareFunctionOutputIndexParams` helpers now back both the create_index path and this DDL. - **Dead fan-out removed**: the ordinary `CreateIndex` broadcast is reverted to control-channel-only; the vchannel fan-out had no consumer and wrote one dead WAL entry per vchannel per index creation. - **No milvus-proto change**: the request already carried `FieldInfo.index_name`/`extra_params` (previously unused); this feature activates them. - e2e suites updated to the new semantics: `test_add_function_field_feature.py` rewritten (explicit `index_params` everywhere, post-DDL `create_index` blocks removed since the bound index conflicts with a second distinct index per existing semantics, new negative cases for the mandatory checks); the external-table negative case updated as well. ## Verification - New/updated unit tests: rootcoord DDL callbacks (mandatory/AUTOINDEX/unknown-index-type rejections, bound-index materialization + ack-callback application via a recording fake), proxy task validation (invalid index name, normalization write-back), shared helper tests. - End-to-end on a live cluster (StorageV3 + bump compaction enabled): request without params rejected; index meta visible with create_index-aligned params immediately after the DDL returns; bump compaction rewrote 2000-row sealed segments and the bound index built on the results (the exact chain that previously stalled); BM25 search over pre-DDL rows succeeds once the compacted segments are loaded. ## Limitations (follow-ups) - Live propagation of the new index meta to already-loaded QueryNode delegators is intentionally out of scope; a loaded collection observes the bound index through the segment load path (e.g. reload / compacted-segment load). Tracked as a separate follow-up PR. - The python e2e suites require the companion pymilvus change: milvus-io/pymilvus#3670 is merged and `tests/python_client/requirements.txt` is bumped to `pymilvus==3.1.0rc61` in this PR (all 22 rewritten cases + the external-table case verified locally against the released rc61). - During a rolling-upgrade window an old coordinator replaying the new message ignores `bound_field_indexes` (proto3 unknown field), i.e. pre-existing behavior; avoid the new argument in mixed-version clusters. - `AddCollectionFunction` on an existing field and plain add-vector-field keep their current behavior (the WAL carrier is generic for future extension). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>