## What this PR does
Strengthens compaction coverage in the PR gate and removes
false-positive and flaky paths.
- preserves terminal success/failure/timeout outcomes after task cleanup
and uses `EndTime` (with a legacy `StartTime` fallback) for post-finish
GC retention
- resolves target-based manual compaction through durable target
metadata: an executable ACTIVE target remains `Executing` across
reconciliation batches, INACTIVE completes only after every task is
classified, inert ACTIVE targets and unknown/GCed IDs remain undefined,
and `compact_id=-1` remains the explicit no-op sentinel
- keeps unknown task states and invalid cleaned terminal states
fail-closed even when their target is runtime-active
- refuses successful aggregation when any task state or cleaned terminal
outcome is unclassified, and covers INACTIVE targets with
still-executing tasks
- makes the Python compaction waiter consume structured plan counts with
context metadata and one monotonic deadline across raw RPC attempts,
reconnect, and backoff
- restores channel recovery for `UNAVAILABLE` through the synchronous,
timeout-aware SDK `reconnect()` interface without leaving a recovery
worker that can mutate the handler after timeout
- polls compaction-eligible segments through a single raw
`GetPersistentSegmentInfo` RPC per observation, preserving call context
and status mapping without the SDK retry decorator reusing a stale
timeout
- retries transient gRPC and rate-limit failures within the remaining
deadline, and recognizes both current `code=600` and legacy
`error_code=SegmentNotFound` wire formats
- retries recoverable `Status.CONNECT_FAILED` reconnect-handshake
failures in segment polling and PK pagination within the existing
deadline/backoff; non-recoverable handshake errors still propagate
- disables auto compaction before topology/concurrency setup and
validates MixCompaction source/target multiplicity, exact topology, row
count, PK uniqueness, and exact PK set
- proves explicit ForceMerge `target_size` behavior with a fixed
`segment.maxSize == 64MB` fixture: equivalent datasets must produce 3
outputs at 64MB and 2 outputs at 128MB
- exposes finite ForceMerge QueryNode/DataNode memory factors in
DataCoord system metrics; non-finite dynamic values fall back to the
default before serialization
- reads the unfiltered system topology and skips the target-size
comparison when QueryNode/DataNode metrics are erroneous or the observed
machine-safe ceiling cannot preserve the larger effective target
- directly regression-tests the L3 machine-safe wrapper with healthy
typed nodes plus an untyped metrics error, preventing a future return to
filtered topology input
- verifies every planned ForceMerge source leaves the active topology,
every exposed target becomes active, the complete PK set survives, and
physical rewrite bytes are conserved
- releases and reloads the collection before MixCompaction and
ForceMerge PK validation, then verifies the complete loaded segment set,
retired-source exclusion, and node assignments so readable old inputs
cannot mask missing compaction outputs
- submits reload asynchronously through single-attempt SDK calls and
polls loading progress with fresh remaining budgets; release, load
submission, progress, and topology checks share one monotonic deadline
- replaces `QueryIterator` validation with deterministic PK-range pages;
every raw query RPC receives a freshly calculated remaining timeout,
retains MilvusClient `cluster_id`, and retries transient gRPC/rate-limit
failures within the same total deadline
- verifies reconnect receives only the remaining pagination deadline, an
over-budget reconnect cannot issue another query RPC, ignored gRPC codes
and real rc83 non-rate-limit business statuses fail fast, and both
current and legacy `(code=0, compatible_code=49)` rate-limit mappings
retry within budget
- preserves valid ForceMerge QueryNode/DataNode memory factors, while
non-finite values fall back and all finite factors below 1—including
positive fractional values—clamp to 1
- sustains checked insert/CreateIndex/search workloads across compaction
and propagates every worker failure, including the last in-flight
request during cleanup; an Executing sample counts as overlap evidence
only when the same workload call spans the observation
- waits for two matching Flushed/L1/sorted source snapshots before the
index workload, then expects unselected sources plus Mix targets;
selected sources must retire and every loaded segment must have an index
and node assignment
- validates final PKs, indexed targets, and search results before
marking an unobserved overlap window as insufficient coverage (pytest
skip); repeated CreateIndex requests establish request concurrency, not
repeated index builds
- validates complete binary-search results against exact Jaccard ground
truth
- adds DataCoord aggregation/target lifecycle UT and compaction
stats-loader UT coverage
`CompactionMergeInfo` exposes one legacy `target` field although a
ForceMerge task may split into multiple result segments. Split-output
validation therefore requires each exposed target to be active and
separately requires every planned source to be retired; complete
active-output multiplicity is verified through the persistent-segment
topology.
Fixes#53272
## Test plan
- [x] Partial-plan review follow-up (2026-10-22): 82 focused local tests
passed with Python 3.12 / PyMilvus rc98, plus Ruff lint/format and diff
checks. The regression invokes the actual index E2E case with an 8-of-9
Mix plan: the prior implementation times out, the fix accepts the valid
indexed target plus unselected source. Negative cases reject retained
selected sources, missing/unexpected/unindexed/unassigned segments, and
missing PKs; the ORM adapter exercises sorting convergence and context
preservation.
- [ ] Live index concurrency rerun is blocked: both Dev CLI
deployment-list and `deploy status compaction-pr-ci` requests return
HTTP 500. No live-cluster pass is claimed for this follow-up.
- [x] Review follow-up on 2026-10-22: Python 3.12 / pinned PyMilvus
3.1.0rc98 focused suite: 72 passed; Ruff lint/format and diff whitespace
checks passed; all three revised concurrency E2E cases collect
successfully. Fault injection covers fast completion, unobserved
overlap, worker failures before and after terminal observation, and
cleanup. Real SDK loading regressions fail on the previous
implementation (5-second budget returns at t=8) and pass with fresh
per-poll budgets.
- [ ] Re-run the three sustained-workload cases and release/reload
integrity cases against a live cluster; this follow-up has local
regression coverage only.
- [ ] Full `make verifiers` attempted on 2026-10-22 but stopped in
`build-3rdparty`: this macOS host lacks a supported LLVM 14–18
toolchain.
- [x] Review follow-up on 2026-10-20: Python 3.12 / pinned PyMilvus
3.1.0rc98 focused suite: 56 passed; Ruff lint/format, Python syntax, and
diff whitespace checks passed. Reconnect regressions exercise the actual
SDK reconnect/readiness functions; topology tests inject
delayed/partial/stuck loading, reload failures, and late responses.
- [ ] Re-run live MixCompaction / ForceMerge integrity E2E with the new
release/reload validation; the review follow-up was verified locally
with fault-injection tests and server-path inspection.
- [x] Go formatting, Python 3.12 compile, Ruff check/format, protobuf
submodule compile, and `git diff --check`
- [x] Local Python 3.12 waiter/raw-adapter/pagination/machine-safe
matrix: 38 passed
- [x] Local focused paramtable + metrics UT: `go test -tags dynamic,test
-gcflags='all=-N -l'`
- [x] Dev CLI exact non-finite memory-factor/serialization UT:
`go-ut-local-yanliang-test-compaction-pr-5aa7d24-ce47cff0-u3805`
- [x] Dev CLI exact full memory-factor preservation/fallback matrix:
`go-ut-local-yanliang-test-compaction-pr-fab7e57-ce47cff0-u3805`
- [x] Dev CLI exact fractional-boundary memory-factor matrix:
`go-ut-local-yanliang-test-compaction-pr-3152b2a-ce47cff0-u3805`
- [ ] Dev CLI targeted retry L0 could not start pytest because the
runner Pod could not resolve the COS source host:
`pysdk-e2e-local-yanliang-test-compaction-d642871-420126f0-u3805`; the
same 3-case rc83 test passes locally
- [x] Dev CLI current server image build:
`build-local-yanliang-qia-test-compac-5aa7d24-bf331245b298-u3805`
- [x] Deploy
`harbor.tencent-stage.zilliz.cn/manta/milvus:local-u3805-test-compaction-pr-ci-coverage-5aa7d24-bf331245b298`
to `compaction-pr-ci`; verify `Healthy` and `updated=true`
- [x] Dev CLI current DataCoord + metrics UT:
`go-ut-local-yanliang-test-compaction-pr-d3b086b-57710439-u3805`
- [x] Dev CLI exact GC-retention suite:
`go-ut-local-yanliang-test-compaction-pr-c9996b9-29281152-u3805`
- [x] Dev CLI full `internal/compaction` + `internal/datacoord` Go UT
for the broader PR changes:
`go-ut-local-yanliang-test-compaction-pr-7b125fe-6ba90598-u3805`
- [x] Dev CLI real no-op trigger plus unknown-ID E2E (3/3):
`pysdk-e2e-local-yanliang-test-compaction-710c3bc-cef493b1-u3805`
- [x] Dev CLI exact binary-search validation (1/1):
`pysdk-e2e-local-yanliang-test-compaction-0728a4d-714e12a8-u3805`
- [ ] Run the final fixed 64/128MB ForceMerge topology/full-PK E2E. The
latest local-source runner still fails before pytest because its Pod
cannot resolve the COS source host:
`pysdk-e2e-local-yanliang-test-compaction-5aa7d24-4e733021-u3805`; the
PR-source channel independently cannot resolve GitHub:
`pysdk-e2e-pr-53273-5aa7d24-8424db8b-u3805`
- [ ] Re-run the final Mix multiplicity assertion and insert/index
result-binding cases after the same source-runner DNS incident is
resolved
## Notes
Upgrade, chaos, performance, and experimental-configuration coverage
remain lower-priority follow-up work.
Signed-off-by: Yanliang Qiao <yanliang.qiao@zilliz.com>
### What problem
master e2e never covers two schema-bump additive intersections: (1) an
`add_function_field` whose input VARCHAR was itself added via
`add_collection_field` after the data was sealed (the exact crash shape
of issue #52159); (2) an `add_function_field` bump triggered while the
collection already holds a TEXT LOB column. Today
`test_add_function_field_feature.py` uses only create-time VARCHAR
inputs (no `add_collection_field`), and `test_milvus_client_text_lob.py`
has no `add_function_field` at all — the two concerns never meet.
### What this does
Add two pymilvus e2e cases into the existing files (reusing their
helpers, no new file):
- `test_add_bm25_function_field_on_backfilled_varchar_input`: seal rows
without the varchar, `add_collection_field(VARCHAR)`, then
`add_function_field` BM25 whose input is that added field. Asserts: old
rows read back null; sibling columns (code/vec) are byte-exact per row
against the inserted values; schema_version increments across both
bumps; null-input rows are never matched by BM25.
- `test_text_lob_add_function_field_bump_preserves_lob`: sealed TEXT LOB
rows in four states (external/inline/empty/null) plus an
`add_function_field` BM25 bump. Asserts: every `content` payload is
preserved byte-exact; sibling columns (tag/vector) stay intact per row;
a post-bump insert is still exact.
Add base helper `wait_for_schema_version_consistency`: gate bump
completion on the DataCoord-emitted `schema_version_consistent_segments
== schema_version_total_segments` stats (via `get_collection_stats`)
instead of a fixed sleep; combined with a `describe_collection`
schema_version increment assertion so the gate cannot pass on the
pre-bump state.
### Dependency / notes
- Depends on the master fix landing first: `pr: #52484` (issue #52159).
Until it merges,
`test_add_bm25_function_field_on_backfilled_varchar_input` reproduces a
`panic: no such field` on the datanode (exactly the regression this test
guards), so CI is expected RED — this is intended.
- Validated locally with a with-fix / without-fix two-arm comparison:
the without-fix binary reproduces the panic (RED), the with-fix binary
passes both tests (GREEN). External LOB goes through the `.vx` ref and
the bump uses REUSE_ALL (LOB path unchanged), both confirmed against
on-disk storage.
- Requires `dataCoord.compaction.bumpSchemaVersion.enabled: true`
(already set in the three e2e helm values).
- Related enhancement tracking: #51649.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Also folds in a one-line repair of a stale L2 assertion (issue: #52878):
#52219 intentionally dropped the computed expected dim from the minhash
dim-mismatch message (overflow-safe division check), so the assertion no
longer expects the literal `512`.
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 <noreply@anthropic.com>
issue: #52114
Makes `ruff check` and `ruff format --check` pass over all of `tests/`,
so the `Ruff (changed files only)` job stops failing PRs for violations
they did not introduce (most recently #51903, which got 69 errors from
an 8-line change).
Almost all of this is mechanical. The behavioural changes are called out
individually below: one `NameError` fix, three shadowed duplicate
definitions, 13 assertion gaps, and two undefined names that only became
visible once star imports were expanded.
### Layout
Seven reviewable commits:
| commit | what | files |
|---|---|---|
| 1 | `ruff format` | 170 |
| 2 | `ruff check --fix` (safe fixes only, no `--unsafe-fixes`) | 156 |
| 3 | F821 fix + `tests/ruff.toml` | 4 |
| 4 | three shadowed duplicate definitions | 4 |
| 5 | close 13 assertion gaps behind F841 | 7 |
| 6 | expand star imports, re-enable F403/F405/F821 | 33 |
| 7 | enable E402, E712, E722 | 16 |
1745 violations → 0. Only `tests/` is touched.
### The undefined-name bug
`chaos/test_chaos_memory_stress.py`:
`TestMemoryStressReplica.prepare_collection` referenced bare `nb` and
`dim`. Those are class attributes, never module-level names, so the
fixture raised `NameError` the moment it ran — and the line directly
above it already used `self.dim`. Changed to `self.nb` / `self.dim`.
This was the only F821 in `tests/`; **the rule stays enabled**.
### Three shadowed duplicate definitions
Three places define the same name twice in the same scope. Python keeps
the later one, so the earlier definition is dead while still reading as
live code — a test that looks like it exists and never runs.
**`restful_client/testcases/test_vector_operations.py` —
`TestDeleteVector`**
```python
@pytest.mark.L0
class TestDeleteVector(TestBase): # line 778 — shadowed, never ran
def test_delete_vector_default(...) # 2x2 parametrized
@pytest.mark.L1
class TestDeleteVector(TestBase): # line 838 — wins
def test_delete_vector_with_invalid_api_key(...)
def test_delete_vector_with_invalid_collection_name(...)
def test_delete_vector_with_non_primary_key(...)
```
The second holds only negative cases, and the file already uses
`TestInsertVectorNegative` / `TestSearchVectorNegative`, so this is a
copy whose class name was never updated. Renamed it to
`TestDeleteVectorNegative`.
**This is the one to look at:** it brings back an `L0` case, so 4
parametrizations of `test_delete_vector_default` start running in PR CI.
They have never executed, so if the restful delete path has drifted
since they were written, this is where it surfaces.
**`python_client/testcases/test_compaction.py` —
`test_compact_after_delete`**
Defined twice in `TestCompactionParams`, covering different things: the
shadowed one (L3, `@parametrize("delete_pos", [1, tmp_nb // 2])`) covers
*partial* deletes plus a compaction-plan check; the surviving one (L2)
deletes everything. There is already a separate
`test_compact_after_delete_all`, so partial-delete coverage existed only
in the shadowed case and was not running. Renamed it to
`test_compact_after_delete_partial`; the surviving definition is
untouched.
**`python_client/utils/util_pymilvus.py` — `add_vector_field`**
Defined twice with different signatures:
```python
def add_vector_field(entities, is_normal=False): # line 455 — shadowed
...
entities.append(field)
return entities
def add_vector_field(nb, dimension=default_dim): # line 527 — wins
field_name = gen_unique_str()
field = {...} # built, then discarded
return field_name # returns only the name
```
Removed the second — both the broken one and the one that was winning.
Grepped the whole repo: **neither definition has a caller**; every
`add_vector_field` hit is something else
(`test_milvus_client_add_vector_field` test methods, the
`Op.add_vector_field` enum member in `chaos/checker.py`, a `Checker`
method of the same name). Dead-code removal, not an API change.
**Net effect on what executes:** `test_delete_vector_default` ×4
(**L0**) and `test_compact_after_delete_partial` ×2 (L3) start running;
nothing stops running; the 3 negative cases keep running under the new
class name. Computed by parsing each file before and after and resolving
shadowing the way Python does, not by reading the diff.
F811 is now clean across `tests/` with no per-file suppressions.
### Star imports expanded, F403/F405/F821 re-enabled
33 of 307 files used `from utils.util_pymilvus import *` and friends.
Inside those, pyflakes cannot tell a typo from a star-imported name — it
downgrades everything unresolved to F405, and **F821 never fires**. The
`NameError` fixed in commit 3 was only catchable because that file
happens not to use a star import.
The 379 F405 hits were only **58 distinct names**. All are now explicit,
resolved by walking the star modules in source order (later import wins,
matching Python). Names that `util_pymilvus` merely re-exports come from
their real source now — `DataType` from `pymilvus`, `np` from `numpy`,
`log` from `utils.util_log` — rather than being laundered through it.
Turning F821 on immediately caught **two real undefined names**, both in
`rate_limit/test_rate_limit.py` (L3, so it has never run in CI):
```python
for collection_num in range(collectionRateLimit): # no such name
# ^ the parametrize arg is `collection_rate_limit`, and it is
# a string ("10"/"500"), so range() needs int().
# The same test uses the correct name 30 lines below.
mil.uninstall(release_name, namespace=NAMESPACE) # no such name
# ^ MilvusOperator.uninstall has exactly this signature, and other suites do the
# same under `mic`/`milvus_op` — the instantiation was just missing.
```
Both fixed. `test_milvus_client_data_integrity_new.py` needed 15 `noqa`
markers instead: its `prepare_data` fixture injects those names via
`globals()[field] = ...`, which no static analysis can follow. That
block has other problems — a `float is None` check against the builtin,
an always-true `if "is null" or "IS NULL" in expression`, leftover
`binbin_debug1` logging — but rewriting fixture logic does not belong
here; noted in #52116.
### 13 assertion gaps closed
Of ~80 F841 hits, most are harmless scratch values. 13 were a call whose
result was bound and then never checked. Highlights:
- `test_milvus_client_database.py`: two tests apply a non-existent
property key (`data.replica.number`, note `data.` not `database.`),
describe the database, and drop it without checking anything. Their
docstrings say `actual: run successfully` / `nothing changed` — the
intent was to prove nothing changed. Now asserted.
- `test_milvus_client_alias.py`: two assertions were commented out,
**and they were commented out for a reason** — `list_aliases()[0]`
returns a dict, so `assert alias in aliases` would have tested dict
keys. Rewritten against `[0]["aliases"]` (how the rest of the file does
it) and re-enabled. In the skipped `alias_default` test the calls were
also missing the required `collection_name` argument.
The remaining ~65 F841 stay, and the rule stays off.
### E402, E712, E722 also enabled
**E722** (37 sites): mechanical `except:` → `except Exception:`. Checked
the two ways this could change behaviour, both empty: no bare-except
try-block can raise `SystemExit`/`KeyboardInterrupt`, and **none calls
`pytest.skip`/`fail`/`xfail`**. That second one matters — pytest's
`Skipped` derives from `BaseException`, so a bare except silently eats
it while `except Exception` would not. 0 of 37, so behaviour is
unchanged.
**E402** (5 sites): imports that were just in the wrong place — `import
re` sat 70 lines into `check/param_check.py`, and the two
`resource_group` modules imported `rich` after their module-level
constants. Nothing to do with `sys.path`, which is what an earlier
version of this description wrongly claimed.
**E712** (5 sites), gone through one at a time rather than
bulk-rewritten:
| site | value | change |
|---|---|---|
| `test_milvus_client_partition.py` | `has_partition()` | → `is False` —
stricter than `not result`, which would pass on `None` |
| `test_restart.py` | JSON status flag | → `is True` |
| `test_restart.py` | local `break_flag` | → plain truth test, it is a
real bool |
| `test_query.py` | wrapper `check_result` | → `is True` |
| `test_milvus_client_ttl.py` | `hit.get(bool_field)` | **kept as `==
False` + noqa** |
That last one is the reason this rule cannot be bulk-fixed: `.get()`
returns `None` for a missing field, so `not ...` would pass exactly
where the assertion should fail, and `is False` would break on a numpy
bool.
### What ruff.toml still disables
| rule | n | reason |
|---|---|---|
| F841 | ~65 | tests deliberately keep call results; the 13 that were
genuine assertion gaps are fixed above |
| UP031 | 95 | pure style rewrite that catches no bugs, and 27 sites use
`% (...)` tuples where it is not equivalent |
Everything else pyflakes/pycodestyle checks is on, including `F401`,
`F811` and `F821`, so new code cannot introduce dead, duplicate or
undefined names. Tracked in #52116.
### Verification
Not just "ruff is green" — the mechanical passes were checked for
semantic drift:
- **Format pass:** parsed all 307 files before and after and compared
ASTs with string constants normalized away → **zero structural
differences**. The only AST-visible deltas are 146 string-constant
changes, every one whitespace-only (docstring indentation / trailing
whitespace); zero non-whitespace string changes.
- **Import removals:** 142 imports were dropped. For each, checked
whether the bound name still appears anywhere in its file. 115 have no
remaining reference at all. The 27 that still matched textually were
each read by hand and are all comments (`# pdb.set_trace()`, the
commented-out checker dict in `test_load_with_checker.py`), docstring
prose (`"input random collection name"`, `"Tuple[List[Dict]]"`), string
literals (`"Collection ID"`, the `"config"` dict key), a different
binding (`from time import sleep`), or attribute access through another
module (`ct.CaseLabel`, `np.random`). **No real reference was removed.**
- **Python version:** the pyupgrade rewrites emit `datetime.UTC` (3.11+)
and `X | None` (3.10+). `tests/.python-version` is 3.12,
`tests/scripts/python-env.sh` defaults to 3.12, and
`tests/docker/Dockerfile` is `python:3.12-bullseye`.
- **`compileall`** over all 307 files under Python 3.12 passes.
- No external reference to `TestDeleteVector`,
`test_compact_after_delete`, or `add_vector_field` exists anywhere in
the repo (`*.py`, `*.sh`, `*.yaml`, `*.json`).
### Conflict surface
Of the 100 currently-open PRs, 9 touch `tests/*.py` and only **2**
overlap with this change: #51903 (3 of its 5 files) and #51856 (1 file).
The other 7 touch files that were already format-clean. Worth merging
promptly to keep that number small.
`.pre-commit-config.yaml` needs no change — it already runs with
`--config=tests/ruff.toml`, so local hooks and CI stay in sync.
---------
Signed-off-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.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>
## What
- Add drop field e2e coverage for MilvusClient.
- Add function field e2e coverage.
- Refine schema evolution e2e coverage.
- Bump pymilvus test dependency to 3.1.0rc49.
## Test
- Not run in this turn; PR contains Python e2e test coverage changes.
---------
Signed-off-by: lyyyuna <yiyang.li@zilliz.com>
### What this PR does
Adds Python client and REST coverage for struct array nullable fields,
including dynamic field scenarios and nullable vector config variants.
### Tests
Not run; PR creation only.
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
Related to #50416
### What changed
- Add MilvusClient E2E coverage for external table
`add_collection_field` on scalar and vector fields.
- Cover refresh/load/search/query behavior after adding fields,
including about 10% NULL values in newly added scalar/vector fields.
- Add non-Parquet source coverage for add scalar field on Lance,
Iceberg, and Vortex.
- Add full supported DataType matrix coverage for external add field,
including scalar, array, geometry, and vector fields.
- Add negative coverage for missing/duplicate `external_field`, type/dim
mismatch, unsupported public data types, add/drop function, and drop
field APIs.
- Update Python client test dependency pin to `pymilvus==3.1.0rc35`.
### Verification
- `python -m py_compile
tests/python_client/milvus_client/test_milvus_client_external_table.py
tests/python_client/common/external_table_common.py
tests/python_client/base/client_v2_base.py
tests/python_client/check/func_check.py`
- `python -m ruff check
tests/python_client/milvus_client/test_milvus_client_external_table.py
tests/python_client/common/external_table_common.py
tests/python_client/base/client_v2_base.py
tests/python_client/check/func_check.py`
- `python -m pytest -n 4
milvus_client/test_milvus_client_external_table.py --host 10.104.18.101
--port 19530 --minio_host 10.104.18.27 --minio_bucket yanliang-mas2
--tb=short -q -s`
Result:
```text
182 passed, 1 xfailed in 318.94s (0:05:18)
```
The xfailed case is linked to #50416 and validates that old-field search
still works before the server returns the current internal QueryNode
assert for newly added fields before refresh.
---------
Signed-off-by: Yanliang Qiao <yanliang.qiao@zilliz.com>
issue: #47420
## What this PR does
Project-wide migration of raw `fmt.Errorf` / `errors.New` in function
bodies onto
the `merr` framework, plus the Sys-vs-Input error classification and the
machinery it drives (retriability, fine-grained metrics, segcore
unification),
plus the convention docs and a linter that keeps it from regressing.
Scope: storage, proxy, coordinators (root/data/query), query node, data
node,
`pkg/util` & `internal/util`, expression parser, message queue,
streaming, and
misc packages. Bare raw-error usages went from ~3000 to a ~340 allowlist
(package-level sentinels / build-tag / test sites).
---
## How to review this PR
It is large but the vast majority is mechanical. Changes fall into three
tiers;
spend review budget on Part 2 and Part 3.
### Part 1 — Mechanical standardization (low risk, verify by rule)
Each converted call follows one of a small fixed set of rules. To
review, check
that each site obeys the matching rule rather than reading every line:
| Pattern | Rule |
|---|---|
| `fmt.Errorf("...")` originating a new error | →
`merr.WrapErrXxxMsg("...")` with a code matching the failure's meaning |
| Adding context to an existing typed error | → `merr.Wrap(err, "...")`
/ `merr.Wrapf(...)` — **preserves** the inner code (never `WrapErr*Err`,
which overwrites it) |
| Errors inside the streaming subsystem | → `status.New*` factories
(StreamingError), **not** merr — this is the component-internal dialect
(see `docs/dev/error_handling_guide.md`) |
| Low-level / control-flow signal caught by `errors.Is` | → kept as a
package-level `errors.New` sentinel (lowercase, same-package) |
Conventions are documented in `docs/dev/error_handling_guide.md`
(how-to) and
`docs/dev/error_sentinel_convention.md` (rules + audit). A
`gocritic`/`ruleguard`
rule (`rawmerrerror`, in `rules.go`) enforces "no raw `return
errors.New/fmt.Errorf`"
under `make verifiers`.
### Part 2 — Behavior changes (review these closely)
These are the sites where the wire contract or runtime behavior changes,
not just
the source text. Listed by category; representative locations given,
full set in
the diff.
**A. gRPC wire-code shifts: `UnexpectedError(1)/Code 65535` → typed
code.**
Where a handler previously returned a raw error (collapsed to
`Code=65535` on the
wire), it now returns a typed merr, so the client sees a real code. The
most
common shift is to `IllegalArgument(5)/Code 1100` (ParameterInvalid).
Touch
points include datanode task handlers (CreateTask/Query/Drop), proxy
Upsert,
querynode GetMetrics, datacoord CreateIndex, httpserver query-response
builder,
and typeutil schema validation. One code refinement: an index-param
validation
moved `1100` → `1101` (ParameterMissing). **Client/SDK assertions and
any code
that switched on `Code=65535` for these paths must be re-checked** (the
go_client
e2e assertions were already aligned in this PR).
**B. Prometheus `status` label contract change (externally visible).**
The proxy metric's coarse `fail` / `rejected` values are split into
`fail_input` / `fail_system` and `rejected_user` / `rejected_system` (in
`requestutil.ParseMetricLabel`; auth/privilege rejections count as
`rejected_user`), so dashboards can attribute a failure to caller vs
operator.
**Dashboards/alerts querying `status="fail"` must migrate to
`status=~"fail_.*"`, and `status="rejected"` to
`status=~"rejected_.*"`.** The
in-repo Grafana dashboard is already migrated; external dashboards built
on the
old values silently go empty after upgrade. This is the one change that
requires an ops-side migration.
**C. Retriability semantics.**
- C1: `merr.Status(err)` now forces `Retriable=false` when the error is
an
`InputError` — a malformed request can never succeed on blind retry, so
clients
never get the self-contradictory "your input is wrong but you may
retry".
- C2: `retry.Do` short-circuits an `InputError` (non-retriable) — **but
only when
the caller did not pass a `RetryErr` predicate**. The check is an
`if c.isRetryErr != nil { ... } else if InputError { ... }` *mutually
exclusive*
branch (`pkg/util/retry/retry.go`): an explicit `RetryErr` takes
precedence and
bypasses the InputError abort. `retry.Handle` deliberately does **not**
apply
the InputError abort (its callers signal abort via `shouldRetry=false`).
Four
flusher startup callsites that must retry through transient "not ready"
errors
were given explicit `RetryErr` escape hatches.
**D. segcore (C++→Go) error classification.**
A single shared Go-side table (`pkg/util/merr/segcore.go`) maps each
segcore code
to a merr sentinel + InputError/signal category, replacing scattered
hand-written
`if errorCode == ...` switches in the cgo wrappers. **Wire `Code` values
change
for every segcore pass-through error, not just the remapped ones.**
Named
sentinels remap (C++ `2003` → merr `2001`, `2033` → `2002`,
Folly/Knowhere codes
likewise); **all remaining pass-through codes (`2004`–`2043`, previously
surfaced to clients as raw C++ enum values) now serialize as `2000`**
(`ErrSegcore`), with the original C++ code preserved in the `Reason`
text
(`segcoreCode=...`); unknown/future codes collapse to `2000` as well
(pinned by
the `wire_code_projection` test). Transient segcore classes (object
storage /
file IO / OOM / mmap / FieldNotLoaded — 11 codes) now report
`Retriable=true`.
**Any client switching on raw segcore codes in the `2004`–`2043` range
must be
re-checked**; the in-Reason code remains available for diagnostics.
Signal
codes (PretendFinished / FollyCancel) are recognized centrally.
`errors.Is`-based
control flow on these (e.g. scheduler skip/retry) is preserved.
**E. InputError classification (25 sentinels + dynamic marks).**
25 sentinels in `errors.go` carry `WithErrorType(InputError)` (the
Collection /
ResourceGroup / Database families, `ErrIndexDuplicate`,
`ErrParameterInvalid`,
`ErrPrivilegeNotAuthenticated`, `ErrImportFailed`, `ErrQueryPlan`, ...),
plus dynamic
marks for the 8 segcore input codes (ExprInvalid, DimNotMatch,
MetricTypeInvalid, FieldIDInvalid, ...) and
`WrapErrAsInputError`. The widest blast radius is `ErrParameterInvalid`
(1100):
~2335 `WrapErrParameterInvalid*` callsites now classify as input /
non-retriable. Because of C1/C2 this changes retriability for
any path that returns these. **The audit to confirm no transient path
was
mis-marked is the single most important review item** (see Part 3). One
reverse
correction: storage field-stats parsing moved from `ErrParameterInvalid`
(input)
to `ErrDataIntegrity` — a corrupted stored stat is data corruption, not
user
input.
### Part 3 — Known risks & traps (called out proactively)
1. **`merr.Wrap` vs `WrapErr*Err` (code-masking).** `WrapErr*Err` builds
a
`wrappedMilvusError{sentinel: ErrServiceInternal}` whose `code()`
returns the
*outer* sentinel — it overwrites the inner typed code and hides the
`errors.Is`
chain. This is intentional (use it to *deliberately* downgrade), but it
was a
recurring conversion defect; the rule "add context with `merr.Wrap`,
downgrade
with `WrapErr*Err`" is enforced by convention and reviewed across the
diff.
2. **InputError × `retry.Do` blast radius.** Marking a sentinel
`InputError` makes
any `retry.Do(...)` without a `RetryErr` predicate stop retrying it.
Reviewers
should sanity-check that no transient use of the 19 newly-marked
sentinels
(especially `ErrParameterInvalid`) sits inside a retry loop that needed
to keep
spinning. The known flusher cases were handled (see C2).
3. **The ~340 raw-error allowlist.** What remains as bare `errors.New`
is, by
design: package-level sentinels (caught by `errors.Is`), `//go:build
test`
sites, and out-of-band trees (`cmd/`, `tests/`, codegen, walimpls). The
linter
only bans the *direct-return* form; assignment-then-return escapes and
the full
no-exceptions ban are deferred to an AST-based linter (Tier 2,
documented).
4. **segcore C++ second step deferred.** This PR unifies classification
on the Go
side; splitting the dual-semantic C++ codes at the source is a
follow-up.
---
## Validation
- `make verifiers`: Go side clean (gofmt + static-check across modules,
including
the new `rawmerrerror` rule with a 0-hit baseline repo-wide).
- `make test-go`: passing; the one real regression introduced (a
datanode
`invalid_task_type` assertion shifting `1` → `5` from a ParameterInvalid
conversion) was fixed in-tree.
- go_client e2e CreateIndex assertions aligned to the new merr messages.
---------
Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
- Expand Python MilvusClient external_table E2E coverage for schema
validation, lifecycle, DQL, refresh, indexing, read-only/write-blocked
behavior, and multi-format datasets.
- Cover parquet, lance-table, iceberg-table, and vortex basic format
flows with shared data builders and stricter assertions.
- Add REST v2 external collection coverage for external job APIs,
nullable vectors, custom database flow, and format matrix behavior.
- Move Vortex data generation to the Python 3.12 test environment and
remove the old sidecar helper scripts.
- Add Python 3.12 environment setup in `tests/scripts` so E2E scripts
create and activate the expected test runtime.
## Notes
- Vortex full-matrix coverage excludes fields that the current
`vortex-data==0.56.0` writer or Milvus refresh path cannot represent
consistently (`FixedSizeBinary`-backed vector fields and known
string/JSON fields).
- Chaos fault-injection coverage is included in the PR but was not run
against the shared validation instance because it mutates the
deployment.
## Validation
- `uv venv -p 3.12 .venv`
- `uv pip install --python .venv/bin/python -r requirements.txt`
- `pytest -q -x -n 6 milvus_client/test_milvus_client_external_table.py
--host 10.100.36.207 --port 19530 --minio_host 10.100.36.172
--minio_bucket external-table-pr49378-master`
- `162 passed, 2 skipped in 176.78s`
- `pytest -q -x -n 6 testcases/test_external_collection_operations.py
--endpoint http://10.100.36.207:19530 --token root:Milvus --minio_host
10.100.36.172 --bucket_name external-table-pr49378-master`
- `22 passed, 4 xfailed in 66.04s`
- `git diff --check`
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary
- Align snapshot wrapper and test call signatures with the latest
PyMilvus snapshot API.
- Update Python client test dependency to `pymilvus==3.1.0rc5`.
- Make snapshot test cleanup safe under `pytest -n 6` by isolating
cleanup lists and tracking RBAC users/roles per test.
- Skip the `CreateSnapshot`/`DropCollection` race case and link it to
#49761.
- Remove stale xfail marks for snapshot RBAC privilege group cases now
covered by the fixed server behavior.
## Test plan
- `.venv/bin/python -m py_compile base/client_base.py
base/client_v2_base.py milvus_client/test_milvus_client_snapshot.py`
- `git diff --check`
- `.venv/bin/python -m pytest -q
milvus_client/test_milvus_client_snapshot.py --dist loadgroup -n 6
--host 10.100.36.198 --port 19530 --tb=short`
- `119 passed, 1 skipped in 293.48s`
- Cleanup audit after the full run:
- `collections []`
- `users ['root']`
- `roles ['admin', 'public']`
- `databases ['default']`
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
## Summary
- Add 40 E2E tests for FileResource CRUD APIs and analyzer integration
(jieba, synonym, stop words, decompounder)
- Verify tokenization correctness with baseline comparison (with vs
without file resources)
- Test data generated at runtime, no static files committed
/kind improvement
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary
- Add 15 new test cases for snapshot feature covering lifecycle edge
cases, alias support, and cross-database restore
- Verify fix for #47578 (DropSnapshot during active restore)
- All tests verified on latest master image
(`master-20260224-091b147e-amd64`)
### New Test Classes
| Class | Tests | Level | Description |
|-------|-------|-------|-------------|
| `TestMilvusClientSnapshotDropInvalid` | +1 | L1 | Drop during active
restore (#47578) |
| `TestMilvusClientSnapshotLifecycle` | +8 | L2/L3 | Collection
lifecycle + snapshot interactions |
| `TestMilvusClientSnapshotAlias` | +6 | L2 | Snapshot ops via
collection aliases |
### Test Details
**Lifecycle tests (L2):**
- Drop target collection during restore
- Rename source collection after snapshot creation
- Create snapshot on collection being restored to
- Restore failure leaves no resource leak
- Concurrent drop of same snapshot (idempotent)
- Create snapshot during drop of source collection
- Cross-database snapshot restore
- Drop and restore race condition (L3)
**Alias tests (L2):**
- Create snapshot via alias → describe shows real collection name
- List snapshots via alias == list via real name
- Restore from alias-created snapshot with data integrity
- List restore jobs via alias on restored collection
- Dropped alias → create snapshot fails with "not found"
- Alter alias retarget → list snapshots reflects new target
## Test plan
- [x] All 14 L1/L2 tests pass on standalone
(master-20260224-091b147e-amd64)
- [x] L3 race test works but may timeout on standalone due to resource
exhaustion
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
## Summary
- Refactor snapshot test cases to align with PR #48143 (bind snapshot
lifecycle to collection)
- Update base class helpers: add `collection_name` to
`drop_snapshot`/`describe_snapshot`, add `source_collection_name` to
`restore_snapshot`
- Remove 18 `@pytest.mark.skip` decorators since pymilvus SDK now
supports collection_name
- Update all snapshot API calls (58 drop, 4 describe, 46 restore) to
pass required collection_name
- Rewrite `test_snapshot_list_after_drop_collection` →
`test_snapshot_cascade_delete_on_drop_collection` for new cascade delete
semantics
- Add 3 comprehensive data type tests: struct array + BM25 text match +
MinHash, BFloat16Vector, all 8 array element types
## Test plan
- [x] L0: 2 passed (basic lifecycle + restore)
- [x] L1: 19 passed (invalid params + list/describe + restore state)
- [x] L2: 49 passed (data types, indexes, partitions, concurrency, data
ops, collection properties)
- [x] All 70 tests pass against master-20260416-e87cc36e (the commit
that introduced collection-scoped snapshots)
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary
- Migrate 50+ partition test cases from `testcases/test_partition.py`
(v1 ORM style) to `milvus_client/test_milvus_client_partition.py` (v2
MilvusClient style)
- Add `gen_default_rows_data_for_upsert` helper in `common_func.py` for
row-based upsert data generation
- Enhance existing v2 tests with partition name validation and `None`
partition name coverage
- Remove migrated test cases from v1 file, keeping only v2-incompatible
cases (description params, dataframe insert)
## Related Issues
#48048
## Test plan
- [x] v1 remaining tests pass on standalone (19530) and cluster (19531)
- [x] v2 full test suite passes on standalone (148 passed, 3 failed -
replica tests need cluster)
- [x] v2 full test suite passes on cluster (151 passed, 0 failed)
- [x] v2 tests pass with 8-worker concurrency (`-n 8`) on both
environments
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: lyyyuna <yiyang.li@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Related to #46199
## Summary
- Remove unused Python test dependencies: `ndg-httpsclient`,
`pyopenssl`, `pyasn1`, `pytest-parallel`, `timeout-decorator`
- Restore `pytest-assume==2.4.3` and `Unidecode==1.3.8` (actively used
in test code)
- Clean up dead `timeout_decorator` import and commented-out code in
`collection_wrapper.py`
## Performance impact
No production performance impact. All changes are in
`tests/python_client/` — test dependencies and test utility code only.
Milvus has no Python in its runtime (Go + C++ only).
| Change | Scope | Impact |
|--------|-------|--------|
| Remove 5 unused pip packages | Test `requirements.txt` | Faster test
env setup (~5 fewer packages to install) |
| Remove dead `timeout_decorator` import + commented-out code |
`collection_wrapper.py` | Cleaner test code, no behavior change |
## Test plan
- [ ] CI E2E tests pass (Python test suite runs successfully with
updated dependencies)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Signed-off-by: Li Liu <li.liu@zilliz.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Summary
- Add comprehensive test cases for Milvus snapshot API functionality
- Cover snapshot create, list, describe, drop, restore operations
- Test various data types, index types, partition scenarios
- Add concurrent operation and data integrity tests
- Update pymilvus to 2.7.0rc122 for snapshot API support
## Test Coverage
- L0 smoke tests: basic snapshot lifecycle
- L1 tests: data types, partitions, indexes
- L2 tests: boundary conditions, negative cases, concurrency
## Related Issue
https://github.com/milvus-io/milvus/issues/44358
## Test plan
- [x] Run tests with `-n 6` parallel execution
- [x] Verify all tests pass (67 tests: 62 passed, 4 xfail)
---------
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
- Refactor connection logic to prioritize uri and token parameters over
host/port/user/password for a more modern connection approach
- Add explicit limit parameter (limit=5) to search and query operations
in chaos checkers to avoid returning unlimited results
- Migrate test_all_collections_after_chaos.py from Collection wrapper to
MilvusClient API style
- Update pytest fixtures in chaos test files to support uri/token params
Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com>
related issue: #44425
1. split insert.py into a few files: upsert.py, insert.py,
partial_upsert.py ...
2. add test for allow insert auto id
---------
Signed-off-by: yanliang567 <yanliang.qiao@zilliz.com>
issue: #43590
Migrate duplicate collection name test cases from TestcaseBase to
TestMilvusClientV2Base
@yanliang567
---------
Signed-off-by: Orpheus Wang <orpheus.wang@zilliz.com>
related issue #40698
1. add diskann and hnsw index test
2. update gen_row_data and gen_column_data functions
---------
Signed-off-by: yanliang567 <yanliang.qiao@zilliz.com>
related issue: #40698
1. use vector datat types instead of hard code datatpe names
2. update search pagination tests
3. remove checking distances in search results checking, for knowhere
customize the distances for different metrics and indexes. Now only
assert the distances are sorted correct.
---------
Signed-off-by: yanliang567 <yanliang.qiao@zilliz.com>
1. gen collection name by testcase name
2. update search pagination test with milvus client v2
3. use collection shared mode for some tests
related issue: #40698
---------
Signed-off-by: yanliang567 <yanliang.qiao@zilliz.com>
yanliang567