mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 10:15:43 +00:00
master
914
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
e8b9dbff2f |
fix: fill collection_name and keep request id in DescribeCollection queried by id (#51313)
issue: #51253 > Rebased on master after #51254 merged. The `db_name`/`db_id` part of the original PR is now covered by #51254; this PR is reduced to the two remaining gaps on the same query-by-id path. ## Problem When `DescribeCollection` is called with **only a `collectionID`** — no collection name, no db name (e.g. the HTTP management API on `:9091`) — two gaps remain after #51254: 1. **Top-level `collection_name` is empty** on both the cached provider path and the `describeCollectionTask` path. The response is initialized from the (empty) `request.CollectionName` before the name is resolved, and never backfilled. 2. **The cached provider discards the caller-provided collection id.** After deriving the name from the id, it re-resolves the id from that name via `GetCollectionID(db, name)`. Id-only lookups are cached under the request db name — empty here — so with same-name collections in different databases, a concurrent cache refresh can rebind the entry under the `""` key and the request would silently describe the wrong collection (the derived name matches, the id does not). ## Fix - `service_provider.go`: - Backfill `resp.CollectionName` from the resolved cached schema when the request carried no name. Requests that pass a name (including aliases) still echo it unchanged. - Skip the name→id re-resolution when the caller already supplied the id. `GetCollectionInfo` already validates the cache entry against the id (`collInfo.collID != collectionID` → refresh by id), so the caller-provided id is authoritative end to end. Name+id requests keep the existing name-precedence behavior. - Resolve identifiers on local copies instead of rewriting `request.CollectionName`/`CollectionID` in place — the access log interceptor and the success metric labels serialize the request after the handler returns, and previously recorded the resolved values instead of what the client sent. - `task.go`: backfill `t.result.CollectionName` from the coordinator result on the non-cached path. - `meta_cache.go`: fix a stale comment on `ResolveCollectionAlias` — `update()` now always keys `collInfo` by the real collection name (aliases live in the separate alias map), the old comment described pre-refactor behavior. - Tests: - `TestCachedProxyServiceProvider_DescribeCollection_ByIDFillsNameAndUsesRequestID`: drives the id-only path; `GetCollectionID` is deliberately not mocked, so any regression back to re-resolving the id panics the test. - `TestDescribeCollectionTask_FillsNameFromResultWhenQueriedByID`: same assertion for the non-cached path. - Removed the now-unused `GetCollectionID` expectation from the test added in #51254 (that call no longer happens on the id-only path). ## Follow-up commit: entity-keyed meta cache with a cluster-wide id index Reviewing this path surfaced a deeper issue in the proxy meta cache, fixed in the second commit: - By-id lookups (`GetCollectionName`, `GetCollectionInfo` with an empty name) linearly scanned a whole db bucket under the read lock, on every request even on cache hits. - The bucket scanned/filled was keyed by the *request* db name — empty for id-only calls — so entries landed in a bogus `""` bucket: a collection also cached under its real db was duplicated, and same-name collections of different databases evicted each other from the single `""`-keyed slot. The cache is now entity-keyed instead of request-keyed: - Fills land under the collection's **actual database** (carried in the describe response); the `""` bucket no longer exists. - A cluster-wide `collectionID → entry` index serves by-id lookups in **O(1)** regardless of the request db — matching rootcoord, which resolves by-id describes straight from `collID2Meta` without consulting the db name. - Name lookups normalize an empty db name to `default`, mirroring rootcoord's backward-compat normalization (`meta_table.getCollectionByNameInternal`). This also closes a latent cross-database mis-hit: a name lookup with an empty db could previously return whichever same-name collection was last id-cached under the `""` bucket. - All removal paths maintain the index (pointer-identity-guarded unindexing); invalidation sweeps stay exhaustive. ## Verification - Ran locally against a current master core build: the full `TestMetaCache*`, `TestCachedProxyServiceProvider*`, `TestDescribeCollectionTask*`, alias and partition test sets all pass, plus the **full `internal/proxy` suite** (only the pre-existing etcd-dependent `TestProxyRpcLimit` fails locally — it needs a live etcd, unrelated to this change). - New tests: `TestCachedProxyServiceProvider_DescribeCollection_ByIDFillsNameAndUsesRequestID` (id-only path; also asserts the request is not rewritten), `TestDescribeCollectionTask_FillsNameFromResultWhenQueriedByID`, `TestMetaCache_ByIDIndexRealDBBucket` (same-name collections in two databases filled by id: no eviction, entries under real dbs, by-name reuses by-id fill, every removal path cleans the index), `TestMetaCache_EmptyDBNameSharesDefaultEntry`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ## Update: proxy meta cache rebuilt around an id-primary store (2 new commits) Following review discussions on invalidation cost and the alias-DDL races, this PR now also rebuilds the cache (issue: #51533, design doc: `docs/design-docs/design_docs/20260716-proxy-metacache-id-inverted-index.md` in this PR): 1. **`fix: forward the pre-alter alias target id in the AlterAlias expiration`** — rootcoord resolves the pre-alter target under its database lock and carries it in the new additive `AlterAliasMessageHeader.old_collection_id`; the ack callback emits a second expiration entry so proxies evict BOTH AlterAlias targets by id. Closes the race where a concurrent Describe re-points the proxy's alias resolution before the expiration arrives, leaving the old target permanently stale. Older rootcoords (field 0) fall back to the proxy's hint resolution. 2. **`enhance: rebuild the proxy meta cache around an id-primary store`** — the primary store is keyed by the cluster-unique collection id (single source of truth, with a per-database generation); name/alias resolution become hints validated against the primary on read; partition caches are keyed by id; fills are ordered against invalidations by a fill RWMutex (drain in-flight describes before evicting), replacing the per-collection timestamp floor. Every invalidation becomes O(1) — no more full-cache scans under the write lock on Load/Release/Drop/Rename/Alter broadcasts, and DropDatabase/AlterDatabase becomes a generation bump. Also removes a latent stale read (alias-keyed partition entries surviving DropCollection). Verification: targeted cache suites green (rename, alias re-point under concurrent describe, drop+recreate with a reused name, db-generation invalidation, cross-db isolation, gated-mock fill/invalidation ordering); three negative controls (hint validation / dbGen check / fill drain disabled) each fail exactly the test guarding them; full-package `-race` delegated to CI. --- ## Update: simplification series (final form) Following design review, the cache was iteratively simplified to an **id-primary store with declared hints** — every mechanism that could be replaced by an invariant was deleted (net-negative diffs throughout): - Primary store keyed by the cluster-unique collection id; liveness IS presence. Name/alias resolution are hints written ONLY together with their entry (declared aliases) and validated against the primary on every read — a stale hint can only cost one extra describe, never a stale read. - Fill/invalidation ordering via a fill RWMutex (drain in-flight describes before evicting); the per-collection timestamp floor, database generations, background GC, alias negative cache, per-partition cache, and the resolve-and-forget DescribeAlias path are all **deleted**. Evictions clean everything the entry owns synchronously; **no invalidation path performs any RPC**. - Old-rootcoord fallbacks: id-describes without DbName are served uncached; alias-DDL broadcasts without ids trigger hint resolution plus a holder scan **gated on that exact fingerprint** (dead code once rootcoords are upgraded), closing the ghost-alias case with no healing event. - **Accepted gaps (WONT-FIX)** are recorded in the design doc §6 — notably the upgrade-window new-target `Aliases` display lag (the association exists only at rootcoord; self-heals on first use; display-only). See `docs/design-docs/design_docs/20260716-proxy-metacache-id-inverted-index.md` for the full design, invariants and trade-offs. Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
5def5ced4a |
enhance: enforce function-field binding principle for function DDL (#51360)
## What Enforce a function–field binding principle for function DDL (add / drop / alter function on an existing collection), so a function and its output field stay coupled and already-stored vectors can never be silently invalidated by DDL. BM25 and MinHash follow the strict binding: add/drop only via the unified `add_function_field` / `drop_function_field`, output field always coupled. **TextEmbedding is a temporary carve-out** — its legacy `AddCollectionFunction` / `DropCollectionFunction` paths are retained, so attach-over-existing-field and detach are still possible for it, because `add_function_field` embedding backfill is not yet implemented. It will be folded into the unified path in a follow-up. So the "always coupled" invariant currently holds for BM25/MinHash, not TextEmbedding. ## Changes - **AlterCollectionSchema invariant**: reject standalone add-function (a function must be added together with its new output field), reject detaching a function without dropping its output field, and reject function cascade (a new function's input being another function's output). - **Legacy RPCs**: `AddCollectionFunction` / `DropCollectionFunction` are rejected for BM25/MinHash (must use `add_function_field` / `drop_function_field`) and retained only for TextEmbedding (see carve-out). `AlterCollectionFunction` is kept and gains the whitelist below. In the alter and legacy-add paths, function field IDs are always re-derived from field names (never trusted from the request), so a request cannot inject an unrelated field ID that a later `drop_function_field` would delete. - **alter_function whitelist**: only connection/runtime params may change (TextEmbedding: `url`, `credential`, `timeout_ms`, `max_client_batch_size`, `region`, `location`, `projectid`, `user`); BM25/MinHash have no alterable params. Function identity (type, name, input/output fields) and output-shaping params (`dim`, `model_name`, `endpoint` — the TEI model identity, `normalize`, `truncate*`, prompts, ...) are immutable. Params are normalized (keys lowercased, duplicate keys rejected) so a crafted duplicate key cannot bypass the diff. - **drop_function_field**: allow any output-producing function (dropping needs no backfill), removing the previous BM25/MinHash-only restriction. ## Not in scope / follow-up - Extending `add_function_field` to TextEmbedding (post-creation add would require backfilling every existing row through the external embedding model) — deferred; folding the TextEmbedding legacy paths into the unified binding follows this. - MinHash input-field analyzer immutability lives on a different DDL path (`AlterCollectionField`); tracked as a separate follow-up. ## Breaking changes - `DropCollectionFunction` on a **MinHash** function (detach — remove the function but keep its output field) is no longer supported; use `drop_function_field` instead (drops the function together with its output field). If a released version supported MinHash detach, migrate any such call to `drop_function_field`. issue: #51348 🤖 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 Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
66069bfc13 |
fix: validate collection schema mutations in rootcoord (#51368)
## Problem Milvus supports online collection-schema mutations — add/drop field, add/drop function, alter field, enable dynamic field — through several RootCoord DDL paths. Existing segments are written under the committed schema, so a mutation that reinterprets an existing field (its data type, nullability, or structural role) or relabels existing data as a function output makes already-written binlogs read incorrectly. Before this change there was no single authoritative check that a proposed schema is a **safe structural successor** of the committed one; validation was scattered and incomplete across DDL paths, so an unsafe mutation could reach the WAL and silently corrupt or misinterpret existing data. ## What this PR does Adds an authoritative, validation-only, fail-closed gate `ValidateSchemaEvolution(oldSchema, newSchema)`, invoked in every RootCoord schema-mutating callback **before** any side effect (analyzer reservation, bound-index allocation, WAL broadcast). It rejects: - in-place field reinterpretation — name, data type, element type, nullability, or structural role (primary / partition / clustering key, autoID, dynamic); - relabeling an existing field as a function output — function outputs must use brand-new fields, never existing data; - unsafe field additions — a newly added field may not arrive already marked primary / partition / clustering key, function output, or dynamic; - unsafe drops of protected fields; - more than one clustering key; - a dynamic field that is also a function output; - corruption of the reserved `max_field_id` watermark. Property-only / TTL updates, analyzer rollback, BM25 / MinHash `add_function_field`, and legacy function Drop / detach paths are preserved unchanged. ## Scope Non-function structural invariants only. Function-graph validation (single producer, output binding, alter-time immutability, cascade) is intentionally **out of scope here and owned by #51360**. This PR does not include schema propagation, Proxy barriers, DML draining, backfill / readiness, index promotion, TextEmbedding enablement, protobuf, configuration, or C++ changes. ## Validation - `go test -tags dynamic,test -gcflags='all=-N -l' ./internal/util/schemautil -count=1` — PASS (the gate's own unit tests) - `git diff --check origin/master...HEAD` — PASS issue: #51340 --------- Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
f662cec16d |
feat: add local format metadata and split policy (#51204)
relate: #50304 ## Summary Add the local format design doc, use the storage writer format constant, and introduce local_format metadata propagation with column-group split policy support. --------- Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
|
a50765f1c9 |
enhance: reduce coordinator metadata lock contention (#51255)
issue: #51333 ## Summary - serialize DataCoord checkpoint mutations per channel so unrelated channel catalog writes can proceed concurrently - keep overlapping single, batch, mark, and drop operations serialized with deterministic multi-channel lock ordering - use a read lock for the read-only RootCoord `DescribeAlias` path - exclude all newly added metrics and profiling instrumentation --------- Signed-off-by: sunby <sunbingyi1992@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
|
ee943a1bf5 |
enhance: allow updating inactive field analyzer params (#50492)
issue: #50961 ## Summary Allow AlterCollectionField to set or delete analyzer configuration on string fields before they are used by text match or BM25. Keep analyzer params immutable once text match is enabled or a BM25 function depends on the field. When analyzer params change, RootCoord revalidates the new analyzer config, reserves newly referenced file resources before broadcasting, and lets MetaTable.AlterCollection reconcile file resource refCnt for request, replay, replicated, and recovery paths. ## Verification - `git diff --check` - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./util/typeutil -run TestIsBm25FunctionInputField` from `pkg/` - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./internal/rootcoord -run 'TestMetaTableAlterCollectionFileResourceRefCnt|TestDDLCallbacksAlterCollectionFieldAnalyzerValidation'` blocked locally: missing `milvus_core.pc` - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./internal/proxy -run TestAlterCollectionField` blocked locally: missing `milvus_core.pc` and `milvus-storage.pc` Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
|
aea5d701bd |
enhance: bind index meta to add function field DDL (#51105)
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> |
||
|
XuanYang-cnandGitHub
|
d9bb262d71 |
enhance: simplify tsoutil timestamp composition (#51120)
Make ComposeTSByTime the logical-zero time-based timestamp API, add ComposeTSByTimeWithLogical for explicit logical composition in tests, and remove the duplicate GetCurrentTime wrapper. See also: #51119 Signed-off-by: yangxuan <xuan.yang@zilliz.com> Signed-off-by: yangxuan <xuan.yang@zilliz.com> |
||
|
|
590a14fdf7 |
fix: reject bare NULL literal in expressions instead of misparsing it as a field (#50889)
issue: #50882 ## Problem `NULL` is not a grammar token in the plan parser, so the lexer treats a bare `NULL`/`null` as an ordinary identifier. When it appears where a value is expected — e.g. inside an `in [...]` value list: ``` id in [6560, NULL, 6722, -7856, -6757] ``` `VisitIdentifier` runs a field lookup on `NULL`: - **without a dynamic field** it fails with the misleading `field NULL not exist`, which misleads users into thinking they must add a column named `NULL` to their schema; - **with a dynamic field** it is silently mistaken for a JSON key and accepted. This affects both `delete()` and `query()` (they share the same `ParseExpr` path). ## Fix Treat bare `null`/`NULL` (case-insensitive) as a reserved word in `VisitIdentifier` and reject it up-front with an actionable message, regardless of whether a dynamic field exists: ``` NULL literal is not supported in expressions; use '<field> is null' or '<field> is not null' instead ``` This implements option 1 (robust rejection) from the issue. `<field> is null` / `is not null` are unaffected (they parse via dedicated tokens, not `VisitIdentifier`). **The guard is schema-aware for backward compatibility** (review feedback): `null` only becomes a create-time keyword in this PR, so a legacy collection may own a field literally named `null`, and the bare identifier is the only syntax that can reference a top-level scalar field. The rejection is therefore gated on a strict `GetFieldFromName` lookup — a real declared field named `null` resolves exactly as before this PR (including `null is null`, which now parses as a valid predicate on that field), while the dynamic-field fallback (the source of the original misparse) still rejects. A JSON **sub-key** literally named `null` additionally remains reachable via quoting, e.g. `field["null"]` / `$meta["null"]`, whose base identifier is the field name, not `null`. ## Test Added `TestExpr_NullLiteral` covering rejection of `NULL` inside `in [...]`, as a comparison operand, and as a left operand, plus confirmation that `is null` / `is not null` still work. Added `TestExpr_NullLiteral_LegacyNullField` locking the legacy path: a scalar field named `null` stays queryable via the bare identifier, a JSON field named `null` stays reachable as `null["x"]`, and any casing that doesn't name a real field keeps the reserved-word rejection. Full `planparserv2` package tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Note: field-name validation tightened Alongside the expression-level rejection, `validateFieldName` (proxy) and `validateAddedStructFieldName` (rootcoord) now go through `common.IsFieldNameKeyword`, which rejects **any casing** of `null` (`null`/`NULL`/`nUlL`/…) as a field name — consistent with how a bare `NULL` literal is rejected in expressions. Previously `FieldNameKeywords` had no `null` entry, so a field literally named `null` could be created. This only affects **new** field creation (validation runs at create time); existing collections/fields are unaffected — and thanks to the schema-aware guard above, a legacy field literally named `null` also stays queryable. --------- Signed-off-by: xiaofanluan <xf@hjjaq.com> Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Zhen YeandGitHub
|
e203c56f0a |
fix: clean up mlog call formatting (#51027)
issue: #35917 - mlog formatting: remove blank lines inserted inside migrated log calls - streaming logs: collapse short mlog calls in streaming coordinator/node helpers for readability Signed-off-by: chyezh <chyezh@outlook.com> |
||
|
|
629c0a58d2 |
fix: log intra-cluster no-identity requests at debug level in rootcoord (#50987)
## What
`getCurrentUserVisible{Databases,Collections}` in rootcoord logged an
expected situation as a misleading warning. With authorization enabled,
every intra-cluster ListDatabases/ShowCollections call (datacoord /
querycoord brokers, in-process calls inside mixcoord) carries no user
identity, hits the fallback branch, and emitted:
```
[WARN] ["get current user from context failed"]
[error="fail to get authorization from the md, authorization:[token]: invalid parameter"]
```
The wording suggests a request input error, but there is no failed user
request at all: the RPC deliberately falls back to full visibility and
succeeds (see the RCA in #50974).
## How
- `contextutil.IsIntraClusterRequest(ctx)`: classifies the request by
the shape of the incoming gRPC metadata (see table below). The
authorization key always wins: a request carrying user identity is never
treated as intra-cluster.
- rootcoord logs the intra-cluster case at debug level with an explicit
"fall back to full visibility" message; any other identity-resolution
failure keeps the warning.
| incoming metadata | classified as | log |
|---|---|---|
| carries `authorization` | user request | (unchanged RBAC path) |
| no metadata at all | in-process intra-cluster call | debug |
| `ServerID`/`Cluster` keys, no `authorization` | intra-cluster gRPC |
debug |
| metadata present, none of the keys | unknown source | warn (kept) |
No behavior change: the AnyWord visibility fallback, the error
construction in contextutil (shared with user-facing paths and asserted
on by e2e), and all merr codes are untouched.
## Verification
Live-verified on a 2.6 build of this patch (standalone,
authorizationEnabled=true):
- startup intra-cluster calls (no-metadata variant): debug line emitted,
zero occurrences of the old warning in the whole log;
- gRPC probe to rootcoord with Cluster metadata and no authorization
(the variant seen in the 2.6-nightly CI logs): debug;
- gRPC probe with unrelated metadata only: warning kept;
- authenticated REST request: succeeds, no new log lines;
- unauthenticated external request: still rejected at the proxy edge and
never reaches rootcoord.
On master: package compile + unit tests (`pkg/util/contextutil` incl.
new `TestIsIntraClusterRequest`, `internal/rootcoord` visibility task
tests).
issue: #50974
Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Bingyi SunandGitHub
|
02b158cfcb |
enhance: route partition namespace requests (#50598)
issue: #50748 ## Summary - route namespace requests to partition names when `namespace.mode=partition` - keep the default namespace mode using `$namespace_id` plan filtering - prevent altering `namespace.mode` after collection creation Signed-off-by: sunby <sunbingyi1992@gmail.com> |
||
|
wei liuandGitHub
|
4d24af7063 |
feat: support Milvus snapshot as external table source (#49914)
issue: #45881 ## Summary Add the `milvus-table` external format so external collection refresh can consume Milvus snapshot metadata and StorageV3 segment manifests directly. This change lets refresh resolve source manifest row counts, rebuild target StorageV3 manifests from source column groups, copy source deltalogs with deterministic preallocated IDs, and keep real external primary-key semantics for milvus-table segments. ## Why Snapshot-backed external tables are not generic file tables. The refresh path needs to preserve Milvus StorageV3 manifest structure, deltalogs, and real PK behavior instead of falling back to virtual PK semantics or format-agnostic fragment handling. ## Details - Add milvus-table external spec parsing and snapshot manifest resolution. - Add StorageV3 manifest translation for source Milvus segment manifests. - Copy milvus-table deltalogs into target manifests with deterministic LogIDs. - Wire QueryNode real-PK lookup/load handling for milvus-table segments. - Add unit and Go client coverage for spec parsing, manifest resolution, refresh, and deltalog handling. ## Validation - `git diff --cached --check` - No proto changes detected - No mockery patterns in changed test files - `source ~/.profile && source scripts/setenv.sh && make milvus` - Reset Milvus local standalone before Go tests - `go test -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${RPATH}" github.com/milvus-io/milvus/internal/datanode/external github.com/milvus-io/milvus/internal/storagev2/packed -count=1 -timeout 300s` - `source ~/.profile && source scripts/setenv.sh && make lint-fix` Related: [#45881](https://github.com/milvus-io/milvus/issues/45881) --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
Zhen YeandGitHub
|
b07c62d53c |
enhance: replace log package with context-aware mlog (#50094)
issue: #35917 - mlog package: move logger initialization, zap core, async buffered writes, field helpers, and scoped logger binding into pkg/mlog while removing pkg/log. - logging callsites: migrate Milvus logging usage to context-aware mlog APIs and simplify redundant With chains across components, utilities, tests, and tools. - trace propagation: replace logutil trace interceptors with mlog/tracer integration and add client_request_id fallback propagation for server stats handlers. --------- Signed-off-by: chyezh <chyezh@outlook.com> |
||
|
Bingyi SunandGitHub
|
4409bf1b63 |
enhance: support partition namespace creation (#50594)
## Summary - Skip injecting `$namespace_id` when `namespace.mode=partition`. - Avoid appending `partitionkey.isolation=true` for partition namespace mode. - Validate namespace mode before namespace field handling. Signed-off-by: sunby <sunbingyi1992@gmail.com> |
||
|
sijie-ni-0214andGitHub
|
7ac57bf2f6 |
fix: Split function drop semantics by request flag (#50471)
Fixes #50424 ## Summary This PR uses the `DropFunctionOutputFields` request flag to let `AlterCollectionSchema` distinguish two client-side drop paths: - `drop_collection_function` detaches the function and keeps its output fields as normal schema fields. - `drop_function_field` sets the flag and cascades supported BM25/MinHash output fields. MinHash detach keeps the output field because the MinHash signature is a materialized `BinaryVector` field. Insert and compaction paths persist the generated signature, raw retrieval is only blocked for BM25 function outputs, and after detach the MinHash output field is kept with `IsFunctionOutput=false` so it can stand alone as a normal vector field. The proxy validation and RootCoord schema mutation logic now follow the requested semantics, with tests covering detach-only and output-field-drop behavior. ## Tests - `make generated-proto-without-cpp` - `make` - `go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" ./internal/proxy -run "TestAlterCollectionSchemaTask_PreExecute|TestValidateDropFunction" -timeout 300s` - `go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" ./internal/rootcoord -run "TestBuildSchemaForDetachFunction|TestBuildSchemaForDropFunctionField" -timeout 300s` - `/Users/zilliz/dev/milvus_py`: `pytest test_drop_collection_field.py::test_drop_function_field_bm25_cascades_output_field test_drop_collection_field.py::test_drop_collection_function_detaches_minhash_output_field test_drop_collection_field.py::test_drop_whole_struct_array_field_basic -v -s` Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> |
||
|
sijie-ni-0214andGitHub
|
ea76e2a7ec |
fix: support alter schema add request validation (#50118)
## Summary - Allow AlterCollectionSchema AddRequest to carry a field-only add request while keeping the added field as a non-function output. - Support function-only add requests against existing output fields, and validate the merged function schema through the shared function validator. - Keep the BM25 field-plus-function path explicit, reject invalid BM25 add forms, and normalize TIMESTAMPTZ default values before broadcasting field-only schema changes. issue: #50119 ## Test Plan - [x] source scripts/setenv.sh && go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${MILVUS_WORK_DIR}/cmake_build/lib -r ${MILVUS_WORK_DIR}/internal/core/output/lib" ./internal/proxy -run 'TestAlterCollectionSchemaTask($|_)' -timeout 300s - [x] gofumpt -l internal/proxy/function_task.go internal/proxy/function_task_test.go internal/proxy/task.go internal/proxy/task_test.go internal/proxy/util.go internal/proxy/util_test.go internal/rootcoord/ddl_callbacks_alter_collection_schema.go internal/rootcoord/ddl_callbacks_alter_collection_schema_test.go internal/util/function/validator/validator.go - [x] git diff --check upstream/master..HEAD - [ ] source scripts/setenv.sh && go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${MILVUS_WORK_DIR}/cmake_build/lib -r ${MILVUS_WORK_DIR}/internal/core/output/lib" ./internal/rootcoord -run 'TestDDLCallbacksBroadcastAlterCollectionSchema|TestDDLCallbacksAlterCollectionSchemaAddSkipsSchemaDropReady' -timeout 300s (blocked locally at link time by stale C++/Loon symbols such as _FreeCFieldMemSizeList and _loon_segment_writer_* in local core libraries) Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> |
||
|
|
85e6523f36 |
enhance: validate TEXT add field and optimize LOB text indexing (#50494)
issue: #50425 Signed-off-by: luzhang <luzhang@zilliz.com> Co-authored-by: luzhang <luzhang@zilliz.com> |
||
|
Bingyi SunandGitHub
|
664c1f5076 |
enhance: add namespace sharding collection property (#50541)
issue: #50557 Summary: - Add and validate `namespace.sharding.enabled` collection property. - Default unset property to `false`. - Reject changing or deleting it via alter collection properties. Signed-off-by: sunby <sunbingyi1992@gmail.com> |
||
|
Bingyi SunandGitHub
|
c4d98d6d88 |
enhance: add namespace mode property (#50504)
## Summary - Add `namespace.mode` collection schema property helpers with `partition_key` as the default mode. - Accept only `partition_key` and `partition` namespace modes; reject invalid values such as `multitenant`. - Validate namespace mode during collection creation so valid values are persisted in `CollectionSchema.Properties`. issue: #50589 --------- Signed-off-by: sunby <sunbingyi1992@gmail.com> |
||
|
sthuangandGitHub
|
ce5d6b5088 |
enhance: [RBAC] support role descriptions (#50184)
- Persist role descriptions when creating roles, expose them in role list/describe results, and support updating descriptions through the AlterRole path without changing role names. - Store role descriptions in the role value body while keeping role names in keys, and tolerate legacy empty role values plus undecodable stored values by returning an empty description for that role row. - Reject built-in/default roles and over-limit descriptions before WAL writes, and share the role-description length validator between proxy and rootcoord. related: #50183 --------- Signed-off-by: shaoting-huang <shaoting.huang@zilliz.com> |
||
|
sthuangandGitHub
|
5d80a9fd81 |
feat: [RBAC] support user description in credentials (#50186)
- Wire credential descriptions through create/update/read paths, including proxy, HTTP v2, Go SDK options, length validation, optional update presence handling, and internal credential proto/model persistence. - Preserve existing encrypted passwords and descriptions during RootCoord credential updates when either field is omitted, reject all-empty or partial direct password updates, and keep sha256 passwords cache-only. - Skip auth-cache refresh for description-only credential updates so existing authentication state is not blanked, while still refreshing caches for password updates. - Return persisted user descriptions from select_user and HTTP v2 describe_user, including include_role_info=false, and reuse already-loaded credential rows in user listing and RBAC backup to avoid duplicate etcd reads while ignoring malformed credential keys. - Use the upstream milvus-proto go-api/v3 version that includes the credential description proto change; the temporary fork replaces in root, pkg, client, and tests/go_client modules have been removed. design doc: docs/design-docs/design_docs/20260601-rbac-user-description.md related: #50179 --------- Signed-off-by: shaoting-huang <shaoting.huang@zilliz.com> |
||
|
|
e2787d3981 |
enhance: standardize error handling on merr + Sys/Input classification (#50221)
issue: #47420 ## What this PR does Project-wide migration of raw `fmt.Errorf` / `errors.New` in function bodies onto the `merr` framework, plus the Sys-vs-Input error classification and the machinery it drives (retriability, fine-grained metrics, segcore unification), plus the convention docs and a linter that keeps it from regressing. Scope: storage, proxy, coordinators (root/data/query), query node, data node, `pkg/util` & `internal/util`, expression parser, message queue, streaming, and misc packages. Bare raw-error usages went from ~3000 to a ~340 allowlist (package-level sentinels / build-tag / test sites). --- ## How to review this PR It is large but the vast majority is mechanical. Changes fall into three tiers; spend review budget on Part 2 and Part 3. ### Part 1 — Mechanical standardization (low risk, verify by rule) Each converted call follows one of a small fixed set of rules. To review, check that each site obeys the matching rule rather than reading every line: | Pattern | Rule | |---|---| | `fmt.Errorf("...")` originating a new error | → `merr.WrapErrXxxMsg("...")` with a code matching the failure's meaning | | Adding context to an existing typed error | → `merr.Wrap(err, "...")` / `merr.Wrapf(...)` — **preserves** the inner code (never `WrapErr*Err`, which overwrites it) | | Errors inside the streaming subsystem | → `status.New*` factories (StreamingError), **not** merr — this is the component-internal dialect (see `docs/dev/error_handling_guide.md`) | | Low-level / control-flow signal caught by `errors.Is` | → kept as a package-level `errors.New` sentinel (lowercase, same-package) | Conventions are documented in `docs/dev/error_handling_guide.md` (how-to) and `docs/dev/error_sentinel_convention.md` (rules + audit). A `gocritic`/`ruleguard` rule (`rawmerrerror`, in `rules.go`) enforces "no raw `return errors.New/fmt.Errorf`" under `make verifiers`. ### Part 2 — Behavior changes (review these closely) These are the sites where the wire contract or runtime behavior changes, not just the source text. Listed by category; representative locations given, full set in the diff. **A. gRPC wire-code shifts: `UnexpectedError(1)/Code 65535` → typed code.** Where a handler previously returned a raw error (collapsed to `Code=65535` on the wire), it now returns a typed merr, so the client sees a real code. The most common shift is to `IllegalArgument(5)/Code 1100` (ParameterInvalid). Touch points include datanode task handlers (CreateTask/Query/Drop), proxy Upsert, querynode GetMetrics, datacoord CreateIndex, httpserver query-response builder, and typeutil schema validation. One code refinement: an index-param validation moved `1100` → `1101` (ParameterMissing). **Client/SDK assertions and any code that switched on `Code=65535` for these paths must be re-checked** (the go_client e2e assertions were already aligned in this PR). **B. Prometheus `status` label contract change (externally visible).** The proxy metric's coarse `fail` / `rejected` values are split into `fail_input` / `fail_system` and `rejected_user` / `rejected_system` (in `requestutil.ParseMetricLabel`; auth/privilege rejections count as `rejected_user`), so dashboards can attribute a failure to caller vs operator. **Dashboards/alerts querying `status="fail"` must migrate to `status=~"fail_.*"`, and `status="rejected"` to `status=~"rejected_.*"`.** The in-repo Grafana dashboard is already migrated; external dashboards built on the old values silently go empty after upgrade. This is the one change that requires an ops-side migration. **C. Retriability semantics.** - C1: `merr.Status(err)` now forces `Retriable=false` when the error is an `InputError` — a malformed request can never succeed on blind retry, so clients never get the self-contradictory "your input is wrong but you may retry". - C2: `retry.Do` short-circuits an `InputError` (non-retriable) — **but only when the caller did not pass a `RetryErr` predicate**. The check is an `if c.isRetryErr != nil { ... } else if InputError { ... }` *mutually exclusive* branch (`pkg/util/retry/retry.go`): an explicit `RetryErr` takes precedence and bypasses the InputError abort. `retry.Handle` deliberately does **not** apply the InputError abort (its callers signal abort via `shouldRetry=false`). Four flusher startup callsites that must retry through transient "not ready" errors were given explicit `RetryErr` escape hatches. **D. segcore (C++→Go) error classification.** A single shared Go-side table (`pkg/util/merr/segcore.go`) maps each segcore code to a merr sentinel + InputError/signal category, replacing scattered hand-written `if errorCode == ...` switches in the cgo wrappers. **Wire `Code` values change for every segcore pass-through error, not just the remapped ones.** Named sentinels remap (C++ `2003` → merr `2001`, `2033` → `2002`, Folly/Knowhere codes likewise); **all remaining pass-through codes (`2004`–`2043`, previously surfaced to clients as raw C++ enum values) now serialize as `2000`** (`ErrSegcore`), with the original C++ code preserved in the `Reason` text (`segcoreCode=...`); unknown/future codes collapse to `2000` as well (pinned by the `wire_code_projection` test). Transient segcore classes (object storage / file IO / OOM / mmap / FieldNotLoaded — 11 codes) now report `Retriable=true`. **Any client switching on raw segcore codes in the `2004`–`2043` range must be re-checked**; the in-Reason code remains available for diagnostics. Signal codes (PretendFinished / FollyCancel) are recognized centrally. `errors.Is`-based control flow on these (e.g. scheduler skip/retry) is preserved. **E. InputError classification (25 sentinels + dynamic marks).** 25 sentinels in `errors.go` carry `WithErrorType(InputError)` (the Collection / ResourceGroup / Database families, `ErrIndexDuplicate`, `ErrParameterInvalid`, `ErrPrivilegeNotAuthenticated`, `ErrImportFailed`, `ErrQueryPlan`, ...), plus dynamic marks for the 8 segcore input codes (ExprInvalid, DimNotMatch, MetricTypeInvalid, FieldIDInvalid, ...) and `WrapErrAsInputError`. The widest blast radius is `ErrParameterInvalid` (1100): ~2335 `WrapErrParameterInvalid*` callsites now classify as input / non-retriable. Because of C1/C2 this changes retriability for any path that returns these. **The audit to confirm no transient path was mis-marked is the single most important review item** (see Part 3). One reverse correction: storage field-stats parsing moved from `ErrParameterInvalid` (input) to `ErrDataIntegrity` — a corrupted stored stat is data corruption, not user input. ### Part 3 — Known risks & traps (called out proactively) 1. **`merr.Wrap` vs `WrapErr*Err` (code-masking).** `WrapErr*Err` builds a `wrappedMilvusError{sentinel: ErrServiceInternal}` whose `code()` returns the *outer* sentinel — it overwrites the inner typed code and hides the `errors.Is` chain. This is intentional (use it to *deliberately* downgrade), but it was a recurring conversion defect; the rule "add context with `merr.Wrap`, downgrade with `WrapErr*Err`" is enforced by convention and reviewed across the diff. 2. **InputError × `retry.Do` blast radius.** Marking a sentinel `InputError` makes any `retry.Do(...)` without a `RetryErr` predicate stop retrying it. Reviewers should sanity-check that no transient use of the 19 newly-marked sentinels (especially `ErrParameterInvalid`) sits inside a retry loop that needed to keep spinning. The known flusher cases were handled (see C2). 3. **The ~340 raw-error allowlist.** What remains as bare `errors.New` is, by design: package-level sentinels (caught by `errors.Is`), `//go:build test` sites, and out-of-band trees (`cmd/`, `tests/`, codegen, walimpls). The linter only bans the *direct-return* form; assignment-then-return escapes and the full no-exceptions ban are deferred to an AST-based linter (Tier 2, documented). 4. **segcore C++ second step deferred.** This PR unifies classification on the Go side; splitting the dual-semantic C++ codes at the source is a follow-up. --- ## Validation - `make verifiers`: Go side clean (gofmt + static-check across modules, including the new `rawmerrerror` rule with a 0-hit baseline repo-wide). - `make test-go`: passing; the one real regression introduced (a datanode `invalid_task_type` assertion shifting `1` → `5` from a ParameterInvalid conversion) was fixed in-tree. - go_client e2e CreateIndex assertions aligned to the new merr messages. --------- Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
f756c52a22 |
enhance:refactor flush source (#50300)
issue: #50425 Signed-off-by: luzhang <luzhang@zilliz.com> Co-authored-by: luzhang <luzhang@zilliz.com> |
||
|
|
59401b0fa4 |
enhance: support minhash schema bump backfill (#50330)
## What this PR does This PR supports end-to-end add-MinHash-function schema evolution by: - allowing RootCoord alter-schema to accept MinHash functions with strict arity validation; - adding a concrete MinHash record materializer in DataNode compactor; - extending bump schema version compaction to materialize missing MinHash BinaryVector outputs while keeping BM25 stats BM25-only. ## Tests - source ./scripts/setenv.sh && go test -v -count=1 -gcflags="all=-N -l" -tags dynamic,test ./internal/datanode/compactor -run 'TestBumpSchemaVersionCompactionTaskSuite/TestBumpSchemaVersionCompactionMaterializesMinHashOutput' - source ./scripts/setenv.sh && go test -v -count=1 -gcflags="all=-N -l" -tags dynamic,test ./internal/datanode/compactor -run 'Test(BumpSchemaVersionCompactionTaskSuite|RecordMaterializer|BM25FunctionMaterializer|MinHashFunctionMaterializer)' - source ./scripts/setenv.sh && go test -v -count=1 -gcflags="all=-N -l" -tags dynamic,test ./internal/rootcoord -run TestDDLCallbacksBroadcastAlterCollectionSchema issue: #48808 🤖 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 Sonnet 4.6 <noreply@anthropic.com> |
||
|
sijie-ni-0214andGitHub
|
d9bbbc13d0 |
feat: support drop field via AlterCollectionSchema (#48988)
## Summary - Support dropping fields from collection schema via `AlterCollectionSchema` RPC with `DropRequest` - Support dropping function fields (cascade delete output fields and indexes) - Support disabling dynamic field via `AlterCollectionSchema` (reuse existing `AlterCollection` logic) - C++ segcore defense-in-depth: skip dropped fields during segment loading issue: #48983 design doc: docs/design-docs/design_docs/20260413-drop-collection-field-design.md ## Test plan - [x] Unit tests for rootcoord drop field/function logic - [x] Unit tests for proxy task validation (drop field constraints) - [x] Unit tests for datacoord index service (dropped field handling) - [x] Unit tests for querynode segment loader (dropped field skipping) - [x] C++ unit tests for SegmentLoadInfo ComputeDiff with dropped fields - [ ] E2E tests: drop scalar field, drop vector field, drop field with index, drop function, disable dynamic field (deferred to QA pending pymilvus RC) 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> |
||
|
wei liuandGitHub
|
6615811dee |
enhance: Support add-column refresh for external collections (#50082)
issue: #45881 Design doc: docs/design-docs/design_docs/20260526-external_table_add_column_refresh.md This PR lets external collection refresh handle additive schema changes, especially adding columns to parquet-backed external collections after segments have already been refreshed. What changed: - DataNode detects existing same-fragment external segments that are missing newly added external columns, appends manifest column groups idempotently, rebuilds fake binlogs, and returns same-ID updated segments. - DataCoord applies kept and updated refresh task payloads as validated segment upserts, including both patched existing segments and newly created segments. - Refresh apply intentionally does not use a job/task schema-version gate for the current additive-only scope. An older-schema refresh may complete without the newest fields, and the next refresh self-heals missing external columns. Segment-level validation still rejects schema-version rollback. - Proxy and RootCoord validate external add-field and alter-schema flows after field IDs are resolved, rejecting duplicate external_field mappings, generated output collisions, and unsupported external field types. - StorageV2 packed manifest helpers can read existing column groups and append missing columns through CommitManifestUpdates. - Patched same-fragment segments include function-output bytes in fake binlog MemorySize so memory estimates include generated columns. - Added the add-column refresh design doc and linked it from the external table design doc. - Added a go-client e2e that adds parquet column score after the first refresh and verifies returned query values, including 0..4 -> 0..0.04 and 100..104 -> 1.00..1.04. Validation: - make generated-proto-without-cpp - make milvus - make lint-fix - git diff --check - git diff --check milvus/master...HEAD - No-new-mockery diff scan - reset Milvus before scoped Go test runs - go test -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${RPATH}" for targeted typeutil, proxy, datacoord, and storagev2/packed tests - go test -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${RPATH}" github.com/milvus-io/milvus/internal/rootcoord -run '^$' - go test -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${RPATH}" github.com/milvus-io/milvus/internal/datanode/external -run TestRefreshExternalCollectionTaskSuite -testify.m TestOrganizeSegments_PatchesMissingExternalColumns - go test -tags dynamic,test -gcflags="all=-N -l" -ldflags="-r ${RPATH}" github.com/milvus-io/milvus/internal/storagev2/packed github.com/milvus-io/milvus/pkg/v3/util/typeutil -run 'TestAppendSegmentManifestColumnsValidationPaths|TestAppendSegmentManifestColumnsReadAndNoopPaths|TestNormalizeAndValidateExternalCollectionSchema' - internal/datacoord full package coverage hit the known local macOS DISKANN subcase; targeted refresh task tests passed after reset Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
congqixiaandGitHub
|
ba538c6b78 |
enhance: add cluster-wide read task queue clearing (#50090)
Related to #50089 Add a Proxy management entry point that routes through MixCoord to clear queued read tasks across all Proxy and QueryNode instances. Queued tasks are failed with canceled semantics and per-component counts are returned so operators can recover from read-task backlog without attempting unsafe active-task preemption. --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
|
7841f8a9b0 |
enhance: use bump_schema_version to involve backfill and drop field compaction(#48808) (#49759)
related: https://github.com/milvus-io/milvus/issues/48808 Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> |
||
|
wei liuandGitHub
|
7bdd40d692 |
enhance: [ExternalTable Part10] enable function output fields on external collections (#49307)
issue: #45881 ## Summary Part 10 of the External Table series. Enables BM25 / MinHash / TextEmbedding function output fields on external collections, and makes external-table text_match work with persisted text indexes. Related: [#45881](https://github.com/milvus-io/milvus/issues/45881) (External Collection Lakehouse Integration tracking). ### What's in this PR **Schema & segcore** - Allow `Function` declarations on external schemas; function-output fields skip `external_field_mapping` validation - `Schema` tracks function-output field IDs; `ChunkedSegmentSealedImpl` and `ManifestGroupTranslator` resolve function-output columns by field name - Fast paths for retrieve / search load function-output columns by field name; external columns continue using `external_field_mapping` - `Util.cpp::GetFieldDatasFromManifest` resolves function-output columns by field name so indexbuilder reads the correct packed column **Refresh pipeline (format-agnostic via loon FFI)** 1. `CreateSegmentManifestWithBasePath` creates the input manifest that references external original files as CG0 2. `FFIPackedReader(v1)` streams input columns into `InsertData` via the shared `ArrowRecordToInsertData` helper 3. `embedding.RunAll` populates function-output fields in-place 4. `FFIPackedWriter` writes output fields as a new column group on top of v1 5. `AddStatsToManifest` registers BM25 stats into the final manifest 6. Memory peak stays around one Arrow batch, defaulting to 64 MiB **RootPath isolation** - Function-output input manifests, output manifests, BM25 stats, and text index artifacts now use the same `RootPath/insert_log/<collection>/<partition>/<segment>` layout as normal external table StorageV3 manifests - This avoids writing function-output artifacts under bucket-root `external/...` paths when multiple clusters share the same bucket **Text match** - External text fields with `enable_match` persist text index artifacts during refresh and load them through the regular external segment path - English and Chinese analyzer coverage are both included in the E2E test **Cross-bucket** - `NewPackedFFIReaderWithManifest` accepts `ExternalReaderContext` and injects per-collection `extfs.{collID}.*` aliases when `external_source` is set. Existing callers pass `{}` and are unaffected **VirtualPK** - `VirtualPKChunkedColumn` implements `GetChunk` / `GetAllChunks` so proxy requery filter-by-PK works for external collections with virtual primary keys **Shared embedding runner** - New `internal/util/function/embedding/runner.go` provides canonical `RunAll(ctx, schema, data, opts)` shared by import and external-table refresh paths - Supports BM25 output vector types including Float, BFloat16, Float16, Binary, and Sparse ### Test Plan - [x] Go unit tests for `internal/datanode/external` with 99.8% package coverage - [x] Go unit tests for changed import, packed, embedding, and schema helpers from the existing branch validation - [x] C++ rebuild + `milvus_storage` / `milvus_core` lib install from the existing branch validation - [x] E2E function-output tests in `tests/go_client/testcases/external_table_function_test.go` - `TestExternalTableBM25Function` - `TestExternalTableMinHashFunction` - `TestExternalTableTextEmbeddingFunction` - [x] `TestExternalTableTextMatch` with 10 files x 5000 rows = 50000 rows, English and Chinese text fields, persisted text index object checks in MinIO, load readiness check, and query correctness checks Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
wtandGitHub
|
90fff49eb6 |
fix: count only available partitions for create partition limit (#49902)
### What changed - Use `collMeta.GetPartitionNum(true)` for the `CreatePartition` max partition limit check so dropped/unavailable partitions are not counted against `RootCoordCfg.MaxPartitionNum`. - Keep the partition-name index lookup path introduced by #47486. - Add a RootCoord regression test that drops a partition and verifies a new partition can still be created when only available partitions are below the limit. ### Why `CreatePartition` loads collection meta with `allowUnavailable=true`, so `collMeta.Partitions` can include dropped partitions. Using `len(collMeta.Partitions)` made dropped partitions count as live partitions and blocked partition creation at the max limit. issue: #49832 ### Verification - Argo benchmark validation passed with image `fix-rootcoord-create-partition-count-available-20260515-38e7e8cf79-v2-amd64`: [main logs](https://argo-workflows.zilliz.cc/artifacts-by-uid/5fddca83-21ee-42f1-9dc3-122edb5883ef/fouramf-vgtzw-3278604834/main-logs) Signed-off-by: wangting0128 <ting.wang@zilliz.com> |
||
|
aoiasdandGitHub
|
b05ce85630 |
fix: sync up drop collection ack (#49412)
relate: #48612 ## Summary Enable AckSyncUp for all DropCollection broadcasts so drop acknowledgement waits for downstream DDL handling instead of using the default FastAck path. Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> |
||
|
Spade AandGitHub
|
b944be4a31 |
feat: impl StructArray -- support dynamic add struct field (#49807)
issue: https://github.com/milvus-io/milvus/issues/42148 design doc: docs/design-docs/design_docs/20260306-struct.md this PR also fixes https://github.com/milvus-io/milvus/issues/49693 --------- Signed-off-by: SpadeA <tangchenjie1210@gmail.com> |
||
|
Spade AandGitHub
|
3fb7c9d63b |
feat: impl StructArray --- support null for struct array and vector array (#48553)
issue: https://github.com/milvus-io/milvus/issues/42148 design doc: https://github.com/milvus-io/milvus-design-docs/blob/main/design_docs/20260306-struct.md --------- Signed-off-by: SpadeA <tangchenjie1210@gmail.com> |
||
|
Bingyi SunandGitHub
|
f484f3c8cf |
enhance: improve the preformance of create partitions (#47486)
issue: https://github.com/milvus-io/milvus/issues/47403 this pr contains performance optimization for concurrent CreatePartition: 1. add a version cache on proxy for partition level cache 2. avoid repeated updates of target when there're many CreatePartition requests. 3. avoid unnecessary copy of meta data. --------- Signed-off-by: sunby <sunbingyi1992@gmail.com> |
||
|
Buqian ZhengandGitHub
|
4884987b0c |
enhance: general optimizations (#49381)
* Optimize metadata and request handling hot paths * Make replication pending-message queue capacity and max size configurable instead of hard-coded. * Batch QueryCoord collection metadata saves with MultiSave to avoid oversized single writes while still reducing per-key etcd operations. * Reduce avoidable allocations in DataCoord catalog tests, access log list formatting, HTTP array joining, and HTTP JSON field extraction. * Use WAL-specific message ID decoding during flusher recovery. * Use set-based RootCoord collection-name filtering and structured collection-not-found errors. issue: #44452 Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com> |
||
|
congqixiaandGitHub
|
7311d450a0 |
enhance: bump Go dependencies to v3 modules (#49485)
Related to #49398 Bump Go module references from pkg/v2 and milvus-proto/go-api/v2 to pkg/v3 and milvus-proto/go-api/v3 so the client tracks the Milvus 3.x release line. This prepares the repository for the upcoming 3.x.y release by aligning imports, module dependencies, and proto API references with the new major-version module paths. --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
wei liuandGitHub
|
3ae4cb28f5 |
fix: external table stability pack for alter tuple, refresh, spec validation, and datanode edge cases (#49366)
issue: #49335, #48830, #49338, #49334, #49292, #49336 ## Summary This PR delivers a stability hardening pack for external table lifecycle and spec handling. ### 1) Alter/Refresh metadata correctness - Guard invalid tuple transitions in alter-collection path. - Persist refreshed external tuple on refresh completion. - Disable unsupported `TruncateCollection` on external collections. ### 2) ExternalSpec safety and contract clarity - Require explicit `cloud_provider`; remove scheme-based inference. - Redact credentials from `DescribeCollection`. - Reserve system field names (including `__virtual_pk__`) from user schema. ### 3) Runtime robustness and edge-case handling - Skip anonymous entries in extfs Layer0 zero-init. - Align DataNode explore-manifest sort + format-filter behavior. - Prevent datanode crash on zero-row parquet. - Surface real sampling error in `RefreshFailed`. - Improve compatibility for vortex schemaless view and large Arrow type variants. ## Why External-table alter/refresh flows had validation and persistence gaps, plus multiple edge-case failures in runtime paths. This PR closes those gaps to make behavior safer, deterministic, and debuggable. ## Test Plan - Unit: - `pkg/util/externalspec/external_spec_test.go` - `internal/rootcoord/create_collection_task_test.go` - `internal/rootcoord/ddl_callbacks_alter_collection_properties_test.go` - `internal/core/unittest/test_external_take.cpp` - E2E / go client: - `tests/go_client/testcases/external_table_test.go` - `tests/go_client/testcases/external_table_iceberg_e2e_test.go` --------- Signed-off-by: wei liu <wei.liu@zilliz.com> Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
congqixiaandGitHub
|
8413af60c5 |
fix: stop dropping ExternalSource and other 2.6.5+ schema fields on rootcoord broadcasts (#49360)
schemapb.CollectionSchema gained five new fields in 2.6.5+ —
ExternalSource, ExternalSpec, DoPhysicalBackfill, FileResourceIds and
EnableNamespace (proto fields 11–15). rootcoord constructs the proto by
hand at 8 sites: the AddField / AlterField / AlterSchema / AlterFunction
/ AlterCollection DDL callbacks, the BroadcastAlteredCollection RPC to
DataCoord, the TTL- and dynamic-field paths in AlterCollection, and the
DescribeCollection response. Each literal has to enumerate every field
it copies, and over several PRs they have drifted: 6 of the 8 paths drop
ExternalSource on every broadcast, 7 drop EnableNamespace, and the loss
pattern for DoPhysicalBackfill / FileResourceIds is ad-hoc. The
rootcoord-side *model.Collection still has the data; only the schema
snapshot it broadcasts to DataCoord and writes into the AlterCollection
WAL messages is missing fields. Downstream components that consume those
fields silently see zeros after any benign DDL.
Convert the literals to a single helper on the model:
- Add (*model.Collection).ToCollectionSchemaPB() in
internal/metastore/model/collection.go. It copies every schema-level
field once (Name, Description, AutoID, Fields, StructArrayFields,
Functions, EnableDynamicField, EnableNamespace, Properties, DbName,
SchemaVersion, FileResourceIds, ExternalSource, ExternalSpec,
DoPhysicalBackfill). Future additions live here only.
- Replace all 8 literal sites in internal/rootcoord/ with the helper.
Operation-specific overrides become explicit field assignments on the
returned schema:
* Version = SchemaVersion + 1 for AddField / AlterField / AlterSchema /
AlterFunction / dynamic-field-enable; unchanged for the TTL property
snapshot and BroadcastAlteredCollection.
* Properties = newPropsKeyValuePairs for the TTL path.
* EnableDynamicField = targetValue for the dynamic-field-enable path.
* DoPhysicalBackfill = addRequest.GetDoPhysicalBackfill() for
AlterSchema (request-driven, not coll-driven).
* Fields/Functions get appended after construction where the DDL adds
new ones. DescribeCollection keeps overriding DbName with the
request-resolved dbName for resp.DbName/resp.Schema.DbName consistency.
- Drop the now-redundant explicit ExternalSource/ExternalSpec
assignments the TTL path used to need.
- Add TestCollection_ToCollectionSchemaPB asserting every field round-
trips. This is the tripwire: any future addition that forgets to wire
the helper will fail this test before it reaches a broadcast path.
Behavior change for DataCoord: BroadcastAlteredCollection.Schema now
carries ExternalSource, ExternalSpec, FileResourceIds, EnableNamespace,
EnableDynamicField and DbName which it had been silently losing.
DataCoord consumers only read the fields they care about, so populating
extras is a fix, not a regression.
issue: #49359
Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
|
||
|
wei liuandGitHub
|
d5e39bc240 |
enhance: [ExternalTable Part9] AWS STS + GCP impersonation + extfs refine (#49292)
issue: #45881 ## Summary Part 9 of ExternalTable. Three coalesced enhancements: ### 1. AWS STS AssumeRole (`role_arn` credential mode) Short-lived credentials via `STS:AssumeRole` for cross-account S3. - extfs keys: `role_arn`, `external_id`, `region` - Mutually exclusive with AK/SK, `use_iam`, `anonymous`, `gcp_target_service_account` ### 2. GCP cross-tenant impersonation (`gcp_target_service_account` mode) Milvus VM-default SA calls `iamcredentials.generateAccessToken` to mint impersonated bearer tokens for target-tenant GCS buckets. - Requires `roles/iam.serviceAccountTokenCreator` on target SA - Scheme gated to `{gs,gcs}` or `cloud_provider=gcp` - Email must end with `.gserviceaccount.com` ### 3. extfs refine — URI normalize + namespace isolation - AWS-style URIs (`s3://bucket/key`) with lazy FFI-boundary normalization via 3-tier priority: `extfs.address` → `DeriveEndpoint(cloud_provider, region)` → URI host - Normalize sites: DataCoord `task_index`, QueryNode `PutOrRef`, DataCoord refresh explore, DataNode refresh worker, packed sample/fetch - `extfs.{collID}.*` namespace isolated from Milvus-internal `fs.*` config - Fixes `extfs.{cid}.use_iam=true` leak causing 403s against non-IAM user buckets - `spec.extfs` empty values no longer clobber Layer-1 URI-derived defaults Bumps milvus-storage to `9c4e211`. Go + C++ validator + extfs allowlist in lockstep. ## Test plan - [x] Unit: `pkg/util/externalspec` (97.6% coverage) - [x] Unit: `internal/storagev2/packed` - [x] Unit: `internal/rootcoord` (createCollectionTask validateSchema) - [ ] E2E: `tests/go_client/testcases/external_table_arn_e2e_test.go` - [ ] E2E: `tests/go_client/testcases/external_table_iceberg_e2e_test.go` --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
wei liuandGitHub
|
fece10a762 |
fix: external table corner cases for invalid arguments (#49302)
## Summary Harden external collection RPCs and refresh pipeline against invalid argument corner cases reported in the part8 series. Each fix is one commit; every issue is independently testable. | Commit | Issue | Fix | |---|---|---| | 1 | [#49225](https://github.com/milvus-io/milvus/issues/49225) | reject zero-row external sources at task creation; classify as non-retriable so the job goes RefreshFailed instead of crashing datanode with integer divide by zero | | 2 | [#48619](https://github.com/milvus-io/milvus/issues/48619) | set element_type on ARRAY bulk_subscript responses for external segments so clients can decode Array(Int32) / Array(VarChar) outputs | | 3 | [#49235](https://github.com/milvus-io/milvus/issues/49235) | reject duplicate external_field mapping at create-time schema validation | | 4 | [#49229](https://github.com/milvus-io/milvus/issues/49229) | treat (external_source, external_spec) as an immutable atomic tuple: alter rejects both keys, create requires both-or-neither, refresh requires both-or-neither, apply path only overwrites non-empty fields | | 5 | [#49224](https://github.com/milvus-io/milvus/issues/49224) | enforce external_source scheme allowlist on RefreshExternalCollection (closes the alter-bypass SSRF surface for refresh) | | 6 | [#49230](https://github.com/milvus-io/milvus/issues/49230) | reject RefreshExternalCollection when the collection has no persisted external_source/external_spec; previously the job entered permanent retry | | 7 | [#49233](https://github.com/milvus-io/milvus/issues/49233) | classify FFI explore failures as non-retriable refresh job errors so NoSuchBucket / AccessDenied / DNS NXDOMAIN / malformed URI surface as RefreshFailed instead of looping forever | | 8 | [#49231](https://github.com/milvus-io/milvus/issues/49231) | surface in-progress refresh jobID at the synchronous RPC edge with ErrTaskDuplicate, so concurrent refresh callers get the existing jobID instead of a fresh one that the WAL ack callback silently drops | ## Test plan - [x] go unit tests added per fix: - `internal/datacoord/external_collection_refresh_manager_test.go` — zero-row, FFI explore failure - `internal/core` — ChunkedSegmentSealedImpl ARRAY branches (covered by go_client e2e) - `pkg/util/typeutil/schema_test.go` — atomic tuple, dup external_field - `internal/metastore/model/collection_test.go` — apply guard - `internal/proxy/impl_test.go` — refresh atomic / scheme / persisted source - `internal/rootcoord/ddl_callbacks_alter_collection_properties_test.go` — alter reject + TTL preserve - `internal/datacoord/services_test.go` — RPC dup detection - [x] go_client e2e: `TestExternalCollectionMultipleDataTypes` (Array+Geometry coverage), `TestRefreshExternalCollectionZeroRowParquet` issue: #49225 #48619 #49235 #49229 #49224 #49230 #49233 #49231 --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
|
3a4e37ad90 |
fix: bind file resource refCnt to collection lifecycle to prevent panic (#48893)
relate: https://github.com/milvus-io/milvus/issues/48612 ## Summary - Increment `fileResourceRefCnt` during `validateSchema` instead of in the async ack callback's `AddCollection`, closing the TOCTOU race where `RemoveFileResource` could delete a resource between validation and `AddCollection` - On failure before `Broadcast`, refCnt is decremented immediately; on restart, refCnt for pending broadcast tasks is recovered from etcd before rootcoord becomes Healthy - Remove refCnt++ from `addCollectionMeta` since it's now done at validation time (reload path unchanged) --------- Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
wei liuandGitHub
|
c703b27ae4 |
enhance: [ExternalTable Part8 - 2/4] control plane — DDL validation, refresh infrastructure (#49062)
## Summary Part 2/4 of the External Table Part 8 series. End-to-end control plane for external collections. **Depends on [#49061](https://github.com/milvus-io/milvus/pull/49061) (Part 1/4 foundation).** ### RootCoord / Proxy — DDL validation - Validate \`externalSource\` / \`externalSpec\` at create_collection_task (defense-in-depth) - Alter collection properties: detect changes, reject invalid transitions, attach \`FieldMask\` to WAL broadcast - New property keys: \`CollectionExternalSource\`, \`CollectionExternalSpec\`; streaming \`FieldMaskCollectionExternalSpec\` ### DataCoord refresh infrastructure - **refresh manager**: per-collection job state machine - **refresh checker**: periodic tick scanning collections for changed external source - **refresh inspector**: coordinates checker + manager, \`wrapTask\` factory - **refresh task**: encapsulates single refresh job (plan fragments, allocate segments, submit to DataNode, observe) - **schema updater**: broadcasts schema DDL detected by refresh back to RootCoord via WAL ### DataNode refresh task (RefreshExternalCollectionTask) - Reads explore manifest, fetches fragments in assigned file range - Parallelizes segment ID allocation from pre-allocated range ### externalspec — DDL validation helpers - \`ValidateExternalSource\`: scheme allowlist (s3/s3a/aws/minio/oss/gs/gcs/file), rejects userinfo - \`ValidateSourceAndSpec\`: combined Proxy/RootCoord entry point - \`RedactExternalSpec\`: removes sensitive extfs keys before logging - \`BuildFormatProperties\`: per-format reader/writer properties ### Unified job path + mutator meta - Single \`processJob\` routine for both periodic checker + eager task-completion - \`notifiedJobs\` dedup map prevents double-broadcast under concurrent invocations - \`Mutate(fn)\` helper centralizes all meta mutations Related: [#45881](https://github.com/milvus-io/milvus/issues/45881) (External Collection Lakehouse Integration tracking). ## Test plan - [x] Full Go build (\`go build -tags dynamic,test ./...\`) — 0 errors - [x] All test files compile (\`go test -tags dynamic,test -run=^$\` on all internal/...) — 0 build failures - [x] externalspec tests — 19 tests pass, 100% coverage on control-plane helpers - [x] golangci-lint on all changed packages — 0 issues - [ ] CI validation (ut-go / ut-cpp / integration-test / e2e) ## Review order Please review after #49061 (1/4) lands. After this (2/4) merges, Part 3/4 (Iceberg) and 4/4 (load optimization) will follow. Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
|
cc809208e9 |
feat: add function field backfill - Part 3 (#48809) (#48831)
DataCoord/DataNode compaction execution layer for function field backfill. - DataCoord: BackfillCompactionPolicy (trigger when schema version inconsistent), BackfillCompactionTask (coord-side state machine), schema version consistency gating in GetCollectionStatistics, clustering compaction guard - DataNode: BackfillCompactionTask (executor-side), backfillCompactor for physical data transformation of function output fields - storagev2: AsNewColumnGroups() API for packed writer transactions related: https://github.com/milvus-io/milvus/issues/48808 design doc: https://github.com/milvus-io/milvus-design-docs/blob/main/design_docs/20260129-add-function-field-design.md --------- Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
87b04e8a63 |
fix: remove golangci-lint v2 exclusion rules and fix ~1500 lint violations (#48586)
## Summary Remove all temporary exclusion rules added during the golangci-lint v1→v2 upgrade (PR #48286), fixing ~1500 lint violations: - **QF series (~1080)**: auto-fixed with `--fix` (embedded field selectors, if/else→switch, De Morgan's law) - **ST1005 (148)**: lowercase error strings per Go conventions - **gosec (116)**: fix G118 context cancel leaks, G306 file permissions; nolint for G602/G120/G705 false positives - **revive (41)**: `Json`→`JSON`, `Url`→`URL` naming conventions - **ineffassign (14)**: remove unused assignments - **unconvert (13)**: remove unnecessary type conversions - **depguard (9)**: replace banned `errors` import with `cockroachdb/errors` - **gocritic (14)**: fix ruleguard violations - **Other staticcheck (12)**: S1034, S1008, SA3001 etc. Kept disabled: govet `printf` analyzer (known Go 1.25 + golangci-lint v2 panic bug) issue: #48574 pr: #48286 ## Test plan - [x] `golangci-lint run` passes with 0 issues locally - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Li Liu <li.liu@zilliz.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
wei liuandGitHub
|
e87cc36e82 |
enhance: bind snapshot lifecycle to collection (#48143)
## Summary - Refactor snapshot name uniqueness from global to per-collection scope - Add cascade delete: DropCollection triggers DropSnapshotsByCollection - Add orphan snapshot GC for deleted collections - Add database-level filtering for ListSnapshots and ListRestoreSnapshotJobs - Distinguish source/target collection in RestoreSnapshot API - Move snapshot privileges from Global level to Collection level - Update client SDK and documentation for new API semantics issue: #44358 issue: #47890 issue: #47883 issue: #47855 ## Test plan - [x] Unit tests for snapshot_meta (DropSnapshotsByCollection, per-collection isolation, partial failure) - [x] Unit tests for snapshot_manager (DropSnapshotsByCollection, getDBCollectionIDs) - [x] Unit tests for services (ListSnapshots/ListRestoreJobs with dbID, RestoreSnapshot with source collectionID) - [x] Unit tests for ddl_callbacks_snapshot (new dropSnapshotsByCollection callback) - [x] Unit tests for garbage_collector (orphan snapshot GC) - [x] E2E tests for cross-database snapshot isolation - [ ] CI validation ## Note on skipped Python E2E tests All 18 snapshot test classes in `tests/python_client/milvus_client/test_milvus_client_snapshot.py` are temporarily skipped with `@pytest.mark.skip`. Reason: this PR changes snapshot APIs (DropSnapshot, DescribeSnapshot, RestoreSnapshot) to require `collection_name` as a mandatory parameter, but the pymilvus SDK used in CI has not been updated to pass this parameter yet. The tests will be re-enabled once pymilvus SDK is updated to match the new API contract. 🤖 Generated with [Claude Code](https://claude.com/claude-code) design doc: https://github.com/milvus-io/milvus-design-docs/blob/main/design_docs/20251114-snapshot_design.md --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
congqixiaandGitHub
|
c32ee00c86 |
fix: tolerate legacy SuffixSnapshot tombstone values in RootCoord meta (#49029)
issue: #49028
Before the SuffixSnapshot removal in
|
||
|
sthuangandGitHub
|
4e8483c55d |
fix: [RBAC] preserve wildcard privilege in RBAC backup/restore (#48978)
- Catalog.RestoreRBAC routed IsAnyWord through the
IsPrivilegeNameDefined /else branches and ended up calling
PrivilegeGroupNameForMetastore("*"), writing
'grantee-id/<id>/PrivilegeGroup*'.
- MetaTable.CheckIfRBACRestorable rejected wildcard grants with
'privilege [*] does not exist' before broadcastRestoreRBACV2 ever
reached the catalog, so the catalog fix alone was unreachable on the
in-process restore path.
related: #48963
Signed-off-by: shaoting-huang <shaoting.huang@zilliz.com>
|
||
|
|
e0873a65d4 |
enhance: remove SuffixSnapshot mechanism from RootCoord metadata storage (#47960)
## Summary Remove the entire SuffixSnapshot time-travel layer from RootCoord metadata storage, replacing it with plain KV operations. Replace SnapShotKV with TxnKV in Catalog (~30 call sites) Delete suffix_snapshot.go (~750 lines) and tests (~917 lines) Remove SnapShotKV interface from pkg/kv/kv.go Remove snapshot GC config params (ttl, reserveTime) Keep IsTombstone/ComposeSnapshotKey constants for migration tool compatibility Why: Investigation confirmed snapshot keys are write-only artifacts that nobody reads: No client/proxy sends non-zero timestamp for time-travel metadata queries Tombstone sweeper calls RemoveCollection(ts=0), bypassing snapshot tombstone writes entirely Snapshot keys accumulate as orphans — the old GC couldn't fully clean them because ts=0 deletion path never wrote tombstone markers Net reduction: -2180 lines related: #48697 ## Test plan - [x] Unit tests for GC batch logic (expiry, cursor progress, latest-per-group protection, tombstone cleanup) - [x] Unit tests for Save/Load with time-travel verification - [x] `go vet` passes on all changed packages - [ ] CI passes Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
|
8746f52efb |
feat: add function field backfill - Part 2 (#48808) (#48810)
Proxy/RootCoord/Schema layer for AlterCollectionSchema API. - Proxy: AlterCollectionSchema handler (alterCollectionSchemaTask, validateAddFieldRequest), schema version consistency gate (checkSchemaVersionConsistency), external collection guard - RootCoord: broadcastAlterCollectionSchema DDL callback with schema version increment - metastore/model: DoPhysicalBackfill field in Collection, MinSchemaVersion field in Index - pkg/common: SchemaVersionConsistentSegmentsKey/TotalSegmentsKey related: #48808 design doc: https://github.com/milvus-io/milvus-design-docs/blob/main/design_docs/20260129-add-function-field-design.md Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |