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>
This commit is contained in:
James
2026-07-20 22:34:42 +08:00
committed by GitHub
co-authored by xiaofanluan Claude Opus 4.8
parent f6bffd8de5
commit e8b9dbff2f
38 changed files with 6256 additions and 2165 deletions
-1
View File
@@ -417,7 +417,6 @@ proxy:
slowQuerySpanInSeconds: 1 # threshold for slow query detection in seconds. For Search/HybridSearch requests, the time is divided by nq for more accurate per-query measurement. Triggers slow log, WebUI display, and metrics.
queryNodePooling:
size: 10 # the size for shardleader(querynode) client pool
metaCacheGCTimeInterval: 300 # the time interval for meta cache GC, in seconds
partialResultRequiredDataRatio: 1 # partial result required data ratio, default to 1 which means disable partial result, otherwise, it will be used as the minimum data ratio for partial result
http:
enabled: true # Whether to enable the http server
@@ -0,0 +1,128 @@
# Proxy MetaCache: id-primary store with lazily-validated hints
- Status: Implemented (this PR)
- Date: 2026-07-16
- Scope: `internal/proxy/meta_cache.go`, `internal/proxy/version_cache.go`, `internal/rootcoord/ddl_callbacks_alter_alias.go`, `pkg/proto/messages.proto`
- Related: 10M-collections initiative; follow-up to the DescribeCollection-by-id / alias-invalidation fixes (#51313 / #51369)
## 1. Problem
An audit of the proxy meta cache found every id- or alias-driven invalidation doing a linear scan under the global write lock:
| Scan | Scope | Trigger |
|---|---|---|
| `removeCollectionByID` finds an id's name-keys | O(all cached collections) | **every** Load/Release/Drop/Rename/Alter broadcast |
| partition sweep (`StaleIf` prefix match) | O(all cached partition entries) | nested in every eviction |
| alias cleanup (`removeAliasesForCollectionLocked`) | O(db's aliases) | every eviction |
| partition-DDL name expansion | O(db's collections + aliases) | partition DDL |
| `RemoveDatabase` | O(db's entries) + O(all partition entries) | database DDL |
At a large cached working set (target: 1M collections) each broadcast stalls every query on the proxy for a full-cache sweep. The scans all existed for one reason: the primary store was keyed by *(db, name)* while invalidations arrive by *id*, *alias*, or *db* — so eviction had to search, and secondary structures (alias entries, partition entries under alias names) had to be found and torn down transactionally.
Additionally, cache fills (describe RPC + write-back) raced invalidation broadcasts on two unordered channels, guarded by a per-collection timestamp floor (`collectionCacheVersion`) with known gaps (alias write-backs unversioned, `RequestTime == 0` compat bypass, `RemoveDatabase` never raised it) and a tombstone-lifecycle problem.
## 2. Design
Two principles, each replacing a family of mechanisms:
**P1 — The primary store is keyed by the cluster-unique collection id and is the single source of truth.** Everything else — name resolution, alias resolution, partition entries — is a *hint* that resolves to an id and is validated against the primary on every read. A failed validation is a cache miss (one extra describe), never a stale read. Hints are written ONLY together with their entry (the name, the entry's declared aliases) and every eviction removes exactly that set through one unified routine — so every hint has a live owner, nothing dangles, and no background reclamation exists.
**P2 — Fills are ordered against invalidations pessimistically, not by timestamps.** A global `fillMu sync.RWMutex`: fills (RPC + write-back) hold the read side for their whole duration; invalidations take the write side, draining in-flight fills before evicting. A pre-DDL snapshot always lands *before* the eviction that cleans it; a fill issued after the eviction reads post-DDL state (rootcoord commits before broadcasting).
### 2.1 Data structures
```
collections map[id]*collectionInfo PRIMARY. liveness IS presence: every invalidation deletes explicitly
nameIdx map[db]map[realName]id hint; read validates: id live, entry.name == name, entry db == db
aliasInfo map[db]map[alias]string hint; alias -> target real name. Written ONLY from the target
entry's declared aliases (update()), so every hint is
discoverable from its entry and eviction always cleans it;
chains through nameIdx for reads
dbInfo map[db]*databaseInfo
partitionCache map["id"]partitionInfos ONE cache, keyed by collection id: the full list (with a
name->info map and the partition-key routing order); point
lookups answer from name2Info, list consumers take it whole.
The filler always fetched the full list anyway, so a separate
per-partition cache only duplicated it. A PLAIN map guarded by
mu and ordered against invalidations by fillMu, exactly like
the primary store -- list values are immutable and handed out
by pointer (GC-kept), so no refcount / stale-marker / version
floor / background reclamation. A plain delete on invalidation
is safe even while a request still routes over a list it read.
fillMu RWMutex P2 (fills=R, invalidations=W); cache hits never touch it
mu RWMutex map consistency; short critical sections, never across an RPC
```
Deleted: the (db,name)-keyed primary, the single-pointer by-id index and its I0 eager-eviction machinery, the alias reverse-index, the `collectionCacheVersion` floor and its `removeVersion` lifecycle, the whole `VersionCache` type (refcount + stale-marker + version floor) — the partition cache is now a plain `fillMu`-ordered map, so `backgroundGCLoop` is gone and there is NO background reclamation anywhere — and every invalidation-time scan.
### 2.2 Operations
- **Read** (hit): by-id — `collections[id]`; by-name — `nameIdx` → id → primary, validated (name and db must still match); by-alias — `aliasInfo` → name → `nameIdx` → primary. All O(1), `mu.RLock` only.
- **Fill** (miss): `fillMu.RLock` across describe + write-back. Writes primary + hints; overwrites nothing else — stale hints (old name after a rename, old id after drop+recreate) self-invalidate on read. An id-only describe whose response omits the db (older rootcoord during a rolling upgrade) is served UNCACHED: its database is unknown, so no database DDL could ever find and invalidate a cached copy. Repeat lookups re-describe until the rootcoords are upgraded — the entry class exists only in the upgrade window, and correctness beats a compat caching mechanism.
- **Invalidate by id** (Load/Release/Drop/Rename/Alter, hot): the unified eviction routine (`evictCollectionEntryLocked`) removes the primary entry TOGETHER with everything it owns — its name hint (only while still pointing at this id), its declared alias hints, its partition list — all O(1)-reachable from the entry in hand. EVERY deletion from the primary goes through this routine; a raw delete strands the entry's hints, and a later drop + same-name recreate resurrects a dead alias through them.
- **Invalidate partition DDL**: resolve the id from the broadcast or the LOCAL hints only — no RPC on any invalidation path. Complete by construction: a partition list exists only while its collection's primary entry lives, and a live entry always has its hints; unresolvable name ⇒ nothing cached ⇒ nothing to invalidate. Then delete the primary entry (NumPartitions changed) + stale the exact `"id"` key.
- **Drop database**: delete the db's primary entries explicitly — its `nameIdx` bucket reaches every one of them (completeness invariant: every live entry has a name hint, written together at fill time and only removed by the eviction that removes its entry) — then drop the hint buckets. O(the db's cached collections), on a rare operation — and the fill write lock is held only for the drain plus the O(1) bucket drop, NOT across the walk: the batched walk runs afterwards through the SAME unified eviction routine (a fill racing the walk may rebuild an entry with fresh hints; evicting it removes entry and hints together — one refetch, never a dangling hint; a tests-only hook covers this window deterministically), so a database drop blocks the proxy's fills no longer than any other invalidation's drain, and `mu` is held O(batch) at a time. This is the one deliberately non-O(1) invalidation left: paying a per-db walk on rare DropDatabase buys the removal of the whole generation concept (no per-entry generation stamps, no generation map, no generation-dead-but-physically-present state).
- **Alter database**: invalidate only the database-level `dbInfo` entry, under `fillMu` so a pre-alter `DescribeDatabase` flight cannot resurrect old properties after the expiration returns. Collection metadata, name/alias hints and partition lists do not contain database properties and remain valid, so clearing them would add an O(the db's cached collections) refill storm with no correctness benefit. If replica-number or resource-group properties change, RootCoord separately calls QueryCoord's `UpdateLoadConfig`; QueryCoord invalidates affected proxy shard-leader entries through its own collection-id path. AlterDatabase cache invalidation is therefore O(1).
- **Alias DDL**: rootcoord resolves the alias's pre-alter target under its database lock and forwards BOTH target ids in the expiration (`AlterAliasMessageHeader.old_collection_id`, additive proto field) — the proxy evicts old and new targets by id, immune to the race where a concurrent describe re-points the proxy's single-valued alias resolution before the expiration arrives. The ack callback routes the OLD-target eviction on `old_collection_id`, and the field's **zero value is deliberately the compat-safe scan path** because CreateAlias shares this message type and older rootcoords never set the field:
- **`> 0`** — old target known: evict it by id, no scan.
- **`== 0`** — old target UNKNOWN: fall back to the proxy's alias-hint resolution PLUS a holder scan (an invalidate with CollectionID == 0 + the alias name, gated `msgType != DropAlias`). Zero is reached two ways, both of which MUST scan: (a) a new AlterAlias whose broadcast could not resolve the pre-alter target (a meta inconsistency — near-impossible), and (b) a **replayed old-rootcoord AlterAlias** that predates the field. Making zero the scan path is what lets an upgrade-window replay still close its ghost instead of silently skipping it.
- **`< 0`** (`aliasNoOldTarget` sentinel, set by CreateAlias) — provably no old target: no scan. This is what keeps a normal CreateAlias off the O(N) scan; it cannot be zero, because zero must mean "scan."
- **DropAlias is EXCLUDED** from the scan entirely (it carries no id in every version; its single-target hint resolution is race-free — nothing re-points an alias concurrently with its drop — so scanning there would reintroduce a permanent O(N) stall on the normal path).
The scan evicts every cached entry declaring the alias, closing the one window case with no healing event (a re-pointed hint would leave the old target's ghost alias stale indefinitely on a stable, name-addressed collection). It is O(cached collections) but runs only on the genuinely-unknown old-target case — a near-impossible meta inconsistency, or transient upgrade-window replay — and is dead code once rootcoords are upgraded and no old messages remain in the WAL.
- **No background GC.** Every eviction cleans everything the entry owns synchronously, all O(1)-reachable from the entry in hand: its name hint, its declared alias hints (also closing the cascade-dropped-alias resurrection: a lazily-kept alias hint would validate again after a same-name recreate), and its partition list (Load/Release/Alter pay one extra ShowPartitions on the next partition access -- the accepted price). A fill that observes a rename deletes the previous name hint likewise. `ResolveCollectionAlias`'s RPC fallback deliberately writes NO hint (it would be undeclared by its target and thus undiscoverable at eviction -- the stray class), so there is no residue at all: every hint's lifecycle is bound to its entry. There is **no background loop at all**: the partition cache is a plain `fillMu`-ordered map whose entries are deleted synchronously by the same eviction, and its list values are immutable and handed out by pointer, so an invalidation's plain delete is safe with no refcount/stale/prune bookkeeping (the former `VersionCache` and its `backgroundGCLoop` are removed entirely).
- **DescribeCollection projection parity.** Cached and remote providers use one public projection. Not-found responses preserve their precise typed error (`ErrDatabaseNotFound` versus `ErrCollectionNotFound`) and receive the same proxy-boundary InputError classification. Successful responses filter internal/dynamic/namespace fields consistently, restore struct field names on detached messages, and return the same schema properties/version regardless of `proxy.enableCachedServiceProvider`. MetaCache retains canonical TIMESTAMPTZ defaults as UTC microseconds; only affected cached fields are cloned before the shared API response hook rewrites them to strings and redacts `ExternalSpec`.
### 2.3 Complexity
| Operation | Before | After |
|---|---|---|
| read hit (name / id / alias) | O(1) | O(1) (+1 map hop by name) |
| invalidate by id (hot path) | O(all cached) + O(all partitions) + O(db aliases) | **O(1)** |
| partition DDL invalidation | O(db collections + aliases) | **O(1)** |
| DropDatabase invalidation | O(db entries) + O(all partitions) | O(db's cached collections), batched (rare op) |
| AlterDatabase invalidation | O(db entries) + O(all partitions) | **O(1)** (`dbInfo` only) |
| write-lock hold, any invalidation | O(N) | **O(1)** (plus draining in-flight fills, caller-context-governed) |
| background reclamation | — | none (evictions clean synchronously) |
## 3. Concurrency
- Lock order: `fillMu``m.mu`. RPCs are never made under `m.mu`; fills hold only `fillMu.RLock` across their RPC.
- Cache hits touch neither `fillMu` nor any invalidation state; fills don't block each other; only rare invalidations take the exclusive side.
- **Caller-owned fill deadline.** Collection, partition and database fills pass the API context through unchanged; MetaCache does not impose a shorter timeout than the caller selected. An invalidation therefore waits until the slowest in-flight fill completes or its caller/transport cancels it. Because Go's `RWMutex` blocks NEW readers once a writer is queued (`a blocked Lock call excludes new readers`), a deadline-less RPC wedged below the transport can pin `fillMu`, the RootCoord synchronous cache-expiration ack behind it, and subsequent fills. This is an explicit contract choice: request/RPC deadline policy belongs at the API or coordinator-client boundary, not inside a metadata cache.
- Deadlock-freedom vs rootcoord: DDL ack callbacks release the meta lock before the expiration fan-out, so a fill's describe RPC is never blocked by the DDL whose invalidation is draining that fill. Proxy-side, `RemovePartition`'s ensure-describe runs *before* taking the write side.
- Trade-offs, accepted by design: an invalidation (and hence the broadcast ack) waits for the slowest in-flight fill; duplicate/late broadcasts over-invalidate (one extra re-describe) instead of being version-filtered.
## 4. Alternatives rejected
- **Multi-key id index / alias reverse index / partition group index** (earlier drafts): kept secondary structures transactionally in sync with evictions — every new structure added a teardown path and a way to get it wrong. Hints + read-validation make the sync problem vanish: a hint can only cost an extra describe.
- **Per-collection version floor** (previous implementation): timestamp comparison at the write-back is exact but leaves unversioned write-backs (alias resolution), a compat bypass (`RequestTime == 0`), and a tombstone lifecycle. The fill lock subsumes all three; its cost lands on rare invalidations instead of hot reads.
- **Copy-on-write / per-map sync.Map**: O(N) writes, or no cross-map consistency.
## 5. Remaining, out of scope
1. **Unbounded cache**: primary entries live until invalidated; needs an LRU/TTL bound for working sets beyond memory.
1. **Shard `fillMu` per collection/database**: today one global fill gate means a fill for database A gates an invalidation for database B until that fill returns under its caller/transport context. Sharding the gate so a fill only gates invalidations for its own collection/database would remove the cross-tenant coupling entirely. Deliberately NOT done yet: invalidations are rare, and a sharded gate must still preserve the "drain all fills of this database before a database DDL" semantics; adopt if cross-tenant wait shows up in practice.
2. **Lock-free reads via concurrent maps**: the hint-plus-validation design makes the read chain tolerant of torn multi-map views (entries are immutable once written and ids are never reused, so a torn read can only demote live→miss and never dead→hit, and a lost/early hint just costs one describe) — so `collections`/`nameIdx`/`aliasInfo` can each become a `typeutil.ConcurrentMap` (Go 1.24+ `sync.Map`) with eviction using `CompareAndDelete` to unlink a hint only while it still points at the evicted entry, removing `m.mu` from the hot read path entirely. This was impossible in the old transactionally-synced design (the doc's earlier "no cross-map consistency" objection) and is now a mechanical swap. Deliberately NOT done yet: `mu.RLock` on hits is tens of ns and invalidations are rare; adopt only when profiling shows reader contention (prefer this over lock sharding). `fillMu` is orthogonal and stays either way.
3. **WAL-subscription meta replication** (the endgame): proxies tail the control channel and apply meta changes from ONE ordered stream — fills and invalidations stop racing entirely, and `fillMu` and the expiration RPC fan-out (`LegacyProxyCollectionMetaCache` — the name anticipates this) both disappear. Belongs to the 10M-collections initiative.
## 6. Accepted gaps — WONT-FIX
Deliberate, documented trade-offs. None affects data, routing or RBAC; none leaves state that survives its healing event. **We do not plan to address them.** Each gap's exact trigger conditions:
| # | Gap | Trigger (ALL conditions required) | Who observes what | Duration / heals when | Why won't fix |
|---|---|---|---|---|---|
| 1 | New alias missing from the target's `Aliases` list | (a) rolling-upgrade window: proxy new, rootcoord OLD (broadcast carries no target id); (b) the target collection was cached BEFORE the CreateAlias/AlterAlias; (c) the alias has not been used for data access yet | `DescribeCollection` of the target under-reports the new alias. Addressing BY the alias works correctly from the first request | Heals on the FIRST use of the alias (the fill overwrites the entry, declaring it) or on any eviction of the target | The alias↔target association exists only at rootcoord; no local predicate — not even a full scan — can select the target (its cached snapshot predates the alias). The only local alternative is evicting the whole database per window alias DDL: a refill storm to cure an under-reported display field |
| 2 | By-id lookups uncached | (a) upgrade window: rootcoord OLD; (b) the lookup is id-only (e.g. the 9091 admin API) — the describe response then omits DbName | Every by-id lookup issues a describe RPC (correct data, no caching) | Entire mixed window; full caching resumes the moment rootcoords are upgraded | Caching an entry whose database is unknown would create state no database event could ever reach — a permanent-staleness hole traded for a window-only cache miss |
| 3 | Junk-name resolutions RPC every time | The resolved name is neither a cached collection nor a declared alias — nonexistent names, typos, requests rejected before the data path ever caches the target | rootcoord sees one describe per such resolution (returns not-found) | Permanent for names that never become real; a real name stops triggering it the moment its collection is cached | A negative cache needs its own TTL/invalidation semantics for a cold/error path; mechanism cost > benefit |
| 4 | Partition list refetched after Load/Release/Alter | Any collection-level invalidation (they share the id-eviction path with Drop), then the next partition-addressed access | One extra ShowPartitions per (invalidation, collection) pair | One request; the refetch re-fills the list | The price of evictions cleaning everything they own synchronously — which is what allows having NO background GC at all |
| 5 | No LRU/TTL bound | A proxy that describes a very large working set (target scale: 1M collections) holds it all resident | Memory growth proportional to the described working set | Until restart or the designated follow-up lands | Designated follow-up, not this PR. A TTL bound would also hard-cap every residual display-staleness window above |
## 7. Rollout
Proxy-side changes are in-memory and proxy-local. The one wire change is the additive `old_collection_id` field in `AlterAliasMessageHeader`: older rootcoords leave it 0 and the proxy falls back to hint resolution plus the gated holder scan (see Alias DDL above), so even a proxy-first upgrade leaves at most lazily-healing Aliases display gaps. The "upgrade rootcoord before proxies; avoid alias DDL during mixed deployment" guidance remains the zero-gap path.
## 8. Testing
- Primary/hint coherence checker (`assertMetaCacheByIDConsistent`) asserts "every hint has a live owner" in all four directions: every primary entry has a name hint targeting a live entry and a hint for each declared alias; every name hint targets a live primary (a rename's old-name hint may point at the live renamed entry — read-time validation rejects it — but a hint to a DEAD id means an eviction bypassed the unified routine); every alias hint chains to a live entry that DECLARES it (the alias-resurrection bugs were exactly violations of this).
- Fill/invalidation ordering: a collection describe parked in flight must be drained by a concurrent `RemoveCollectionsByID` / `RemoveDatabase`, and its pre-DDL write-back must not survive the eviction. A database-info describe parked in flight must likewise be drained by `RemoveDatabaseInfo`, so AlterDatabase cannot be followed by resurrection of pre-alter properties (gated-mock concurrency tests).
- Rename (old name misses immediately via hint validation), drop+recreate under a reused name (new id; old partition entries unreachable), alias re-point under concurrent describe (both targets evicted via forwarded ids), database invalidation via the hint-bucket walk, cross-db isolation, `-race` on the full proxy suite.
+232 -291
View File
@@ -19,15 +19,15 @@ package proxy
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
"github.com/milvus-io/milvus-proto/go-api/v3/schemapb"
"github.com/milvus-io/milvus/internal/mocks"
"github.com/milvus-io/milvus/pkg/v3/util/conc"
"github.com/milvus-io/milvus/pkg/v3/util/merr"
"github.com/milvus-io/milvus/pkg/v3/util/paramtable"
)
@@ -36,15 +36,17 @@ import (
func TestResolveCollectionAlias_WildcardAndEmptySkippedByInterceptor(t *testing.T) {
// The interceptor guard (objectName != util.AnyWord && objectName != "") ensures
// that "*" and "" never reach resolveCollectionAlias. This test verifies those
// values are correctly guarded at the interceptor level by checking that the
// cache's DescribeAlias RPC is never called for these sentinel values.
// values are correctly guarded at the interceptor level, then checks a normal
// name resolves through the standard collection-cache fill (one
// DescribeCollection RPC; rootcoord resolves the alias server-side).
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{"default": {}},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
oldCache := globalMetaCache
@@ -60,16 +62,20 @@ func TestResolveCollectionAlias_WildcardAndEmptySkippedByInterceptor(t *testing.
assert.True(t, shouldSkip, "sentinel %q should be skipped by interceptor guard", sentinel)
}
// Normal name should resolve via RPC
mockCoord.On("DescribeAlias", mock.Anything, mock.Anything).Return(&milvuspb.DescribeAliasResponse{
Status: &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success},
Collection: "real_col",
// Normal name resolves via the collection-cache fill: rootcoord answers the
// alias-addressed describe with the real collection.
mockCoord.EXPECT().DescribeCollection(mock.Anything, mock.Anything).Return(&milvuspb.DescribeCollectionResponse{
Status: merr.Success(),
CollectionID: 11,
DbName: "default",
Schema: &schemapb.CollectionSchema{Name: "real_col"},
Aliases: []string{"some_alias"},
}, nil)
result, err := resolveCollectionAlias(ctx, "default", "some_alias")
assert.NoError(t, err)
assert.Equal(t, "real_col", result)
mockCoord.AssertCalled(t, "DescribeAlias", mock.Anything, mock.Anything)
mockCoord.AssertCalled(t, "DescribeCollection", mock.Anything, mock.Anything)
}
func TestResolveCollectionAlias_CachedCollection(t *testing.T) {
@@ -77,90 +83,72 @@ func TestResolveCollectionAlias_CachedCollection(t *testing.T) {
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {
"test_collection": {
collID: 1,
schema: &schemaInfo{},
},
},
},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// plant the collection into the primary store + name hint the way update() would
seedCollection(cache, "default", "test_collection", 1)
result, err := cache.ResolveCollectionAlias(ctx, "default", "test_collection")
assert.NoError(t, err)
assert.Equal(t, "test_collection", result)
mockCoord.AssertNotCalled(t, "DescribeAlias")
mockCoord.AssertNotCalled(t, "DescribeCollection")
}
func TestResolveCollectionAlias_ValidAlias(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
mockCoord.On("DescribeAlias", mock.Anything, mock.MatchedBy(func(req *milvuspb.DescribeAliasRequest) bool {
return req.DbName == "default" && req.Alias == "my_alias"
})).Return(&milvuspb.DescribeAliasResponse{
Status: &commonpb.Status{
ErrorCode: commonpb.ErrorCode_Success,
},
Collection: "actual_collection",
}, nil)
// ONE fill: rootcoord resolves the alias server-side. The .Once() proves the
// second resolution below is served from the cache written by the first fill.
mockCoord.EXPECT().DescribeCollection(mock.Anything, mock.MatchedBy(func(req *milvuspb.DescribeCollectionRequest) bool {
return req.GetDbName() == "default" && req.GetCollectionName() == "my_alias"
})).Return(&milvuspb.DescribeCollectionResponse{
Status: merr.Success(),
CollectionID: 100,
DbName: "default",
Schema: &schemapb.CollectionSchema{Name: "actual_collection"},
Aliases: []string{"my_alias"},
}, nil).Once()
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// the first resolution fills the cache (one DescribeCollection RPC) ...
result, err := cache.ResolveCollectionAlias(ctx, "default", "my_alias")
assert.NoError(t, err)
assert.Equal(t, "actual_collection", result)
}
func TestResolveCollectionAlias_NotAnAlias(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
mockCoord.On("DescribeAlias", mock.Anything, mock.MatchedBy(func(req *milvuspb.DescribeAliasRequest) bool {
return req.DbName == "default" && req.Alias == "not_an_alias"
})).Return(&milvuspb.DescribeAliasResponse{
Status: &commonpb.Status{
ErrorCode: commonpb.ErrorCode_CollectionNotExists,
Reason: "alias not found",
},
}, nil)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
// ... and the fill wrote the DECLARED alias hint, so the next resolution is a
// pure cache hit -- the .Once() above enforces no extra RPC.
target, ok := getAlias(cache, "default", "my_alias")
if assert.True(t, ok, "the declared alias hint must be cached by the fill") {
assert.Equal(t, "actual_collection", target)
}
// Should return original name when alias not found (not error)
result, err := cache.ResolveCollectionAlias(ctx, "default", "not_an_alias")
result, err = cache.ResolveCollectionAlias(ctx, "default", "my_alias")
assert.NoError(t, err)
assert.Equal(t, "not_an_alias", result)
assert.Equal(t, "actual_collection", result)
}
func TestResolveCollectionAlias_RPCError(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
mockCoord.On("DescribeAlias", mock.Anything, mock.Anything).
mockCoord.EXPECT().DescribeCollection(mock.Anything, mock.Anything).
Return(nil, assert.AnError)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
result, err := cache.ResolveCollectionAlias(ctx, "default", "some_name")
@@ -172,8 +160,9 @@ func TestResolveCollectionAlias_InternalServerError(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
// Internal server error should be propagated, not swallowed
mockCoord.On("DescribeAlias", mock.Anything, mock.Anything).Return(&milvuspb.DescribeAliasResponse{
// A non-not-found error status from the describe fill should be propagated,
// not swallowed as "resolve to the literal name".
mockCoord.EXPECT().DescribeCollection(mock.Anything, mock.Anything).Return(&milvuspb.DescribeCollectionResponse{
Status: &commonpb.Status{
ErrorCode: commonpb.ErrorCode_UnexpectedError,
Reason: "internal error",
@@ -181,11 +170,10 @@ func TestResolveCollectionAlias_InternalServerError(t *testing.T) {
}, nil)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
result, err := cache.ResolveCollectionAlias(ctx, "default", "some_name")
@@ -193,31 +181,6 @@ func TestResolveCollectionAlias_InternalServerError(t *testing.T) {
assert.Equal(t, "", result)
}
func TestResolveCollectionAlias_EmptyCollectionInResponse(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
// Status is success but collection field is empty
mockCoord.On("DescribeAlias", mock.Anything, mock.Anything).Return(&milvuspb.DescribeAliasResponse{
Status: &commonpb.Status{
ErrorCode: commonpb.ErrorCode_Success,
},
Collection: "",
}, nil)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
}
result, err := cache.ResolveCollectionAlias(ctx, "default", "some_alias")
assert.NoError(t, err)
assert.Equal(t, "some_alias", result)
}
func TestResolveCollectionAlias_NilGlobalMetaCache(t *testing.T) {
ctx := context.Background()
oldCache := globalMetaCache
@@ -234,110 +197,62 @@ func TestResolveCollectionAlias_AliasCacheHit(t *testing.T) {
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"my_alias": {collectionName: "real_collection", cachedAt: time.Now()},
},
},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// hint-declared seeding, the way a real fill writes it: the primary entry
// declares the alias, and the alias hint chains alias -> real name -> entry
real := seedCollection(cache, "default", "real_collection", 7)
real.aliases = []string{"my_alias"}
cache.setAliasLocked("default", "my_alias", "real_collection")
// Should return cached alias without RPC call
// Should resolve through the cached hint chain without any RPC call
result, err := cache.ResolveCollectionAlias(ctx, "default", "my_alias")
assert.NoError(t, err)
assert.Equal(t, "real_collection", result)
mockCoord.AssertNotCalled(t, "DescribeAlias")
mockCoord.AssertNotCalled(t, "DescribeCollection")
}
func TestResolveCollectionAlias_NegativeCacheHit(t *testing.T) {
// TestResolveCollectionAlias_NotAnAliasIsNotCached: a name that resolves to no
// collection (junk, or an alias unknown to rootcoord) is returned unchanged and
// caches NOTHING -- a not-found fill has no entry to write (no negative cache by
// design). Every resolution of such a name re-asks rootcoord.
func TestResolveCollectionAlias_NotAnAliasIsNotCached(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"regular_name": {collectionName: "", cachedAt: time.Now()},
},
},
}
// Negative cache: not an alias, return as-is without RPC
result, err := cache.ResolveCollectionAlias(ctx, "default", "regular_name")
assert.NoError(t, err)
assert.Equal(t, "regular_name", result)
mockCoord.AssertNotCalled(t, "DescribeAlias")
}
func TestResolveCollectionAlias_PopulatesPositiveCache(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
mockCoord.On("DescribeAlias", mock.Anything, mock.Anything).Return(&milvuspb.DescribeAliasResponse{
Status: &commonpb.Status{
ErrorCode: commonpb.ErrorCode_Success,
},
Collection: "real_collection",
}, nil).Once()
mockCoord.EXPECT().DescribeCollection(mock.Anything, mock.MatchedBy(func(req *milvuspb.DescribeCollectionRequest) bool {
return req.GetDbName() == "default" && req.GetCollectionName() == "not_alias"
})).Return(&milvuspb.DescribeCollectionResponse{
Status: merr.Status(merr.WrapErrCollectionNotFound("not_alias")),
}, nil).Twice()
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// First call triggers RPC
result, err := cache.ResolveCollectionAlias(ctx, "default", "my_alias")
assert.NoError(t, err)
assert.Equal(t, "real_collection", result)
// Second call should hit cache, no additional RPC
result, err = cache.ResolveCollectionAlias(ctx, "default", "my_alias")
assert.NoError(t, err)
assert.Equal(t, "real_collection", result)
// DescribeAlias should have been called exactly once
mockCoord.AssertNumberOfCalls(t, "DescribeAlias", 1)
}
func TestResolveCollectionAlias_PopulatesNegativeCache(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
mockCoord.On("DescribeAlias", mock.Anything, mock.Anything).Return(&milvuspb.DescribeAliasResponse{
Status: &commonpb.Status{
ErrorCode: commonpb.ErrorCode_CollectionNotExists,
Reason: "alias not found",
},
}, nil).Once()
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{},
// both calls resolve to the name as-is (no error), and BOTH issue the RPC
for i := 0; i < 2; i++ {
result, err := cache.ResolveCollectionAlias(ctx, "default", "not_alias")
assert.NoError(t, err)
assert.Equal(t, "not_alias", result)
}
mockCoord.AssertNumberOfCalls(t, "DescribeCollection", 2)
// First call triggers RPC
result, err := cache.ResolveCollectionAlias(ctx, "default", "not_alias")
assert.NoError(t, err)
assert.Equal(t, "not_alias", result)
// Second call should hit negative cache
result, err = cache.ResolveCollectionAlias(ctx, "default", "not_alias")
assert.NoError(t, err)
assert.Equal(t, "not_alias", result)
mockCoord.AssertNumberOfCalls(t, "DescribeAlias", 1)
// nothing was cached: no alias hint, no name hint, no primary entry
cache.mu.RLock()
_, hasAliasHint := cache.aliasInfo["default"]["not_alias"]
_, hasNameHint := cache.nameIdx["default"]["not_alias"]
numCached := len(cache.collections)
cache.mu.RUnlock()
assert.False(t, hasAliasHint, "a non-resolving name must not write an alias hint")
assert.False(t, hasNameHint, "a non-resolving name must not write a name hint")
assert.Zero(t, numCached, "a not-found fill must cache nothing")
}
func TestRemoveAlias_InvalidatesCache(t *testing.T) {
@@ -345,13 +260,12 @@ func TestRemoveAlias_InvalidatesCache(t *testing.T) {
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{
"default": {
"my_alias": {collectionName: "real_collection", cachedAt: time.Now()},
"my_alias": "real_collection",
},
},
}
@@ -360,86 +274,110 @@ func TestRemoveAlias_InvalidatesCache(t *testing.T) {
cache.RemoveAlias(ctx, "default", "my_alias")
// Verify the alias was removed from cache
_, ok := cache.getAlias("default", "my_alias")
_, ok := getAlias(cache, "default", "my_alias")
assert.False(t, ok)
}
func TestRemoveCollectionByID_CleansUpAliases(t *testing.T) {
// TestRemoveCollectionByID_DeclaredAliasHintsCleanedStrayMisses covers the
// eviction contract in two parts. PRODUCTION path: eviction removes every alias
// hint the entry DECLARES (alias1), synchronously — there is no background GC.
// DEFENSIVE part: a "stray" hint (one pointing at the collection but NOT in its
// declared aliases, alias2) is an IMPOSSIBLE state under the current design —
// aliasInfo is written ONLY from update()'s declared-aliases loop, so a hint
// can never lack a declaring owner — but even were one to exist it must not
// cause a stale read: a lookup through it still misses the evicted collection.
func TestRemoveCollectionByID_DeclaredAliasHintsCleanedStrayMisses(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {
"my_collection": {
collID: 100,
schema: &schemaInfo{},
},
},
},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"alias1": {collectionName: "my_collection", cachedAt: time.Now()},
"alias2": {collectionName: "my_collection", cachedAt: time.Now()},
"alias3": {collectionName: "other_collection", cachedAt: time.Now()},
},
},
collectionCacheVersion: make(map[UniqueID]uint64),
sfGlobal: conc.Singleflight[*collectionInfo]{},
collLevelPartitionCache: NewVersionCache[string, *partitionInfos](),
partitionCache: NewVersionCache[string, *partitionInfo](),
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
partitionCache: map[string]*partitionInfos{},
}
// plant the collection into the primary store + name hint like update() would;
// alias1 is recorded on the entry itself (as a real fill would), alias2 is a
// stray hint (e.g. an upgrade-window DescribeAlias write-back)
doomed := seedCollection(cache, "default", "my_collection", 100)
doomed.aliases = []string{"alias1"}
cache.setAliasLocked("default", "alias1", "my_collection")
cache.setAliasLocked("default", "alias2", "my_collection")
cache.setAliasLocked("default", "alias3", "other_collection")
// Remove collection by ID
cache.RemoveCollectionsByID(ctx, 100, 0, true)
// Remove collection by ID: the primary entry AND everything it owns
// (its listed alias hints, its name hint) go away synchronously.
cache.RemoveCollectionsByID(ctx, 100)
// Aliases pointing to my_collection should be removed
_, ok := cache.getAlias("default", "alias1")
assert.False(t, ok)
_, ok = cache.getAlias("default", "alias2")
assert.False(t, ok)
cache.mu.RLock()
assert.Nil(t, collByIDLive(cache, 100), "the primary entry must be deleted")
assert.Nil(t, cachedEntryLocked(cache, "default", "my_collection"), "a by-name lookup must miss after the drop")
_, hasListed := cache.aliasInfo["default"]["alias1"]
cache.mu.RUnlock()
assert.False(t, hasListed, "an alias listed on the entry is cleaned at eviction")
// Alias pointing to other_collection should remain
entry, ok := cache.getAlias("default", "alias3")
// DEFENSIVE: a stray hint (impossible in production -- not declared by the
// entry) survives the eviction, but a lookup through it must still MISS the
// dead collection.
entry, ok := getAlias(cache, "default", "alias2")
if assert.True(t, ok, "the (impossible-in-production) stray hint is not reached by declared-alias cleanup") {
assert.Equal(t, "my_collection", entry)
}
_, ok = cache.getCollection("default", "alias2", 0)
assert.False(t, ok, "a lookup through the stray alias must not reach the dead collection")
// Alias pointing to other_collection is untouched.
entry, ok = getAlias(cache, "default", "alias3")
assert.True(t, ok)
assert.Equal(t, "other_collection", entry.collectionName)
assert.Equal(t, "other_collection", entry)
}
func TestRemoveCollection_CleansUpAliasesWhenNotCached(t *testing.T) {
// TestRemoveCollection_DefensiveDanglingAliasBranch is a DEFENSIVE test: alias
// hints whose target is not in the primary store are IMPOSSIBLE under the
// current design (aliasInfo is written only from an entry's declared aliases,
// and an entry's eviction removes them), so this state is hand-seeded. It
// exercises removeCollectionByAliasLocked's one-line defensive branch:
// RemoveCollection for an ALIAS name whose target is uncached deletes just that
// alias hint, so it cannot mis-resolve a later ResolveCollectionAlias.
func TestRemoveCollection_DefensiveDanglingAliasBranch(t *testing.T) {
ctx := context.Background()
mockCoord := mocks.NewMockMixCoordClient(t)
// Collection is NOT in collInfo cache, but aliases pointing to it exist
// Target collection is NOT in the primary store, but alias hints exist.
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"default": {},
},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"alias1": {collectionName: "uncached_collection", cachedAt: time.Now()},
"alias2": {collectionName: "uncached_collection", cachedAt: time.Now()},
"alias3": {collectionName: "other_collection", cachedAt: time.Now()},
},
},
collectionCacheVersion: make(map[UniqueID]uint64),
sfGlobal: conc.Singleflight[*collectionInfo]{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
cache.setAliasLocked("default", "alias1", "uncached_collection")
cache.setAliasLocked("default", "alias2", "uncached_collection")
cache.setAliasLocked("default", "alias3", "other_collection")
// RemoveCollection for an uncached collection
cache.RemoveCollection(ctx, "default", "uncached_collection", 0)
// Aliases pointing to uncached_collection should be removed
_, ok := cache.getAlias("default", "alias1")
assert.False(t, ok)
_, ok = cache.getAlias("default", "alias2")
assert.False(t, ok)
// Alias pointing to other_collection should remain
entry, ok := cache.getAlias("default", "alias3")
// RemoveCollection with the (uncached) TARGET name does not touch the
// hand-seeded hints pointing at it (no reverse alias sweep exists).
cache.RemoveCollection(ctx, "default", "uncached_collection")
_, ok := getAlias(cache, "default", "alias1")
assert.True(t, ok, "no reverse-alias sweep for an uncached target name")
_, ok = getAlias(cache, "default", "alias2")
assert.True(t, ok)
assert.Equal(t, "other_collection", entry.collectionName)
// RemoveCollection with the ALIAS name deletes just that hint (defensive branch).
cache.RemoveCollection(ctx, "default", "alias1")
_, ok = getAlias(cache, "default", "alias1")
assert.False(t, ok, "the dangling alias hint itself must be deleted")
// The sibling alias to the same uncached target is untouched.
entry, ok := getAlias(cache, "default", "alias2")
if assert.True(t, ok) {
assert.Equal(t, "uncached_collection", entry)
}
// Aliases of other targets are untouched.
entry, ok = getAlias(cache, "default", "alias3")
if assert.True(t, ok) {
assert.Equal(t, "other_collection", entry)
}
}
func TestRemoveDatabase_CleansUpAliases(t *testing.T) {
@@ -447,26 +385,24 @@ func TestRemoveDatabase_CleansUpAliases(t *testing.T) {
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{
"mydb": {},
},
aliasInfo: map[string]map[string]*aliasEntry{
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
partitionCache: map[string]*partitionInfos{},
aliasInfo: map[string]map[string]string{
"mydb": {
"alias1": {collectionName: "coll1", cachedAt: time.Now()},
"alias1": "coll1",
},
},
dbInfo: map[string]*databaseInfo{
"mydb": {},
},
collLevelPartitionCache: NewVersionCache[string, *partitionInfos](),
partitionCache: NewVersionCache[string, *partitionInfo](),
}
cache.RemoveDatabase(ctx, "mydb")
// Alias cache for the database should be gone
_, ok := cache.getAlias("mydb", "alias1")
_, ok := getAlias(cache, "mydb", "alias1")
assert.False(t, ok)
}
@@ -476,14 +412,15 @@ func TestCreateAliasTask_ResolvesCollectionAlias(t *testing.T) {
// Set up cache: "existing_alias" -> "real_collection"
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{"default": {}},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"existing_alias": {collectionName: "real_collection", cachedAt: time.Now()},
},
},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// hint-declared seeding: the entry declares the alias, so "existing_alias"
// resolves to "real_collection" from the cache with zero RPC
seedCollection(cache, "default", "real_collection", 1).aliases = []string{"existing_alias"}
cache.setAliasLocked("default", "existing_alias", "real_collection")
oldCache := globalMetaCache
globalMetaCache = cache
@@ -515,14 +452,15 @@ func TestAlterAliasTask_ResolvesCollectionAlias(t *testing.T) {
// Set up cache: "existing_alias" -> "real_collection"
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{"default": {}},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"existing_alias": {collectionName: "real_collection", cachedAt: time.Now()},
},
},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// hint-declared seeding: the entry declares the alias, so "existing_alias"
// resolves to "real_collection" from the cache with zero RPC
seedCollection(cache, "default", "real_collection", 1).aliases = []string{"existing_alias"}
cache.setAliasLocked("default", "existing_alias", "real_collection")
oldCache := globalMetaCache
globalMetaCache = cache
@@ -555,14 +493,15 @@ func TestCreateAliasTask_ResolvesEvenWhenRBACFlagDisabled(t *testing.T) {
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{"default": {}},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"existing_alias": {collectionName: "real_collection", cachedAt: time.Now()},
},
},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// hint-declared seeding: the entry declares the alias, so "existing_alias"
// resolves to "real_collection" from the cache with zero RPC
seedCollection(cache, "default", "real_collection", 1).aliases = []string{"existing_alias"}
cache.setAliasLocked("default", "existing_alias", "real_collection")
oldCache := globalMetaCache
globalMetaCache = cache
@@ -596,14 +535,15 @@ func TestListAliasesTask_ResolvesCollectionAlias(t *testing.T) {
// Set up cache: "existing_alias" -> "real_collection"
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{"default": {}},
aliasInfo: map[string]map[string]*aliasEntry{
"default": {
"existing_alias": {collectionName: "real_collection", cachedAt: time.Now()},
},
},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
// hint-declared seeding: the entry declares the alias, so "existing_alias"
// resolves to "real_collection" from the cache with zero RPC
seedCollection(cache, "default", "real_collection", 1).aliases = []string{"existing_alias"}
cache.setAliasLocked("default", "existing_alias", "real_collection")
oldCache := globalMetaCache
globalMetaCache = cache
@@ -633,9 +573,10 @@ func TestListAliasesTask_NoResolveWhenCollectionNameEmpty(t *testing.T) {
mockCoord := mocks.NewMockMixCoordClient(t)
cache := &MetaCache{
mixCoord: mockCoord,
collInfo: map[string]map[string]*collectionInfo{"default": {}},
aliasInfo: map[string]map[string]*aliasEntry{},
mixCoord: mockCoord,
collections: map[UniqueID]*collectionInfo{},
nameIdx: map[string]map[string]UniqueID{},
aliasInfo: map[string]map[string]string{},
}
oldCache := globalMetaCache
@@ -658,5 +599,5 @@ func TestListAliasesTask_NoResolveWhenCollectionNameEmpty(t *testing.T) {
assert.NoError(t, err)
// CollectionName should remain empty
assert.Equal(t, "", task.CollectionName)
mockCoord.AssertNotCalled(t, "DescribeAlias")
mockCoord.AssertNotCalled(t, "DescribeCollection")
}
+8 -1
View File
@@ -49,7 +49,14 @@ func fillDatabase(ctx context.Context, req interface{}) (context.Context, interf
}
return ctx, r
case *milvuspb.DescribeCollectionRequest:
if r.DbName == "" {
// Collection ids are cluster-unique, so an id-only describe does not use
// the database to resolve its target. Preserve an omitted db name here:
// filling it with the connection default would lose the distinction
// between "unknown until resolved by id" and an explicit default-db
// request. Name-based describes keep the historical empty => default
// behavior.
idOnly := r.GetCollectionID() > 0 && r.GetCollectionName() == ""
if r.DbName == "" && !idOnly {
r.DbName = GetCurDBNameFromContextOrDefault(ctx)
}
return ctx, r
@@ -45,6 +45,24 @@ func TestDatabaseInterceptor(t *testing.T) {
assert.NoError(t, err)
})
t.Run("id-only describe keeps omitted database empty", func(t *testing.T) {
md := metadata.Pairs(util.HeaderDBName, "db-from-header")
ctx := metadata.NewIncomingContext(context.Background(), md)
req := &milvuspb.DescribeCollectionRequest{CollectionID: 100}
_, err := interceptor(ctx, req, &grpc.UnaryServerInfo{}, handler)
assert.NoError(t, err)
assert.Empty(t, req.GetDbName())
})
t.Run("name-based describe still gets database from metadata", func(t *testing.T) {
md := metadata.Pairs(util.HeaderDBName, "db-from-header")
ctx := metadata.NewIncomingContext(context.Background(), md)
req := &milvuspb.DescribeCollectionRequest{CollectionName: "collection"}
_, err := interceptor(ctx, req, &grpc.UnaryServerInfo{}, handler)
assert.NoError(t, err)
assert.Equal(t, "db-from-header", req.GetDbName())
})
t.Run("external snapshot requests get db from metadata", func(t *testing.T) {
md := metadata.Pairs(util.HeaderDBName, "db")
ctx := metadata.NewIncomingContext(context.Background(), md)
@@ -244,7 +244,7 @@ func TestNewFunctionChainRerankMeta(t *testing.T) {
}
func newFunctionChainTestSchema() *schemaInfo {
return newSchemaInfo(&schemapb.CollectionSchema{
return mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "pk", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
{FieldID: 101, Name: "ts", DataType: schemapb.DataType_Int64},
@@ -257,7 +257,7 @@ func newFunctionChainTestSchema() *schemaInfo {
}
func newFunctionChainStructTestSchema() *schemaInfo {
return newSchemaInfo(&schemapb.CollectionSchema{
return mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "pk", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
},
+57 -61
View File
@@ -132,80 +132,86 @@ func (node *Proxy) InvalidateCollectionMetaCache(ctx context.Context, request *p
ctx, sp := otel.Tracer(typeutil.ProxyRole).Start(ctx, "Proxy-InvalidateCollectionMetaCache")
defer sp.End()
mlog.Info(context.TODO(), "received request to invalidate collection meta cache")
mlog.Info(ctx, "received request to invalidate collection meta cache")
dbName := request.DbName
collectionName := request.CollectionName
collectionID := request.CollectionID
msgType := request.GetBase().GetMsgType()
var aliasName []string
deprecateShardCaches := func(names ...string) {
seen := make(map[string]struct{}, len(names))
for _, name := range names {
if name == "" {
continue
}
if _, ok := seen[name]; ok {
continue
}
seen[name] = struct{}{}
node.shardMgr.DeprecateShardCache(request.GetDbName(), name)
}
}
if globalMetaCache != nil {
switch msgType {
case commonpb.MsgType_DropCollection, commonpb.MsgType_RenameCollection, commonpb.MsgType_DropAlias, commonpb.MsgType_AlterAlias, commonpb.MsgType_CreateAlias:
// remove collection by name first, otherwise the drop collection remove version will be failed.
if collectionName != "" {
globalMetaCache.RemoveCollection(ctx, request.GetDbName(), collectionName, request.GetBase().GetTimestamp()) // no need to return error, though collection may be not cached
node.shardMgr.DeprecateShardCache(request.GetDbName(), collectionName)
}
if request.CollectionID != UniqueID(0) {
aliasName = globalMetaCache.RemoveCollectionsByID(ctx, collectionID, request.GetBase().GetTimestamp(), msgType == commonpb.MsgType_DropCollection)
for _, name := range aliasName {
node.shardMgr.DeprecateShardCache(request.GetDbName(), name)
aliasOperation := msgType == commonpb.MsgType_CreateAlias ||
msgType == commonpb.MsgType_AlterAlias || msgType == commonpb.MsgType_DropAlias
aliasName = globalMetaCache.InvalidateCollectionMeta(
ctx, request.GetDbName(), collectionName, collectionID, aliasOperation)
deprecateShardCaches(append(aliasName, collectionName)...)
if aliasOperation && collectionName != "" {
if request.CollectionID == UniqueID(0) && msgType != commonpb.MsgType_DropAlias {
// An alias-DDL broadcast with no target id (id==0) means
// the old target could not be precisely located, so fall
// back to scanning for cached holders of the alias --
// otherwise a concurrent-describe race could leave the old
// target's Aliases list stale with no healing event. Two
// sources reach here, so this is a PERMANENT safety net,
// not upgrade-window compat: (a) an old rootcoord that
// predates old_collection_id, and (b) a new rootcoord that
// could not resolve the pre-alter AlterAlias target (a meta
// inconsistency). DropAlias is EXCLUDED: it legitimately
// carries no id in every version (its single-target hint
// resolution is race-free -- nothing re-points an alias
// concurrently with its drop), so scanning there would
// reintroduce a permanent O(N) stall on the normal path.
globalMetaCache.RemoveAliasHolders(ctx, request.GetDbName(), collectionName)
}
}
// Invalidate alias cache for alias operations
if msgType == commonpb.MsgType_CreateAlias || msgType == commonpb.MsgType_AlterAlias || msgType == commonpb.MsgType_DropAlias {
if collectionName != "" {
globalMetaCache.RemoveAlias(ctx, request.GetDbName(), collectionName)
}
}
mlog.Info(context.TODO(), "complete to invalidate collection meta cache with collection name", mlog.String("type", request.GetBase().GetMsgType().String()))
mlog.Info(ctx, "complete to invalidate collection meta cache with collection name", mlog.String("type", request.GetBase().GetMsgType().String()))
case commonpb.MsgType_LoadCollection, commonpb.MsgType_ReleaseCollection:
// All the request from query use collectionID
if request.CollectionID != UniqueID(0) {
aliasName = globalMetaCache.RemoveCollectionsByID(ctx, collectionID, 0, false)
for _, name := range aliasName {
node.shardMgr.DeprecateShardCache(request.GetDbName(), name)
}
aliasName = globalMetaCache.InvalidateCollectionMeta(ctx, request.GetDbName(), "", collectionID, false)
deprecateShardCaches(aliasName...)
}
mlog.Info(context.TODO(), "complete to invalidate collection meta cache", mlog.String("type", request.GetBase().GetMsgType().String()))
mlog.Info(ctx, "complete to invalidate collection meta cache", mlog.String("type", request.GetBase().GetMsgType().String()))
case commonpb.MsgType_CreatePartition, commonpb.MsgType_DropPartition:
if request.GetPartitionName() == "" {
mlog.Warn(context.TODO(), "invalidate collection meta cache failed. partitionName is empty")
return &commonpb.Status{ErrorCode: commonpb.ErrorCode_UnexpectedError}, nil
err := merr.WrapErrServiceInternalMsg("partition cache invalidation received empty partition name")
mlog.Warn(ctx, "failed to invalidate partition meta cache", mlog.Err(err),
mlog.FieldDbName(request.GetDbName()),
mlog.FieldCollectionName(collectionName),
mlog.FieldCollectionID(collectionID))
return merr.Status(err), nil
}
globalMetaCache.RemovePartition(ctx, request.GetDbName(), collectionID, collectionName, request.GetPartitionName(), request.GetBase().GetTimestamp())
mlog.Info(context.TODO(), "complete to invalidate collection meta cache", mlog.String("type", request.GetBase().GetMsgType().String()))
globalMetaCache.RemovePartition(ctx, request.GetDbName(), collectionID, collectionName, request.GetPartitionName())
mlog.Info(ctx, "complete to invalidate collection meta cache", mlog.String("type", request.GetBase().GetMsgType().String()))
case commonpb.MsgType_DropDatabase:
node.shardMgr.RemoveDatabase(request.GetDbName())
fallthrough
case commonpb.MsgType_AlterDatabase:
globalMetaCache.RemoveDatabase(ctx, request.GetDbName())
case commonpb.MsgType_AlterDatabase:
globalMetaCache.RemoveDatabaseInfo(ctx, request.GetDbName())
case commonpb.MsgType_AlterCollection, commonpb.MsgType_AlterCollectionField:
if request.CollectionID != UniqueID(0) {
aliasName = globalMetaCache.RemoveCollectionsByID(ctx, collectionID, 0, false)
for _, name := range aliasName {
node.shardMgr.DeprecateShardCache(request.GetDbName(), name)
}
}
if collectionName != "" {
globalMetaCache.RemoveCollection(ctx, request.GetDbName(), collectionName, request.GetBase().GetTimestamp())
}
mlog.Info(context.TODO(), "complete to invalidate collection meta cache", mlog.String("type", request.GetBase().GetMsgType().String()))
aliasName = globalMetaCache.InvalidateCollectionMeta(ctx, request.GetDbName(), collectionName, collectionID, false)
deprecateShardCaches(append(aliasName, collectionName)...)
mlog.Info(ctx, "complete to invalidate collection meta cache", mlog.String("type", request.GetBase().GetMsgType().String()))
default:
mlog.Warn(context.TODO(), "receive unexpected msgType of invalidate collection meta cache", mlog.String("msgType", request.GetBase().GetMsgType().String()))
if request.CollectionID != UniqueID(0) {
aliasName = globalMetaCache.RemoveCollectionsByID(ctx, collectionID, request.GetBase().GetTimestamp(), false)
for _, name := range aliasName {
node.shardMgr.DeprecateShardCache(request.GetDbName(), name)
}
}
if collectionName != "" {
globalMetaCache.RemoveCollection(ctx, request.GetDbName(), collectionName, request.GetBase().GetTimestamp()) // no need to return error, though collection may be not cached
node.shardMgr.DeprecateShardCache(request.GetDbName(), collectionName)
}
mlog.Warn(ctx, "receive unexpected msgType of invalidate collection meta cache", mlog.String("msgType", request.GetBase().GetMsgType().String()))
aliasName = globalMetaCache.InvalidateCollectionMeta(ctx, request.GetDbName(), collectionName, collectionID, false)
deprecateShardCaches(append(aliasName, collectionName)...)
}
}
@@ -223,7 +229,7 @@ func (node *Proxy) InvalidateCollectionMetaCache(ctx context.Context, request *p
metrics.CleanupProxyDBMetrics(paramtable.GetNodeID(), request.GetDbName())
DeregisterSubLabel(ratelimitutil.GetDBSubLabel(request.GetDbName()))
}
mlog.Info(context.TODO(), "complete to invalidate collection meta cache")
mlog.Info(ctx, "complete to invalidate collection meta cache")
return merr.Success(), nil
}
@@ -854,16 +860,6 @@ func (node *Proxy) DescribeCollection(ctx context.Context, request *milvuspb.Des
}, nil
}
resp, err := interceptor.Call(ctx, request)
// Single-point redaction at the API edge. The persisted spec carries
// extfs credentials (access_key_id / access_key_value / ssl_ca_cert)
// that internal callers (refresh / load) need raw via the same
// mixCoord.DescribeCollection RPC; redacting at source would break
// FFI auth. Sanitize here so every provider path (cached / remote)
// converges through one spot — adding a new provider does not
// re-introduce the leak.
if resp != nil && resp.GetSchema() != nil {
resp.Schema.ExternalSpec = externalspec.RedactExternalSpec(resp.Schema.ExternalSpec)
}
return resp, err
}
+144 -13
View File
@@ -88,6 +88,127 @@ func TestProxy_InvalidateCollectionMetaCache_remove_stream(t *testing.T) {
assert.Equal(t, commonpb.ErrorCode_Success, status.GetErrorCode())
}
// TestProxy_InvalidateCollectionMetaCache_AliasScanGating locks in the
// holder-scan fallback fingerprint: ONLY Create/AlterAlias broadcasts WITHOUT
// target ids (an old rootcoord) may scan. DropAlias legitimately carries no id
// in every version -- scanning there would reintroduce a permanent O(N) stall
// on the normal drop path -- and id-carrying broadcasts never scan. The strict
// mock fails the test on any unexpected RemoveAliasHolders call.
func TestProxy_InvalidateCollectionMetaCache_AliasScanGating(t *testing.T) {
paramtable.Init()
oldCache := globalMetaCache
defer func() { globalMetaCache = oldCache }()
ctx := context.Background()
const ts = uint64(42)
newNode := func(t *testing.T) (*Proxy, *MockCache) {
cache := NewMockCache(t)
globalMetaCache = cache
shard := shardclient.NewMockShardClientManager(t)
shard.EXPECT().DeprecateShardCache(mock.Anything, mock.Anything).Return().Maybe()
node := &Proxy{shardMgr: shard}
node.UpdateStateCode(commonpb.StateCode_Healthy)
return node, cache
}
req := func(mt commonpb.MsgType, id int64) *proxypb.InvalidateCollMetaCacheRequest {
return &proxypb.InvalidateCollMetaCacheRequest{
Base: &commonpb.MsgBase{MsgType: mt, Timestamp: ts},
DbName: "db",
CollectionName: "a",
CollectionID: id,
}
}
t.Run("DropAlias without id must NOT scan", func(t *testing.T) {
node, cache := newNode(t)
cache.EXPECT().InvalidateCollectionMeta(mock.Anything, "db", "a", int64(0), true).Return(nil)
// no RemoveAliasHolders expectation: the strict mock fails on a call
status, err := node.InvalidateCollectionMetaCache(ctx, req(commonpb.MsgType_DropAlias, 0))
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, status.GetErrorCode())
})
for _, mt := range []commonpb.MsgType{commonpb.MsgType_CreateAlias, commonpb.MsgType_AlterAlias} {
t.Run(mt.String()+" without id (old rootcoord) must scan", func(t *testing.T) {
node, cache := newNode(t)
cache.EXPECT().InvalidateCollectionMeta(mock.Anything, "db", "a", int64(0), true).Return(nil)
cache.EXPECT().RemoveAliasHolders(mock.Anything, "db", "a").Return().Once()
status, err := node.InvalidateCollectionMetaCache(ctx, req(mt, 0))
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, status.GetErrorCode())
})
}
t.Run("AlterAlias WITH id (new rootcoord) must NOT scan", func(t *testing.T) {
node, cache := newNode(t)
cache.EXPECT().InvalidateCollectionMeta(mock.Anything, "db", "a", int64(7), true).Return(nil)
status, err := node.InvalidateCollectionMetaCache(ctx, req(commonpb.MsgType_AlterAlias, 7))
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, status.GetErrorCode())
})
}
func TestProxy_InvalidateCollectionMetaCache_DatabaseInvalidationScope(t *testing.T) {
paramtable.Init()
oldCache := globalMetaCache
defer func() { globalMetaCache = oldCache }()
t.Run("alter database removes database info only", func(t *testing.T) {
cache := NewMockCache(t)
cache.EXPECT().RemoveDatabaseInfo(mock.Anything, "db").Return().Once()
globalMetaCache = cache
node := &Proxy{}
node.UpdateStateCode(commonpb.StateCode_Healthy)
status, err := node.InvalidateCollectionMetaCache(context.Background(), &proxypb.InvalidateCollMetaCacheRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_AlterDatabase},
DbName: "db",
})
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, status.GetErrorCode())
})
t.Run("drop database still removes all database metadata", func(t *testing.T) {
cache := NewMockCache(t)
cache.EXPECT().RemoveDatabase(mock.Anything, "db").Return().Once()
globalMetaCache = cache
shardMgr := shardclient.NewMockShardClientManager(t)
shardMgr.EXPECT().RemoveDatabase("db").Return().Once()
node := &Proxy{shardMgr: shardMgr}
assert.NoError(t, node.initRateCollector())
node.UpdateStateCode(commonpb.StateCode_Healthy)
status, err := node.InvalidateCollectionMetaCache(context.Background(), &proxypb.InvalidateCollMetaCacheRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_DropDatabase},
DbName: "db",
})
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, status.GetErrorCode())
})
}
func TestProxy_InvalidateCollectionMetaCache_EmptyPartitionNameReturnsTypedStatus(t *testing.T) {
paramtable.Init()
oldCache := globalMetaCache
defer func() { globalMetaCache = oldCache }()
globalMetaCache = NewMockCache(t)
node := &Proxy{}
node.UpdateStateCode(commonpb.StateCode_Healthy)
status, err := node.InvalidateCollectionMetaCache(context.Background(), &proxypb.InvalidateCollMetaCacheRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_CreatePartition},
DbName: "db",
CollectionName: "collection",
CollectionID: 100,
})
assert.NoError(t, err)
assert.ErrorIs(t, merr.Error(status), merr.ErrServiceInternal)
assert.Equal(t, merr.Code(merr.ErrServiceInternal), status.GetCode())
assert.Contains(t, status.GetReason(), "empty partition name")
assert.NotEqual(t, "true", status.GetExtraInfo()[merr.InputErrorFlagKey])
}
func TestProxy_CheckHealth(t *testing.T) {
t.Run("not healthy", func(t *testing.T) {
node := &Proxy{session: &sessionutil.Session{SessionRaw: sessionutil.SessionRaw{ServerID: 1}}}
@@ -1262,6 +1383,9 @@ func TestProxyDescribeCollection(t *testing.T) {
},
},
}, nil).Maybe()
mixCoord.On("DescribeCollection", mock.Anything, mock.MatchedBy(func(req *milvuspb.DescribeCollectionRequest) bool {
return req.DbName == "db_not_exists"
})).Return(nil, merr.WrapErrDatabaseNotFound("db_not_exists")).Maybe()
mixCoord.On("ShowPartitions", mock.Anything, mock.Anything).Return(&milvuspb.ShowPartitionsResponse{
Status: merr.Success(),
PartitionNames: []string{"default"},
@@ -1293,6 +1417,8 @@ func TestProxyDescribeCollection(t *testing.T) {
assert.NoError(t, err)
assert.Contains(t, resp.GetStatus().GetReason(), "can't find collection[database=test_1][collection=test_not_exists]")
assert.Equal(t, commonpb.ErrorCode_CollectionNotExists, resp.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrCollectionNotFound), resp.GetStatus().GetCode())
assert.Equal(t, "true", resp.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey])
})
t.Run("collection id not exists", func(t *testing.T) {
@@ -1303,6 +1429,8 @@ func TestProxyDescribeCollection(t *testing.T) {
assert.NoError(t, err)
assert.Contains(t, resp.GetStatus().GetReason(), "can't find collection[database=test_1][collection=]")
assert.Equal(t, commonpb.ErrorCode_CollectionNotExists, resp.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrCollectionNotFound), resp.GetStatus().GetCode())
assert.Equal(t, "true", resp.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey])
})
t.Run("db not exists", func(t *testing.T) {
@@ -1311,8 +1439,11 @@ func TestProxyDescribeCollection(t *testing.T) {
CollectionName: "test_collection",
})
assert.NoError(t, err)
assert.Contains(t, resp.GetStatus().GetReason(), "can't find collection[database=db_not_exists][collection=test_collection]")
assert.Equal(t, commonpb.ErrorCode_CollectionNotExists, resp.GetStatus().GetErrorCode())
assert.Contains(t, resp.GetStatus().GetReason(), "database not found")
assert.Equal(t, commonpb.ErrorCode_UnexpectedError, resp.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrDatabaseNotFound), resp.GetStatus().GetCode())
assert.ErrorIs(t, merr.Error(resp.GetStatus()), merr.ErrDatabaseNotFound)
assert.Equal(t, "true", resp.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey])
})
t.Run("describe collection ok", func(t *testing.T) {
@@ -1413,7 +1544,7 @@ func TestProxy_Delete(t *testing.T) {
},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
basicInfo := &collectionInfo{
collID: collectionID,
}
@@ -2233,7 +2364,7 @@ func TestHandleIfSearchByPK_PreservesNamespaceInInternalQuery(t *testing.T) {
cache := NewMockCache(t)
cache.EXPECT().
GetCollectionInfo(mock.Anything, "default", "test_collection", int64(0)).
Return(&collectionInfo{schema: newSchemaInfo(schema)}, nil)
Return(&collectionInfo{schema: mustNewSchemaInfo(schema)}, nil)
globalMetaCache = cache
var capturedNamespace *string
@@ -2308,7 +2439,7 @@ func TestProxy_ManualCompaction_ExternalCollection(t *testing.T) {
m1 := mockey.Mock((*MetaCache).GetCollectionID).Return(int64(1), nil).Build()
m2 := mockey.Mock((*MetaCache).GetCollectionInfo).Return(&collectionInfo{
schema: newSchemaInfo(externalSchema),
schema: mustNewSchemaInfo(externalSchema),
}, nil).Build()
defer m1.UnPatch()
defer m2.UnPatch()
@@ -2338,7 +2469,7 @@ func TestProxy_Insert_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2367,7 +2498,7 @@ func TestProxy_Delete_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2397,7 +2528,7 @@ func TestProxy_Upsert_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2426,7 +2557,7 @@ func TestProxy_Flush_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2455,7 +2586,7 @@ func TestProxy_CreatePartition_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2485,7 +2616,7 @@ func TestProxy_DropPartition_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2515,7 +2646,7 @@ func TestProxy_ImportV2_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
@@ -2782,7 +2913,7 @@ func TestProxy_AlterCollectionField_ExternalCollection(t *testing.T) {
},
}
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(externalSchema), nil).Build()
m1 := mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(externalSchema), nil).Build()
defer m1.UnPatch()
proxy := &Proxy{}
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
+147 -30
View File
@@ -751,6 +751,58 @@ func (_c *MockCache_HasDatabase_Call) RunAndReturn(run func(context.Context, str
return _c
}
// InvalidateCollectionMeta provides a mock function with given fields: ctx, database, collectionName, collectionID, removeAlias
func (_m *MockCache) InvalidateCollectionMeta(ctx context.Context, database string, collectionName string, collectionID int64, removeAlias bool) []string {
ret := _m.Called(ctx, database, collectionName, collectionID, removeAlias)
if len(ret) == 0 {
panic("no return value specified for InvalidateCollectionMeta")
}
var r0 []string
if rf, ok := ret.Get(0).(func(context.Context, string, string, int64, bool) []string); ok {
r0 = rf(ctx, database, collectionName, collectionID, removeAlias)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]string)
}
}
return r0
}
// MockCache_InvalidateCollectionMeta_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'InvalidateCollectionMeta'
type MockCache_InvalidateCollectionMeta_Call struct {
*mock.Call
}
// InvalidateCollectionMeta is a helper method to define mock.On call
// - ctx context.Context
// - database string
// - collectionName string
// - collectionID int64
// - removeAlias bool
func (_e *MockCache_Expecter) InvalidateCollectionMeta(ctx interface{}, database interface{}, collectionName interface{}, collectionID interface{}, removeAlias interface{}) *MockCache_InvalidateCollectionMeta_Call {
return &MockCache_InvalidateCollectionMeta_Call{Call: _e.mock.On("InvalidateCollectionMeta", ctx, database, collectionName, collectionID, removeAlias)}
}
func (_c *MockCache_InvalidateCollectionMeta_Call) Run(run func(ctx context.Context, database string, collectionName string, collectionID int64, removeAlias bool)) *MockCache_InvalidateCollectionMeta_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(int64), args[4].(bool))
})
return _c
}
func (_c *MockCache_InvalidateCollectionMeta_Call) Return(_a0 []string) *MockCache_InvalidateCollectionMeta_Call {
_c.Call.Return(_a0)
return _c
}
func (_c *MockCache_InvalidateCollectionMeta_Call) RunAndReturn(run func(context.Context, string, string, int64, bool) []string) *MockCache_InvalidateCollectionMeta_Call {
_c.Call.Return(run)
return _c
}
// RemoveAlias provides a mock function with given fields: ctx, database, alias
func (_m *MockCache) RemoveAlias(ctx context.Context, database string, alias string) {
_m.Called(ctx, database, alias)
@@ -786,9 +838,44 @@ func (_c *MockCache_RemoveAlias_Call) RunAndReturn(run func(context.Context, str
return _c
}
// RemoveCollection provides a mock function with given fields: ctx, database, collectionName, version
func (_m *MockCache) RemoveCollection(ctx context.Context, database string, collectionName string, version uint64) {
_m.Called(ctx, database, collectionName, version)
// RemoveAliasHolders provides a mock function with given fields: ctx, database, alias
func (_m *MockCache) RemoveAliasHolders(ctx context.Context, database string, alias string) {
_m.Called(ctx, database, alias)
}
// MockCache_RemoveAliasHolders_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveAliasHolders'
type MockCache_RemoveAliasHolders_Call struct {
*mock.Call
}
// RemoveAliasHolders is a helper method to define mock.On call
// - ctx context.Context
// - database string
// - alias string
func (_e *MockCache_Expecter) RemoveAliasHolders(ctx interface{}, database interface{}, alias interface{}) *MockCache_RemoveAliasHolders_Call {
return &MockCache_RemoveAliasHolders_Call{Call: _e.mock.On("RemoveAliasHolders", ctx, database, alias)}
}
func (_c *MockCache_RemoveAliasHolders_Call) Run(run func(ctx context.Context, database string, alias string)) *MockCache_RemoveAliasHolders_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string), args[2].(string))
})
return _c
}
func (_c *MockCache_RemoveAliasHolders_Call) Return() *MockCache_RemoveAliasHolders_Call {
_c.Call.Return()
return _c
}
func (_c *MockCache_RemoveAliasHolders_Call) RunAndReturn(run func(context.Context, string, string)) *MockCache_RemoveAliasHolders_Call {
_c.Run(run)
return _c
}
// RemoveCollection provides a mock function with given fields: ctx, database, collectionName
func (_m *MockCache) RemoveCollection(ctx context.Context, database string, collectionName string) {
_m.Called(ctx, database, collectionName)
}
// MockCache_RemoveCollection_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveCollection'
@@ -800,14 +887,13 @@ type MockCache_RemoveCollection_Call struct {
// - ctx context.Context
// - database string
// - collectionName string
// - version uint64
func (_e *MockCache_Expecter) RemoveCollection(ctx interface{}, database interface{}, collectionName interface{}, version interface{}) *MockCache_RemoveCollection_Call {
return &MockCache_RemoveCollection_Call{Call: _e.mock.On("RemoveCollection", ctx, database, collectionName, version)}
func (_e *MockCache_Expecter) RemoveCollection(ctx interface{}, database interface{}, collectionName interface{}) *MockCache_RemoveCollection_Call {
return &MockCache_RemoveCollection_Call{Call: _e.mock.On("RemoveCollection", ctx, database, collectionName)}
}
func (_c *MockCache_RemoveCollection_Call) Run(run func(ctx context.Context, database string, collectionName string, version uint64)) *MockCache_RemoveCollection_Call {
func (_c *MockCache_RemoveCollection_Call) Run(run func(ctx context.Context, database string, collectionName string)) *MockCache_RemoveCollection_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(uint64))
run(args[0].(context.Context), args[1].(string), args[2].(string))
})
return _c
}
@@ -817,22 +903,22 @@ func (_c *MockCache_RemoveCollection_Call) Return() *MockCache_RemoveCollection_
return _c
}
func (_c *MockCache_RemoveCollection_Call) RunAndReturn(run func(context.Context, string, string, uint64)) *MockCache_RemoveCollection_Call {
func (_c *MockCache_RemoveCollection_Call) RunAndReturn(run func(context.Context, string, string)) *MockCache_RemoveCollection_Call {
_c.Run(run)
return _c
}
// RemoveCollectionsByID provides a mock function with given fields: ctx, collectionID, version, removeVersion
func (_m *MockCache) RemoveCollectionsByID(ctx context.Context, collectionID int64, version uint64, removeVersion bool) []string {
ret := _m.Called(ctx, collectionID, version, removeVersion)
// RemoveCollectionsByID provides a mock function with given fields: ctx, collectionID
func (_m *MockCache) RemoveCollectionsByID(ctx context.Context, collectionID int64) []string {
ret := _m.Called(ctx, collectionID)
if len(ret) == 0 {
panic("no return value specified for RemoveCollectionsByID")
}
var r0 []string
if rf, ok := ret.Get(0).(func(context.Context, int64, uint64, bool) []string); ok {
r0 = rf(ctx, collectionID, version, removeVersion)
if rf, ok := ret.Get(0).(func(context.Context, int64) []string); ok {
r0 = rf(ctx, collectionID)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]string)
@@ -850,15 +936,13 @@ type MockCache_RemoveCollectionsByID_Call struct {
// RemoveCollectionsByID is a helper method to define mock.On call
// - ctx context.Context
// - collectionID int64
// - version uint64
// - removeVersion bool
func (_e *MockCache_Expecter) RemoveCollectionsByID(ctx interface{}, collectionID interface{}, version interface{}, removeVersion interface{}) *MockCache_RemoveCollectionsByID_Call {
return &MockCache_RemoveCollectionsByID_Call{Call: _e.mock.On("RemoveCollectionsByID", ctx, collectionID, version, removeVersion)}
func (_e *MockCache_Expecter) RemoveCollectionsByID(ctx interface{}, collectionID interface{}) *MockCache_RemoveCollectionsByID_Call {
return &MockCache_RemoveCollectionsByID_Call{Call: _e.mock.On("RemoveCollectionsByID", ctx, collectionID)}
}
func (_c *MockCache_RemoveCollectionsByID_Call) Run(run func(ctx context.Context, collectionID int64, version uint64, removeVersion bool)) *MockCache_RemoveCollectionsByID_Call {
func (_c *MockCache_RemoveCollectionsByID_Call) Run(run func(ctx context.Context, collectionID int64)) *MockCache_RemoveCollectionsByID_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(int64), args[2].(uint64), args[3].(bool))
run(args[0].(context.Context), args[1].(int64))
})
return _c
}
@@ -868,7 +952,7 @@ func (_c *MockCache_RemoveCollectionsByID_Call) Return(_a0 []string) *MockCache_
return _c
}
func (_c *MockCache_RemoveCollectionsByID_Call) RunAndReturn(run func(context.Context, int64, uint64, bool) []string) *MockCache_RemoveCollectionsByID_Call {
func (_c *MockCache_RemoveCollectionsByID_Call) RunAndReturn(run func(context.Context, int64) []string) *MockCache_RemoveCollectionsByID_Call {
_c.Call.Return(run)
return _c
}
@@ -907,9 +991,43 @@ func (_c *MockCache_RemoveDatabase_Call) RunAndReturn(run func(context.Context,
return _c
}
// RemovePartition provides a mock function with given fields: ctx, database, collectionID, collectionName, partitionName, version
func (_m *MockCache) RemovePartition(ctx context.Context, database string, collectionID int64, collectionName string, partitionName string, version uint64) {
_m.Called(ctx, database, collectionID, collectionName, partitionName, version)
// RemoveDatabaseInfo provides a mock function with given fields: ctx, database
func (_m *MockCache) RemoveDatabaseInfo(ctx context.Context, database string) {
_m.Called(ctx, database)
}
// MockCache_RemoveDatabaseInfo_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveDatabaseInfo'
type MockCache_RemoveDatabaseInfo_Call struct {
*mock.Call
}
// RemoveDatabaseInfo is a helper method to define mock.On call
// - ctx context.Context
// - database string
func (_e *MockCache_Expecter) RemoveDatabaseInfo(ctx interface{}, database interface{}) *MockCache_RemoveDatabaseInfo_Call {
return &MockCache_RemoveDatabaseInfo_Call{Call: _e.mock.On("RemoveDatabaseInfo", ctx, database)}
}
func (_c *MockCache_RemoveDatabaseInfo_Call) Run(run func(ctx context.Context, database string)) *MockCache_RemoveDatabaseInfo_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string))
})
return _c
}
func (_c *MockCache_RemoveDatabaseInfo_Call) Return() *MockCache_RemoveDatabaseInfo_Call {
_c.Call.Return()
return _c
}
func (_c *MockCache_RemoveDatabaseInfo_Call) RunAndReturn(run func(context.Context, string)) *MockCache_RemoveDatabaseInfo_Call {
_c.Run(run)
return _c
}
// RemovePartition provides a mock function with given fields: ctx, database, collectionID, collectionName, partitionName
func (_m *MockCache) RemovePartition(ctx context.Context, database string, collectionID int64, collectionName string, partitionName string) {
_m.Called(ctx, database, collectionID, collectionName, partitionName)
}
// MockCache_RemovePartition_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemovePartition'
@@ -923,14 +1041,13 @@ type MockCache_RemovePartition_Call struct {
// - collectionID int64
// - collectionName string
// - partitionName string
// - version uint64
func (_e *MockCache_Expecter) RemovePartition(ctx interface{}, database interface{}, collectionID interface{}, collectionName interface{}, partitionName interface{}, version interface{}) *MockCache_RemovePartition_Call {
return &MockCache_RemovePartition_Call{Call: _e.mock.On("RemovePartition", ctx, database, collectionID, collectionName, partitionName, version)}
func (_e *MockCache_Expecter) RemovePartition(ctx interface{}, database interface{}, collectionID interface{}, collectionName interface{}, partitionName interface{}) *MockCache_RemovePartition_Call {
return &MockCache_RemovePartition_Call{Call: _e.mock.On("RemovePartition", ctx, database, collectionID, collectionName, partitionName)}
}
func (_c *MockCache_RemovePartition_Call) Run(run func(ctx context.Context, database string, collectionID int64, collectionName string, partitionName string, version uint64)) *MockCache_RemovePartition_Call {
func (_c *MockCache_RemovePartition_Call) Run(run func(ctx context.Context, database string, collectionID int64, collectionName string, partitionName string)) *MockCache_RemovePartition_Call {
_c.Call.Run(func(args mock.Arguments) {
run(args[0].(context.Context), args[1].(string), args[2].(int64), args[3].(string), args[4].(string), args[5].(uint64))
run(args[0].(context.Context), args[1].(string), args[2].(int64), args[3].(string), args[4].(string))
})
return _c
}
@@ -940,7 +1057,7 @@ func (_c *MockCache_RemovePartition_Call) Return() *MockCache_RemovePartition_Ca
return _c
}
func (_c *MockCache_RemovePartition_Call) RunAndReturn(run func(context.Context, string, int64, string, string, uint64)) *MockCache_RemovePartition_Call {
func (_c *MockCache_RemovePartition_Call) RunAndReturn(run func(context.Context, string, int64, string, string)) *MockCache_RemovePartition_Call {
_c.Run(run)
return _c
}
+1 -2
View File
@@ -5,9 +5,8 @@ package proxy
import (
context "context"
grpc "google.golang.org/grpc"
mock "github.com/stretchr/testify/mock"
grpc "google.golang.org/grpc"
rootcoordpb "github.com/milvus-io/milvus/pkg/v3/proto/rootcoordpb"
)
+9
View File
@@ -4582,6 +4582,15 @@ func TestProxy_RelatedPrivilege(t *testing.T) {
}
ctx := GetContext(context.Background(), "root:123456")
// OperatePrivilege resolves the collection alias before granting (default
// ResolveAliasForPrivilege on); pin globalMetaCache to a mock that resolves
// "col1" as-is, so the test does not depend on leftover global cache state.
originCache := globalMetaCache
metaCache := NewMockCache(t)
metaCache.EXPECT().ResolveCollectionAlias(mock.Anything, mock.Anything, "col1").Return("col1", nil).Maybe()
globalMetaCache = metaCache
defer func() { globalMetaCache = originCache }()
t.Run("related privilege grpc error", func(t *testing.T) {
mixCoord := mocks.NewMockMixCoordClient(t)
proxy := &Proxy{mixCoord: mixCoord}
@@ -304,7 +304,7 @@ func TestRateLimitInterceptor(t *testing.T) {
createdTimestamp: 1,
}, nil).Times(4)
mockCache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(int64(1), nil).Times(4)
mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(newSchemaInfo(schema), nil).Times(4)
mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(mustNewSchemaInfo(schema), nil).Times(4)
mockCache.EXPECT().GetPartitionInfo(mock.Anything, mock.Anything, mock.Anything, namespace).Return(&partitionInfo{
name: namespace,
partitionID: 20,
+8 -1
View File
@@ -812,12 +812,19 @@ func (coord *MixCoordMock) ShowPartitions(ctx context.Context, req *milvuspb.Sho
coord.collMtx.RLock()
defer coord.collMtx.RUnlock()
// mirror DescribeCollection: an empty CollectionName means the caller
// addressed the collection by id -- the proxy's id-primary partition fill
// shows partitions BY id (empty name) -- so resolve from req.CollectionID.
usingID := req.CollectionName == ""
collID, exist := coord.collName2ID[req.CollectionName]
if !exist {
if !exist && !usingID {
return &milvuspb.ShowPartitionsResponse{
Status: merr.Status(merr.WrapErrCollectionNotFound(req.CollectionName)),
}, nil
}
if usingID {
collID = req.CollectionID
}
coord.partitionMtx.RLock()
defer coord.partitionMtx.RUnlock()
+3 -3
View File
@@ -1279,7 +1279,7 @@ func (s *SearchPipelineSuite) TestHighlightOp() {
tasks: highlightTasks,
},
lb: mockLb,
schema: newSchemaInfo(schema),
schema: mustNewSchemaInfo(schema),
request: req,
collectionName: collName,
SearchRequest: &internalpb.SearchRequest{
@@ -1398,7 +1398,7 @@ func (s *SearchPipelineSuite) TestLexicalHighlightOpNullableStringKeepsEmptyHigh
tasks: highlightTasks,
},
lb: mockLb,
schema: newSchemaInfo(schema),
schema: mustNewSchemaInfo(schema),
request: &milvuspb.SearchRequest{CollectionName: collName, DbName: "default"},
collectionName: collName,
SearchRequest: &internalpb.SearchRequest{CollectionID: 0},
@@ -3404,7 +3404,7 @@ func (s *SearchPipelineSuite) TestNewSearchReduceOperatorUsesPipelineOffsetParam
CollectionID: 100,
PartitionIDs: []int64{10},
},
schema: newSchemaInfo(&schemapb.CollectionSchema{
schema: mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
{FieldID: 101, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
},
+209 -119
View File
@@ -6,8 +6,8 @@ import (
"strconv"
"github.com/cockroachdb/errors"
"github.com/samber/lo"
"go.opentelemetry.io/otel"
"google.golang.org/protobuf/proto"
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
@@ -15,6 +15,7 @@ import (
"github.com/milvus-io/milvus/pkg/v3/common"
"github.com/milvus-io/milvus/pkg/v3/metrics"
"github.com/milvus-io/milvus/pkg/v3/mlog"
"github.com/milvus-io/milvus/pkg/v3/util/externalspec"
"github.com/milvus-io/milvus/pkg/v3/util/merr"
"github.com/milvus-io/milvus/pkg/v3/util/paramtable"
"github.com/milvus-io/milvus/pkg/v3/util/timerecord"
@@ -35,6 +36,35 @@ type ServiceInterceptor[Req Request, Resp Response] interface {
Call(ctx context.Context, request Req) (Resp, error)
}
// serviceProviderMetricError lets a provider preserve a non-failure metric
// outcome, such as "abandon" for a request that could not be enqueued, while
// keeping the original error identity and merr code intact for the response.
type serviceProviderMetricError struct {
err error
status string
cause string
}
func (e *serviceProviderMetricError) Error() string {
return e.err.Error()
}
func (e *serviceProviderMetricError) Unwrap() error {
return e.err
}
func withServiceProviderMetric(err error, status, cause string) error {
return &serviceProviderMetricError{err: err, status: status, cause: cause}
}
func serviceCallMetricLabel(err error) (string, string) {
var metricErr *serviceProviderMetricError
if errors.As(err, &metricErr) {
return metricErr.status, metricErr.cause
}
return failMetricLabel(err)
}
func NewInterceptor[Req Request, Resp Response](proxy *Proxy, method string) (*InterceptorImpl[Req, Resp], error) {
var provider milvuspb.MilvusServiceServer
cached := paramtable.Get().ProxyCfg.EnableCachedServiceProvider.GetAsBool()
@@ -46,9 +76,10 @@ func NewInterceptor[Req Request, Resp Response](proxy *Proxy, method string) (*I
switch method {
case "DescribeCollection":
interceptor := &InterceptorImpl[*milvuspb.DescribeCollectionRequest, *milvuspb.DescribeCollectionResponse]{
proxy: proxy,
method: method,
onCall: provider.DescribeCollection,
proxy: proxy,
method: method,
onCall: provider.DescribeCollection,
onResponse: finalizeDescribeCollectionResponse,
onError: func(err error) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{
Status: merr.Status(err),
@@ -62,10 +93,11 @@ func NewInterceptor[Req Request, Resp Response](proxy *Proxy, method string) (*I
}
type InterceptorImpl[Req Request, Resp Response] struct {
proxy *Proxy
method string
onCall func(ctx context.Context, request Req) (Resp, error)
onError func(err error) (Resp, error)
proxy *Proxy
method string
onCall func(ctx context.Context, request Req) (Resp, error)
onResponse func(resp Resp) error
onError func(err error) (Resp, error)
}
func (i *InterceptorImpl[Req, Resp]) Call(ctx context.Context, request Req,
@@ -77,65 +109,141 @@ func (i *InterceptorImpl[Req, Resp]) Call(ctx context.Context, request Req,
ctx, sp := otel.Tracer(typeutil.ProxyRole).Start(ctx, fmt.Sprintf("Proxy-%s", i.method))
defer sp.End()
tr := timerecord.NewTimeRecorder(i.method)
metrics.ProxyFunctionCall.WithLabelValues(strconv.FormatInt(paramtable.GetNodeID(), 10), i.method,
metrics.TotalLabel, metrics.CauseNA, request.GetDbName(), request.GetCollectionName()).Inc()
nodeID := strconv.FormatInt(paramtable.GetNodeID(), 10)
dbName := request.GetDbName()
collectionName := request.GetCollectionName()
metrics.ProxyFunctionCall.WithLabelValues(nodeID, i.method,
metrics.TotalLabel, metrics.CauseNA, dbName, collectionName).Inc()
defer func() {
metrics.ProxyReqLatency.WithLabelValues(nodeID, i.method).
Observe(float64(tr.ElapseSpan().Milliseconds()))
}()
resp, err := i.onCall(ctx, request)
if err != nil {
status, cause := serviceCallMetricLabel(err)
metrics.ProxyFunctionCall.WithLabelValues(nodeID, i.method,
status, cause, dbName, collectionName).Inc()
return i.onError(err)
}
metrics.ProxyFunctionCall.WithLabelValues(strconv.FormatInt(paramtable.GetNodeID(), 10), i.method,
metrics.SuccessLabel, metrics.CauseNA, request.GetDbName(), request.GetCollectionName()).Inc()
metrics.ProxyReqLatency.WithLabelValues(strconv.FormatInt(paramtable.GetNodeID(), 10), i.method).Observe(float64(tr.ElapseSpan().Milliseconds()))
if i.onResponse != nil && merr.Ok(resp.GetStatus()) {
if err := i.onResponse(resp); err != nil {
status, cause := serviceCallMetricLabel(err)
metrics.ProxyFunctionCall.WithLabelValues(nodeID, i.method,
status, cause, dbName, collectionName).Inc()
return i.onError(err)
}
}
return resp, err
if !merr.Ok(resp.GetStatus()) {
status, cause := failMetricLabel(merr.Error(resp.GetStatus()))
metrics.ProxyFunctionCall.WithLabelValues(nodeID, i.method,
status, cause, dbName, collectionName).Inc()
return resp, nil
}
metrics.ProxyFunctionCall.WithLabelValues(nodeID, i.method,
metrics.SuccessLabel, metrics.CauseNA, dbName, collectionName).Inc()
return resp, nil
}
type CachedProxyServiceProvider struct {
*Proxy
}
// cloneStructArrayFields creates a deep copy of struct array fields to avoid modifying cached data
func cloneStructArrayFields(fields []*schemapb.StructArrayFieldSchema) []*schemapb.StructArrayFieldSchema {
if fields == nil {
func describeCollectionErrorStatus(err error, database, collectionName string) *commonpb.Status {
// A user-facing DescribeCollection miss is an input error, but keep the
// precise source code: a missing database is ErrDatabaseNotFound while a
// missing collection is ErrCollectionNotFound. The sentinels remain system
// errors globally because internal refresh/retry paths also use them. The
// deprecated ErrorCode enum has no database-not-found value, so Code=800 is
// authoritative there and ErrorCode keeps merr's standard UnexpectedError
// compatibility fallback.
err = merr.WrapErrAsInputErrorWhen(err, merr.ErrCollectionNotFound, merr.ErrDatabaseNotFound)
status := merr.Status(err)
if errors.Is(err, merr.ErrCollectionNotFound) {
// Preserve the established SDK-visible collection-not-found message while
// letting merr.Status own both the typed Code and legacy ErrorCode mapping.
reason := fmt.Sprintf("can't find collection[database=%s][collection=%s]", database, collectionName)
status.Reason = reason
status.Detail = reason
}
return status
}
func needsTimestamptzDefaultProjection(field *schemapb.FieldSchema) bool {
if field.GetDataType() != schemapb.DataType_Timestamptz || field.GetDefaultValue() == nil {
return false
}
_, ok := field.GetDefaultValue().GetData().(*schemapb.ValueField_TimestamptzData)
return ok
}
// projectDescribeCollectionSchema defines the public DescribeCollection schema
// shape shared by cached and remote providers. sourceShared is true for
// MetaCache-owned schemas: only fields that the public TIMESTAMPTZ rewrite will
// mutate are cloned, keeping the canonical cached int64 representation intact.
func projectDescribeCollectionSchema(source *schemapb.CollectionSchema, sourceShared bool) (*schemapb.CollectionSchema, error) {
if source == nil {
return nil, merr.WrapErrServiceInternalMsg("describe collection returned a nil collection schema")
}
projected := &schemapb.CollectionSchema{
Name: source.GetName(),
Description: source.GetDescription(),
AutoID: source.GetAutoID(),
Fields: make([]*schemapb.FieldSchema, 0, len(source.GetFields())),
EnableDynamicField: source.GetEnableDynamicField(),
Properties: append([]*commonpb.KeyValuePair(nil), source.GetProperties()...),
Functions: append([]*schemapb.FunctionSchema(nil), source.GetFunctions()...),
DbName: source.GetDbName(),
StructArrayFields: make([]*schemapb.StructArrayFieldSchema, 0, len(source.GetStructArrayFields())),
Version: source.GetVersion(),
ExternalSource: source.GetExternalSource(),
ExternalSpec: source.GetExternalSpec(),
EnableNamespace: source.GetEnableNamespace(),
}
for _, field := range source.GetFields() {
if field.GetIsDynamic() || field.GetName() == common.NamespaceFieldName ||
field.GetFieldID() < common.StartOfUserFieldID {
continue
}
outputField := field
if sourceShared && needsTimestamptzDefaultProjection(field) {
outputField = proto.Clone(field).(*schemapb.FieldSchema)
}
projected.Fields = append(projected.Fields, outputField)
}
// Struct field names are restored in place, so these messages must always be
// detached from both RootCoord's response and MetaCache's canonical schema.
for _, field := range source.GetStructArrayFields() {
projected.StructArrayFields = append(projected.StructArrayFields,
proto.Clone(field).(*schemapb.StructArrayFieldSchema))
}
if err := restoreStructFieldNames(projected); err != nil {
return nil, merr.WrapErrServiceInternalErr(err, "failed to restore struct field names")
}
return projected, nil
}
func finalizeDescribeCollectionResponse(resp *milvuspb.DescribeCollectionResponse) error {
if resp == nil || !merr.Ok(resp.GetStatus()) {
return nil
}
cloned := make([]*schemapb.StructArrayFieldSchema, len(fields))
for i, field := range fields {
cloned[i] = &schemapb.StructArrayFieldSchema{
FieldID: field.FieldID,
Name: field.Name,
Description: field.Description,
Fields: make([]*schemapb.FieldSchema, len(field.Fields)),
Nullable: field.Nullable,
}
// Deep copy sub-fields
for j, subField := range field.Fields {
cloned[i].Fields[j] = &schemapb.FieldSchema{
FieldID: subField.FieldID,
Name: subField.Name,
IsPrimaryKey: subField.IsPrimaryKey,
Description: subField.Description,
DataType: subField.DataType,
TypeParams: subField.TypeParams,
IndexParams: subField.IndexParams,
AutoID: subField.AutoID,
State: subField.State,
ElementType: subField.ElementType,
DefaultValue: subField.DefaultValue,
IsDynamic: subField.IsDynamic,
IsPartitionKey: subField.IsPartitionKey,
IsClusteringKey: subField.IsClusteringKey,
Nullable: subField.Nullable,
IsFunctionOutput: subField.IsFunctionOutput,
}
}
if resp.GetSchema() == nil {
return merr.WrapErrServiceInternalMsg("describe collection returned a nil projected schema")
}
return cloned
if err := timestamptz.RewriteTimestampTzDefaultValueToString(resp.Schema); err != nil {
return merr.WrapErrServiceInternalErr(err, "failed to project TIMESTAMPTZ default value")
}
resp.Schema.ExternalSpec = externalspec.RedactExternalSpec(resp.Schema.GetExternalSpec())
return nil
}
func (node *CachedProxyServiceProvider) DescribeCollection(ctx context.Context,
@@ -157,75 +265,63 @@ func (node *CachedProxyServiceProvider) DescribeCollection(ctx context.Context,
DbName: request.DbName,
}
wrapErrorStatus := func(err error) *commonpb.Status {
status := &commonpb.Status{}
if errors.Is(err, merr.ErrCollectionNotFound) {
// nolint
status.ErrorCode = commonpb.ErrorCode_CollectionNotExists
// nolint
status.Reason = fmt.Sprintf("can't find collection[database=%s][collection=%s]", request.DbName, request.CollectionName)
status.ExtraInfo = map[string]string{merr.InputErrorFlagKey: "true"}
} else {
status = merr.Status(err)
}
return status
}
// Resolve identifiers on local copies instead of mutating the request: the
// access log interceptor and the success metric labels read the request
// after the handler returns, and must see what the client actually sent.
collectionName := request.CollectionName
collectionID := request.CollectionID
var c *collectionInfo
if request.CollectionName == "" && request.CollectionID > 0 {
collName, err := globalMetaCache.GetCollectionName(ctx, request.DbName, request.CollectionID)
resolvedNameByID := collectionName == "" && collectionID > 0
if resolvedNameByID {
// Resolve the complete entry by id once. Calling GetCollectionName first
// discarded the returned collectionInfo and forced a second describe on a
// rolling-upgrade old RootCoord response that omitted DbName (such entries
// are deliberately returned uncached because their database is unknown).
c, err = globalMetaCache.GetCollectionInfo(ctx, request.DbName, "", collectionID)
if err != nil {
resp.Status = wrapErrorStatus(err)
resp.Status = describeCollectionErrorStatus(err, request.DbName, collectionName)
return resp, nil
}
request.CollectionName = collName
collectionName = c.schema.GetName()
}
// validate collection name, ref describeCollectionTask.PreExecute
if err = validateCollectionName(request.CollectionName); err != nil {
resp.Status = wrapErrorStatus(err)
if err = validateCollectionName(collectionName); err != nil {
resp.Status = describeCollectionErrorStatus(err, request.DbName, collectionName)
return resp, nil
}
request.CollectionID, err = globalMetaCache.GetCollectionID(ctx, request.DbName, request.CollectionName)
// Resolve the id and complete entry from the name only when the caller did
// not provide an id. The id-only path already has the complete entry above.
if !resolvedNameByID {
collectionID, err = globalMetaCache.GetCollectionID(ctx, request.DbName, collectionName)
if err != nil {
resp.Status = describeCollectionErrorStatus(err, request.DbName, collectionName)
return resp, nil
}
c, err = globalMetaCache.GetCollectionInfo(ctx, request.DbName, collectionName, collectionID)
if err != nil {
resp.Status = describeCollectionErrorStatus(err, request.DbName, collectionName)
return resp, nil
}
}
// The base logger was built from the raw request, whose name is empty for
// id-only requests; surface the resolved name so downstream log lines stay
// traceable. The request itself is left untouched -- the access log and
// metric labels read it as the client sent it.
if resolvedNameByID {
log = log.With(mlog.String("resolvedCollection", collectionName))
}
if resp.CollectionName == "" {
resp.CollectionName = c.schema.Name
}
resp.Schema, err = projectDescribeCollectionSchema(c.schema.CollectionSchema, true)
if err != nil {
resp.Status = wrapErrorStatus(err)
return resp, nil
}
c, err := globalMetaCache.GetCollectionInfo(ctx, request.DbName, request.CollectionName, request.CollectionID)
if err != nil {
resp.Status = wrapErrorStatus(err)
return resp, nil
}
// skip dynamic fields, see describeCollectionTask.Execute
resp.Schema = &schemapb.CollectionSchema{
Name: c.schema.Name,
Description: c.schema.Description,
AutoID: c.schema.AutoID,
Fields: lo.Filter(c.schema.Fields, func(field *schemapb.FieldSchema, _ int) bool {
return !field.IsDynamic && field.Name != common.NamespaceFieldName
}),
StructArrayFields: cloneStructArrayFields(c.schema.StructArrayFields),
EnableDynamicField: c.schema.EnableDynamicField,
EnableNamespace: c.schema.EnableNamespace,
Properties: c.schema.Properties,
Functions: c.schema.Functions,
DbName: c.schema.DbName,
ExternalSource: c.schema.ExternalSource,
ExternalSpec: c.schema.ExternalSpec,
Version: c.schema.Version,
}
// Restore struct field names from internal format (structName[fieldName]) to original format
if err := restoreStructFieldNames(resp.Schema); err != nil {
log.Error(ctx, "failed to restore struct field names", mlog.Err(err))
return nil, err
}
err = timestamptz.RewriteTimestampTzDefaultValueToString(resp.Schema)
if err != nil {
log.Info(ctx, "failed to rewrite timestamp value", mlog.Err(err))
log.Error(ctx, "failed to project collection schema", mlog.Err(err))
return nil, err
}
@@ -237,7 +333,7 @@ func (node *CachedProxyServiceProvider) DescribeCollection(ctx context.Context,
resp.DbId = c.dbID
resp.CollectionID = c.collID
resp.UpdateTimestamp = c.updateTimestamp
resp.UpdateTimestampStr = fmt.Sprintf("%d", c.updateTimestamp)
resp.UpdateTimestampStr = strconv.FormatUint(c.updateTimestamp, 10)
resp.CreatedTimestamp = c.createdTimestamp
resp.CreatedUtcTimestamp = c.createdUtcTimestamp
resp.ConsistencyLevel = c.consistencyLevel
@@ -249,7 +345,8 @@ func (node *CachedProxyServiceProvider) DescribeCollection(ctx context.Context,
resp.Properties = c.properties
log.Debug(ctx, "DescribeCollection done",
mlog.FieldCollectionID(resp.GetCollectionID()),
mlog.Any("schema", resp.GetSchema()),
mlog.Int("fieldCount", len(resp.GetSchema().GetFields())),
mlog.Int("schemaVersion", int(resp.GetSchema().GetVersion())),
)
return resp, nil
}
@@ -273,16 +370,13 @@ func (node *RemoteProxyServiceProvider) DescribeCollection(ctx context.Context,
mlog.String("db", request.DbName),
mlog.String("collection", request.CollectionName))
method := "DescribeCollection"
log.Debug(ctx, "DescribeCollection received")
if err := node.sched.ddQueue.Enqueue(dct); err != nil {
log.Warn(ctx, "DescribeCollection failed to enqueue",
mlog.Err(err))
metrics.ProxyFunctionCall.WithLabelValues(strconv.FormatInt(paramtable.GetNodeID(), 10), method,
metrics.AbandonLabel, metrics.CauseNA, request.GetDbName(), request.GetCollectionName()).Inc()
return nil, err
return nil, withServiceProviderMetric(err, metrics.AbandonLabel, metrics.CauseNA)
}
log.Debug(ctx, "DescribeCollection enqueued",
@@ -295,10 +389,6 @@ func (node *RemoteProxyServiceProvider) DescribeCollection(ctx context.Context,
mlog.Uint64("BeginTS", dct.BeginTs()),
mlog.Uint64("EndTS", dct.EndTs()))
failStatus, failCause := failMetricLabel(err)
metrics.ProxyFunctionCall.WithLabelValues(strconv.FormatInt(paramtable.GetNodeID(), 10), method,
failStatus, failCause, request.GetDbName(), request.GetCollectionName()).Inc()
return nil, err
}
+527 -6
View File
@@ -2,10 +2,16 @@ package proxy
import (
"context"
"sync/atomic"
"testing"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
dto "github.com/prometheus/client_model/go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"google.golang.org/grpc"
"google.golang.org/protobuf/proto"
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
@@ -13,9 +19,18 @@ import (
"github.com/milvus-io/milvus/internal/mocks"
"github.com/milvus-io/milvus/internal/util/sessionutil"
"github.com/milvus-io/milvus/pkg/v3/common"
"github.com/milvus-io/milvus/pkg/v3/metrics"
"github.com/milvus-io/milvus/pkg/v3/util/merr"
"github.com/milvus-io/milvus/pkg/v3/util/paramtable"
)
func proxyHistogramSampleCount(t *testing.T, observer prometheus.Observer) uint64 {
t.Helper()
metric := &dto.Metric{}
assert.NoError(t, observer.(prometheus.Metric).Write(metric))
return metric.GetHistogram().GetSampleCount()
}
func TestNewInterceptor(t *testing.T) {
mixc := &mocks.MockMixCoordClient{}
mixc.EXPECT().CheckHealth(mock.Anything, mock.Anything).Return(&milvuspb.CheckHealthResponse{IsHealthy: false}, nil)
@@ -63,7 +78,7 @@ func TestCachedProxyServiceProvider_DescribeCollection_IgnoresLegacyDoPhysicalBa
mockCache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(collectionID, nil)
mockCache.EXPECT().GetCollectionInfo(mock.Anything, dbName, collectionName, collectionID).Return(&collectionInfo{
collID: collectionID,
schema: newSchemaInfo(schema),
schema: mustNewSchemaInfo(schema),
shardsNum: common.DefaultShardsNum,
}, nil)
globalMetaCache = mockCache
@@ -78,6 +93,58 @@ func TestCachedProxyServiceProvider_DescribeCollection_IgnoresLegacyDoPhysicalBa
assert.False(t, resp.GetSchema().GetDoPhysicalBackfill())
}
func TestCachedProxyServiceProvider_DescribeCollection_ByIDFillsNameAndUsesRequestID(t *testing.T) {
ctx := context.Background()
origCache := globalMetaCache
defer func() { globalMetaCache = origCache }()
dbName := "db1"
dbID := int64(3)
collectionName := "coll1"
collectionID := int64(449574)
schema := &schemapb.CollectionSchema{
Name: collectionName,
Fields: []*schemapb.FieldSchema{{
FieldID: common.StartOfUserFieldID,
Name: "id",
IsPrimaryKey: true,
DataType: schemapb.DataType_Int64,
}},
}
// Query by collection id only: no collection name, no db name (e.g. the HTTP
// management API). The provider must fill the top-level collection_name from
// the cache instead of echoing the empty request.CollectionName, and must
// keep the caller-provided id. Neither GetCollectionName nor GetCollectionID
// is mocked, so any redundant identifier round-trip would fail the test.
mockCache := &MockCache{}
mockCache.EXPECT().GetCollectionInfo(mock.Anything, "", "", collectionID).Return(&collectionInfo{
collID: collectionID,
dbName: dbName,
dbID: dbID,
schema: mustNewSchemaInfo(schema),
shardsNum: common.DefaultShardsNum,
}, nil)
globalMetaCache = mockCache
provider := &CachedProxyServiceProvider{Proxy: &Proxy{}}
request := &milvuspb.DescribeCollectionRequest{
CollectionID: collectionID,
}
resp, err := provider.DescribeCollection(ctx, request)
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, resp.GetStatus().GetErrorCode())
assert.Equal(t, collectionName, resp.GetCollectionName())
assert.Equal(t, collectionID, resp.GetCollectionID())
assert.Equal(t, dbName, resp.GetDbName())
assert.Equal(t, dbID, resp.GetDbId())
// the handler must not rewrite the request in place: access logs and metric
// labels serialize it after the call and must see what the client sent
assert.Empty(t, request.GetCollectionName())
assert.Equal(t, collectionID, request.GetCollectionID())
}
func TestCachedProxyServiceProvider_DescribeCollection_FilterNamespaceField(t *testing.T) {
ctx := context.Background()
@@ -117,7 +184,7 @@ func TestCachedProxyServiceProvider_DescribeCollection_FilterNamespaceField(t *t
mockCache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(collectionID, nil)
mockCache.EXPECT().GetCollectionInfo(mock.Anything, dbName, collectionName, collectionID).Return(&collectionInfo{
collID: collectionID,
schema: newSchemaInfo(schema),
schema: mustNewSchemaInfo(schema),
shardsNum: common.DefaultShardsNum,
}, nil)
globalMetaCache = mockCache
@@ -161,14 +228,12 @@ func TestCachedProxyServiceProvider_DescribeCollection_ByIDReturnsActualDbName(t
}
mockCache := &MockCache{}
mockCache.EXPECT().GetCollectionName(mock.Anything, "", collectionID).Return("coll1", nil)
mockCache.EXPECT().GetCollectionID(mock.Anything, "", "coll1").Return(collectionID, nil)
mockCache.EXPECT().GetCollectionInfo(mock.Anything, "", "coll1", collectionID).Return(&collectionInfo{
mockCache.EXPECT().GetCollectionInfo(mock.Anything, "", "", collectionID).Return(&collectionInfo{
collID: collectionID,
// resolved by the coordinator and carried in the cache entry
dbID: 7,
dbName: "db1",
schema: newSchemaInfo(schema),
schema: mustNewSchemaInfo(schema),
shardsNum: common.DefaultShardsNum,
}, nil)
globalMetaCache = mockCache
@@ -182,3 +247,459 @@ func TestCachedProxyServiceProvider_DescribeCollection_ByIDReturnsActualDbName(t
assert.Equal(t, "db1", resp.GetDbName())
assert.Equal(t, int64(7), resp.GetDbId())
}
// TestCachedProxyServiceProvider_DescribeCollection_ByIDNotFoundUsesCollectionNotExistsCode
// covers the direct id-only lookup failing with a not-found. Both the typed code
// and deprecated ErrorCode must carry the collection-not-found meaning.
func TestCachedProxyServiceProvider_DescribeCollection_ByIDNotFoundUsesCollectionNotExistsCode(t *testing.T) {
ctx := context.Background()
origCache := globalMetaCache
defer func() { globalMetaCache = origCache }()
collectionID := int64(4242)
mockCache := &MockCache{}
mockCache.EXPECT().GetCollectionInfo(mock.Anything, "", "", collectionID).
Return(nil, merr.WrapErrCollectionNotFound(collectionID))
globalMetaCache = mockCache
provider := &CachedProxyServiceProvider{Proxy: &Proxy{}}
resp, err := provider.DescribeCollection(ctx, &milvuspb.DescribeCollectionRequest{CollectionID: collectionID})
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_CollectionNotExists, resp.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrCollectionNotFound), resp.GetStatus().GetCode())
assert.Equal(t, "true", resp.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey],
"collection not found must be flagged as an input error")
assert.False(t, resp.GetStatus().GetRetriable())
}
// TestCachedProxyServiceProvider_DescribeCollection_EmptyRequestFailsValidation:
// name=="" and id==0 does NOT enter the id-only branch (id>0 is false), so the
// empty name reaches validateCollectionName and must fail without touching the
// cache.
func TestCachedProxyServiceProvider_DescribeCollection_EmptyRequestFailsValidation(t *testing.T) {
ctx := context.Background()
origCache := globalMetaCache
defer func() { globalMetaCache = origCache }()
// no expectations: any cache call is an unexpected-call failure
globalMetaCache = &MockCache{}
provider := &CachedProxyServiceProvider{Proxy: &Proxy{}}
resp, err := provider.DescribeCollection(ctx, &milvuspb.DescribeCollectionRequest{})
assert.NoError(t, err)
assert.NotEqual(t, commonpb.ErrorCode_Success, resp.GetStatus().GetErrorCode(),
"an empty collection name must fail validation")
}
// TestCachedProxyServiceProvider_DescribeCollection_NameOnlyDoesNotMutateRequest
// mirrors the id-only no-mutation guarantee for the name-only path: the handler
// resolves the id on a local copy and must not write it back into the request,
// which the access log and metric labels read after the call.
func TestCachedProxyServiceProvider_DescribeCollection_NameOnlyDoesNotMutateRequest(t *testing.T) {
ctx := context.Background()
origCache := globalMetaCache
defer func() { globalMetaCache = origCache }()
dbName := "db"
collectionName := "coll1"
collectionID := int64(918)
schema := &schemapb.CollectionSchema{
Name: collectionName,
Fields: []*schemapb.FieldSchema{{
FieldID: common.StartOfUserFieldID,
Name: "id",
IsPrimaryKey: true,
DataType: schemapb.DataType_Int64,
}},
}
mockCache := &MockCache{}
mockCache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(collectionID, nil)
mockCache.EXPECT().GetCollectionInfo(mock.Anything, dbName, collectionName, collectionID).Return(&collectionInfo{
collID: collectionID,
schema: mustNewSchemaInfo(schema),
shardsNum: common.DefaultShardsNum,
}, nil)
globalMetaCache = mockCache
provider := &CachedProxyServiceProvider{Proxy: &Proxy{}}
request := &milvuspb.DescribeCollectionRequest{DbName: dbName, CollectionName: collectionName}
resp, err := provider.DescribeCollection(ctx, request)
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_Success, resp.GetStatus().GetErrorCode())
// the resolved id must not leak back into the request
assert.Equal(t, int64(0), request.GetCollectionID())
assert.Equal(t, collectionName, request.GetCollectionName())
}
func TestCachedProxyServiceProvider_DescribeCollection_IDOnlyOldRootCoordUsesOneDescribeAndKeepsUnknownDB(t *testing.T) {
ctx := context.Background()
origCache := globalMetaCache
defer func() { globalMetaCache = origCache }()
const collectionID = int64(101)
var describeCount atomic.Int32
mixCoord := mocks.NewMockMixCoordClient(t)
mixCoord.EXPECT().DescribeCollection(mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, req *milvuspb.DescribeCollectionRequest, opts ...grpc.CallOption) (*milvuspb.DescribeCollectionResponse, error) {
describeCount.Add(1)
assert.Equal(t, collectionID, req.GetCollectionID())
assert.Empty(t, req.GetDbName())
assert.Empty(t, req.GetCollectionName())
return &milvuspb.DescribeCollectionResponse{
Status: merr.Success(),
CollectionID: collectionID,
// Rolling-upgrade old RootCoord resolves the id but cannot report
// the collection's real database.
DbName: "",
Schema: &schemapb.CollectionSchema{
Name: "foo",
},
}, nil
}).Once()
cache, err := NewMetaCache(mixCoord)
assert.NoError(t, err)
defer cache.Close()
globalMetaCache = cache
provider := &CachedProxyServiceProvider{Proxy: &Proxy{}}
request := &milvuspb.DescribeCollectionRequest{CollectionID: collectionID}
interceptor := DatabaseInterceptor()
result, err := interceptor(ctx, request, &grpc.UnaryServerInfo{}, func(ctx context.Context, req interface{}) (interface{}, error) {
return provider.DescribeCollection(ctx, req.(*milvuspb.DescribeCollectionRequest))
})
assert.NoError(t, err)
resp := result.(*milvuspb.DescribeCollectionResponse)
assert.Equal(t, commonpb.ErrorCode_Success, resp.GetStatus().GetErrorCode())
assert.Equal(t, collectionID, resp.GetCollectionID())
assert.Equal(t, "foo", resp.GetCollectionName())
assert.Empty(t, request.GetDbName(), "the interceptor must preserve an omitted db for id-only requests")
assert.Empty(t, resp.GetDbName(), "an old RootCoord's unknown db must not be reported as default")
assert.Equal(t, int32(1), describeCount.Load())
}
func TestProjectDescribeCollectionSchema_CopyOnWriteAndFinalization(t *testing.T) {
timestamptzField := &schemapb.FieldSchema{
FieldID: common.StartOfUserFieldID + 1,
Name: "created_at",
DataType: schemapb.DataType_Timestamptz,
DefaultValue: &schemapb.ValueField{
Data: &schemapb.ValueField_TimestamptzData{TimestamptzData: 1_700_000_000_000_000},
},
}
ordinaryField := &schemapb.FieldSchema{
FieldID: common.StartOfUserFieldID,
Name: "id",
DataType: schemapb.DataType_Int64,
IsPrimaryKey: true,
State: schemapb.FieldState_FieldDropping,
ExternalField: "external_id",
}
source := &schemapb.CollectionSchema{
Name: "collection",
DbName: "db",
Version: 7,
Properties: []*commonpb.KeyValuePair{{Key: common.TimezoneKey, Value: "UTC"}},
ExternalSpec: `{"format":"parquet","extfs":{"access_key_id":"AKIA","access_key_value":"SECRET"}}`,
Fields: []*schemapb.FieldSchema{
{FieldID: common.RowIDField, Name: common.RowIDFieldName, DataType: schemapb.DataType_Int64},
ordinaryField,
timestamptzField,
{FieldID: common.StartOfUserFieldID + 2, Name: common.MetaFieldName, DataType: schemapb.DataType_JSON, IsDynamic: true},
{FieldID: common.StartOfUserFieldID + 3, Name: common.NamespaceFieldName, DataType: schemapb.DataType_VarChar},
},
StructArrayFields: []*schemapb.StructArrayFieldSchema{{
FieldID: common.StartOfUserFieldID + 4,
Name: "profile",
Fields: []*schemapb.FieldSchema{{
FieldID: common.StartOfUserFieldID + 5,
Name: "profile[age]",
DataType: schemapb.DataType_Int64,
State: schemapb.FieldState_FieldDropping,
ExternalField: "external_age",
}},
}},
}
projected, err := projectDescribeCollectionSchema(source, true)
assert.NoError(t, err)
assert.Len(t, projected.GetFields(), 2)
assert.Same(t, ordinaryField, projected.GetFields()[0], "ordinary immutable fields should not be cloned")
assert.NotSame(t, timestamptzField, projected.GetFields()[1], "the field rewritten for the API must be cloned")
assert.Equal(t, schemapb.FieldState_FieldDropping, projected.GetFields()[0].GetState())
assert.Equal(t, "external_id", projected.GetFields()[0].GetExternalField())
assert.Equal(t, "age", projected.GetStructArrayFields()[0].GetFields()[0].GetName())
assert.Equal(t, schemapb.FieldState_FieldDropping, projected.GetStructArrayFields()[0].GetFields()[0].GetState())
assert.Equal(t, "external_age", projected.GetStructArrayFields()[0].GetFields()[0].GetExternalField())
resp := &milvuspb.DescribeCollectionResponse{Status: merr.Success(), Schema: projected}
assert.NoError(t, finalizeDescribeCollectionResponse(resp))
assert.IsType(t, &schemapb.ValueField_StringData{}, projected.GetFields()[1].GetDefaultValue().GetData())
assert.IsType(t, &schemapb.ValueField_TimestamptzData{}, timestamptzField.GetDefaultValue().GetData(),
"public response conversion must not mutate the cached canonical default")
assert.NotContains(t, projected.GetExternalSpec(), "AKIA")
assert.NotContains(t, projected.GetExternalSpec(), "SECRET")
assert.Equal(t, int32(7), projected.GetVersion())
assert.Equal(t, "db", projected.GetDbName())
}
func TestDescribeCollectionCachedAndRemoteProjectionEquivalent(t *testing.T) {
ctx := context.Background()
const (
database = "db"
collectionName = "collection"
collectionID = int64(100)
)
raw := &milvuspb.DescribeCollectionResponse{
Status: merr.Success(),
CollectionID: collectionID,
CollectionName: collectionName,
DbName: database,
DbId: 10,
CreatedTimestamp: 11,
CreatedUtcTimestamp: 12,
UpdateTimestamp: 13,
ShardsNum: 2,
NumPartitions: 3,
ConsistencyLevel: commonpb.ConsistencyLevel_Bounded,
VirtualChannelNames: []string{"v1"},
PhysicalChannelNames: []string{"p1"},
Aliases: []string{"alias"},
Properties: []*commonpb.KeyValuePair{{Key: "collection_property", Value: "value"}},
Schema: &schemapb.CollectionSchema{
Name: collectionName,
Description: "description",
DbName: database,
Version: 9,
EnableDynamicField: true,
EnableNamespace: true,
Properties: []*commonpb.KeyValuePair{{Key: common.TimezoneKey, Value: "UTC"}},
ExternalSource: "s3://bucket/path",
ExternalSpec: `{"format":"parquet","extfs":{"access_key_id":"AKIA","access_key_value":"SECRET"}}`,
Fields: []*schemapb.FieldSchema{
{FieldID: common.RowIDField, Name: common.RowIDFieldName, DataType: schemapb.DataType_Int64},
{FieldID: common.StartOfUserFieldID, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true, State: schemapb.FieldState_FieldDropping, ExternalField: "external_id"},
{FieldID: common.StartOfUserFieldID + 1, Name: "created_at", DataType: schemapb.DataType_Timestamptz, DefaultValue: &schemapb.ValueField{Data: &schemapb.ValueField_TimestamptzData{TimestamptzData: 1_700_000_000_000_000}}},
{FieldID: common.StartOfUserFieldID + 2, Name: common.MetaFieldName, DataType: schemapb.DataType_JSON, IsDynamic: true},
{FieldID: common.StartOfUserFieldID + 3, Name: common.NamespaceFieldName, DataType: schemapb.DataType_VarChar},
},
StructArrayFields: []*schemapb.StructArrayFieldSchema{{
FieldID: common.StartOfUserFieldID + 4,
Name: "profile",
Fields: []*schemapb.FieldSchema{{
FieldID: common.StartOfUserFieldID + 5, Name: "profile[age]", DataType: schemapb.DataType_Int64,
State: schemapb.FieldState_FieldDropping, ExternalField: "external_age",
}},
}},
},
}
mix := NewMixCoordMock()
mix.SetDescribeCollectionFunc(func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return proto.Clone(raw).(*milvuspb.DescribeCollectionResponse), nil
})
remoteTask := &describeCollectionTask{
Condition: NewTaskCondition(ctx),
DescribeCollectionRequest: &milvuspb.DescribeCollectionRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_DescribeCollection},
DbName: database, CollectionName: collectionName,
},
ctx: ctx, mixCoord: mix,
}
assert.NoError(t, remoteTask.PreExecute(ctx))
assert.NoError(t, remoteTask.Execute(ctx))
remoteResp := remoteTask.result
assert.NoError(t, finalizeDescribeCollectionResponse(remoteResp))
cacheSchema := proto.Clone(raw.GetSchema()).(*schemapb.CollectionSchema)
cache := NewMockCache(t)
cache.EXPECT().GetCollectionID(mock.Anything, database, collectionName).Return(collectionID, nil)
cache.EXPECT().GetCollectionInfo(mock.Anything, database, collectionName, collectionID).Return(&collectionInfo{
collID: collectionID,
dbID: raw.GetDbId(),
dbName: database,
schema: mustNewSchemaInfo(cacheSchema),
createdTimestamp: raw.GetCreatedTimestamp(),
createdUtcTimestamp: raw.GetCreatedUtcTimestamp(),
consistencyLevel: raw.GetConsistencyLevel(),
updateTimestamp: raw.GetUpdateTimestamp(),
vChannels: raw.GetVirtualChannelNames(),
pChannels: raw.GetPhysicalChannelNames(),
numPartitions: raw.GetNumPartitions(),
shardsNum: raw.GetShardsNum(),
aliases: raw.GetAliases(),
properties: raw.GetProperties(),
}, nil)
oldCache := globalMetaCache
globalMetaCache = cache
defer func() { globalMetaCache = oldCache }()
cachedResp, err := (&CachedProxyServiceProvider{Proxy: &Proxy{}}).DescribeCollection(ctx,
&milvuspb.DescribeCollectionRequest{DbName: database, CollectionName: collectionName})
assert.NoError(t, err)
assert.NoError(t, finalizeDescribeCollectionResponse(cachedResp))
assert.True(t, proto.Equal(remoteResp, cachedResp), "cached and remote responses differ:\nremote=%s\ncached=%s", remoteResp, cachedResp)
assert.IsType(t, &schemapb.ValueField_TimestamptzData{}, cacheSchema.GetFields()[2].GetDefaultValue().GetData(),
"cached provider must preserve the canonical TIMESTAMPTZ default")
}
func TestDescribeCollectionCachedAndRemoteNotFoundStatusEquivalent(t *testing.T) {
ctx := context.Background()
const (
database = "db"
collectionName = "missing"
)
notFound := merr.WrapErrCollectionNotFound(collectionName)
mix := NewMixCoordMock()
mix.SetDescribeCollectionFunc(func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{Status: merr.Status(notFound)}, nil
})
remoteTask := &describeCollectionTask{
Condition: NewTaskCondition(ctx),
DescribeCollectionRequest: &milvuspb.DescribeCollectionRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_DescribeCollection},
DbName: database, CollectionName: collectionName,
},
ctx: ctx, mixCoord: mix,
}
assert.NoError(t, remoteTask.PreExecute(ctx))
assert.NoError(t, remoteTask.Execute(ctx))
cache := NewMockCache(t)
cache.EXPECT().GetCollectionID(mock.Anything, database, collectionName).Return(int64(0), notFound)
oldCache := globalMetaCache
globalMetaCache = cache
defer func() { globalMetaCache = oldCache }()
cachedResp, err := (&CachedProxyServiceProvider{Proxy: &Proxy{}}).DescribeCollection(ctx,
&milvuspb.DescribeCollectionRequest{DbName: database, CollectionName: collectionName})
assert.NoError(t, err)
assert.True(t, proto.Equal(remoteTask.result.GetStatus(), cachedResp.GetStatus()))
assert.Equal(t, commonpb.ErrorCode_CollectionNotExists, cachedResp.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrCollectionNotFound), cachedResp.GetStatus().GetCode())
assert.Equal(t, "true", cachedResp.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey])
assert.False(t, cachedResp.GetStatus().GetRetriable())
}
func TestDescribeCollectionCachedAndRemoteDatabaseNotFoundStatusEquivalent(t *testing.T) {
ctx := context.Background()
const (
database = "missing_db"
collectionName = "collection"
)
notFound := merr.WrapErrDatabaseNotFound(database)
mix := NewMixCoordMock()
mix.SetDescribeCollectionFunc(func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{Status: merr.Status(notFound)}, nil
})
remoteTask := &describeCollectionTask{
Condition: NewTaskCondition(ctx),
DescribeCollectionRequest: &milvuspb.DescribeCollectionRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_DescribeCollection},
DbName: database, CollectionName: collectionName,
},
ctx: ctx, mixCoord: mix,
}
assert.NoError(t, remoteTask.PreExecute(ctx))
assert.NoError(t, remoteTask.Execute(ctx))
cache := NewMockCache(t)
cache.EXPECT().GetCollectionID(mock.Anything, database, collectionName).Return(int64(0), notFound)
oldCache := globalMetaCache
globalMetaCache = cache
defer func() { globalMetaCache = oldCache }()
cachedResp, err := (&CachedProxyServiceProvider{Proxy: &Proxy{}}).DescribeCollection(ctx,
&milvuspb.DescribeCollectionRequest{DbName: database, CollectionName: collectionName})
assert.NoError(t, err)
assert.True(t, proto.Equal(remoteTask.result.GetStatus(), cachedResp.GetStatus()))
assert.ErrorIs(t, merr.Error(cachedResp.GetStatus()), merr.ErrDatabaseNotFound)
// The deprecated ErrorCode enum cannot represent database-not-found; the
// typed Code below is the authoritative wire value.
assert.Equal(t, commonpb.ErrorCode_UnexpectedError, cachedResp.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrDatabaseNotFound), cachedResp.GetStatus().GetCode())
assert.Contains(t, cachedResp.GetStatus().GetReason(), "database not found")
assert.Equal(t, "true", cachedResp.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey])
assert.False(t, cachedResp.GetStatus().GetRetriable())
}
func TestInterceptorImpl_RecordsOutcomeFromErrorAndResponseStatus(t *testing.T) {
paramtable.Init()
node := &Proxy{}
node.UpdateStateCode(commonpb.StateCode_Healthy)
request := &milvuspb.DescribeCollectionRequest{DbName: "db", CollectionName: "collection"}
nodeID := paramtable.GetStringNodeID()
tests := []struct {
name string
onCall func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error)
expectStatus string
expectCause string
}{
{
name: "response input error",
onCall: func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{Status: merr.Status(merr.WrapErrParameterInvalidMsg("bad request"))}, nil
},
expectStatus: metrics.FailLabel,
expectCause: metrics.CauseUser,
},
{
name: "go system error",
onCall: func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return nil, merr.WrapErrServiceInternalMsg("internal failure")
},
expectStatus: metrics.FailLabel,
expectCause: metrics.CauseSystem,
},
{
name: "provider abandon",
onCall: func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return nil, withServiceProviderMetric(
merr.WrapErrServiceInternalMsg("enqueue failed"), metrics.AbandonLabel, metrics.CauseNA)
},
expectStatus: metrics.AbandonLabel,
expectCause: metrics.CauseNA,
},
{
name: "success",
onCall: func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{Status: merr.Success()}, nil
},
expectStatus: metrics.SuccessLabel,
expectCause: metrics.CauseNA,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
method := "DescribeCollectionMetric_" + tc.name
outcomeCounter := metrics.ProxyFunctionCall.WithLabelValues(
nodeID, method, tc.expectStatus, tc.expectCause, request.GetDbName(), request.GetCollectionName())
totalCounter := metrics.ProxyFunctionCall.WithLabelValues(
nodeID, method, metrics.TotalLabel, metrics.CauseNA, request.GetDbName(), request.GetCollectionName())
latency := metrics.ProxyReqLatency.WithLabelValues(nodeID, method)
beforeOutcome := testutil.ToFloat64(outcomeCounter)
beforeTotal := testutil.ToFloat64(totalCounter)
beforeLatency := proxyHistogramSampleCount(t, latency)
interceptor := &InterceptorImpl[*milvuspb.DescribeCollectionRequest, *milvuspb.DescribeCollectionResponse]{
proxy: node,
method: method,
onCall: tc.onCall,
onError: func(err error) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{Status: merr.Status(err)}, nil
},
}
resp, err := interceptor.Call(context.Background(), request)
assert.NoError(t, err)
assert.NotNil(t, resp)
assert.Equal(t, beforeOutcome+1, testutil.ToFloat64(outcomeCounter))
assert.Equal(t, beforeTotal+1, testutil.ToFloat64(totalCounter))
assert.Equal(t, beforeLatency+1, proxyHistogramSampleCount(t, latency))
})
}
}
+15 -84
View File
@@ -1742,15 +1742,7 @@ func (t *describeCollectionTask) PreExecute(ctx context.Context) error {
func (t *describeCollectionTask) Execute(ctx context.Context) error {
var err error
t.result = &milvuspb.DescribeCollectionResponse{
Status: merr.Success(),
Schema: &schemapb.CollectionSchema{
Name: "",
Description: "",
AutoID: false,
Fields: make([]*schemapb.FieldSchema, 0),
Functions: make([]*schemapb.FunctionSchema, 0),
StructArrayFields: make([]*schemapb.StructArrayFieldSchema, 0),
},
Status: merr.Success(),
CollectionID: 0,
VirtualChannelNames: nil,
PhysicalChannelNames: nil,
@@ -1764,32 +1756,19 @@ func (t *describeCollectionTask) Execute(ctx context.Context) error {
return err
}
if result.GetStatus().GetErrorCode() != commonpb.ErrorCode_Success {
t.result.Status = result.Status
// compatibility with PyMilvus existing implementation
err := merr.Error(t.result.GetStatus())
if errors.Is(err, merr.ErrCollectionNotFound) {
// nolint
t.result.Status.ErrorCode = commonpb.ErrorCode_UnexpectedError
// nolint
t.result.Status.Reason = fmt.Sprintf("can't find collection[database=%s][collection=%s]", t.GetDbName(), t.GetCollectionName())
t.result.Status.ExtraInfo = map[string]string{merr.InputErrorFlagKey: "true"}
}
if !merr.Ok(result.GetStatus()) {
t.result.Status = describeCollectionErrorStatus(
merr.Error(result.GetStatus()), t.GetDbName(), t.GetCollectionName())
return nil
}
if t.result.CollectionName == "" {
t.result.CollectionName = result.GetCollectionName()
}
t.result.Schema.Name = result.Schema.Name
t.result.Schema.Description = result.Schema.Description
t.result.Schema.AutoID = result.Schema.AutoID
t.result.Schema.EnableDynamicField = result.Schema.EnableDynamicField
t.result.Schema.ExternalSource = result.Schema.ExternalSource
// Pass spec through unredacted; the public proxy.DescribeCollection
// entry point applies RedactExternalSpec uniformly across cached and
// remote provider paths so internal-only callers of this task path
// (if any) still observe raw creds for FFI auth.
t.result.Schema.ExternalSpec = result.Schema.ExternalSpec
t.result.Schema.EnableNamespace = result.Schema.EnableNamespace
t.result.Schema, err = projectDescribeCollectionSchema(result.GetSchema(), false)
if err != nil {
return err
}
t.result.CollectionID = result.CollectionID
t.result.VirtualChannelNames = result.VirtualChannelNames
t.result.PhysicalChannelNames = result.PhysicalChannelNames
@@ -1799,61 +1778,13 @@ func (t *describeCollectionTask) Execute(ctx context.Context) error {
t.result.ConsistencyLevel = result.ConsistencyLevel
t.result.Aliases = result.Aliases
t.result.Properties = result.Properties
t.result.DbName = result.GetDbName()
if result.GetDbName() != "" {
t.result.DbName = result.GetDbName()
}
t.result.DbId = result.GetDbId()
t.result.NumPartitions = result.NumPartitions
t.result.UpdateTimestamp = result.UpdateTimestamp
t.result.UpdateTimestampStr = result.UpdateTimestampStr
copyFieldSchema := func(field *schemapb.FieldSchema) *schemapb.FieldSchema {
return &schemapb.FieldSchema{
FieldID: field.FieldID,
Name: field.Name,
IsPrimaryKey: field.IsPrimaryKey,
AutoID: field.AutoID,
Description: field.Description,
DataType: field.DataType,
TypeParams: field.TypeParams,
IndexParams: field.IndexParams,
IsDynamic: field.IsDynamic,
IsPartitionKey: field.IsPartitionKey,
IsClusteringKey: field.IsClusteringKey,
DefaultValue: field.DefaultValue,
ElementType: field.ElementType,
Nullable: field.Nullable,
IsFunctionOutput: field.IsFunctionOutput,
ExternalField: field.GetExternalField(),
}
}
for _, field := range result.Schema.Fields {
if field.IsDynamic || field.Name == common.NamespaceFieldName {
continue
}
if field.FieldID >= common.StartOfUserFieldID {
t.result.Schema.Fields = append(t.result.Schema.Fields, copyFieldSchema(field))
}
}
for i, structArrayField := range result.Schema.StructArrayFields {
t.result.Schema.StructArrayFields = append(t.result.Schema.StructArrayFields, &schemapb.StructArrayFieldSchema{
FieldID: structArrayField.FieldID,
Name: structArrayField.Name,
Description: structArrayField.Description,
Fields: make([]*schemapb.FieldSchema, 0, len(structArrayField.Fields)),
Nullable: structArrayField.Nullable,
})
for _, field := range structArrayField.Fields {
t.result.Schema.StructArrayFields[i].Fields = append(t.result.Schema.StructArrayFields[i].Fields, copyFieldSchema(field))
}
}
for _, function := range result.Schema.Functions {
t.result.Schema.Functions = append(t.result.Schema.Functions, proto.Clone(function).(*schemapb.FunctionSchema))
}
if err := restoreStructFieldNames(t.result.Schema); err != nil {
return merr.WrapErrParameterInvalidMsg("failed to restore struct field names: %v", err)
}
t.result.UpdateTimestampStr = strconv.FormatUint(result.UpdateTimestamp, 10)
return nil
}
+3
View File
@@ -149,6 +149,9 @@ func (ddt *dropDatabaseTask) Execute(ctx context.Context) error {
err = merr.CheckRPCCall(ddt.result, err)
if err == nil {
// Local best-effort cleanup on the issuing proxy; the authoritative
// eviction is the DropDatabase broadcast handled in
// InvalidateCollectionMetaCache.
globalMetaCache.RemoveDatabase(ctx, ddt.DbName)
}
return err
+9 -9
View File
@@ -272,7 +272,7 @@ func (s *DeleteRunnerSuite) SetupSuite() {
},
},
}
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
s.mockCache = NewMockCache(s.T())
}
@@ -356,7 +356,7 @@ func (s *DeleteRunnerSuite) TestInitSuccess() {
&schemapb.FieldSchema{FieldID: common.StartOfUserFieldID, Name: "pk", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
&schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 1, Name: "non_pk", DataType: schemapb.DataType_Int64},
)
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
dr := deleteRunner{
req: &milvuspb.DeleteRequest{
CollectionName: s.collectionName,
@@ -409,7 +409,7 @@ func (s *DeleteRunnerSuite) TestInitSuccess() {
},
},
}
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
s.mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(s.schema, nil).Once()
mockChMgr.EXPECT().getVChannels(mock.Anything).Return([]string{"vchan1"}, nil)
@@ -450,7 +450,7 @@ func (s *DeleteRunnerSuite) TestInitSuccess() {
},
},
}
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
s.mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(s.schema, nil).Once()
mockChMgr.EXPECT().getVChannels(mock.Anything).Return([]string{"vchan1"}, nil)
s.mockCache.EXPECT().GetPartitionID(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(int64(1000), nil)
@@ -496,7 +496,7 @@ func (s *DeleteRunnerSuite) TestInitSuccess() {
},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil).Once()
s.mockCache.EXPECT().GetPartitionID(mock.Anything, mock.Anything, mock.Anything, namespace).Return(int64(1000), nil)
mockChMgr.EXPECT().getVChannels(mock.Anything).Return([]string{"vchan1"}, nil)
@@ -538,7 +538,7 @@ func (s *DeleteRunnerSuite) TestInitFailure() {
},
},
}
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
s.mockCache.EXPECT().GetDatabaseInfo(mock.Anything, mock.Anything).Return(&databaseInfo{dbID: 0}, nil)
s.mockCache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(s.collectionID, nil)
s.mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(s.schema, nil)
@@ -668,7 +668,7 @@ func (s *DeleteRunnerSuite) TestInitFailure() {
},
},
}
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
s.mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).
Return(s.schema, nil)
@@ -702,7 +702,7 @@ func (s *DeleteRunnerSuite) TestInitFailure() {
},
},
}
s.schema = newSchemaInfo(schema)
s.schema = mustNewSchemaInfo(schema)
s.mockCache.EXPECT().GetDatabaseInfo(mock.Anything, mock.Anything).Return(&databaseInfo{dbID: 0}, nil)
s.mockCache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(s.collectionID, nil)
s.mockCache.EXPECT().GetCollectionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&collectionInfo{}, nil)
@@ -772,7 +772,7 @@ func TestDeleteRunner_Run(t *testing.T) {
},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
metaCache := NewMockCache(t)
metaCache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(collectionID, nil).Maybe()
+1 -1
View File
@@ -215,7 +215,7 @@ func TestCreateIndexTask_PreExecute(t *testing.T) {
mock.Anything, // context.Context
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(newTestSchema()), nil)
).Return(mustNewSchemaInfo(newTestSchema()), nil)
mockCache.On("GetCollectionInfo",
mock.Anything, // context.Context
mock.AnythingOfType("string"),
+8 -8
View File
@@ -92,7 +92,7 @@ func TestInsertTaskPreExecuteTextRequiresStorageV3(t *testing.T) {
dbName = "db"
collectionName = "text_collection"
)
schema := newSchemaInfo(newTextSchemaForStorageV3Test(collectionName))
schema := mustNewSchemaInfo(newTextSchemaForStorageV3Test(collectionName))
cache := NewMockCache(t)
cache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(int64(100), nil)
cache.EXPECT().GetCollectionInfo(mock.Anything, dbName, collectionName, int64(100)).Return(&collectionInfo{}, nil)
@@ -479,7 +479,7 @@ func TestInsertTask_KeepUserPK_WhenAllowInsertAutoIDTrue(t *testing.T) {
idAllocator: idAllocator,
}
info := newSchemaInfo(schema)
info := mustNewSchemaInfo(schema)
cache := NewMockCache(t)
collectionID := UniqueID(0)
cache.On("GetCollectionID",
@@ -616,7 +616,7 @@ func TestInsertTask_Function(t *testing.T) {
idAllocator: idAllocator,
}
info := newSchemaInfo(schema)
info := mustNewSchemaInfo(schema)
cache := NewMockCache(t)
collectionID := UniqueID(0)
cache.On("GetCollectionID",
@@ -674,7 +674,7 @@ func TestInsertTaskForSchemaMismatch(t *testing.T) {
mockCache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(0, nil)
mockCache.EXPECT().GetCollectionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&collectionInfo{
updateTimestamp: 100,
schema: newSchemaInfo(&schemapb.CollectionSchema{
schema: mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "fooooo",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
@@ -731,9 +731,9 @@ func TestInsertTask_Namespace(t *testing.T) {
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Unset()
cache.EXPECT().GetPartitionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset()
cache.EXPECT().GetCollectionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&collectionInfo{
schema: newSchemaInfo(schemaWithNamespaceEnabled),
schema: mustNewSchemaInfo(schemaWithNamespaceEnabled),
}, nil).Maybe()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(newSchemaInfo(schemaWithNamespaceEnabled), nil).Maybe()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(mustNewSchemaInfo(schemaWithNamespaceEnabled), nil).Maybe()
cache.EXPECT().GetPartitionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&partitionInfo{
name: "p1",
partitionID: 10,
@@ -778,9 +778,9 @@ func TestInsertTask_Namespace(t *testing.T) {
cache.EXPECT().GetCollectionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Unset()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Unset()
cache.EXPECT().GetCollectionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&collectionInfo{
schema: newSchemaInfo(schemaWithNamespaceDisabled),
schema: mustNewSchemaInfo(schemaWithNamespaceDisabled),
}, nil).Maybe()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(newSchemaInfo(schemaWithNamespaceDisabled), nil).Maybe()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(mustNewSchemaInfo(schemaWithNamespaceDisabled), nil).Maybe()
cache.EXPECT().GetPartitionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&partitionInfo{
name: "p1",
partitionID: 10,
+3 -3
View File
@@ -71,7 +71,7 @@ func TestQueryTask_PlanNamespace_AfterPreExecute(t *testing.T) {
},
EnableNamespace: true,
}
return newSchemaInfo(schema), nil
return mustNewSchemaInfo(schema), nil
}).Build()
// Capture plan to verify namespace by mocking plan creation inside createPlan
@@ -94,7 +94,7 @@ func TestQueryTask_PlanNamespace_AfterPreExecute(t *testing.T) {
ns := "ns-1"
task.request.Namespace = &ns
// Pre-set schema for namespace check before cache fetch inside PreExecute
task.schema = newSchemaInfo(&schemapb.CollectionSchema{
task.schema = mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_collection",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -127,7 +127,7 @@ func TestQueryTask_NamespaceSetsPartitionIDs(t *testing.T) {
mockey.Mock((*MetaCache).GetCollectionID).Return(int64(1001), nil).Build()
mockey.Mock((*MetaCache).GetCollectionInfo).Return(&collectionInfo{updateTimestamp: 12345, consistencyLevel: commonpb.ConsistencyLevel_Strong}, nil).Build()
mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(schema), nil).Build()
mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(schema), nil).Build()
mockey.Mock((*MetaCache).GetPartitionsIndex).Return(partitionNames, nil).Build()
mockey.Mock((*MetaCache).GetPartitions).Return(partitionIDs, nil).Build()
mockey.Mock(validatePartitionTag).Return(nil).Build()
+3 -3
View File
@@ -1690,7 +1690,7 @@ func Test_createCntPlan(t *testing.T) {
func Test_queryTask_createPlan(t *testing.T) {
collSchema := newTestSchema()
t.Run("query without expression", func(t *testing.T) {
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
tsk := &queryTask{
request: &milvuspb.QueryRequest{
OutputFields: []string{"Int64"},
@@ -1702,7 +1702,7 @@ func Test_queryTask_createPlan(t *testing.T) {
})
t.Run("invalid expression", func(t *testing.T) {
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
tsk := &queryTask{
schema: schema,
@@ -1716,7 +1716,7 @@ func Test_queryTask_createPlan(t *testing.T) {
})
t.Run("invalid output fields", func(t *testing.T) {
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
tsk := &queryTask{
schema: schema,
+3 -3
View File
@@ -37,7 +37,7 @@ func TestSearchTask_PlanNamespace_AfterPreExecute(t *testing.T) {
},
EnableNamespace: true,
}
return newSchemaInfo(schema), nil
return mustNewSchemaInfo(schema), nil
}).Build()
// Patch checkNq to bypass placeholder parsing
@@ -85,7 +85,7 @@ func TestSearchTask_NamespaceSetsPartitionIDs(t *testing.T) {
mockey.Mock((*MetaCache).GetCollectionID).Return(int64(1001), nil).Build()
mockey.Mock((*MetaCache).GetCollectionInfo).Return(&collectionInfo{updateTimestamp: 12345, consistencyLevel: commonpb.ConsistencyLevel_Strong}, nil).Build()
mockey.Mock((*MetaCache).GetCollectionSchema).Return(newSchemaInfo(schema), nil).Build()
mockey.Mock((*MetaCache).GetCollectionSchema).Return(mustNewSchemaInfo(schema), nil).Build()
mockey.Mock((*MetaCache).GetPartitionsIndex).Return(partitionNames, nil).Build()
mockey.Mock((*MetaCache).GetPartitions).Return(partitionIDs, nil).Build()
mockey.Mock(isIgnoreGrowing).Return(false, nil).Build()
@@ -133,7 +133,7 @@ func TestSearchTask_RequeryPlanNamespace(t *testing.T) {
tsk := &searchTask{
Condition: NewTaskCondition(context.Background()),
ctx: context.Background(),
schema: newSchemaInfo(schema),
schema: mustNewSchemaInfo(schema),
request: &milvuspb.SearchRequest{CollectionName: "test_collection"},
SearchRequest: &internalpb.SearchRequest{Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_Search}},
node: &Proxy{},
+31 -31
View File
@@ -97,7 +97,7 @@ func TestSearchTaskPreExecuteTextRequiresStorageV3(t *testing.T) {
collectionName = "text_collection"
collectionID = int64(100)
)
schema := newSchemaInfo(newTextSchemaForStorageV3Test(collectionName))
schema := mustNewSchemaInfo(newTextSchemaForStorageV3Test(collectionName))
cache := NewMockCache(t)
cache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(collectionID, nil)
cache.EXPECT().GetCollectionSchema(mock.Anything, dbName, collectionName).Return(schema, nil)
@@ -214,7 +214,7 @@ func TestSearchTask_PostExecute(t *testing.T) {
},
Nq: 1,
},
schema: newSchemaInfo(collSchema),
schema: mustNewSchemaInfo(collSchema),
request: &milvuspb.SearchRequest{
CollectionName: collName,
},
@@ -597,12 +597,12 @@ func TestSearchTask_NamespacePartitionModeSkipsRequery(t *testing.T) {
{FieldID: 101, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: common.DimKey, Value: "4"}}},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
t.Run("namespace disabled does not skip", func(t *testing.T) {
disabledSchema := proto.Clone(schema).(*schemapb.CollectionSchema)
disabledSchema.EnableNamespace = false
task := &searchTask{schema: newSchemaInfo(disabledSchema)}
task := &searchTask{schema: mustNewSchemaInfo(disabledSchema)}
require.False(t, task.skipRequeryByNamespacePartitionMode())
})
@@ -1477,7 +1477,7 @@ func TestSearchTask_WithFunctions(t *testing.T) {
collectionID := UniqueID(1000)
cache := NewMockCache(t)
info := newSchemaInfo(schema)
info := mustNewSchemaInfo(schema)
cache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(collectionID, nil).Maybe()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(info, nil).Maybe()
cache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"_default": UniqueID(1)}, nil).Maybe()
@@ -4522,7 +4522,7 @@ func TestSearchTask_Requery(t *testing.T) {
collectionID := UniqueID(0)
cache := NewMockCache(t)
collSchema := constructCollectionSchema(pkField, vecField, dim, collection)
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
cache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(collectionID, nil).Maybe()
cache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schema, nil).Maybe()
cache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"_default": UniqueID(1)}, nil).Maybe()
@@ -4541,7 +4541,7 @@ func TestSearchTask_Requery(t *testing.T) {
t.Run("Test normal", func(t *testing.T) {
collSchema := constructCollectionSchema(pkField, vecField, dim, collection)
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
qn := mocks.NewMockQueryNodeClient(t)
qn.EXPECT().Query(mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, request *querypb.QueryRequest, option ...grpc.CallOption) (*internalpb.RetrieveResults, error) {
@@ -4641,7 +4641,7 @@ func TestSearchTask_Requery(t *testing.T) {
t.Run("Test no primary key", func(t *testing.T) {
collSchema := &schemapb.CollectionSchema{}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
node := mocks.NewMockProxy(t)
@@ -4666,7 +4666,7 @@ func TestSearchTask_Requery(t *testing.T) {
t.Run("Test requery failed", func(t *testing.T) {
collSchema := constructCollectionSchema(pkField, vecField, dim, collection)
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
qn := mocks.NewMockQueryNodeClient(t)
qn.EXPECT().Query(mock.Anything, mock.Anything).
Return(nil, errors.New("mock err 1"))
@@ -4703,7 +4703,7 @@ func TestSearchTask_Requery(t *testing.T) {
t.Run("Test postExecute with requery failed", func(t *testing.T) {
collSchema := constructCollectionSchema(pkField, vecField, dim, collection)
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
qn := mocks.NewMockQueryNodeClient(t)
qn.EXPECT().Query(mock.Anything, mock.Anything).
Return(nil, errors.New("mock err 1"))
@@ -5125,7 +5125,7 @@ func (s *MaterializedViewTestSuite) TestMvNotEnabledWithNoPartitionKey() {
task.enableMaterializedView = false
schema := constructCollectionSchemaByDataType(s.colName, s.fieldName2Types, testInt64Field, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
err := task.PreExecute(s.ctx)
@@ -5140,7 +5140,7 @@ func (s *MaterializedViewTestSuite) TestMvNotEnabledWithPartitionKey() {
task.enableMaterializedView = false
task.request.Dsl = testInt64Field + " == 1"
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testInt64Field, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
s.mockMetaCache.EXPECT().GetPartitionsIndex(mock.Anything, mock.Anything, mock.Anything).Return([]string{"partition_1", "partition_2"}, nil)
s.mockMetaCache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"partition_1": 1, "partition_2": 2}, nil)
@@ -5156,7 +5156,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledNoPartitionKey() {
task := s.getSearchTask()
task.enableMaterializedView = true
schema := constructCollectionSchemaByDataType(s.colName, s.fieldName2Types, testInt64Field, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
err := task.PreExecute(s.ctx)
@@ -5171,7 +5171,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledPartitionKeyOnInt64() {
task.enableMaterializedView = true
task.request.Dsl = testInt64Field + " == 1"
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testInt64Field, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
s.mockMetaCache.EXPECT().GetPartitionsIndex(mock.Anything, mock.Anything, mock.Anything).Return([]string{"partition_1", "partition_2"}, nil)
s.mockMetaCache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"partition_1": 1, "partition_2": 2}, nil)
@@ -5188,7 +5188,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledPartitionKeyOnVarChar() {
task.enableMaterializedView = true
task.request.Dsl = testVarCharField + " == \"a\""
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testVarCharField, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
s.mockMetaCache.EXPECT().GetPartitionsIndex(mock.Anything, mock.Anything, mock.Anything).Return([]string{"partition_1", "partition_2"}, nil)
s.mockMetaCache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"partition_1": 1, "partition_2": 2}, nil)
@@ -5208,7 +5208,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledPartitionKeyOnVarCharWithIsolat
task.request.Dsl = testVarCharField + " == \"a\""
task.IsAdvanced = isAdvanced
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testVarCharField, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
s.mockMetaCache.EXPECT().GetPartitionsIndex(mock.Anything, mock.Anything, mock.Anything).Return([]string{"partition_1", "partition_2"}, nil)
s.mockMetaCache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"partition_1": 1, "partition_2": 2}, nil)
@@ -5228,7 +5228,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledPartitionKeyOnVarCharWithIsolat
task.IsAdvanced = isAdvanced
task.request.Dsl = testVarCharField + " in [\"a\", \"b\", \"c\"]"
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testVarCharField, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
s.ErrorContains(task.PreExecute(s.ctx), "partition key isolation does not support IN")
}
@@ -5236,7 +5236,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledPartitionKeyOnVarCharWithIsolat
func (s *MaterializedViewTestSuite) TestHybridSearchPartitionKeyIsolationWithoutMaterializedView() {
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testVarCharField, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil).Maybe()
s.mockMetaCache.EXPECT().GetPartitionsIndex(mock.Anything, mock.Anything, mock.Anything).Return([]string{"partition_1", "partition_2"}, nil).Maybe()
s.mockMetaCache.EXPECT().GetPartitions(mock.Anything, mock.Anything, mock.Anything).Return(map[string]int64{"partition_1": 1, "partition_2": 2}, nil).Maybe()
@@ -5276,7 +5276,7 @@ func (s *MaterializedViewTestSuite) TestMvEnabledPartitionKeyOnVarCharWithIsolat
task.IsAdvanced = isAdvanced
task.request.Dsl = testVarCharField + " == \"a\" || " + testVarCharField + " == \"b\" || " + testVarCharField + " == \"c\""
schema := ConstructCollectionSchemaWithPartitionKey(s.colName, s.fieldName2Types, testInt64Field, testVarCharField, false)
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
s.mockMetaCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(schemaInfo, nil)
s.ErrorContains(task.PreExecute(s.ctx), "partition key isolation does not support IN")
}
@@ -5371,7 +5371,7 @@ func TestSearchTask_InitSearchRequestWithStructArrayFields(t *testing.T) {
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
tests := []struct {
name string
@@ -5470,7 +5470,7 @@ func TestSearchTask_FunctionChainRerankMeta(t *testing.T) {
{FieldID: 102, Name: "vec", DataType: schemapb.DataType_FloatVector, TypeParams: []*commonpb.KeyValuePair{{Key: common.DimKey, Value: "128"}}},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
newRequest := func() *milvuspb.SearchRequest {
return &milvuspb.SearchRequest{
@@ -5711,7 +5711,7 @@ func TestSearchTask_AddHighlightTask(t *testing.T) {
},
}
info := newSchemaInfo(schema)
info := mustNewSchemaInfo(schema)
placeholder := &commonpb.PlaceholderGroup{
Placeholders: []*commonpb.PlaceholderValue{{
@@ -5764,7 +5764,7 @@ func TestSearchTask_AddHighlightTask(t *testing.T) {
})
task := &searchTask{
schema: newSchemaInfo(multiAnalyzerSchema),
schema: mustNewSchemaInfo(multiAnalyzerSchema),
}
highlighter := &commonpb.Highlighter{
@@ -5809,7 +5809,7 @@ func TestSearchTask_AddHighlightTask(t *testing.T) {
})
task := &searchTask{
schema: newSchemaInfo(multiAnalyzerSchema),
schema: mustNewSchemaInfo(multiAnalyzerSchema),
}
highlighter := &commonpb.Highlighter{
@@ -5893,7 +5893,7 @@ func TestSearchTask_AddHighlightTask(t *testing.T) {
},
}
info := newSchemaInfo(schemaWithoutBM25)
info := mustNewSchemaInfo(schemaWithoutBM25)
task := &searchTask{
schema: info,
}
@@ -6320,7 +6320,7 @@ func TestSearchTask_InitSearchRequestWithHighlighter(t *testing.T) {
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
t.Run("highlighter adds RequiredFieldIDs to OutputFieldsId", func(t *testing.T) {
task := &searchTask{
@@ -6584,7 +6584,7 @@ func TestSearchTask_ArrayOfVectorGroupBy(t *testing.T) {
},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
makePlaceholderGroup := func(phType commonpb.PlaceholderType) []byte {
phg := &commonpb.PlaceholderGroup{
@@ -6717,7 +6717,7 @@ func TestSearchTask_ArrayOfVectorSimpleSearch(t *testing.T) {
},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
makePlaceholderGroup := func(phType commonpb.PlaceholderType) []byte {
phg := &commonpb.PlaceholderGroup{
@@ -6882,7 +6882,7 @@ func TestSearchTask_ArrayOfVectorHybridSearch(t *testing.T) {
},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
makePlaceholderGroup := func(phType commonpb.PlaceholderType) []byte {
phg := &commonpb.PlaceholderGroup{
@@ -7179,7 +7179,7 @@ func TestSearchTask_StructHybridElementScopeValidation(t *testing.T) {
},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
makePlaceholderGroup := func(phType commonpb.PlaceholderType) []byte {
phg := &commonpb.PlaceholderGroup{
@@ -7413,7 +7413,7 @@ func TestSearchTask_SearchRequeryPolicy(t *testing.T) {
{FieldID: 102, Name: "title", DataType: schemapb.DataType_VarChar, TypeParams: []*commonpb.KeyValuePair{{Key: common.MaxLengthKey, Value: "256"}}},
},
}
schemaInfo := newSchemaInfo(schema)
schemaInfo := mustNewSchemaInfo(schema)
buildTask := func(outputFields []string) *searchTask {
return &searchTask{
+60 -17
View File
@@ -708,7 +708,7 @@ func TestTranslateOutputFields(t *testing.T) {
},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
// Test empty output fields
outputFields, userOutputFields, userDynamicFields, _, requestedPK, err = translateOutputFields([]string{}, schema, false)
@@ -843,7 +843,7 @@ func TestTranslateOutputFields(t *testing.T) {
{Name: common.MetaFieldName, FieldID: 102, DataType: schemapb.DataType_JSON, IsDynamic: true},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
outputFields, userOutputFields, userDynamicFields, _, requestedPK, err = translateOutputFields([]string{"A", idFieldName}, schema, true)
assert.NoError(t, err)
@@ -919,7 +919,7 @@ func TestTranslateOutputFields_StructArrayField(t *testing.T) {
},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
// Test struct array field
outputFields, userOutputFields, userDynamicFields, _, requestedPK, err = translateOutputFields([]string{"sub_vector_field"}, schema, false)
@@ -2251,7 +2251,7 @@ func TestHasCollectionTask(t *testing.T) {
task.CollectionName = collectionName
// invalidate collection cache, trigger rootcoord rpc
globalMetaCache.RemoveCollection(ctx, dbName, collectionName, 0)
globalMetaCache.RemoveCollection(ctx, dbName, collectionName)
// rc return collection not found error
mixc.describeCollectionFunc = func(ctx context.Context, request *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
@@ -2326,7 +2326,9 @@ func TestDescribeCollectionTask(t *testing.T) {
assert.NoError(t, err)
err = task.Execute(ctx)
assert.NoError(t, err)
assert.Equal(t, commonpb.ErrorCode_UnexpectedError, task.result.GetStatus().GetErrorCode())
assert.Equal(t, commonpb.ErrorCode_CollectionNotExists, task.result.GetStatus().GetErrorCode())
assert.Equal(t, merr.Code(merr.ErrCollectionNotFound), task.result.GetStatus().GetCode())
assert.Equal(t, "true", task.result.GetStatus().GetExtraInfo()[merr.InputErrorFlagKey])
}
func TestDescribeCollectionTask_ShardsNum1(t *testing.T) {
@@ -2537,6 +2539,47 @@ func TestDescribeCollectionTask_FilterNamespaceField(t *testing.T) {
assert.Equal(t, collectionName, task.result.GetCollectionName())
}
func TestDescribeCollectionTask_FillsNameFromResultWhenQueriedByID(t *testing.T) {
ctx := context.Background()
mix := NewMixCoordMock()
const (
collectionName = "collection_from_result"
collectionID = int64(449574)
dbName = "db_from_result"
dbID = int64(3)
)
mix.SetDescribeCollectionFunc(func(context.Context, *milvuspb.DescribeCollectionRequest) (*milvuspb.DescribeCollectionResponse, error) {
return &milvuspb.DescribeCollectionResponse{
Status: merr.Success(),
CollectionName: collectionName,
CollectionID: collectionID,
DbName: dbName,
DbId: dbID,
Schema: &schemapb.CollectionSchema{
Name: collectionName,
},
}, nil
})
task := &describeCollectionTask{
Condition: NewTaskCondition(ctx),
DescribeCollectionRequest: &milvuspb.DescribeCollectionRequest{
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_DescribeCollection},
CollectionID: collectionID,
},
ctx: ctx,
mixCoord: mix,
}
assert.NoError(t, task.PreExecute(ctx))
assert.NoError(t, task.Execute(ctx))
assert.Equal(t, collectionName, task.result.GetCollectionName())
assert.Equal(t, collectionID, task.result.GetCollectionID())
assert.Equal(t, dbName, task.result.GetDbName())
assert.Equal(t, dbID, task.result.GetDbId())
}
// Security regression: DescribeCollection must redact credentials embedded in
// ExternalSpec.extfs (access_key_id / access_key_value / ssl_ca_cert) before
// returning to the client. Any caller with Describe privilege would otherwise
@@ -2586,9 +2629,9 @@ func TestDescribeCollectionTask_RedactsExternalSpecCredentials(t *testing.T) {
assert.NoError(t, task.PreExecute(ctx))
assert.NoError(t, task.Execute(ctx))
// describeCollectionTask now passes ExternalSpec through unredacted —
// redaction lives at the public proxy.DescribeCollection edge so both
// cached and remote provider paths converge through one sanitizer.
// describeCollectionTask passes ExternalSpec through unredacted — the
// DescribeCollection interceptor response hook sanitizes both cached and
// remote provider results before metrics and API return.
// The task-level result must still carry raw creds so that the
// post-task wrapper can choose to redact (user-facing) or not
// (internal callers, if added later, that need raw spec for FFI).
@@ -2662,7 +2705,7 @@ func TestCreatePartitionTask(t *testing.T) {
rc := mocks.NewMockMixCoordClient(t)
mockCache := NewMockCache(t)
mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(newSchemaInfo(&schemapb.CollectionSchema{
mockCache.EXPECT().GetCollectionSchema(mock.Anything, mock.Anything, mock.Anything).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{
EnableDynamicField: false,
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "ID", DataType: schemapb.DataType_Int64},
@@ -2746,7 +2789,7 @@ func TestDropPartitionTask(t *testing.T) {
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{}), nil)
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{}), nil)
globalMetaCache = mockCache
task := &dropPartitionTask{
@@ -2797,7 +2840,7 @@ func TestDropPartitionTask(t *testing.T) {
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{}), nil)
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{}), nil)
globalMetaCache = mockCache
task.PartitionName = "partition1"
err = task.PreExecute(ctx)
@@ -2824,7 +2867,7 @@ func TestDropPartitionTask(t *testing.T) {
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{}), nil)
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{}), nil)
globalMetaCache = mockCache
err = task.PreExecute(ctx)
assert.NoError(t, err)
@@ -2850,7 +2893,7 @@ func TestDropPartitionTask(t *testing.T) {
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{}), nil)
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{}), nil)
globalMetaCache = mockCache
err = task.PreExecute(ctx)
assert.Error(t, err)
@@ -3493,7 +3536,7 @@ func Test_createIndexTask_getIndexedFieldAndFunction(t *testing.T) {
mock.Anything, // context.Context
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
idField,
vectorField,
@@ -3526,7 +3569,7 @@ func Test_createIndexTask_getIndexedFieldAndFunction(t *testing.T) {
mock.Anything, // context.Context
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
idField,
otherField,
@@ -3675,7 +3718,7 @@ func Test_createIndexTask_PreExecute(t *testing.T) {
mock.Anything, // context.Context
mock.AnythingOfType("string"),
mock.AnythingOfType("string"),
).Return(newSchemaInfo(&schemapb.CollectionSchema{
).Return(mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
{
FieldID: 100,
@@ -4070,7 +4113,7 @@ func TestLoadCollectionTaskExecuteTextRequiresStorageV3(t *testing.T) {
collectionName = "text_collection"
collectionID = int64(100)
)
schema := newSchemaInfo(newTextSchemaForStorageV3Test(collectionName))
schema := mustNewSchemaInfo(newTextSchemaForStorageV3Test(collectionName))
cache := NewMockCache(t)
cache.EXPECT().GetCollectionID(mock.Anything, dbName, collectionName).Return(collectionID, nil)
cache.EXPECT().GetCollectionSchema(mock.Anything, dbName, collectionName).Return(schema, nil)
+28 -28
View File
@@ -77,16 +77,16 @@ func TestUpsertTask_CheckAligned(t *testing.T) {
// checkFieldsDataBySchema was already checked by TestUpsertTask_checkFieldsDataBySchema
boolFieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Bool}
int8FieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Int8}
int16FieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Int16}
int32FieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Int32}
int64FieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Int64}
floatFieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Float}
doubleFieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_Double}
floatVectorFieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_FloatVector}
binaryVectorFieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_BinaryVector}
varCharFieldSchema := &schemapb.FieldSchema{DataType: schemapb.DataType_VarChar}
boolFieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID, Name: "Bool", DataType: schemapb.DataType_Bool}
int8FieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 1, Name: "Int8", DataType: schemapb.DataType_Int8}
int16FieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 2, Name: "Int16", DataType: schemapb.DataType_Int16}
int32FieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 3, Name: "Int32", DataType: schemapb.DataType_Int32}
int64FieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 4, Name: "Int64", DataType: schemapb.DataType_Int64}
floatFieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 5, Name: "Float", DataType: schemapb.DataType_Float}
doubleFieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 6, Name: "Double", DataType: schemapb.DataType_Double}
floatVectorFieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 7, Name: "FloatVector", DataType: schemapb.DataType_FloatVector}
binaryVectorFieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 8, Name: "BinaryVector", DataType: schemapb.DataType_BinaryVector}
varCharFieldSchema := &schemapb.FieldSchema{FieldID: common.StartOfUserFieldID + 9, Name: "VarChar", DataType: schemapb.DataType_VarChar}
numRows := 20
dim := 128
@@ -107,7 +107,7 @@ func TestUpsertTask_CheckAligned(t *testing.T) {
varCharFieldSchema,
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
case2 := upsertTask{
req: &milvuspb.UpsertRequest{
NumRows: uint32(numRows),
@@ -429,7 +429,7 @@ func TestUpsertTask_Function(t *testing.T) {
},
}
info := newSchemaInfo(schema)
info := mustNewSchemaInfo(schema)
collectionID := UniqueID(0)
cache := NewMockCache(t)
globalMetaCache = cache
@@ -504,7 +504,7 @@ func TestUpsertTaskForSchemaMismatch(t *testing.T) {
mockCache.EXPECT().GetCollectionID(mock.Anything, mock.Anything, mock.Anything).Return(0, nil)
mockCache.EXPECT().GetCollectionInfo(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&collectionInfo{
updateTimestamp: 100,
schema: newSchemaInfo(&schemapb.CollectionSchema{
schema: mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "col-0",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", DataType: schemapb.DataType_Int64, IsPrimaryKey: true},
@@ -607,7 +607,7 @@ func createTestSchema() *schemaInfo {
},
},
}
return newSchemaInfo(schema)
return mustNewSchemaInfo(schema)
}
func TestRetrieveByPKs_Success(t *testing.T) {
@@ -1072,7 +1072,7 @@ func TestUpdateTask_PreExecute_QueryPreExecuteError(t *testing.T) {
func TestUpsertTask_queryPreExecute_MixLogic(t *testing.T) {
// Schema for the test collection
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_merge_collection",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -1165,7 +1165,7 @@ func TestUpsertTask_queryPreExecute_MixLogic(t *testing.T) {
func TestUpsertTask_queryPreExecute_PureInsert(t *testing.T) {
// Schema for the test collection
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_merge_collection",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -1254,7 +1254,7 @@ func TestUpsertTask_queryPreExecute_PureInsert(t *testing.T) {
func TestUpsertTask_queryPreExecute_PureUpdate(t *testing.T) {
// Schema for the test collection
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_merge_collection",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -1341,7 +1341,7 @@ func TestUpsertTask_queryPreExecute_PureUpdate(t *testing.T) {
}
func TestUpsertTask_queryPreExecute_StructWholeReplace(t *testing.T) {
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_struct_partial_update",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -2196,7 +2196,7 @@ func TestUpsertTask_queryPreExecute_EmptyDataArray(t *testing.T) {
t.Run("scalar field with empty data array nullable field", func(t *testing.T) {
// Schema with nullable scalar field c
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_empty_data_array",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "a", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -2314,7 +2314,7 @@ func TestUpsertTask_queryPreExecute_EmptyDataArray(t *testing.T) {
t.Run("scalar field with empty data array - non-nullable field", func(t *testing.T) {
// Schema with non-nullable scalar field c
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_empty_data_array_non_nullable",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "a", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -2448,7 +2448,7 @@ func TestInsertPreExecute_FilterBM25AndMinHashOutputFields(t *testing.T) {
m := mockey.Mock(common.AllocAutoID).Return(int64(1000), int64(1000+numRows), nil).Build()
defer m.UnPatch()
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_filter_bm25_minhash",
AutoID: true,
Fields: []*schemapb.FieldSchema{
@@ -2533,7 +2533,7 @@ func TestInsertPreExecute_FilterBM25AndMinHashOutputFields(t *testing.T) {
defer m.UnPatch()
// Schema with a text embedding function (non-BM25/MinHash)
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_preserve_embedding",
AutoID: true,
Fields: []*schemapb.FieldSchema{
@@ -2599,7 +2599,7 @@ func TestInsertPreExecute_FilterBM25AndMinHashOutputFields(t *testing.T) {
m := mockey.Mock(common.AllocAutoID).Return(int64(1000), int64(1000+numRows), nil).Build()
defer m.UnPatch()
noFuncSchema := newSchemaInfo(&schemapb.CollectionSchema{
noFuncSchema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_no_func",
AutoID: true,
Fields: []*schemapb.FieldSchema{
@@ -2654,7 +2654,7 @@ func TestInsertPreExecute_FilterBM25AndMinHashOutputFields(t *testing.T) {
func TestUpsertTask_queryPreExecute_NullableFields(t *testing.T) {
dim := int64(4)
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_nullable_vec",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -2938,7 +2938,7 @@ func TestUpsertTask_GenNullableFieldData(t *testing.T) {
func TestUpsertTask_queryPreExecute_DefaultValueWithValidData(t *testing.T) {
// Schema with a non-nullable field that has DefaultValue
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_default_value_upsert",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -3027,7 +3027,7 @@ func TestUpsertTask_queryPreExecute_DefaultValueWithValidData(t *testing.T) {
func TestUpsertTask_queryPreExecute_DefaultValueError(t *testing.T) {
// Schema with a non-nullable field that has DefaultValue
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_default_value_error",
Fields: []*schemapb.FieldSchema{
{FieldID: 100, Name: "id", IsPrimaryKey: true, DataType: schemapb.DataType_Int64},
@@ -3107,7 +3107,7 @@ func TestUpsertTask_queryPreExecute_DefaultValueError(t *testing.T) {
func TestUpsertTask_queryPreExecute_DynamicFieldValidData(t *testing.T) {
// Schema with dynamic field enabled, simulating a collection with id + value + $meta
schema := newSchemaInfo(&schemapb.CollectionSchema{
schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_dynamic_validdata",
EnableDynamicField: true,
Fields: []*schemapb.FieldSchema{
@@ -3305,7 +3305,7 @@ func TestUpsertTask_queryPreExecute_DynamicFieldValidData(t *testing.T) {
// After upgrading to 2.6, existing collections retain this schema.
// queryPreExecute must NOT unconditionally fill ValidData for $meta,
// because CheckValidData expects len(ValidData)==0 for non-nullable fields.
v25Schema := newSchemaInfo(&schemapb.CollectionSchema{
v25Schema := mustNewSchemaInfo(&schemapb.CollectionSchema{
Name: "test_v25_compat",
EnableDynamicField: true,
Fields: []*schemapb.FieldSchema{
+3 -3
View File
@@ -862,7 +862,7 @@ func TestValidateMultipleVectorFields(t *testing.T) {
func TestFillFieldIDBySchema(t *testing.T) {
t.Run("column count mismatch", func(t *testing.T) {
collSchema := &schemapb.CollectionSchema{}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
columns := []*schemapb.FieldData{
{
FieldName: "TestFillFieldIDBySchema",
@@ -882,7 +882,7 @@ func TestFillFieldIDBySchema(t *testing.T) {
},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
columns := []*schemapb.FieldData{
{
FieldName: "TestFillFieldIDBySchema",
@@ -907,7 +907,7 @@ func TestFillFieldIDBySchema(t *testing.T) {
},
},
}
schema := newSchemaInfo(collSchema)
schema := mustNewSchemaInfo(collSchema)
columns := []*schemapb.FieldData{
{
FieldName: "FieldB",
+2 -2
View File
@@ -151,7 +151,7 @@ func assertPlaceholderVectorsMatchFloat32(
}
func fp16BF16SchemaInfo(dataType schemapb.DataType, dim int64) *schemaInfo {
return newSchemaInfo(&schemapb.CollectionSchema{
return mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
{
FieldID: 100,
@@ -296,7 +296,7 @@ func TestConvertPlaceholderGroupOverflowToFloat16Infinity(t *testing.T) {
func TestNormalizeFp32ToFp16Bf16VectorFieldData(t *testing.T) {
newTestSchema := func(dataType schemapb.DataType) *schemaInfo {
return newSchemaInfo(&schemapb.CollectionSchema{
return mustNewSchemaInfo(&schemapb.CollectionSchema{
Fields: []*schemapb.FieldSchema{
{
FieldID: 100,
-207
View File
@@ -1,207 +0,0 @@
// Licensed to the LF AI & Data foundation under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package proxy
import "sync"
// EntryState represents the state of a cache entry.
type EntryState int
const (
EntryStateActive EntryState = iota
EntryStateStale
)
type VersionTable[K comparable, V any] struct {
entries map[K]*VersionEntry[K, V]
}
func NewVersionTable[K comparable, V any]() *VersionTable[K, V] {
return &VersionTable[K, V]{
entries: make(map[K]*VersionEntry[K, V]),
}
}
type VersionEntry[key comparable, V any] struct {
key key
value V
version uint64
state EntryState
}
func (t *VersionTable[K, V]) Lookup(key K) (*VersionEntry[K, V], bool) {
entry, ok := t.entries[key]
if !ok {
return nil, false
}
return entry, true
}
func (t *VersionTable[K, V]) Insert(key K, value V, version uint64) *VersionEntry[K, V] {
entry, ok := t.entries[key]
if !ok || version > entry.version {
newEntry := &VersionEntry[K, V]{
key: key,
value: value,
version: version,
state: EntryStateActive,
}
t.entries[key] = newEntry
return newEntry
}
return entry
}
func (t *VersionTable[K, V]) Erase(key K) {
delete(t.entries, key)
}
func (t *VersionTable[K, V]) Stale(key K, version uint64) {
var v V
newEntry := &VersionEntry[K, V]{
key: key,
value: v,
version: version,
state: EntryStateStale,
}
t.entries[key] = newEntry
}
type RefCount[K comparable] map[K]uint64
func (r RefCount[K]) Inc(key K) {
r[key]++
}
func (r RefCount[K]) Dec(key K) {
r[key]--
}
func (r RefCount[K]) Count(key K) uint64 {
return r[key]
}
func (r RefCount[K]) Erase(key K) {
delete(r, key)
}
type VersionCache[K comparable, V any] struct {
sync.Mutex
table *VersionTable[K, V]
refs RefCount[K]
}
// ReleaseFunc is a function that releases the entry.
type ReleaseFunc[K comparable, V any] func(entry *VersionEntry[K, V])
// Lookup returns the entry if it exists and increments the reference count.
// Caller must call Release to decrement the reference count after using the entry.
func (c *VersionCache[K, V]) Lookup(key K) (*VersionEntry[K, V], bool, ReleaseFunc[K, V]) {
c.Lock()
defer c.Unlock()
entry, ok := c.table.Lookup(key)
if ok {
c.refs.Inc(key)
return entry, true, c.Release
}
return nil, false, c.Release
}
// Insert inserts a new entry into the cache and increments the reference count.
// Caller must call Release to decrement the reference count after using the entry.
func (c *VersionCache[K, V]) Insert(key K, value V, version uint64) (*VersionEntry[K, V], ReleaseFunc[K, V]) {
c.Lock()
defer c.Unlock()
entry := c.table.Insert(key, value, version)
c.refs.Inc(key)
return entry, c.Release
}
func (c *VersionCache[K, V]) InsertBatchWithoutRef(keys []K, values []V, versions []uint64) {
c.Lock()
defer c.Unlock()
for i := range keys {
c.table.Insert(keys[i], values[i], versions[i])
}
}
// Release decrements the reference count for the entry
// Users should always call this function to decrement the reference count after using the entry.
func (c *VersionCache[K, V]) Release(entry *VersionEntry[K, V]) {
if entry == nil {
return
}
c.Lock()
defer c.Unlock()
c.refs.Dec(entry.key)
}
// Stale marks the entry as stale or erases it if the reference count is 0.
func (c *VersionCache[K, V]) Stale(key K, version uint64) {
c.Lock()
defer c.Unlock()
if c.refs.Count(key) == 0 {
c.table.Erase(key)
c.refs.Erase(key)
} else {
c.table.Stale(key, version)
}
}
// StaleIf marks entries as stale or erases them based on the predicate.
// For each entry where predicate returns true:
// - if ref count is 0, erase it directly
// - otherwise, mark it as Stale
func (c *VersionCache[K, V]) StaleIf(predicate func(K) bool, version uint64) {
c.Lock()
defer c.Unlock()
for key := range c.table.entries {
if predicate(key) {
if c.refs.Count(key) == 0 {
c.table.Erase(key)
c.refs.Erase(key)
} else {
c.table.Stale(key, version)
}
}
}
}
// Prune erases all entries that are stale and have reference count 0.
// Users should call this function if they care about memory usage.
func (c *VersionCache[K, V]) Prune() {
c.Lock()
defer c.Unlock()
for key, entry := range c.table.entries {
if c.refs.Count(key) == 0 && entry.state == EntryStateStale {
c.table.Erase(key)
c.refs.Erase(key)
}
}
}
func NewVersionCache[K comparable, V any]() *VersionCache[K, V] {
return &VersionCache[K, V]{
table: NewVersionTable[K, V](),
refs: make(RefCount[K]),
}
}
+89 -10
View File
@@ -23,11 +23,23 @@ import (
"github.com/milvus-io/milvus-proto/go-api/v3/commonpb"
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
"github.com/milvus-io/milvus/internal/distributed/streaming"
"github.com/milvus-io/milvus/pkg/v3/mlog"
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message"
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message/ce"
"github.com/milvus-io/milvus/pkg/v3/util/typeutil"
)
// aliasNoOldTarget marks an AlterAlias-family broadcast that provably has no
// pre-alter target to evict (a CreateAlias). It is deliberately distinct from
// the zero value: an old rootcoord (produced before old_collection_id existed)
// leaves the field unset, and a new AlterAlias that cannot resolve its old
// target also leaves it 0 -- both of those MUST fall back to the proxy holder
// scan. So OldCollectionId==0 means "unknown old target, scan to be safe" and
// this sentinel means "definitely nothing to evict, do NOT scan". Keeping the
// zero value as the safe-scan path is what makes a replayed old-rootcoord
// AlterAlias still close its ghost after a rolling upgrade.
const aliasNoOldTarget = int64(-1)
func (c *Core) broadcastCreateAlias(ctx context.Context, req *milvuspb.CreateAliasRequest) error {
req.DbName = strings.TrimSpace(req.DbName)
req.Alias = strings.TrimSpace(req.Alias)
@@ -58,6 +70,9 @@ func (c *Core) broadcastCreateAlias(ctx context.Context, req *milvuspb.CreateAli
CollectionId: collection.CollectionID,
Alias: req.GetAlias(),
CollectionName: req.GetCollectionName(),
// CreateAlias has no pre-existing target: mark it explicitly so the
// ack callback does NOT run the O(N) holder scan on every create.
OldCollectionId: aliasNoOldTarget,
}).
WithBody(&message.AlterAliasMessageBody{}).
WithBroadcast([]string{streaming.WAL().ControlChannel()}).
@@ -89,13 +104,44 @@ func (c *Core) broadcastAlterAlias(ctx context.Context, req *milvuspb.AlterAlias
return err
}
// Resolve the alias's CURRENT (pre-alter) target under the same database
// lock and carry it in the header, so the cache expiration can evict the old
// target by id. The proxy cannot always resolve the old target itself: a
// concurrent describe of the new target may re-point its alias resolution
// before the expiration arrives, which would leave the old target's cached
// Aliases list permanently stale (the proxy's holder-scan fallback only runs
// for old-rootcoord broadcasts with CollectionID==0, so a partial message
// with a nonzero new id and a zero old id silently disables the fix).
//
// GetCollectionByName resolves an alias to its target authoritatively (it
// consults the alias map), allowUnavailable=true so a target already being
// dropped still yields its id. CheckIfAliasAlterable above guarantees the
// alias exists, so this normally resolves to a real (>0) id and the ack
// callback evicts the old target by id -- no scan. If it genuinely cannot (a
// meta inconsistency), DON'T fail the alter: leave the old target UNKNOWN
// (zero) so the ack callback falls back to the proxy holder scan. Zero (not
// a sentinel) is deliberately the safe-scan path -- it is also what an old
// rootcoord (before old_collection_id existed) leaves in a replayed
// AlterAlias, so both cases converge on the scan and neither reopens a
// ghost. Only CreateAlias, which provably has no old target, sets the
// aliasNoOldTarget sentinel to skip the scan. Computed at broadcast time so
// message replay stays deterministic.
oldCollectionID := int64(0)
if oldColl, err := c.meta.GetCollectionByName(ctx, req.GetDbName(), req.GetAlias(), typeutil.MaxTimestamp, true); err == nil {
oldCollectionID = oldColl.CollectionID
} else {
mlog.Warn(ctx, "could not resolve pre-alter alias target; proxy will holder-scan for it",
mlog.String("db", req.GetDbName()), mlog.String("alias", req.GetAlias()), mlog.Err(err))
}
msg := message.NewAlterAliasMessageBuilderV2().
WithHeader(&message.AlterAliasMessageHeader{
DbId: db.ID,
DbName: req.GetDbName(),
CollectionId: collection.CollectionID,
Alias: req.GetAlias(),
CollectionName: req.GetCollectionName(),
DbId: db.ID,
DbName: req.GetDbName(),
CollectionId: collection.CollectionID,
Alias: req.GetAlias(),
CollectionName: req.GetCollectionName(),
OldCollectionId: oldCollectionID,
}).
WithBody(&message.AlterAliasMessageBody{}).
WithBroadcast([]string{streaming.WAL().ControlChannel()}).
@@ -108,9 +154,42 @@ func (c *DDLCallback) alterAliasV2AckCallback(ctx context.Context, result messag
if err := c.meta.AlterAlias(ctx, result); err != nil {
return err
}
return c.ExpireCaches(ctx,
ce.NewBuilder().WithLegacyProxyCollectionMetaCache(
ce.OptLPCMDBName(result.Message.Header().DbName),
ce.OptLPCMCollectionName(result.Message.Header().Alias),
ce.OptLPCMMsgType(commonpb.MsgType_AlterAlias)))
header := result.Message.Header()
builder := ce.NewBuilder().WithLegacyProxyCollectionMetaCache(
ce.OptLPCMDBName(header.DbName),
ce.OptLPCMCollectionName(header.Alias),
// Forward the new target's collection id so the proxy also evicts the
// canonical entry (and id index) of the collection now gaining the
// alias, which the proxy cannot resolve from the alias name (the alias
// did not point at it when it was cached). Without this, an id-only
// Describe of the new target would serve a stale Aliases list.
ce.OptLPCMCollectionID(header.CollectionId),
ce.OptLPCMMsgType(commonpb.MsgType_AlterAlias))
switch {
case header.OldCollectionId > 0 && header.OldCollectionId != header.CollectionId:
// Old target known and distinct from the new one: evict it by id. A
// second expiration entry with no collection name -- the proxy's handler
// then only runs the id eviction, no scan.
builder = builder.WithLegacyProxyCollectionMetaCache(
ce.OptLPCMDBName(header.DbName),
ce.OptLPCMCollectionID(header.OldCollectionId),
ce.OptLPCMMsgType(commonpb.MsgType_AlterAlias))
case header.OldCollectionId == 0:
// Old target UNKNOWN. Two ways to land here, both must scan: (a) a new
// AlterAlias whose broadcast could not resolve the pre-alter target (meta
// inconsistency), and (b) a replayed old-rootcoord AlterAlias that
// predates old_collection_id (field defaults to 0). Emit a
// CollectionID==0 alias-name entry so the proxy runs its holder-scan
// fallback and evicts any cached collection still declaring the alias.
//
// CreateAlias sets the aliasNoOldTarget (<0) sentinel and takes neither
// branch: it provably has no old target, so it must never trigger the
// O(N) holder scan.
builder = builder.WithLegacyProxyCollectionMetaCache(
ce.OptLPCMDBName(header.DbName),
ce.OptLPCMCollectionName(header.Alias),
ce.OptLPCMCollectionID(0),
ce.OptLPCMMsgType(commonpb.MsgType_AlterAlias))
}
return c.ExpireCaches(ctx, builder)
}
@@ -0,0 +1,150 @@
// Licensed to the LF AI & Data foundation under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package rootcoord
import (
"context"
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
mockrootcoord "github.com/milvus-io/milvus/internal/rootcoord/mocks"
"github.com/milvus-io/milvus/internal/util/proxyutil"
"github.com/milvus-io/milvus/pkg/v3/proto/proxypb"
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message"
"github.com/milvus-io/milvus/pkg/v3/util/funcutil"
)
// TestDDLCallbacksAlterAliasV2AckCallback_OldTargetRouting locks in how the
// AlterAlias ack callback routes the OLD target eviction by the header's
// OldCollectionId, which shares the message type with CreateAlias. The zero
// value is the compat-safe scan path so a replayed old-rootcoord AlterAlias
// (which predates old_collection_id and leaves it 0) still closes its ghost:
// - > 0 : old target known -> evict it by id (no scan).
// - == 0 : old target UNKNOWN (new AlterAlias could not resolve it, OR an old
// rootcoord that never set the field) -> emit a CollectionID==0 alias entry
// so the proxy holder-scans (graceful degrade, never fails the alter).
// - < 0 : CreateAlias sentinel (provably no old target) -> NO CollectionID==0
// entry, so a normal create never triggers the O(N) holder scan.
func TestDDLCallbacksAlterAliasV2AckCallback_OldTargetRouting(t *testing.T) {
ctx := context.Background()
const newID = int64(20)
invalidate := func(t *testing.T, oldCollectionID int64) []*proxypb.InvalidateCollMetaCacheRequest {
meta := mockrootcoord.NewIMetaTable(t)
meta.EXPECT().AlterAlias(mock.Anything, mock.Anything).Return(nil)
var got []*proxypb.InvalidateCollMetaCacheRequest
pcm := proxyutil.NewMockProxyClientManager(t)
pcm.EXPECT().InvalidateCollectionMetaCache(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, req *proxypb.InvalidateCollMetaCacheRequest, opts ...proxyutil.ExpireCacheOpt) error {
got = append(got, req)
return nil
}).Maybe()
c := newTestCore(withMeta(meta), withTsoAllocator(newMockTsoAllocator()))
c.proxyClientManager = pcm
cb := &DDLCallback{Core: c}
raw := message.NewAlterAliasMessageBuilderV2().
WithHeader(&message.AlterAliasMessageHeader{
DbName: "db",
Alias: "a",
CollectionId: newID,
OldCollectionId: oldCollectionID,
}).
WithBody(&message.AlterAliasMessageBody{}).
WithBroadcast([]string{funcutil.GetControlChannel("test")}).
MustBuildBroadcast()
msg := message.MustAsBroadcastAlterAliasMessageV2(raw)
require.NoError(t, cb.alterAliasV2AckCallback(ctx, message.BroadcastResultAlterAliasMessageV2{
Message: msg,
Results: map[string]*message.AppendResult{},
}))
return got
}
// helpers over the captured requests
hasIDOnly := func(reqs []*proxypb.InvalidateCollMetaCacheRequest, id int64) bool {
for _, r := range reqs {
if r.GetCollectionID() == id && r.GetCollectionName() == "" {
return true
}
}
return false
}
hasScanTrigger := func(reqs []*proxypb.InvalidateCollMetaCacheRequest) bool {
// the proxy holder-scan is gated on CollectionID==0 + the alias name
for _, r := range reqs {
if r.GetCollectionID() == 0 && r.GetCollectionName() == "a" {
return true
}
}
return false
}
// hasID matches any request carrying the id, regardless of an accompanying
// name (the new-target eviction rides on the first request, which also carries
// the alias name, so hasIDOnly deliberately does NOT match it).
hasID := func(reqs []*proxypb.InvalidateCollMetaCacheRequest, id int64) bool {
for _, r := range reqs {
if r.GetCollectionID() == id {
return true
}
}
return false
}
t.Run("old target known -> evict by id, no scan trigger", func(t *testing.T) {
reqs := invalidate(t, 10)
require.True(t, hasIDOnly(reqs, 10), "old target 10 must be evicted by id")
require.False(t, hasScanTrigger(reqs), "a known old target must not trigger the holder scan")
})
t.Run("old target unknown (0) -> scan trigger, no stray id eviction", func(t *testing.T) {
// zero covers both a new AlterAlias that could not resolve and a replayed
// old-rootcoord AlterAlias that never set old_collection_id.
reqs := invalidate(t, 0)
require.True(t, hasScanTrigger(reqs), "an unknown old target (0) must emit a CollectionID==0 scan trigger")
require.False(t, hasIDOnly(reqs, 0), "zero must never be sent as a real id eviction")
})
t.Run("create alias sentinel (-1) -> no scan trigger", func(t *testing.T) {
reqs := invalidate(t, aliasNoOldTarget)
require.False(t, hasScanTrigger(reqs), "CreateAlias (aliasNoOldTarget) must not trigger an O(N) holder scan")
})
t.Run("new target is always evicted by id", func(t *testing.T) {
// the new target's canonical entry must be expired regardless of the old
// target's fate; a regression dropping the unconditional first eviction
// would leave an id-only Describe of the new target serving a stale
// Aliases list.
for _, oldID := range []int64{10, 0, aliasNoOldTarget} {
reqs := invalidate(t, oldID)
require.True(t, hasID(reqs, newID), "new target %d must be evicted (oldID=%d)", newID, oldID)
}
})
t.Run("old target == new target -> deduped, no redundant eviction, no scan", func(t *testing.T) {
// the guard is OldCollectionId > 0 && OldCollectionId != CollectionId; when
// they coincide the old-target branch must be skipped so we do not emit a
// second, redundant eviction for the same id, and never fall into the scan.
reqs := invalidate(t, newID)
require.True(t, hasID(reqs, newID), "the new target must still be evicted")
require.False(t, hasIDOnly(reqs, newID), "no separate id-only old-target eviction when old==new")
require.False(t, hasScanTrigger(reqs), "a resolved (equal) old target must not trigger the scan")
})
}
@@ -0,0 +1,164 @@
// Licensed to the LF AI & Data foundation under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package rootcoord
import (
"context"
"testing"
"github.com/bytedance/mockey"
"github.com/cockroachdb/errors"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/milvus-io/milvus-proto/go-api/v3/milvuspb"
"github.com/milvus-io/milvus/internal/distributed/streaming"
"github.com/milvus-io/milvus/internal/metastore/model"
"github.com/milvus-io/milvus/internal/mocks/distributed/mock_streaming"
"github.com/milvus-io/milvus/internal/mocks/streamingcoord/server/mock_broadcaster"
mockrootcoord "github.com/milvus-io/milvus/internal/rootcoord/mocks"
"github.com/milvus-io/milvus/pkg/v3/streaming/util/message"
)
// These tests audit the PRODUCTION side of AlterAliasMessageHeader.OldCollectionId
// (the ack-callback consumer is covered by
// TestDDLCallbacksAlterAliasV2AckCallback_OldTargetRouting). They mock the
// broadcast plumbing and capture the built message to assert which sentinel the
// broadcaster stamps: CreateAlias => aliasNoOldTarget (-1), AlterAlias with a
// resolvable pre-alter target => its real id (>0), AlterAlias whose target
// cannot be resolved => 0 (the safe holder-scan path, never a failed alter).
// TestBroadcastCreateAlias_MarksNoOldTargetSentinel: a create provably has no
// pre-existing target, so the header must carry the aliasNoOldTarget sentinel to
// keep the ack callback off the O(N) holder scan.
func TestBroadcastCreateAlias_MarksNoOldTargetSentinel(t *testing.T) {
mockey.PatchConvey("create alias stamps aliasNoOldTarget", t, func() {
meta := mockrootcoord.NewIMetaTable(t)
meta.EXPECT().CheckIfAliasCreatable(mock.Anything, "db", "a", "coll").Return(nil)
meta.EXPECT().GetDatabaseByName(mock.Anything, "db", mock.Anything).Return(&model.Database{ID: 3, Name: "db"}, nil)
meta.EXPECT().GetCollectionByName(mock.Anything, "db", "coll", mock.Anything, false).
Return(&model.Collection{CollectionID: 20, Name: "coll"}, nil)
core := newTestCore(withMeta(meta))
var captured message.BroadcastMutableMessage
broadcastAPI := mock_broadcaster.NewMockBroadcastAPI(t)
broadcastAPI.EXPECT().Broadcast(mock.Anything, mock.Anything).
Run(func(ctx context.Context, msg message.BroadcastMutableMessage) { captured = msg }).
Return(nil, nil).Once()
broadcastAPI.EXPECT().Close().Return().Once()
mockey.Mock(startBroadcastWithDatabaseLock).Return(broadcastAPI, nil).Build()
wal := mock_streaming.NewMockWALAccesser(t)
wal.EXPECT().ControlChannel().Return("by-dev-rootcoord-dml_0").Once()
streaming.SetWALForTest(wal)
defer streaming.SetWALForTest(nil)
err := core.broadcastCreateAlias(context.Background(), &milvuspb.CreateAliasRequest{
DbName: "db",
Alias: "a",
CollectionName: "coll",
})
require.NoError(t, err)
require.NotNil(t, captured)
header := message.MustAsBroadcastAlterAliasMessageV2(captured).Header()
require.Equal(t, aliasNoOldTarget, header.OldCollectionId, "CreateAlias must stamp the no-old-target sentinel")
require.Equal(t, int64(20), header.CollectionId)
})
}
// TestBroadcastAlterAlias_ForwardsResolvedOldTarget: an alter whose pre-alter
// alias target resolves must forward that id so the proxy evicts the old target
// by id (no scan).
func TestBroadcastAlterAlias_ForwardsResolvedOldTarget(t *testing.T) {
mockey.PatchConvey("alter alias forwards the resolved old target id", t, func() {
meta := mockrootcoord.NewIMetaTable(t)
meta.EXPECT().CheckIfAliasAlterable(mock.Anything, "db", "a", "coll").Return(nil)
meta.EXPECT().GetDatabaseByName(mock.Anything, "db", mock.Anything).Return(&model.Database{ID: 3, Name: "db"}, nil)
// the new target (by collection name)
meta.EXPECT().GetCollectionByName(mock.Anything, "db", "coll", mock.Anything, false).
Return(&model.Collection{CollectionID: 20, Name: "coll"}, nil)
// the pre-alter target (by alias name, allowUnavailable=true)
meta.EXPECT().GetCollectionByName(mock.Anything, "db", "a", mock.Anything, true).
Return(&model.Collection{CollectionID: 10, Name: "oldcoll"}, nil)
core := newTestCore(withMeta(meta))
var captured message.BroadcastMutableMessage
broadcastAPI := mock_broadcaster.NewMockBroadcastAPI(t)
broadcastAPI.EXPECT().Broadcast(mock.Anything, mock.Anything).
Run(func(ctx context.Context, msg message.BroadcastMutableMessage) { captured = msg }).
Return(nil, nil).Once()
broadcastAPI.EXPECT().Close().Return().Once()
mockey.Mock(startBroadcastWithDatabaseLock).Return(broadcastAPI, nil).Build()
wal := mock_streaming.NewMockWALAccesser(t)
wal.EXPECT().ControlChannel().Return("by-dev-rootcoord-dml_0").Once()
streaming.SetWALForTest(wal)
defer streaming.SetWALForTest(nil)
err := core.broadcastAlterAlias(context.Background(), &milvuspb.AlterAliasRequest{
DbName: "db",
Alias: "a",
CollectionName: "coll",
})
require.NoError(t, err)
require.NotNil(t, captured)
header := message.MustAsBroadcastAlterAliasMessageV2(captured).Header()
require.Equal(t, int64(10), header.OldCollectionId, "the resolved pre-alter target id must be forwarded")
require.Equal(t, int64(20), header.CollectionId)
})
}
// TestBroadcastAlterAlias_LeavesZeroWhenOldTargetUnresolvable: if the pre-alter
// target cannot be resolved (a meta inconsistency), the alter must NOT fail; it
// leaves OldCollectionId at 0 so the ack callback falls back to the proxy holder
// scan (0 is the safe-scan path, distinct from the -1 CreateAlias sentinel).
func TestBroadcastAlterAlias_LeavesZeroWhenOldTargetUnresolvable(t *testing.T) {
mockey.PatchConvey("alter alias leaves zero when the old target is unresolvable", t, func() {
meta := mockrootcoord.NewIMetaTable(t)
meta.EXPECT().CheckIfAliasAlterable(mock.Anything, "db", "a", "coll").Return(nil)
meta.EXPECT().GetDatabaseByName(mock.Anything, "db", mock.Anything).Return(&model.Database{ID: 3, Name: "db"}, nil)
meta.EXPECT().GetCollectionByName(mock.Anything, "db", "coll", mock.Anything, false).
Return(&model.Collection{CollectionID: 20, Name: "coll"}, nil)
// the pre-alter target cannot be resolved
meta.EXPECT().GetCollectionByName(mock.Anything, "db", "a", mock.Anything, true).
Return(nil, errors.New("meta inconsistency"))
core := newTestCore(withMeta(meta))
var captured message.BroadcastMutableMessage
broadcastAPI := mock_broadcaster.NewMockBroadcastAPI(t)
broadcastAPI.EXPECT().Broadcast(mock.Anything, mock.Anything).
Run(func(ctx context.Context, msg message.BroadcastMutableMessage) { captured = msg }).
Return(nil, nil).Once()
broadcastAPI.EXPECT().Close().Return().Once()
mockey.Mock(startBroadcastWithDatabaseLock).Return(broadcastAPI, nil).Build()
wal := mock_streaming.NewMockWALAccesser(t)
wal.EXPECT().ControlChannel().Return("by-dev-rootcoord-dml_0").Once()
streaming.SetWALForTest(wal)
defer streaming.SetWALForTest(nil)
err := core.broadcastAlterAlias(context.Background(), &milvuspb.AlterAliasRequest{
DbName: "db",
Alias: "a",
CollectionName: "coll",
})
require.NoError(t, err, "an unresolvable old target must NOT fail the alter")
require.NotNil(t, captured)
header := message.MustAsBroadcastAlterAliasMessageV2(captured).Header()
require.Equal(t, int64(0), header.OldCollectionId, "an unresolvable old target must leave 0 (safe-scan path)")
})
}
+13
View File
@@ -404,6 +404,19 @@ message AlterAliasMessageHeader {
int64 collection_id = 3;
string collection_name = 4;
string alias = 5;
// The collection the alias pointed to BEFORE this alter. Sentinel-encoded:
// > 0 the known old target -- cache expiration evicts it by id;
// == 0 UNKNOWN: either a new AlterAlias whose broadcast could not resolve
// the old target, OR an older producer that predates this field. The
// proxy falls back to an O(N) holder scan, so 0 is the safe default;
// < 0 the "no old target" sentinel that CreateAlias sets -- it provably
// has none, so it must NOT trigger the scan. A CreateAlias producer
// MUST use this, NOT 0 (0 would O(N)-scan every create).
// Carried in the header -- computed once under the broadcaster's database
// lock -- so cache expiration can evict the old target by id even when a
// concurrent describe has already re-pointed the proxy's alias resolution to
// the new target, and so replaying the message stays deterministic.
int64 old_collection_id = 6;
}
// AlterAliasMessageBody is the body of alter alias message.
File diff suppressed because it is too large Load Diff
-11
View File
@@ -2128,8 +2128,6 @@ type proxyConfig struct {
QueryNodePoolingSize ParamItem `refreshable:"false"`
HybridSearchRequeryPolicy ParamItem `refreshable:"true"`
MetaCacheGCTimeInterval ParamItem `refreshable:"true"`
}
func (p *proxyConfig) init(base *BaseTable) {
@@ -2716,15 +2714,6 @@ Disabled if the value is less or equal to 0.`,
Export: true,
}
p.QueryNodePoolingSize.Init(base.mgr)
p.MetaCacheGCTimeInterval = ParamItem{
Key: "proxy.metaCacheGCTimeInterval",
Version: "2.6.13",
Doc: "the time interval for meta cache GC, in seconds",
DefaultValue: "300",
Export: true,
}
p.MetaCacheGCTimeInterval.Init(base.mgr)
}
// /////////////////////////////////////////////////////////////////////////////