The `if (!nullable_)` branch had no `return`, so a non-nullable column
fell through into the nullable block and invoked the callback a second
time per row (latent — currently masked by callers that guard with `if
(!IsNullable()) return`). Added the missing `return`.
issue: #51385🤖 Generated with [Claude Code](https://claude.com/claude-code)
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>
`SealedDataGetter::Get`'s indexed branch (`from_data_==false`) threw
`AssertInfo("field data not found")` on a null row instead of returning
`std::nullopt` like the from-data branches, failing the whole group-by
query. Now returns nullopt so the NULL forms a distinct null group. Adds
a nullable group-by regression test (the index-only branch is
CI-verified).
issue: #51385🤖 Generated with [Claude Code](https://claude.com/claude-code)
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>
issue: #51299
## Problem
storage-v3 (loon/vortex) sealed segments load the raw vector column
resident **on top of** a vector index that already carries the raw data
(`HasRawData=true`), roughly **doubling** the per-segment disk footprint
of DiskANN / IVF_FLAT / HNSW(with-raw) segments. On the 100M / 128-dim
DiskANN benchmark (1 replica, 100Gi querynode ephemeral-storage) this
pushes a querynode over the cap and `load_collection` fails with
`segment request resource failed[resourceType=Disk]`.
The v2 binlog path already skips the raw fielddata when the index has
raw data (`ChunkedSegmentSealedImpl`: *"skip fielddata because index has
raw data"*); the v3 manifest path in
`SegmentLoadInfo::ComputeDiffColumnGroups` only marked the column
**lazy**, and `LoadColumnGroup(eager_load=false)` still materialized a
`ProxyChunkColumn` and set `field_data_ready`, so the raw column stayed
resident anyway.
## Fix
In `ComputeDiffColumnGroups`, when a field's index carries raw data and
`prefer_field_data_when_index_has_raw_data == false`, drop the raw
column from the load set entirely (mirroring the v2 binlog path) instead
of lazy-loading it. This covers vector fields and other indexed scalars
(retrieve/filter read the raw value from the index).
The **primary key is excluded**: sorted segments navigate rows through
the pk column (`num_rows_until_chunk` / `get_chunk_by_offset`) and a pk
scalar index is not registered in `scalar_indexings`, so the pk raw
column must stay resident — otherwise a `ColumnExpr` on the pk falls
back to a column scan (`use_index_data_ == false`) and crashes (`field
100 must exist when getting rows until chunk`, regressed go-sdk
`TestCreateSortedScalarIndex`).
The stale-copy drop is scoped to the real no-raw-index -> raw-index
reopen transition. `prefer_field_data` remains the explicit opt-in to
keep both resident.
---------
Signed-off-by: xiaofanluan <xf@hjjaq.com>
Signed-off-by: Li Liu <li.liu@zilliz.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Li Liu <li.liu@zilliz.com>
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>
`insert_helper` in `PhyIterativeFilterNode` indexed the per-query result
window by the **relative** count instead of the **absolute** window end
(`base_idx + count`). For `nq_index >= 1` the shift guard `count > pos`
is always false, so an out-of-order candidate (a closer distance
arriving after farther ones — which approximate vector iterators do
emit) overwrote an existing top-k entry instead of shifting it,
corrupting the top-k of **every query vector after the first** in a
multi-query filtered search.
Extracted the insertion into `Utils.h::topk_binsert` with the correct
absolute-window math, matching the already-correct math in
`PhyIterativeElementFilterNode::CollectResults`.
Deterministic regression test
`IterativeFilterInsert.MultiQueryWindowOrdering` inserts an out-of-order
stream into the second query window and asserts the correct sorted
result.
issue: #51385🤖 Generated with [Claude Code](https://claude.com/claude-code)
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>
issue: #51191
Fixes three robustness defects in the copy-segment restore pipeline
(`internal/datacoord/copy_segment_*.go`), each with a unit test covering
the failure mode.
### 1. `checkPendingJob` cannot resume a partially created job (high)
Task creation is a multi-step sequence (per-group `AllocID` + `AddTask`,
each persisted individually, then the `Pending → Executing` transition).
If any step fails mid-way (etcd hiccup, DataCoord restart), the job
stayed `Pending` with only a subset of tasks persisted, and the old
early-return `if len(tasks) > 0 { return }` fired on every subsequent
round — the restore hung until the job timeout with no way to progress.
Now the transition is resume-safe: it computes the source segments
already covered by persisted tasks, creates tasks only for the uncovered
mappings, and always (re-)applies the idempotent `Executing` transition
(so a round that persisted all tasks but failed the job update also
recovers).
### 2. Transient `QueryCopySegment` RPC error failed the whole restore
(medium)
A single network blip or DataNode restart during a status poll
permanently failed the task and its parent restore job. This was
inconsistent with the import pipeline, where a `QueryImport` RPC error
resets the task for retry instead of failing the job.
Now a query RPC error is logged and the task is kept `InProgress` for
re-query on the next tick. The job-level timeout still bounds a
permanently unreachable node; worker-reported task failures keep the
fail-fast behavior.
### 3. `copySegmentTask.Clone` mutated shared state before persisting
(medium)
`Clone` copied the `atomic.Pointer` value, not the protobuf, so the
"clone" used by `UpdateTask` shared the same `*datapb.CopySegmentTask`
as the cached entry. Update actions mutate the proto in place, so the
in-memory task was mutated *before* `SaveCopySegmentTask` — a failed
catalog save left the coordinator cache reflecting the new state while
etcd kept the old one. Now `Clone` deep-copies with `proto.Clone`,
matching `copySegmentJob.Clone`.
### Testing
- `TestCheckPendingJob_ResumesPartialTaskCreation`,
`TestCheckPendingJob_AllTasksExistTransitionsToExecuting`
- `TestQueryTaskOnWorker_RPCErrorKeepsTaskInProgress`
- `TestClone_DeepCopiesProto`,
`TestUpdateTask_SaveFailureLeavesCacheUnchanged`
- Full `./internal/datacoord/` package: PASS
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Signed-off-by: cai.zhang <cai.zhang@zilliz.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What this PR does
- Adds REST v2 query coverage for orderByFields across ascending,
descending, multi-field, filtering, pagination, nullable fields, and
invalid parameters.
- Uses class-scoped REST clients and shared collection setup consistent
with the existing REST v2 test pattern.
- Verifies that omitting limit applies the REST default of 100 rows.
- Covers duplicate-key pagination behavior and exact validation error
code 1100.
## Test
Against schema-evolution-v3-latest through port-forward:
cd tests/restful_client_v2
../python_client/.venv/bin/python -m pytest
testcases/test_query_order_by.py -v --tb=short -p no:rerunfailures
--endpoint http://127.0.0.1:19530 --token root:Milvus
Result: 16 passed, 16 warnings in 40.87s.
---------
Signed-off-by: lyyyuna <yiyang.li@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
issue: #51381
related: #51414
related: #51416
### What this PR does
- Adds Python client E2E coverage for nullable Struct Array
NULL-versus-empty semantics.
- Covers compaction, snapshot restoration, partial updates, drop and
re-add isolation, imports, regex indexes, aggregation, and dynamic
fields.
- Adds exact ANN oracles for Struct Array element and embedding-list
searches without assuming self-hit behavior.
- Covers hybrid search collapse strategies and element identity
preservation.
- Marks known regressions with strict xfail markers linked to their
corresponding issues while keeping unaffected parameters active.
### Why
Struct Array rows can represent NULL, empty arrays, and non-empty
arrays. These states affect predicates, nested indexes, element offsets,
and vector search candidates differently. The added coverage protects
these semantics across write, reload, compaction, import, and snapshot
lifecycle paths.
### Validation
- `ruff check` passed for all changed Python files.
- `ruff format --check` passed for all changed Python files.
- Python compilation passed for all changed Python files.
- Pytest collection passed.
- The nine known regression nodes executed as expected: `9 xfailed`.
- The three unaffected parameterized cases passed.
- Targeted E2E validation used Milvus master build `09c44f2dbb`.
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
## Summary
Avoid direct captures of structured bindings in
`ChunkedSegmentSealedImpl.cpp`, which is compiled with OpenMP enabled.
- Use an init-capture for the field ID in `LoadBatchTextIndexes`.
- Use an init-capture for the field ID in `FinalizeLoadDiffForReopen`
when dropping JSON indexes.
- Keep the inner text-index commit lambda as a normal value capture
because it captures the ordinary init-capture from the outer lambda, not
a structured binding.
issue: #51231
## Test Plan
- [x] `make`
- [x] `ninja -C cmake_build
src/segcore/CMakeFiles/milvus_segcore.dir/ChunkedSegmentSealedImpl.cpp.o`
- [x] `git diff --check`
Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com>
## What
Add `dataCoord.import.l0ImportDisabled` (default **true**) and reject
`ImportV2` requests carrying `l0_import=true` when it is set.
- `pkg/util/paramtable/component_param.go`: new `L0ImportDisabled` param
(default true).
- `internal/datacoord/services.go`: in `ImportV2`, reject `l0_import`
requests when disabled (fail-closed, before resource allocation).
- `configs/milvus.yaml`: documented config entry.
## Why
Importing L0 (delete-only) segments as separate L0 segments during a
binlog/backup import is incompatible with `commit_timestamp`
(two-phase-commit / replication imports). L0 deletes carry original
timestamps while data segments are stamped with `commit_ts`, so
`delete_ts < insert_ts` and the deletes are **silently dropped** — rows
that should be deleted survive. Because this is silent data corruption,
a fail-closed default is safer than relying on callers/operators to
avoid it. See #51247.
The intended replacement is to fold a backup's L0 deletes into
per-segment deltalogs beforehand (offline L0 compaction), after which a
plain restore drops the rows via the existing per-segment delta path.
Operators who still need the legacy behavior can set the config to
`false`.
## Rollout note
This changes the default so that L0 import is rejected. Deployments
relying on legacy L0 import must either (a) migrate their backups to the
deltalog-folded form first, or (b) set
`dataCoord.import.l0ImportDisabled=false`. The default flip is intended
to be coordinated with the availability of the offline-compaction (wash)
tooling.
## Testing
- `paramtable` builds and its unit tests pass locally (pure-Go module).
- Added `TestImportV2_L0ImportDisabledReturnsError` (rejects `l0_import`
when disabled). The datacoord package requires the C++ core to build;
local core is stale in this dev env, so the datacoord test is left to
CI.
issue: #51247
---------
Signed-off-by: bigsheeper <yihao.dai@zilliz.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
issue: #51497
After a successful WAL switch via `POST /management/wal/alter`, `GET
/management/config/get?keys=mq.type` kept returning the pre-switch value
with `"source":"RuntimeSource"` until the coordinator restarted, even
though the ack callback had already persisted the new mq.type into etcd
(and linearizably refreshed the local EtcdSource). External operators
polling that endpoint to confirm switch completion never saw the new
value.
### Root cause
`InitAndSelectWALName()` saved the startup-resolved WAL name into the
config manager's **runtime overlay**, and `Manager.GetConfig` serves the
overlay with top priority — permanently shadowing the EtcdSource for
every reader in the process. The overlay write existed only to feed
`ProcessImmutableConfigs`, which pins `GetConfig`'s value into etcd
create-if-absent at first boot (without it, the literal `"default"`
would be persisted).
### Fix
Replace the overlay bridge with an explicit per-key **renderer** on the
persistence path, so mq.type reads fall through to the etcd source and
naturally follow a WAL switch:
- `ProcessImmutableConfigs(renderers map[string]func(raw string)
string)`: a renderer converts a placeholder raw value (mq.type's literal
`"default"`) into the concrete value at create-if-absent persist time.
It never runs when etcd already holds the key (a completed switch
survives restart), and it also covers the key-absent-from-all-sources
case (raw is passed as `""`). Generic: any future immutable config with
a placeholder value can plug in its own renderer.
- `InitAndSelectWALName()` no longer writes the runtime overlay and
returns the resolved `message.WALName`; the coordinator startup passes a
mq.type renderer backed by that name.
- `dependency.HealthCheck` resolves the literal `"default"` to the
actually-enabled MQ before probing (this gap predates the overlay Save:
since #36822, deployments running with `mq.type: default` probed nothing
and always reported unhealthy).
This also fixes two other readers of the shadowed value:
- the `HandleAlterWAL` same-type short-circuit: re-posting the completed
target no longer re-broadcasts, and a rollback to the original MQ is no
longer silently swallowed with "already configured";
- the proxy MQ health check (`/_cluster/dependencies`) no longer keeps
probing the old MQ forever after a switch.
### Behavior note
`MQCfg.Type.GetValue()` now follows etcd at runtime instead of being
frozen at process start: the coordinator sees a switch immediately
(linearizable refresh in the ack callback); other nodes converge within
one EtcdSource poll cycle (default `refreshInterval` 5s). Audited
readers: `mustSelectMQType` / `MustSelectWALName` resolve `"default"`
themselves and are startup-scoped; the AlterWAL idempotency check and
the proxy health check are exactly the readers this change fixes.
### Verification
- New tests (all watched failing first): renderer converts placeholder
before first persist / existing etcd value never overwritten nor
re-rendered / no-renderer keys unchanged / key absent from all sources
still pinned via renderer (`pkg/config`); `InitAndSelectWALName` leaves
mq.type served from its original source, not `RuntimeSource`
(`streamingutil/util`); `HealthCheck("default")` resolves to the enabled
MQ (`dependency`).
- Full package runs green locally: `pkg/config`,
`internal/util/streamingutil/util`, `internal/util/dependency` (with
`-tags dynamic,test -gcflags="all=-N -l"`).
- `cmd/roles`, `internal/coordinator`, `internal/distributed/streaming`
type-checked via `go vet` locally (no current-master segcore build on
this machine); relying on CI for full compilation. A live-cluster alter
→ config/get end-to-end pass is still pending — the switch-visibility
chain (etcd write → `GetConfig` returns the new value) is covered at
unit level by the existing `TestAlterConfigsInEtcd`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: tinswzy <zhenyuan.wei@zilliz.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Summary
In CDC replication mode, a 2PC bulk import (`auto_commit=false`) runs
**independently** on the primary (source) and standby clusters against
the same
files. Only the source can broadcast `Commit`/`Rollback`; the standby
replays
them via CDC. When the import fails on **one** cluster but the other
holds a
replicated `Uncommitted` job, that job had no recovery path:
- **Mode 1 (source fails, standby `Uncommitted`):** the failure never
broadcast
a `RollbackImport`, and `AbortImport` on a terminal-`Failed` source was
rejected — so the standby stayed `Uncommitted` indefinitely, holding
invisible
imported segments. (Observed in production: standby stuck 2+ days.)
- **Mode 2 (standby fails, source `Uncommitted`):** if the platform
commits the
source before confirming the standby is prepared, the source commits
while the
standby's commit ack no-ops on its `Failed` job → silent divergence.
issue: #51070
## What this changes (datacoord)
- **Abort a failed source (`AbortImport`):** a `Failed` source (its own
import
failed) is now abortable — it broadcasts `RollbackImport` to release the
peer's
`Uncommitted` job. `Committing`/`Completed` stay rejected. Lets an
operator
recover **immediately**, instead of waiting for GC.
- **GC self-heal (`importChecker.checkGC`):** before GC removes a
`Failed`
replicate-source job, it broadcasts `RollbackImport` so the peer is
released
**automatically at retention expiry**. Transient broadcast failure keeps
the
job and retries on the next tick; a standby's `ErrNotPrimary` proceeds;
**job
removal itself is the idempotency guard** (once gone, never
re-broadcast). This
closes the recovery-window gap without a persisted flag or proto change.
- **Divergence signal (`CommitImport` ack):** a commit ack landing on a
`Failed`
replica now logs at WARN (Mode 2).
- `isReplicatingClusterNow`: the GC decision now **propagates**
replication-check
errors instead of tolerating them — an indeterminate status (transient
balancer
error, `OnShutdownError`) retains the job and retries next tick, since a
false
"not replicating" would irreversibly strand the replicating peer; only
permanent
broadcast errors (`ErrNotPrimary` from a standby,
`ErrCollectionNotFound` after a
replicated drop) proceed to GC. The balancer access is bounded by a 10s
timeout
so an unregistered balancer cannot park the checker loop.
- Minor: the coordinator callbacks the checker needs are bundled into an
`importCheckerHooks` struct instead of a growing positional-arg list.
## Recovery guarantee
The peer's `Uncommitted` job is released either **immediately** via
`AbortImport`,
or **automatically** when the failed source reaches GC retention — no
permanent
stranding, and no dependence on a manual abort within the 3h retention
window.
## Not in scope (follow-ups)
- **Idempotent re-broadcast dedup:** repeated `AbortImport` on a
real-failure
source re-broadcasts each time (harmless — flusher/standby-ack no-op). A
persisted `rollback_broadcasted` flag would dedup this.
- **Eager self-heal:** broadcasting the rollback at *failure* time (in
`checkFailedJob`) rather than at GC time, to shorten peer-stranding to
seconds.
- **Mode 2 prevention:** a standby→coordinator prepared-vote so the
source
refuses to commit unless all replicas are prepared.
## Test plan
- `checkGC` self-heal: failed replicate-source broadcasts-then-removes
with
transient-retry; `ErrNotPrimary` (standby) proceeds; non-replicating
skips the
broadcast.
- `AbortImport`: failed-source broadcasts rollback; `Committing` and
`Completed`
rejected; previously-user-aborted is idempotent.
- CI `ci-v2/ut-go` compiles and runs these tests. (Local datacoord UT
was not run
for this revision due to cgo core drift in the worktree.)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Signed-off-by: bigsheeper <yihao.dai@zilliz.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What\n- Make master Mergify build-ut-cov gate success depend only on
ci-v2/build-ut-cov.\n- Make master Mergify build-ut-cov gate failure
remove ci-passed when ci-v2/build-ut-cov fails, without requiring legacy
split CI failures.\n\n## Why\nWhen legacy split CI jobs are downlined on
master, those checks can be missing instead of failed. The previous
remove rule required both build-ut-cov failure and legacy failure, so
ci-passed could remain after build-ut-cov failed.\n\n## Validation\n-
python3 -c "import yaml;
yaml.safe_load(open('/Users/zilliz/workspace/milvus/zhikunyao_milvus_ci/.github/mergify.yml'));
print('yaml ok')"\n- git diff --check
Signed-off-by: Zhikun Yao <zhikun.yao@zilliz.com>
issue: #50834
Fix no-index sealed vector raw data warmup selection. When a sealed
segment has no usable built or interim vector index, segcore now treats
raw vector field data as the vector search representation and resolves
its warmup from vector-index policy. Once a built or interim index
exists, raw vector data keeps normal vector-field warmup semantics.
Go load paths continue to pass field warmup unchanged; segcore resolves
the effective policy from current segment state.
---------
Signed-off-by: Shawn Wang <shawn.wang@zilliz.com>
## Summary
Fix L0 import storage version selection when `UseLoonFFI` is enabled:
- Force DataCoord to assign StorageV2 for L0 import segments.
- Force L0 import requests to disable Loon FFI.
- Defensively keep DataNode L0 sync tasks on StorageV2 even if the
request carries StorageV3.
- Add regression coverage for L0 import with `UseLoonFFI=true`.
Fixes#51148
## Known mixed-version window (documented, not fixed here)
The `AssignSegments` metadata fix only applies to jobs created on the
new version. A job created by a **pre-upgrade** DataCoord with
`common.storage.useLoonFFI=true` has already persisted its L0 segments
with `StorageVersion=3` in etcd, and segment IDs are reused (not
reallocated) when the task is recovered. If such an in-flight job
completes **after** the upgrade — either the upgraded DataCoord recovers
the task and reassembles the request as V2, or an upgraded DataNode
serves the old coordinator's V3 request (`syncDelete` now hard-codes V2)
— the deltalogs are written as V2 but the segment stays tagged
`StorageVersion=3` with an empty `ManifestPath`: the import completion
path does not reconcile `StorageVersion`, and L0 segments are excluded
from the storage-version compaction policy, so nothing self-heals the
tag.
Impact: harmless for delta load and L0 compaction (both branch on the
manifest, not the tag), but snapshot restore trusts the tag —
`collectSegmentFiles` fails on `storage_version>=3` with an empty
manifest.
Scope: requires a deployment running the experimental `useLoonFFI=true`
with an L0 import job in flight at the moment of the upgrade, later
restored via snapshot. Since the pre-upgrade V3 L0 write path always
fails (#51148), no successfully-written V3 L0 data exists — only the
metadata tag can be stale. If this window ever materializes in practice,
the backfill is a one-operator change in the completion path
(`UpdateStorageVersionOperator(segID, StorageV2)` for L0).
## Test Plan
- [x] `make static-check`
- [x] `go test -tags dynamic,test -gcflags="all=-N -l" -count=1
./internal/datacoord -run
TestImportUtil_L0ImportUsesStorageV2WhenLoonFFIEnabled`
- [x] `go test -tags dynamic,test -gcflags="all=-N -l" -count=1
./internal/datanode/importv2 -run
'TestL0Import/TestL0ImportForcesStorageV2ForSyncTask'`
Note: after rebasing to latest master, the focused local tests could not
be rerun with the reused local C++ output because the local
`libmilvus_core` is older than current headers and misses
`_SegcoreSetPrefetchThreadPoolNum`. The same focused tests passed before
the rebase; CI should build with matching artifacts.
Signed-off-by: Yihao Dai <yihao.dai@zilliz.com>
## What
Enforce a function–field binding principle for function DDL (add / drop
/ alter function on an existing collection), so a function and its
output field stay coupled and already-stored vectors can never be
silently invalidated by DDL.
BM25 and MinHash follow the strict binding: add/drop only via the
unified `add_function_field` / `drop_function_field`, output field
always coupled. **TextEmbedding is a temporary carve-out** — its legacy
`AddCollectionFunction` / `DropCollectionFunction` paths are retained,
so attach-over-existing-field and detach are still possible for it,
because `add_function_field` embedding backfill is not yet implemented.
It will be folded into the unified path in a follow-up. So the "always
coupled" invariant currently holds for BM25/MinHash, not TextEmbedding.
## Changes
- **AlterCollectionSchema invariant**: reject standalone add-function (a
function must be added together with its new output field), reject
detaching a function without dropping its output field, and reject
function cascade (a new function's input being another function's
output).
- **Legacy RPCs**: `AddCollectionFunction` / `DropCollectionFunction`
are rejected for BM25/MinHash (must use `add_function_field` /
`drop_function_field`) and retained only for TextEmbedding (see
carve-out). `AlterCollectionFunction` is kept and gains the whitelist
below. In the alter and legacy-add paths, function field IDs are always
re-derived from field names (never trusted from the request), so a
request cannot inject an unrelated field ID that a later
`drop_function_field` would delete.
- **alter_function whitelist**: only connection/runtime params may
change (TextEmbedding: `url`, `credential`, `timeout_ms`,
`max_client_batch_size`, `region`, `location`, `projectid`, `user`);
BM25/MinHash have no alterable params. Function identity (type, name,
input/output fields) and output-shaping params (`dim`, `model_name`,
`endpoint` — the TEI model identity, `normalize`, `truncate*`, prompts,
...) are immutable. Params are normalized (keys lowercased, duplicate
keys rejected) so a crafted duplicate key cannot bypass the diff.
- **drop_function_field**: allow any output-producing function (dropping
needs no backfill), removing the previous BM25/MinHash-only restriction.
## Not in scope / follow-up
- Extending `add_function_field` to TextEmbedding (post-creation add
would require backfilling every existing row through the external
embedding model) — deferred; folding the TextEmbedding legacy paths into
the unified binding follows this.
- MinHash input-field analyzer immutability lives on a different DDL
path (`AlterCollectionField`); tracked as a separate follow-up.
## Breaking changes
- `DropCollectionFunction` on a **MinHash** function (detach — remove
the function but keep its output field) is no longer supported; use
`drop_function_field` instead (drops the function together with its
output field). If a released version supported MinHash detach, migrate
any such call to `drop_function_field`.
issue: #51348🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: MrPresent-Han <chun.han@gmail.com>
Co-authored-by: MrPresent-Han <chun.han@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
issue: #51418
- flushcommon syncmgr: skip the write-buffer panic callback for stale
growing-source meta errors while preserving failure metrics
- growing-source tests: cover stale channel and missing segment errors
without invoking the failure callback
---------
Signed-off-by: chyezh <chyezh@outlook.com>
Optimize the k-way merge by advancing the heap root in place instead of
popping and pushing it for every candidate. Use typed deduplication keys
for INT64 and VARCHAR primary keys, including element-level and group-by
search paths.
Skip late materialization when the search plan has no non-primary target
fields, while preserving the primary field for mixed output fields
required by proxy reranking.
Ensure submitted segcore output-field tasks are joined on exceptional
paths and add correctness and benchmark coverage for the optimized merge
implementations.
https://github.com/milvus-io/milvus/issues/51315
Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com>
Related to #51419
Update the Go client module path, imports, dependencies, tests,
examples, and documentation from client/v2 to client/v3.
Set the SDK version to 3.0.0 in preparation for the Milvus 3.0 release.
Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
## What this PR does
- Change the default `MILVUS_MINIO_ROOT_PATH` from `files` to `file` in
the Text LOB inline/object layout test.
- Keep the environment-variable override available for deployments using
a custom root path.
issue: #51257
## Verification
- `python3 -m py_compile
tests/python_client/milvus_client/test_milvus_client_text_lob.py`
- Targeted pytest node collection completed successfully.
- Verified the committed diff contains only the requested one-line
default change.
Signed-off-by: Eric Hou <eric.hou@zilliz.com>
Co-authored-by: Eric Hou <eric.hou@zilliz.com>
issue: #49156
## Summary
- Record `cost_time` / `cost_cpu_num` on DataNode index build tasks and
surface them through `QueryTask` response `Properties`.
- Reserve the unified `cost_time` / `cost_cpu_num` keys under
`pkg/taskcommon` so future PRs can wire up the other
`CreateTask`-dispatched task types.
- Introduce small `internal/datanode/taskcost` helper (`NowMs`,
`ElapsedMs`, `EstimateIndexBuildCPUNum`) for index build task cost
accounting.
- Final state/failReason and execution-end cost are written in a single
`TaskManager` critical section, and `QueryTask` reads them from a single
snapshot, so state and cost in one response are always consistent.
## Notes
This PR is producer-side only. DataCoord / proxy / metrics exporter do
not consume these properties yet; downstream consumption should land in
a follow-up linked from #49156.
`cost_cpu_num` is a coarse, statistics-only estimate for observability
(vector index ≈ knowhere build pool size = NumCPU in cluster mode;
scalar index = 1). It is not used for coordinator-side scheduling, so
the standalone pool-ratio deviation (NumCPU × buildIndexThreadPoolRatio)
and pool sharing across concurrent builds are intentionally not modeled.
Supersedes #49157 (that PR's head branch was rebased onto latest master
and force-pushed, so GitHub no longer allows reopening it). Content is
identical, rebased to a single commit on top of current master.
## Test plan
- [x] `go test ./taskcommon/...` from `pkg/`
- [x] `go test -tags dynamic,test -gcflags="all=-N -l" -count=1
./internal/datanode/taskcost`
- [ ] `go test -tags dynamic,test -gcflags="all=-N -l" -count=1
./internal/datanode/index -run
'TestIndexTaskSchedulerRecordsIndexTaskCost|Test_statsTaskInfoSuite/Test_IndexTaskCostMethods'`
- [ ] Manual sanity: `QueryTask` returns `cost_time` / `cost_cpu_num`
for DataNode index build task success and failure paths.
---------
Signed-off-by: cai.zhang <cai.zhang@zilliz.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
## Problem
During schema evolution, SQN can convert an insert payload with the old
schema and then race with a schema update before ProcessInsert creates
and writes the growing segment. If the old payload still contains a
dropped field, the native collection may already use the new schema and
segcore rejects the write with unexpected new field.
## Solution
- Add a collection-local schema-transition RW lock.
- Insert holds the reader lock from schema snapshot use through payload
conversion and delegator.ProcessInsert, so a growing segment is created
and written in the same schema epoch as its payload.
- Schema update and existing-collection load refresh paths take the
writer lock around native schema mutation and Go schema snapshot
publication.
- Acquire a temporary collection lease before releasing
collectionManager.mut. This keeps the collection alive while the writer
waits, without serializing unrelated collection-manager operations
behind a schema transition.
- Preserve dropped-field replay handling: when DDL has already
completed, payload conversion filters fields absent from the current
schema before inserting into a new-schema growing segment.
## Tests
- Add a deterministic native regression: pause after a v950 payload
containing field 580 is converted, queue the v951 drop-field update,
then resume a real NewSegment plus growing.Insert and verify both insert
and DDL succeed.
- Assert the transition reader covers both payload conversion and
ProcessInsert.
- Cover the DDL-first replay path, where field 580 is filtered before
native insertion.
- Cover both schema writer entry points: UpdateSchema and
existing-collection PutOrRef.
## Validation
- make static-check
- make build-go
- Native pipeline regression tests repeated 20 times with dynamic,test
- Collection schema-transition and lease tests repeated 20 times
- internal/querynodev2/pipeline and internal/querynodev2/segments
package tests
- gofumpt -l and git diff --check
Fixes#51436
Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com>
issue: #51493
### What
#50221 split the `status` label values of `milvus_proxy_req_count` /
`milvus_proxy_grpc_latency` in place. Released in **v2.6.19**, that
silently zeroes every existing dashboard panel and alert rule matching
`status="fail"` / `status="rejected"` — an alert that stops firing
rather than erroring.
This keeps the classification but moves it onto its own dimension,
restoring the status domain to what <= v2.6.18 emitted:
```
status: success | fail | rejected | retry | total | abandon (as before v2.6.19)
cause: user | system | cancel | na (new)
```
| v2.6.19 / v2.6.20 | this PR |
|---|---|
| `status="fail_input"` | `status="fail", cause="user"` |
| `status="fail_system"` | `status="fail", cause="system"` |
| `status="rejected_user"` | `status="rejected", cause="user"` |
| `status="rejected_system"` | `status="rejected", cause="system"` |
| `status="cancel"` | `status="fail"` or `"rejected"`, `cause="cancel"`
|
| `success` / `retry` / `total` / `abandon` | unchanged, `cause="na"` |
### Why a label instead of new status values
- **Old queries work unchanged and count each request once.**
`status="fail"` is again every hard failure; Prometheus aggregates over
`cause` for free. That rollup is what a label dimension *is* — the split
forces every consumer who just wants "how many failures" to hand-write
`status=~"fail_.*"`.
- **Cardinality is unchanged.** `cause` is functionally dependent on the
outcome, so the realized `(status, cause)` pairs are exactly the series
the split already produces. Additive, not multiplicative.
- **New consumers lose nothing**: alert on `status="fail",
cause="system"`; route `cause="user"` to the owning application team.
The alternative — emitting old and new `status` values side by side
during a transition — was rejected: it defers the break rather than
removing it (the old values must still be dropped eventually, forcing
the same migration later), and meanwhile double-counts every request in
unfiltered aggregations.
`cancel` is folded into `cause` for the same reason: before v2.6.19
client cancellations were counted as `fail` (cancel in the response
status) or `rejected` (cancel at the interceptor), so promoting it to a
`status` value quietly makes `status="fail"` under-count versus older
releases. As a cause it stays excludable via `cause!="cancel"` without
redefining `status`.
### Also in this PR
- The 7 `snapshot_impl` sites that emitted a bare `fail` with no
classification now report their real cause via `failMetricLabel(err)`
(e.g. `snapshot_metadata_uri is required` is `cause="user"`, not a
system fault). Their `status` is unchanged.
- `deployments/monitor/grafana/milvus-dashboard.json`: the "Faild
Request Rate" panel goes back to `status="fail"` — a verbatim revert of
what #50221 changed, which is the compatibility claim demonstrated.
- Dev docs and stale comments that named the old label values.
### Verification
- `TestParseMetricLabelStatusDomainIsStable` pins the status domain, so
re-splitting it fails CI rather than shipping.
- Adding a label makes every `WithLabelValues` site arity-sensitive **at
runtime, not compile time** — a missed site panics in production. Unit
tests do not reach all of them (coverage shows
`GetRestoreSnapshotState`, `ListRestoreSnapshotJobs`, `PinSnapshotData`,
`UnpinSnapshotData` at 0%), so all **58 emit sites were verified
statically via AST**, not only the ones tests happen to hit.
- Passing: `pkg/metrics`, `pkg/util/requestutil`, `pkg/util/merr`,
`internal/distributed/proxy` (incl. `httpserver`, which covers the REST
emit path), and the `internal/proxy` snapshot / metric-label tests.
`golangci-lint` clean on every touched package.
- Pre-existing and unrelated: `service_test.go` bind failures (`listen
tcp :19529: address already in use`) reproduce identically on unmodified
master — a local process holds the port.
### Rollout
Should be cherry-picked to 2.6 so the window in which the fine-grained
`status` values exist stays confined to v2.6.19–v2.6.20. Users who
already adopted the new values on those two releases need a one-time
query change (`status="fail_system"` -> `status="fail",
cause="system"`); that is a known, bounded set, and preferable to
leaving every pre-2.6.19 dashboard silently broken.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pr: https://github.com/milvus-io/milvus/pull/49922
## Summary
Route generic LIKE Match operations to raw-data scan when a varchar
inverted index is present, while keeping PrefixMatch on the
inverted-index path. Update planner policy and sealed-segment coverage
for a complex LIKE pattern.
Signed-off-by: aoiasd <zhicheng.yue@zilliz.com>
issue: #51376
## What this fixes
The in-place partial-materialization path of
`bump_schema_version_compaction` commits onto a live segment's manifest,
and DataCoord adopted the result on a version-newer check only — it
never verified the result was built on the segment's *current* manifest.
A bump built on a plan-pinned base could therefore be adopted after a
concurrent stats/index task advanced the manifest pointer, silently
dropping that index (correct version number, wrong content lineage). The
loss is permanent and non-self-healing, because the segment meta stats
placeholder stops the stats inspector from re-triggering.
## Fix — application-layer adoption CAS (mirrors the stats path)
- The schema-bump result carries the plan-pinned base manifest
(`CompactionSegment.base_manifest`, like `StatsResult.base_manifest`);
the DataNode fills it with the manifest it materialized on.
- DataCoord adopts the in-place result only if `result.base_manifest ==
segment.ManifestPath` (the current pointer); a missing or stale base is
rejected (`ErrIllegalCompactionPlan`) so the task re-triggers and
rebuilds on the current manifest instead of overwriting the concurrent
commit. This mirrors the stats path's `errStatsResultStale`.
- The storage commit stays on `OverwriteResolver` (unchanged). Because
the CAS is at the adoption layer (against the meta pointer, not
storage-latest), a result lost after a DataNode crash self-heals on
retry: the pointer stays put, so the retry re-pins the same base and is
adopted — no wedge.
- The bump policy is restricted to internal collections (matching every
sibling compaction policy), so a rejected attempt's orphan is reclaimed
by internal dropped-segment GC and bump never churns on external
collections.
## Verification
Both interleavings converge with no data loss: stats-first → bump's base
no longer matches the advanced pointer → reject + retrigger on the
current manifest; bump-first → stats' existing base-check rejects and
retries. Added meta-level adoption tests (matching/stale base) and
updated the in-place adoption tests to carry the base.
Signed-off-by: MrPresent-Han <chun.han@gmail.com>
Co-authored-by: MrPresent-Han <chun.han@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Related to #51419
- migrate required error and utility helpers into the client module
- preserve RPC error codes, retry metadata, and errors.Is semantics
- expose client-local RPC errors and common error sentinels
- remove github.com/milvus-io/milvus/pkg/v3 and server-only dependencies
- lower the client Go version to 1.24.9
---------
Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
## Problem
Milvus supports online collection-schema mutations — add/drop field,
add/drop function, alter field, enable dynamic field — through several
RootCoord DDL paths. Existing segments are written under the committed
schema, so a mutation that reinterprets an existing field (its data
type, nullability, or structural role) or relabels existing data as a
function output makes already-written binlogs read incorrectly. Before
this change there was no single authoritative check that a proposed
schema is a **safe structural successor** of the committed one;
validation was scattered and incomplete across DDL paths, so an unsafe
mutation could reach the WAL and silently corrupt or misinterpret
existing data.
## What this PR does
Adds an authoritative, validation-only, fail-closed gate
`ValidateSchemaEvolution(oldSchema, newSchema)`, invoked in every
RootCoord schema-mutating callback **before** any side effect (analyzer
reservation, bound-index allocation, WAL broadcast). It rejects:
- in-place field reinterpretation — name, data type, element type,
nullability, or structural role (primary / partition / clustering key,
autoID, dynamic);
- relabeling an existing field as a function output — function outputs
must use brand-new fields, never existing data;
- unsafe field additions — a newly added field may not arrive already
marked primary / partition / clustering key, function output, or
dynamic;
- unsafe drops of protected fields;
- more than one clustering key;
- a dynamic field that is also a function output;
- corruption of the reserved `max_field_id` watermark.
Property-only / TTL updates, analyzer rollback, BM25 / MinHash
`add_function_field`, and legacy function Drop / detach paths are
preserved unchanged.
## Scope
Non-function structural invariants only. Function-graph validation
(single producer, output binding, alter-time immutability, cascade) is
intentionally **out of scope here and owned by #51360**. This PR does
not include schema propagation, Proxy barriers, DML draining, backfill /
readiness, index promotion, TextEmbedding enablement, protobuf,
configuration, or C++ changes.
## Validation
- `go test -tags dynamic,test -gcflags='all=-N -l'
./internal/util/schemautil -count=1` — PASS (the gate's own unit tests)
- `git diff --check origin/master...HEAD` — PASS
issue: #51340
---------
Signed-off-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
issue: https://github.com/milvus-io/milvus/issues/51466
## What changed
`PhyUnaryRangeFilterExpr::DetermineExecPath()` previously used a
JSON-specific hard-coded denylist for string predicates:
- `Match`
- `PostfixMatch`
- `InnerMatch`
This bypassed the concrete scalar index's `ShouldUseOp()` policy. As a
result:
- indexes capable of evaluating these predicates, such as `STL_SORT`,
were
forced to scan and parse raw JSON;
- JSON string predicates could use a different execution policy from
ordinary
VARCHAR predicates;
- `RegexMatch` remained on the index path even for indexes whose planner
policy
explicitly preferred raw-data execution.
---------
Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com>
- Rebuild TiKV write transactions on retriable begin, build-stage IO,
and classified-transient commit failures while aborting deterministic
build failures and unsafe commit outcomes.
- Preserve original errors for TiKV undetermined commit results and
caller cancellation before string-wrapping; keep private
`retry.Unrecoverable` markers inside the retry helper so they never leak
to callers.
- Stop `ReliableWriteMetaKv` from replaying undetermined TiKV write
results, correct the TiKV begin/requestTimeout comment, and add
regression tests for commit aborts, retry exhaustion, deadline stopping,
and outer reliable-write behavior.
related: #51425
---------
Signed-off-by: shaoting-huang <shaoting.huang@zilliz.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Summary
Keep raw search distances through segment search, iterator output, index
query, and chunk merging so ranking/reduce order is not affected by
`round_decimal`.
Apply `round_decimal` only when producing the final search response,
after reduce/rerank/order-by has already determined result order.
Returned scores keep the requested precision without changing TopK
selection.
issue: #50347
---------
Signed-off-by: xianliang.li <xianliang.li@zilliz.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## What changed
- Replace Pod deletion with Chaos Mesh `container-kill` in CDC
force-promote and source-down failover tests.
- Kill every regular container in matching Pods while preserving the Pod
objects.
- Verify that Pod UIDs remain unchanged and each container's restart
count increases.
- Clean up the generated `PodChaos` resources after fault injection.
## Why
Deleting Pods changes their identity and tests Pod recreation rather
than container-level recovery. These scenarios need to verify CDC
failover behavior when containers restart inside the existing Pods.
## Impact
CDC failover tests now exercise container failures while explicitly
validating that Kubernetes does not recreate the affected Pods.
## Validation
- `ruff check` passed for all modified files.
- `ruff format --check` passed for all modified files.
- Python compilation passed for all modified files.
- Full CDC E2E was not run locally because it requires a Kubernetes
environment with Chaos Mesh.
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
issue: #45881
## What this PR does
External refresh previously read the segment row target directly from
each DataNode configuration during execution. That made the worker input
implicit and allowed task behavior to depend on the selected node.
This change:
- keeps `dataNode.externalCollection.targetRowsPerSegment` as the
existing configuration source;
- has DataCoord snapshot the configured value into the refresh task
request;
- adds `target_rows_per_segment` to the DataCoord-to-DataNode request;
- validates the transmitted value in DataNode;
- uses the request value for fragment splitting and segment balancing;
- preserves the MilvusTable one-fragment-per-segment behavior.
## Validation
- `make generated-proto-without-cpp`
- `make lint-fix`
- `make build-go`
- `go test -gcflags="all=-N -l" -race -tags dynamic,test
github.com/milvus-io/milvus/pkg/v3/util/paramtable -failfast -count=1
-ldflags="-r ${RPATH}" -run "^TestCachedParam$" -v`
- `go test -gcflags="all=-N -l" -race
-coverprofile=/tmp/refine-rowcount-config-dc.out -tags dynamic,test
github.com/milvus-io/milvus/internal/datacoord -failfast -count=1
-ldflags="-r ${RPATH}" -run
"^TestRefreshExternalCollectionTask_CreateTaskOnWorker$" -v`
- `go test -gcflags="all=-N -l" -race
-coverprofile=/tmp/refine-rowcount-config-dn.out -tags dynamic,test
github.com/milvus-io/milvus/internal/datanode/external -failfast
-count=1 -ldflags="-r ${RPATH}" -run
"^TestRefreshExternalCollectionTaskSuite$" -v`
- `git diff --check`
Signed-off-by: Wei Liu <wei.liu@zilliz.com>
issue: #51250
Keep TEXT segments on StorageV3 while making growing-source flush an
explicit allowed mode instead of a mandatory TEXT path.
Propagate create-segment storage versions through WAL flusher and write
buffer, reject storage-version mismatches in DataCoord, and keep raw
segcore chunks sticky for segments that may be flushed from growing
source.
Signed-off-by: luzhang <luzhang@zilliz.com>
Co-authored-by: luzhang <luzhang@zilliz.com>
relate: #50304
## Summary
Add the local format design doc, use the storage writer format constant,
and introduce local_format metadata propagation with column-group split
policy support.
---------
Signed-off-by: aoiasd <zhicheng.yue@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
issue: #51436
### What this PR does
- Propagates a CreateCollection RecoveryStorage observation failure from
the flusher component to WAL dispatch.
- Stops before creating a DataSyncService when the recovery state was
not recorded.
- Adds coverage for the canceled observation path, including the absence
of schema lookup.
### Verification
- `go test -v -tags dynamic,test -gcflags="all=-N -l" -count=1
./internal/streamingnode/server/flusher/flusherimpl -run
"TestWALFlusher.*CreateCollection"`
- `make static-check`
- `gofumpt -l` on changed Go files
- `git diff --check`
Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com>
issue: #50742
## What this PR does
Optimizes protobuf decoding on Milvus RPC read/write hot paths with a
hand-written decoder in `pkg/util/fastpb`. The final change contains no
gRPC connection-pool or connection-count changes.
## Fast decoder coverage
The gRPC `releaseCodec.Unmarshal` path dispatches these top-level
messages to fastpb:
- `internalpb.RetrieveResults`
- `milvuspb.InsertRequest`
- `milvuspb.UpsertRequest`
`schemapb.SearchResultData` is not a top-level codec-dispatched message.
It is decoded directly from `SlicedBlob` at the search/reduce hot paths
in:
- `internal/proxy/search_reduce_util.go`
- `internal/querynodev2/segments/result.go`
- `internal/querynodev2/local_worker.go`
- `internal/querynodev2/tasks/search_task_go_reduce.go`
Nested hot messages such as `FieldData`, scalar/vector arrays, and IDs
are decoded by the same package.
Main optimizations:
- varchar string arrays use one backing byte arena, reducing per-string
allocations
- packed float vectors use a single memcpy into self-owned memory
- cold, complex, unknown, and mismatched fields delegate or merge
through the official protobuf codec
- no new external dependency or source-buffer aliasing
## Compatibility boundary
The compatibility contract depends on the data path:
- **Untrusted client ingress (`InsertRequest` and `UpsertRequest`)**
validates proto3 strings as UTF-8 and is tested differentially against
`proto.Unmarshal`, including malformed input behavior.
- **Trusted internal result paths (`SearchResultData`,
`RetrieveResults`, and direct `FieldData` decoding)** assume canonical
Milvus/segcore protobuf output and intentionally skip UTF-8 validation
for performance. Therefore these paths do not claim equivalence for
arbitrary adversarial wire containing invalid UTF-8.
Within that boundary, the decoder preserves the protobuf behaviors
exercised by the differential tests:
- proto2 groups fall back to the official codec
- known fields with mismatched wire types fall back to the official
codec
- unknown and future fields are preserved through an official merge pass
- repeated singular messages merge from the raw wire, including
explicitly encoded proto3 default values
- repeated message-valued oneof members merge correctly, while different
variants remain last-wins
- mixed packed and unpacked scalar encodings preserve wire order
- descriptor field-set tripwires force decoder review when the protobuf
schema changes
The unsafe packed-float copy assumes the currently supported
little-endian Milvus targets such as x86-64 and arm64.
## Benchmarks
fastpb vs official `proto.Unmarshal`, using 1000-row payloads:
| path | payload | speedup | allocations/op |
|---|---|---:|---:|
| search / query | varchar | about 2x | about 1000-2035 to about 10-17 |
| search / query | float vector, dim 768 | about 6x | unchanged |
| insert | varchar with UTF-8 validation | about 1.8x | 1019 to about 10
|
## Verification
- unit and differential tests cover field descriptors, UTF-8 contracts,
wire-type mismatches, proto2 groups, unknown fields, packed/unpacked
ordering, oneof behavior, and repeated-message merge semantics
- randomized canonical-message equivalence tests cover search, retrieve,
insert, and upsert payloads
- `go test ./util/fastpb ./util/resource` passes
- `go vet ./util/fastpb ./util/resource` passes
- current full CI and end-to-end status is tracked by the PR checks
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
issue: #51446
- message properties: add an explicit `Unreplicable` marker for concrete
WAL messages and expose builder opt-in through `WithUnreplicable`
- replication: skip unreplicable messages in the CDC sender and
secondary replicate interceptor before callback replay
- DDL producers: mark unsupported snapshot, manifest backfill, and
external collection refresh broadcasts as unreplicable
- streaming docs: document the message-level skip marker and current
unsupported DDL replication behavior
Signed-off-by: chyezh <chyezh@outlook.com>
Related: #50236
- Reuse the initially loaded grantee keys/values when ListPolicy checks
legacy grantee ID ownership.
- Avoid reloading the full grantee-privileges tree once per legacy
grant.
- Add a regression test that verifies ListPolicy performs only one full
grantee-prefix load for legacy grants.
Signed-off-by: shaoting-huang <shaoting.huang@zilliz.com>
issue: #51248
## What changed
- correct the omitted-`target_size` case to assert ordinary manual
compaction semantics
- add a physical Force Merge target-size case using persistent segment
IDs and MinIO insert-log sizes
- add deterministic Go boundary coverage for grouping selection at the
configured threshold and threshold + 1
- add target-size memory clamp coverage for cluster and standalone
co-location modes
- extend Optimize format, signed-int64 boundary, loaded-segment refresh,
and live async lifecycle coverage
- add the async Optimize wrapper and update the PyMilvus test dependency
to 3.1.0rc64
## Why
The previous cases did not prove that an explicit Force Merge target
affected physical output, assumed the wrong grouping threshold, and
depended on manual log inspection for algorithm selection. Loaded
refresh and the public async Optimize workflow also lacked live
end-to-end coverage.
The async case exposed milvus-io/pymilvus#3680 on PyMilvus 3.1.0rc62.
Version 3.1.0rc64 contains the tuple-unpacking fix and passes the
unmodified test.
## Validation
- all 10 planned case IDs pass; 0 failed and 0 blocked
- `milvus-dev-cli` Go UT job
`go-ut-local-zhuwenxi-zhuwenxing-i-4082040-30413568` succeeded; both
test functions and all five subtests passed
- physical target-size L3 case passed against a real cluster and MinIO
in 200.68s
- loaded refresh, target format/boundary, and manual compaction cases
passed against the same server version
- async Optimize L3 passed unmodified with PyMilvus 3.1.0rc64 in 56.91s
- 56 Python nodes collect successfully; Python compilation, Ruff, and
`git diff --check` pass
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
Related to #51068
Store PK index slots and virtual PK offset maps in RuntimeResourceState.
Build Storage V1 and V2 PK indexes through the unified translator and
stage PK replacements until the final state publication.
Make PK lookup, range search, bulk subscript, and delete filtering use a
single published snapshot. Remove the sealed InsertRecord fallback and
clear stale PK resources when external segments become empty.
Add regression coverage for atomic PK replacement and zero-row external
segment cleanup.
Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
