mirror of
https://github.com/milvus-io/milvus.git
synced 2026-10-12 02:19:10 +00:00
master
4098
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 pymilvus-bot
|
1e943dc23b |
test: Increase PyMilvus version to 3.1.0rc92 from master for master branch (#54068)
Automated bump from pymilvus master branch to milvus master branch. Updates tests/python_client/requirements.txt. Signed-off-by: pymilvus-bot <[email protected]> |
||
|
junjiejiangjjj
|
8cfeb14750 |
test: Enable hybrid rerank chain test (#54041)
issue: https://github.com/milvus-io/milvus/issues/46565 Signed-off-by: junjie.jiang <[email protected]> |
||
|
Jamesandxiaofanluan
|
d1cb277f0c |
test: wait for compactable segments in struct partial-update lifecycle (#53964)
issue: #53962 The Struct Array partial-update lifecycle test can request manual compaction while its freshly flushed segments are still being sorted. DataCoord then returns the valid no-plan result `-1`, and the immediate positive-ID assertion fails before the compaction/reload checks run. Wait for vector-index readiness, then retry only `-1` against a monotonic 120-second deadline, passing the remaining budget to each compact RPC. Require an accepted compaction to complete and check the baseline both immediately afterward and after release/load. RPC failures, unexpected IDs, timeouts, and data mismatches remain failures. This follows the existing Struct nullable lifecycle test's approach from #52108. The observed failure is in [PR #53941's E2E run](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-e2e-pipeline-gcp/61102/console); this change does not modify server compaction behavior. Validation: - Ruff lint and format checks, Python syntax compilation, and `git diff --check`. - Executed the changed statements extracted from the actual file with simulated time/RPC responses: retry then success, persistent no-plan timeout, invalid IDs, RPC failure, index/compaction wait failures, and both post-compaction and post-reload data mismatches. - Live E2E has not been run against this fix locally; CI validation is pending. The separately requested rerun of #53941 uses its original head and does not validate this patch. Signed-off-by: xiaofanluan <[email protected]> Co-authored-by: xiaofanluan <[email protected]> |
||
|
|
15fcd798cf |
test: add continuous streaming data integrity draft (#53307) (#53868)
issue: #53307 ## What changed - Add a Phase 2 draft continuous-streaming DataIntegrity L3 workload with concurrent MutationIngress and ImportIngress lanes, no manual flush, and no manual compaction during streaming. - Extend the Phase 1 framework through opt-in sequence, level, and filtered-verification helpers while keeping existing cases unchanged. - Add the Phase 2 design contract and include the draft workload in the existing dedicated serial DataIntegrity job. - After automatic coverage and final ingress verification pass, request one post-ingress compaction and require Phase 1 lineage/serving checkpoints, fenced full-data verification, and release/load verification; this does not establish per-Delete L0 attribution. - Keep fixed epochs without producer/validator coordination; missing suffix progress fails with explicit coverage diagnostics for CI failure-rate measurement. ## Validation - Related DataIntegrity unit tests: 202 passed. - Ruff, format checks, and scoped git diff checks passed. - This review update was verified in medium mode; the new post-ingress compaction phase has not yet been rerun against a live cluster. Signed-off-by: MrPresent-Han <[email protected]> Co-authored-by: MrPresent-Han <[email protected]> Co-authored-by: GPT-5 <[email protected]> |
||
|
zhuwenxing
|
93af23b066 |
test: declare py dependency for REST parallel tests (#53906)
Explicitly pin py==1.11.0 in the REST test requirements. pytest-xdist 2.5.0 imports py.log when starting parallel workers; without py installed, pytest fails before executing test cases. Validation: git diff --check passed. Parallel E2E has not been rerun. Signed-off-by: zhuwenxing <[email protected]> |
||
|
zhuwenxing
|
734acf93f0 |
test: expand import and snapshot restore data integrity coverage (#53820)
## Summary - Add Python client import coverage for nullable values, defaults, row identity, scalar boundaries, vector encodings, dynamic fields, BM25, geometry, and timestamps across supported file formats. - Cover multi-file progress, empty-file commit/abort, malformed input, and preservation of existing data after failed imports. - Correct legacy import fixtures and assertions, including JSON field NULLs and the distinction between non-finite and null array elements. - Verify packed Storage V2 binlog restore after adding nullable dense/sparse vectors. Native Storage V3 uses the snapshot export/restore path instead. - Strengthen the REST v2 L0 snapshot lifecycle test with per-row and schema comparisons, NULL filtering, vector search, restore after dropping the source snapshot and collection, reload checks, and exported-object cleanup. ## Validation Tested against Milvus commit `1ae8642b8b1` using `milvus-dev-cli` E2E runners. - REST v2 snapshot lifecycle: passed on Storage V2 (76.93s) and Storage V3 (77.97s). - Packed Storage V2 binlog restore: 2 passed (dense and sparse nullable vectors). - Nullable timestamp defaults: 3 passed (JSON, CSV, and Parquet). - Targeted import and legacy bulk-insert runs covered nullable values, defaults, partition keys, malformed files, vector encodings, scalar boundaries, dynamic fields, BM25, and invalid array elements. - Python import testcase collection after renaming: 60 cases collected. - Changed snapshot testcase: Ruff formatting/lint, Python compilation, and pytest collection passed. Renamed import testcase: Ruff lint, Python compilation, and pytest collection passed. Validation used targeted runs during development; the complete final branch test selection has not been rerun as one suite. Snapshot validation used serial REST runs. The default CI storage configuration is not changed by this PR. ## Scope Test-only changes. No production behavior changes. --------- Signed-off-by: zhuwenxing <[email protected]> |
||
|
tinswzyandClaude Opus 5
|
d731ae60b6 |
enhance: bump woodpecker to v0.1.45 and expose the compaction bounds it adds (#53468)
issue: #43638 ### What Bumps `github.com/zilliztech/woodpecker` from v0.1.42 to v0.1.45 in the root and `pkg` modules, moves the woodpecker image tag in the three service-mode e2e helm values to match, and **exposes the six configuration keys this range adds** — the previous bumps in this series were dependency-only because woodpecker's config schema had not moved; this one has. `v0.1.42..v0.1.45` is twenty-one commits. The ten through v0.1.44: ``` dc9666e fix(hardware): drop the unused GPU memory helpers (#333) 2c072a4 enhance(logstore): bound what a node takes on, and watch its memory while doing it (#329) 335f151 enhance(log): bound the compaction pass and each attempt against a node (#324) d5f5d09 fix(ci): take MinIO from quay.io, which is where it still exists (#331) 9b65f40 fix(segment): index the fence LAC by the replicas that may be missing it (#326) 31ca09d enhance(logstore): size the shared flush pool from config, not the host's cores (#325) f36986b enhance(compaction): drop one payload copy and the pool around a contiguous read (#322) 068fb0b enhance(log): jitter the auditor's first tick and bound the compaction pass (#323) 7efb0a5 enhance(config): expose the two settings missing from woodpecker.yaml (#321) 0d8ac26 enhance(log): stop the auditor re-logging a no-op compaction pass on local storage (#313) ``` and eleven more in v0.1.45, all observability and operator work this embedding does not reach — new metrics (#342 #343 #344 #345 #355), a monitoring dashboard row and its fixtures (#348 #350), an admin endpoint for locating orphaned local data (#353), and the k8s operator's init image (#334 #336). The exception is #347, which is the only one that touches the module graph; see below. v0.1.44..v0.1.45 adds no configuration keys, so the six exposed here are still the whole set. ### The GPU link fix `#333` deletes woodpecker's unused GPU memory helpers. Their cgo preamble defined `getAllGPUMemoryInfo` without `static`, so the symbol had external linkage and collided with Milvus's own copy in `pkg/util/hardware`. A build tag is a property of a build rather than of a module, so a GPU build compiled woodpecker's file even though woodpecker never passes `-tags cuda` itself. **This branch is not affected** — [#49383](https://github.com/milvus-io/milvus/pull/49383) made its copy `static` in April, leaving one definition and a clean link. 2.6 never received that fix, so its GPU build is broken today; #53495 resolves it with this same bump. What this branch gains is the removal of a CUDA cgo compilation unit from every Milvus GPU build, for code woodpecker never called. ### What matters for Milvus **#326 — a recovery boundary below data a client was told was durable.** `resolveFenceLAC` indexed the sorted fence replies at `len(results)-ackQuorum`, which is the acknowledged boundary only when every replica answered. One failed reply shifted the index down a position: with tails `[0,0,-1]` and `Aq=2`, a lost reply left `[0,-1]` and selected `-1`, completion succeeded at `-1`, compaction sealed an empty object, and a fresh reader could not retrieve an entry the client had already been told was committed. A missing reply is not evidence of an empty replica, so the index is now `writeQuorum-ackQuorum` — the same tolerance the append path spends before giving up. That change forced a second one in the same PR. Completion had required `ackQuorum` replicas to *cover* the boundary locally, which cannot be satisfied once the boundary may be held by fewer, and failing it is not free: `CompleteSegment` has already reached every node by the time the count happens, and in service mode `Finalize` burns the boundary into a durable footer that refuses any later `Finalize` at a different one. So the segment could never complete — and because `OpenLogWriter` fences and completes every active segment before returning a writer, **one wedged segment stops writes on the whole log**. Completion now requires an ack quorum to have *recorded* the boundary rather than to cover it. **#324 / #323 / #329 bound work that was previously unbounded**, all on the auditor's compaction path, which runs its passes in sequence so an unbounded one stalls that log's truncate state, snapshot publication and orphan sweep behind it: - **#324** gives each compaction attempt against a node its own deadline — without one, a node that hangs rather than failing holds the call forever, since the auditor's context carries no deadline and the node's own ceiling is only enforced while the node is still running it. It also bounds how long one cycle spends *starting* compactions. - **#323** caps how many segments one cycle compacts (64, oldest first) and jitters each auditor's first tick, so writers created together — a node restart, a WAL failover, a channel rebalance — stop ticking in lockstep. - **#329** adds node-wide admission control for compaction memory. A node holds segments from many logs, each driven by its own auditor on its own schedule, and nothing bounded how many it took on at once. **#322** removes a payload copy from every merged block and replaces a pooled per-block read with one contiguous `ReadAt`, on the staged-storage compaction path. The remainder are woodpecker's own CI and a local-storage logging fix. ### Config This is the part that differs from the previous bumps. Milvus builds its woodpecker configuration from `config.NewConfiguration()` and overrides selected fields, so a key it does not map silently takes woodpecker's in-code default and cannot be tuned at all. All six new keys are exposed: | Key | Default | Why it is worth reaching | |---|---|---| | `woodpecker.logstore.syncScheduler.maxWorkers` | 32 | The node's flush concurrency. #325 made it configurable precisely because deriving it from the host's core count could not saturate a device's IOPS — leaving it unexposed would waste that change. | | `...segmentCompactionPolicy.maxInflightMemory` | 1000000000 | Memory a node reserves for concurrent compaction. Milvus already exposes `maxSize` and `maxParallelUploads`, which raise what one compaction costs; without this the node has no say in the total. | | `...segmentCompactionPolicy.memoryHighWatermark` | 0.7 | Stops new compactions when the node is near its memory limit for any reason. Pairs with the above. | | `...segmentCompactionPolicy.timeout` | 300s | The node's ceiling on one compaction. | | `woodpecker.client.auditor.compactionAttemptTimeout` | 330s | The client's deadline for one node to answer. **Exposed together with the ceiling above on purpose:** the two combine as a minimum, so exposing either alone offers a knob that cannot take effect. | | `woodpecker.client.auditor.compactionPassBudget` | 60s | How long one auditor cycle keeps starting compactions. | **Every default matches what woodpecker ships, so behaviour is unchanged for a deployment that sets none of them.** `maxInflightMemory` is written as a decimal `1000000000` rather than `"1G"` for that reason — `GetAsSize()` reads `"1G"` as 2^30, which would raise the ceiling 7% above the library's own default. (The pre-existing `segmentCompactionPolicy.maxSize: 2M` has the same 2^20-vs-decimal divergence against woodpecker's 2000000; it is left as it is.) `syncScheduler.maxWorkers` is marked `refreshable:"false"` — it sizes the worker pool built at logstore construction, so a runtime change would not rebuild it. `configs/milvus.yaml` is regenerated by the config tool, not hand-edited. ### Compatibility - **API**: unchanged for every symbol Milvus uses. `pkg/streaming/walimpls/impls/wp` consumes `NewClient` / `NewEmbedClient` / `StopEmbedLogStore` / `LogExists` / `CreateLog` / `OpenLog` / `LogReader` and the message-id helpers; nothing in that surface moved. - **Transitive deps**: woodpecker's `go.mod` does move this time. #347 takes gopsutil from v3.23.7 to v4.25.10, adding `ebitengine/purego` and bumping `perfstat`, `tklauser` and `wmi`. **Milvus's graph is unaffected**: it already requires every one of those at the same or a higher version — `gopsutil/v4 v4.25.10` and `purego v0.9.0` match exactly — so MVS resolves nothing new and the `go.sum` delta is still the two woodpecker lines per module. `gopsutil/v3` stays in `pkg/go.mod`, now reached through pulsar-client-go's testcontainers dependency rather than through woodpecker. - **On-disk format**: unchanged. #326 changes which boundary recovery selects, not what is written. - **Metrics**: #329 adds five `woodpecker_server_compaction_admission_*` series. No dashboard under `deployments/` references any `woodpecker_*` series, so nothing in this repo needs updating. - **New behaviour worth knowing about**, none of which is a regression: - A logstore node under memory pressure or at its compaction ceiling now refuses a compaction with a retryable error. The client tries the next replica and the auditor retries next cycle, so this classifies as `transient` in `WpSegmentCompactionFailuresTotal` rather than as a failure — but a series that was previously always zero can become non-zero under load. - Each auditor's first cycle after a writer opens is delayed by a random fraction of its interval (#323), up to 10s on the default. ### Test `pkg/util/paramtable` gains default-value assertions for the six new keys. Beyond that this is a dependency bump plus configuration plumbing, so the coverage is woodpecker's own tests for the commits above and the existing Milvus e2e suites. Verified locally: - both modules resolve to v0.1.45, and the `go.sum` delta is exactly the two woodpecker lines per module — populated additively with `go list` under `-mod=mod` (which only adds) rather than by `go mod tidy`, and checked with `go mod verify` - `pkg/util/paramtable` builds, vets and tests clean - `pkg/streaming/walimpls/impls/wp` and `pkg/mq/msgstream/mqwrapper/wp` vet clean against v0.1.45 - the six values reach the library end to end: a throwaway test drove `paramtable.Init()` → `builderImpl.getWpConfig()` and asserted each landed in the woodpecker `Configuration` with `Validate()` passing. Removed before commit. - `configs/milvus.yaml` regenerated with the config tool; the diff is 31 added lines and no reordering of existing content No Milvus-side behavioural claim is made beyond the API / config / dependency-graph audit above. The effects of #326, #324, #323 and #329 are woodpecker-internal and have not been reproduced or measured on a Milvus cluster in this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: tinswzy <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
marcelo-cjl
|
ad4887b38c |
fix: support vector null predicates (#52442)
issue: #52239 ## Summary - Support IS NULL and IS NOT NULL predicates for ordinary vector fields. - Read logical-row validity through vector-index HasValidData/IsRowValid when raw field data is unavailable, remaining compatible with #50524's Knowhere IdMap ownership. - Rebase onto current master while preserving master's broader search-by-PK result reordering and tests. ## Test Plan - [x] Rebase onto master `38c4699891` with exactly two commits and no merge commit. - [x] `git range-diff` confirms both patches are unchanged by the rebase. - [x] `git diff --check origin/master...HEAD` - [x] `go test ./internal/parser/planparserv2` - [x] `go test -c -o /tmp/milvus-pr-52442-rebased-testcases.test ./testcases` (from `tests/go_client`) - [x] C++ clang-format, Rust format, and Go format checks. - [x] `bash scripts/check_segcore_error_boundaries.sh` - [x] `ruff check` for the four changed Python test files. - [x] `python3 -m py_compile` for the four changed Python test files. - [x] Pre-rebase private Release+ASan integration build at `d839d36b`; the rebased commits are patch-equivalent. - [ ] CI on rebased head `a9e9d31ff2`. --------- Signed-off-by: marcelo-cjl <[email protected]> |
||
|
Bingyi Sun
|
215c79e293 |
test: wait for indexes in external table source override (#53883)
The external-table add-field source-override test can load a refreshed snapshot before its embedding index is ready. Use a FLAT/L2 index for this data/schema test and wait for `pending_index_rows == 0` before loading each refreshed snapshot. Validation: Python syntax parsing, helper-signature review, clang-format 15, and `git diff --check` pass. The Milvus/MinIO E2E case was not run locally. `make lint-fix` ran Go formatting, then stopped at unrelated native type-check errors in `internal/storagev2` (missing Loon filesystem metrics APIs). Signed-off-by: sunby <[email protected]> |
||
|
dependabot[bot]
|
ca8c70ba6f |
build(deps): bump deepdiff from 8.6.1 to 8.6.2 in /tests/python_client (#53674)
Bumps [deepdiff](https://github.com/qlustered/deepdiff) from 8.6.1 to 8.6.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/qlustered/deepdiff/releases">deepdiff's releases</a>.</em></p> <blockquote> <p>8.6.2 - Fix (CVE-2025-58367)</p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/qlustered/deepdiff/commit/0d07ec21d12b46ef4e489383b363eadc22d990fb"><code>0d07ec2</code></a> Merge commit from fork</li> <li><a href="https://github.com/qlustered/deepdiff/commit/791f5aac51b2da7f90375ab204256ef6a6b40206"><code>791f5aa</code></a> updating CVE number</li> <li><a href="https://github.com/qlustered/deepdiff/commit/a6aafea3ba5498aab6f9c77047c54949e7e968ae"><code>a6aafea</code></a> updating docs</li> <li><a href="https://github.com/qlustered/deepdiff/commit/a0950abbe6263298bc4bdbc3ebb57edc1af079b4"><code>a0950ab</code></a> Bump version: 8.6.1 → 8.6.2</li> <li><a href="https://github.com/qlustered/deepdiff/commit/887128abe5e510341cc4a82f6914a82d5ff1b6a7"><code>887128a</code></a> Fix (CVE-2025-58367)</li> <li>See full diff in <a href="https://github.com/qlustered/deepdiff/compare/8.6.1...8.6.2">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/milvus-io/milvus/network/alerts). </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
zhuwenxing
|
dde236e5f9 |
test: decouple external add-field type matrix from E2E (#53777)
## What changed
- Move the proto and PyMilvus DataType matrix check into a separate test
so enum changes cannot prevent the external add-field E2E from reaching
Milvus.
- Classify the new proto-only UUID type and check that DataType
categories are disjoint and cover both proto and SDK enums.
- Verify each added field's type, external field mapping, and nullable
setting before refresh and index creation.
## Verification
- Ruff check and format check passed.
- The standalone DataType matrix test passed with PyMilvus 3.1.0rc89.
- The targeted external add-field E2E passed against Milvus master
commit
|
||
|
Zhen YeandClaude Opus 4.6
|
06ba9d2298 |
enhance: refactor asynchronous WAL recovery and summary persistence (#53595)
issue: #40451 - WAL recovery: replace the legacy flusher with asynchronous VChannel/Segment persistence, retained-message completion, and owner-fenced snapshots before a single global checkpoint. Publish schemas before VChannel base metadata and use the base checkpoint to filter incomplete schema publication during recovery; delegate task concurrency to the shared NodeScheduler. Preserve transaction state through RecoveryBarrier with one scanner, reconcile legacy segment state during migration, and mark the WAL unavailable if background checkpoint persistence terminates unexpectedly. - WALSummary and idempotency: keep summaries and keyed-write deduplication always available; persist chunks/manifests independently, restore deduplication windows, provide bounded Delete reads, and manage WAL coverage, retention, and bootstrap replay boundaries. Seal on staged size or WAL tail pressure, report terminal Summary failures to the WAL owner, and perform physical GC outside reader/publication locks. Remove obsolete idempotency retention settings. - Data persistence: materialize missing function outputs before growing persistence, publish growing L1 data to DataCoord before completing Inserts, and track commits with DataVersion. Retire empty L1 segments explicitly through DataCoord without generating a manifest or publishing DataView; install the SN tombstone without a DataVersion only after retirement is confirmed, allowing ManualFlush to complete. Materialize retained Deletes into L0 using size, age, recovery pressure, or explicit WAL boundaries; retain the Summary-based materializer for future QueryView integration. Report complete recovery positions and poison work after WAL ownership loss. - Lifecycle completion: synchronize ManualFlush, FlushAll, and snapshot creation through consuming-side L1/L0 completion and broadcast acknowledgements. Let Broadcaster add CChannel centrally while callers supply the required collection VChannels. Rename the import ID-range update message to UpdateImport while preserving its payload and behavior. Preserve coordinator-owned Import visibility and legacy CommitImport completion during upgrades, and flush surviving partitions on DropPartition. Fence dropped objects in memory, publish durable tombstones only after asynchronous dependencies finish, and retire metadata and historical schemas through checkpoint- and Summary-gated GC. Cancel stale QueryCoord orphan-segment cleanup when a loaded collection reassigns the node to a replica. --------- Signed-off-by: chyezh <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
Feilong HouandEric Hou
|
38c4699891 |
fix: Repair Spark backfill toolbox build dependencies (#53770)
The Spark backfill toolbox init container fails when the pinned Avro recipe downloads the unavailable 1.12.1 archive from Apache's current mirror. The source build also needs additional Rust prerequisites, and the clean Spark runtime cannot load JNI unless libaio is included in the artifacts. This change: - Builds Avro C++ 1.12.2 from its official archive with a pinned SHA256. Reuses only the existing pinned recipe's build instructions, exports a distinct local recipe, and forces the storage dependency graph to that exact revision. - Installs pkg-config, libssl-dev, libclang-dev, and protobuf-compiler for the Rust bridge. - Bundles libaio.so.1 alongside the Conan runtime libraries. - Records the Avro recipe reference in build evidence and documents the prerequisites. issue: #52130 ### Validation - Shell syntax, embedded Python syntax, and git diff --check pass. - Verified Conan source download/checksum and repeatable recipe export; the resolved storage graph uses Avro 1.12.2. - Completed a Kubernetes toolbox build with Spark 4.0.1 / Java 21 on Linux AMD64, using spark-milvus commit 37d2217fe64c352048db71e4c928068b8cdaada6. - Verified artifact hashes, all JNI shared-library dependencies (including Avro 1.12.2), Python imports, BackfillApp class loading, and JNI loading in the Spark JVM. - A local Spark/Parquet round trip returned 100 rows with ID sum 4950. The build completed using caches preserved across retries. The final libaio packaging step was tested separately against the built artifacts, followed by the clean-runtime checks above; a fresh full build was not repeated after that packaging-only change. Milvus/S3 backfill was not exercised. Assembly emitted warnings about skipped Java 21/22 multi-release entries; assembly and the listed runtime checks passed. Signed-off-by: Eric Hou <[email protected]> Co-authored-by: Eric Hou <[email protected]> |
||
|
Li LiuandCodex
|
bd7c9df6bd |
fix: enforce DescribeCollection object visibility (#53724)
DescribeCollection can return shared cached metadata without applying the current caller's collection visibility, while ID-only coordinator lookups skip the name-guarded visibility check. Apply the existing visibility rules to the resolved collection in its actual database before returning metadata. - Recheck cached responses through the coordinator on every authenticated request, including cache hits and permission revocation; authorize the canonical collection in its actual database and reject changed IDs. - Apply coordinator visibility to both named and ID-only requests. Preserve internal metadata reads and the existing root/role rules. - Forward the verified user through remote, legacy REST, and console calls, replacing stale outgoing identities. Preserve the operator view for administrators authenticated by the management gate. - Strip outgoing user authorization from shared cache fills as well. Otherwise a stale denied identity can reject a permitted user's cold-cache request before the authoritative visibility check. Preserve request cancellation, deadlines, values, other metadata, and the original context. issue: #53723 Validation: - Actual package compilation and relevant tests passed on Linux arm64 with Go 1.26.6 and native libraries from the exact PR base image `milvusdb/milvus:master-20260921-12e77527-arm64`: ```sh go test -p 2 -tags dynamic,test -gcflags="all=-N -l" -count=1 ./internal/proxy ./internal/rootcoord ./internal/distributed/proxy/httpserver -run "(?i)describe.?collection|TestNewInterceptor|TestInterceptorImpl" ``` - The strengthened cold-cache test fails against the original PR for name, alias and cross-database ID lookups, then passes with the context fix. Request-context tests cover identity replacement, cancellation, deadline, values, other metadata and parent-context immutability. - Added and passed `TestDescribeCollectionVisibility` against a rebuilt server with separate Milvus component processes and real etcd, MinIO and Pulsar. Both cached and remote providers cover cross-database same-name collections, aliases, IDs with wrong/omitted database hints, a root-warmed cache, grant/revoke, and preservation of authorized access. Denied responses contain no schema, collection ID or aliases. - The same integration test binary fails against the official base server in both provider modes, reproducing 18 unauthorized metadata responses; it passes against the rebuilt PR server. The integration binary was compiled with `-tags dynamic,test -gcflags="all=-N -l"` and run with `-test.count=1`. - Existing `pkg/util/contextutil` tests passed with the required flags. Formatting, diff checks, and independent standards/spec reviews passed. REST/console are covered by actual handler tests and source tracing; deployment regression uses public gRPC. These are targeted regression tests, not the full repository test suite. The former local native-dependency blocker has been resolved. Cost and rollout: - Auth-enabled cached DescribeCollection adds one coordinator RPC per response; a cold cache fill may require another. Latency has not been benchmarked. Auth-disabled requests skip the visibility RPC. - Upgrade both Proxy and RootCoord. An older coordinator response without the database needed for an ID lookup fails closed with a retriable service-unavailable error; coordinator-only rollout does not repair an old Proxy's cache path. --------- Signed-off-by: Li Liu <[email protected]> Co-authored-by: Codex <[email protected]> |
||
|
|
50df537ca8 |
fix: validate sparse search rows before copying (#53759)
Search and HybridSearch accepted malformed sparse query rows at Proxy. In segcore, `CopyAndWrapSparseRow` allocated space for whole cells but copied the original byte length before checking alignment, allowing an out-of-bounds write. This change validates every sparse placeholder with `ValidateSparseFloatRows` before Proxy pass-through, returning `ErrParameterInvalid` (1100) for malformed input. It also checks native row lengths before allocation/copy, including callers that disable per-element validation. Existing valid rows, empty rows and text placeholders retain their behavior. Fixes #52425 Related: #52430 (existing Proxy-only proposal). ### Validation - Actual `internal/proxy` package on Linux arm64, with `-tags dynamic,test -gcflags="all=-N -l" -count=1`: pre-fix regressions fail for both Search and HybridSearch; final search/sparse/conversion tests cover later rows, later placeholders, the second Hybrid subrequest, valid inputs and the parameter-invalid status. - Full `pkg/util/typeutil` and `pkg/util/merr` tests pass with the same required flags. - Reduced ASan/UBSan harness using the actual copy functions, pinned Knowhere `SparseRow` definitions and retained C++ regression bodies: the baseline reports a 9-byte heap-buffer-overflow write; the fix passes misaligned, aligned and empty-row cases with validation enabled and disabled. - Formatting and `git diff --check` pass; independent code review found no blockers. The Proxy tests link official Linux native libraries; they do not rebuild this C++ change. The sanitizer harness replaces only the assertion logging/stacktrace dependency with a typed throw. Full segcore `all_tests` and a live RPC deployment were not run locally. Function-generated sparse rows are created after Proxy validation and are protected by the native length guard. --------- Signed-off-by: Li Liu <[email protected]> Signed-off-by: Li Liu <[email protected]> Co-authored-by: Codex <[email protected]> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]> |
||
|
zhuwenxing
|
2dfd51b8d7 |
test: add external table reload checker (#53784)
issue: #53778 ## Changes - Add `ExternalTableLoadChecker` to repeatedly release and load a fixed external collection, then verify its row count without refreshing the source. - Register `external_table_load` as a chaos operation. ## Verification - `ruff check tests/python_client/chaos/checker.py` - Python 3.12 compile and import checks for the checker and operation enum. Signed-off-by: zhuwenxing <[email protected]> |
||
|
Li LiuandClaude Opus 5.5
|
b7b3775aaa |
enhance: retire legacy REST routes and make V1 APIs optional (#53753)
Remove the non-underscore `/api/v1/*` REST routes from the metrics port, including `/api/v1/health`. Add `proxy.http.enableV1` (default `true`, restart required) to control both `/api/v1/_*` on the metrics port and `/v1/vector/*` on the Proxy HTTP port. Setting it to `false` leaves the listeners, `/v2/vectordb/*`, probes, metrics, and other management endpoints available. Retained console APIs keep their existing authentication policy. WebUI and telemetry pages depend on these APIs; their static pages remain controlled separately by `proxy.http.enableWebUI`. This is a breaking removal of the older non-underscore API even with default configuration. Clients must migrate to V2 where equivalent operations exist, or use the gRPC SDK; health probes should use `/healthz` or `/livez` as appropriate. Remove the obsolete legacy REST smoke script, its private fixtures, and its Nightly invocation. The separate V1/V2 REST test runner remains. issue: #49846 Design and migration: [HTTP V1 route retirement](https://github.com/liliu-z/milvus/blob/enhance/http-v1-retirement-20260922/docs/design-docs/design_docs/20260922-http-v1-route-retirement.md) Validation: - Passed HTTP configuration tests (default YAML, missing-key default, explicit disabling with the listener still enabled). - Passed `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./internal/http/...`. - Passed the repository formatter checks and `git diff --check`. - Verified access-log initialization with a disposable native-independent test: the actual middleware panics before initialization and handles the request afterward. The direct HTTP test helper initializes access logging as production startup does. - Added production route-registration regressions for all 47 retired method/path pairs, all 35 console pairs, and all 10 V1 vector pairs, plus live HTTP V1/V2 and console authentication cases. Local Proxy tests cannot compile because `milvus_core`, `milvus-storage`, and `rocksdb` are missing from pkg-config; native-enabled CI must run these tests. - The broader paramtable suite also encounters a local `/var/lib/milvus/data` permission failure in `TestComponentParam_StorageIopsParams`, which disables environment overrides. Focused HTTP config tests pass with `LOCAL_STORAGE_PATH=/tmp/milvus-http-v1-paramtable`. No deployed-cluster or browser acceptance test was run locally. --------- Signed-off-by: Li Liu <[email protected]> Signed-off-by: Li Liu <[email protected]> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]> |
||
|
Li LiuandClaude Opus 5
|
ed9e408686 |
enhance: bump google.golang.org/grpc to v1.83.2 to fix CVE-2026-84304 (#53497)
## What / Why Fixes **CVE-2026-84304** (Uncontrolled Resource Consumption in `google.golang.org/grpc`, HIGH), reported by the daily image scan. Fixed upstream in grpc `v1.83.1`; this bumps the module to `v1.83.2` (latest patch). `go.mod`-only change → covers **all** runtime image variants (ubuntu22.04 / ubuntu20.04 / amazonlinux2023 / rockylinux9 + gpu) since they share the same Go module graph. No per-OS Dockerfile change needed. ## Dependency cascade grpc `v1.83.x` requires `go.opentelemetry.io/otel v1.44.0`, which in turn pulls `otelgrpc v0.61.0`. That release **removed** the deprecated `UnaryServerInterceptor`/`StreamServerInterceptor`, which are still used by: - `go.etcd.io/etcd/server/v3 v3.5.33` (embedded etcd) - `github.com/zilliztech/woodpecker v0.1.44` (latest) To keep those building without a major etcd/woodpecker bump, this PR adds a `replace` pinning `otelgrpc` back to `v0.60.0` (which retains both the legacy interceptors and the new `NewServerHandler`/`NewClientHandler` APIs used by the GCP client libraries). Verified with `go build` on the etcd/grpc/otelgrpc-importing packages. ## Modules touched - `go.mod` / `go.sum` - `pkg/go.mod` / `pkg/go.sum` - `client/go.mod` / `client/go.sum` - `tests/go_client/go.mod` / `tests/go_client/go.sum` (auto-generated CVE fix) issue: #53001 --------- Signed-off-by: Li Liu <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
junjiejiangjjj
|
8c317d2ce1 |
test: fix topology-dependent FunctionChain tests (#53743)
Remove three Python E2E cases that assume L1 rerank runs on globally merged candidates. L1 pagination and GroupBy candidate selection depend on how segments are distributed across workers. Add QueryNode regression tests for worker-local L1 pagination and GroupBy candidate selection before L1 rerank. issue: #53737 Signed-off-by: junjie.jiang <[email protected]> |
||
|
wei liu
|
2fa2b0c090 |
feat: support positional partial updates for array values (#53163)
issue: #53016 ## What changed - Add `PATH_REPLACE` handling for one scalar Array element, one child of an existing StructArray element, or a non-empty subset/all children of one StructArray element. - Resolve and validate one request-wide path per parent field before the old row query, then materialize complete `FieldData` through the existing partial-upsert DML path. - Expose REST v2 and Go SDK request builders with dense singleton operands, strict non-null handling, and literal dynamic JSON key isolation. - Preserve omitted Struct children, Array order, existing nullability, and the current cross-VChannel partial-success boundary. - Add unit and E2E matrices for supported replacements, invalid paths, duplicate/overlapping operations, nulls, missing rows, bounds, dynamic JSON, different Array lengths, and compaction. - Pin all Milvus Go modules to the proto revision merged by https://github.com/milvus-io/milvus-proto/pull/666. ## Design [Positional partial update design](https://github.com/weiliu1031/milvus/blob/partial-update-array/docs/design-docs/design_docs/20260901-array-positional-partial-update.md) The feature intentionally does not add multiple paths for one parent, nested Array-child replacement, element-level nullability, a JSONPath language, or a new request-level atomicity guarantee. ## Validation - `make build-go` - Targeted Proxy tests passed with `-tags dynamic,test`, `-gcflags="all=-N -l"`, and the worktree RPATH. - Targeted REST v2 tests passed with the same required Go test flags. - Targeted Go SDK and column tests passed with the required Go test flags. - Nine targeted Go E2E groups passed against the rebased standalone binary, including compaction and the complete supported/rejected matrix. - A wire-level check with the previously pinned proto binding preserved enum value 3 and reached the old validator's default rejection branch. - `git diff --check` --------- Signed-off-by: Wei Liu <[email protected]> |
||
|
|
5ddad821c9 |
test: add V2 projection data-integrity case (#53307) (#53631)
issue: #53307 ## Summary - Add an independent L3 Storage V2 vector-projection workload: persist deterministic PK-bound narrow and wide vectors, verify the full dataset, drop the binary-vector field with schema bump disabled, and require an adopted MixCompaction rewrite before checking every retained cell. - Reuse the existing exclusive configuration fixture, lineage/serving checkpoints, full-retrieve fences, canonical byte comparisons, and cleanup; leave ordinary workloads, server code, and shared CI runners unchanged. - Pin both PyMilvus requirement entries to `3.1.0rc89`, whose published wheel includes `list_segments`, `list_compaction_tasks`, `list_loaded_segments`, and the required public enums. - Check dropped-index absence by field ownership (`list_indexes(..., field_name=dropped_field)`), with regression coverage for same-name and custom-name stale indexes and an unrelated index sharing the dropped field's name. - Cover deterministic payloads, row-preserving vector corruption, verification ordering, and configuration restoration across success and failure paths in the explicitly collected helper unit-test module; unequal field widths are adversarial inputs, not proof of a particular native reader refill. ## Verification - Isolated published `pymilvus==3.1.0rc89`, not the local editable SDK: 163 helper/oracle unit tests and 50 configuration/reporting/runner tests passed. - All 8 dedicated L3 workloads collect successfully; server-backed E2E was not rerun for this medium-mode update. - Negative control: restoring the old name-only assertion in memory causes 6 expected unit-test failures. - Scoped Ruff checks, Ruff format checks, and `git diff --check` passed. Signed-off-by: MrPresent-Han <[email protected]> Co-authored-by: MrPresent-Han <[email protected]> Co-authored-by: Codex <[email protected]> |
||
|
zhuwenxing
|
3cd8755ee2 |
test: update snapshot restore alias conflict expectation (#53682)
After #53547, restoring a snapshot to an existing alias is rejected during restore admission with code 1100 and an "already exists in database" message. The alias-conflict test still expected the later CreateCollection error. Update `test_restore_target_name_equals_existing_alias_fails` to match the admission error, explicitly assert code 1100, and correct its path documentation. Preserve the snapshot check and subsequent restore to a fresh target. Validation: - Reproduced the original assertion failure against Milvus commit `09d3618c818` with PyMilvus `3.1.0rc83`. - Targeted E2E on the same instance: 3 passed (alias target, existing collection target, and failed-restore resource cleanup). - Ruff lint/format, Python compilation, and `git diff --check` passed. Signed-off-by: zhuwenxing <[email protected]> |
||
|
shaoyue
|
378cf04fbf |
test: Enable s2 gRPC compression in CI E2E Helm values (#53686)
Enable internal gRPC compression with `grpc.client.compressionEnabled: true` and `grpc.client.compressionAlgorithm: s2` in all 18 embedded `user.yaml` configurations used by CI E2E and nightly Helm deployments on master. Companion PRs: #53464 (3.0) and #53467 (2.6). Validation: parsed all 18 outer Helm values documents and embedded `user.yaml` blocks, confirmed the semantic diff contains only the two requested settings, and passed `git diff --check`. Master registers the S2 compressor and accepts the configured algorithm. Runtime E2E validation is pending CI. Signed-off-by: shaoyue <[email protected]> |
||
|
|
eb7e63ebcf |
test: add Spark-Milvus backfill E2E coverage (#52155)
## What this PR does Adds dedicated Spark-Milvus Backfill E2E coverage and the runtime tooling needed to execute it against real Storage V3 and Storage V2 segments. - Adds opt-in, serial-only pytest coverage for Backfill modes, null semantics, snapshot boundaries, schema fencing, manifest versions, compaction, and invalid inputs. - Adds Kubernetes Job and reusable Spark Toolbox runners with credential redaction, RBAC validation, evidence collection, and cleanup. - Builds `zilliztech/spark-milvus` in the toolbox pod and runs Spark locally through the validated wrapper and native-library runtime. - Separates Storage V3 and Storage V2 execution through pytest markers and independent Milvus deployments. - Uses the dedicated `SparkBackfill` tag, so the normal `L0 L1 L2 ClusterOnly` nightly selection does not include this suite. - Documents manual toolbox deployment and dedicated-suite execution. Fixes #52130 ## Pipeline isolation The suite is ignored during ordinary Python client collection unless `--run-spark-backfill` is explicitly provided. The dedicated pipeline additionally selects `--tags SparkBackfill`; existing PR, nightly, and weekly tag sets therefore select zero Spark Backfill cases. V3 and V2 remain separate marker selections because they require different Milvus storage configurations and must run serially without pytest-xdist. ## Test plan - [x] Dedicated Spark Backfill pipeline completed successfully. - [x] Storage V3: 30 cases collected; 26 passed and 4 strict xfails track the known schema-fencing gap in #51318. - [x] Dedicated `SparkBackfill` tag collects 31 cases: 30 V3 and 1 V2. - [x] Normal nightly tags `L0 L1 L2 ClusterOnly` select 0 Spark Backfill cases, even when the opt-in collection gate is opened. - [x] `ruff check tests/python_client/common/common_type.py tests/python_client/conftest.py tests/python_client/spark_backfill` - [x] `git diff upstream/master...HEAD --check` ## Operational notes - Spark execution uses `spark-submit --master local[2]` in one toolbox pod. - The toolbox is discovered by `app=spark-milvus-toolbox` and is managed outside pytest. - V3 and V2 must use independent Milvus deployments. --------- Signed-off-by: Eric Hou <[email protected]> Co-authored-by: Eric Hou <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> |
||
|
zhuwenxing
|
fda7d59e39 |
test: fix missing primary key expectation for AutoID partition upsert (#53611)
AutoID collections support Upsert, but callers must supply a primary key for lookup. `test_partition_upsert_with_auto_id` omits that key, so PyMilvus rejects the request before sending an Upsert RPC. The expectation restored in #53158 incorrectly matched the server-side error. Expect SDK error code `1` and the missing-primary-key message, preserving the existing negative test. Validation: - Ruff lint/format, Python compilation, and pytest collection passed. - Targeted E2E passed with PyMilvus `3.1.0rc83` and Milvus commit `09d3618c818`: the corrected missing-key test passed; two diagnostic cases confirmed Int64/VarChar AutoID Upsert preserves existing IDs, allocates IDs for missing lookup keys, and supports partial updates. Diagnostic cases are not included in this PR. Signed-off-by: zhuwenxing <[email protected]> |
||
|
zhikunyao
|
ed7c7ff0d6 |
fix: initialize Docker bind mount ownership (#53549)
issue: #51221 related: #49775 ## What this changes - Add a one-shot volume initializer to the CPU and GPU standalone Compose files. - Resolve the numeric UID/GID from the same Milvus image and migrate `/var/lib/milvus` before the non-root server starts. - Write a UID/GID-specific marker only after a successful recursive migration, so large existing volumes are not scanned on every start and interrupted migrations remain retryable. - Prepare the bind mount in `standalone_embed.sh`, including when a previously created container is stopped after a permission failure. - Teach Docker deployment health checks to exclude one-shot services. - Document the ownership and restore contract. The initializer has no network, uses a read-only root filesystem, and mounts only the Milvus data directory. The Milvus server continues to run as `milvus:milvus`. This fixes both fresh rootful-Docker installations, where Docker creates the bind source as `root:root`, and upgrades from root-running images with root-owned data or cache files. It avoids the ineffective image-layer `chmod` approach from #51226 because a bind mount hides image-layer ownership and mode. ## Compatibility - The recursive ownership migration runs once for each image UID/GID. - A failed or interrupted migration does not create the marker and is retried. - Root-running older images can still read and write the migrated files on rollback. - After restoring files owned by another user, remove `.milvus-volume-owner-*` before starting Milvus to rerun migration. ## Validation - `git diff --check` - `bash -n scripts/standalone_embed.sh tests/python_client/deploy/utils.sh tests/python_client/deploy/check_healthy.sh` - `docker compose -f deployments/docker/standalone/docker-compose.yml config --quiet` - `docker compose -f deployments/docker/gpu/standalone/docker-compose.yml config --quiet` - Verified the deployment health-count expressions return the three long-running services. - Simulated the existing release bump regexes and confirmed both init/server image references and `MILVUS_IMAGE` are updated. Container-level startup was not run locally because the Docker daemon is unavailable; the Linux deployment CI must exercise the fresh-install and root-owned upgrade paths. --------- Signed-off-by: Zhikun Yao <[email protected]> |
||
|
XuanYang-cn
|
f36cf77d79 |
test: reduce integration import and shared wait overhead (#53525)
## What changed Integration tests spend substantial time waiting for scheduler ticks and repeating collection/import/index/load setup. This PR reduces that overhead while preserving the existing data volumes, supported format combinations, and completion/failure assertions. - Shorten import scheduling and shared readiness polling intervals, and reduce collection teardown overhead. - Group vector imports by vector type: 23 setup workflows become 6, retaining all 23 vector/format combinations and 2,300 rows, with per-format query and search checks. - Combine the four dynamic-field formats into one collection/import job, retaining 400 rows and per-format validation. - Fix binlog-import error classification: invalid backup path counts remain non-retryable `ErrImportFailed` input errors; other listing failures remain service-unavailable errors with their causes preserved. - Replace QueryNode fixed sleeps with readiness checks, and make BloomMatch recall comparisons use identical query vectors. Scheduler changes are test-only. The binlog error-classification fix changes production behavior and has regression coverage. ## Performance and validation The measured head `a250d9c63429` completed the full IT run successfully. Compare the original baseline [PR #53168 / IT #14635](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-ut-integration-pipeline/14635/) with [this PR / IT #14690](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-ut-integration-pipeline/14690/): **Plain IT runtime (Integration Tests stage): 87m21s → 62m16s, saving 25m04s (28.7%).** Package logs confirm the same **45 packages** completed: 44 passed and one had no test files. The final run recorded **427 passing test events, seven skips, and zero failures**. Existing focused checks also passed for invalid binlog inputs, storage-error handling, dynamic-field formats, and the complete vector format matrix. These are measured historical CI runs; machine and run-to-run variation still applies. Displayed times are rounded independently, with reductions calculated from the original seconds. --------- Signed-off-by: yangxuan <[email protected]> |
||
|
Chun HanandMrPresent-Han
|
33d4d826f3 |
enhance: expose compaction lifecycle observability (#53308)
issue: #53307 Protocol dependency: milvus-io/milvus-proto#668 (merged). ## What changed - Reuse the existing RPCs to expose retained segment lifecycle metadata and direct compaction sources; retain the existing sealed/loaded segment reporting contract without adding Growing segments to GetQuerySegmentInfo. - Expose collection-scoped retained compaction tasks, with complete task identity, public type/state enums, source/target sets, and failure reasons, while preserving manual job-ID lookup. - Preserve failure reasons across worker cleanup without changing task cleanup or metadata GC semantics. - Add seven **L3-only** data-integrity workloads: fixed V2/V3 Insert, fixed V2/V3 Import, V2 add/drop-field evolution through MixCompaction with schema bump disabled, V3 schema evolution, and V2-to-V3-to-V2 storage transitions. - Validate deterministic full-row/full-field data against an independent oracle, fenced persistence/serving checkpoints, and actual lineage transitions, allowing either automatic or manual compaction. - Provide `tests/scripts/ci_compaction_integrity.sh` as a dedicated, single-invocation, serial entry; leave the shared `tests/scripts/ci_e2e.sh` unchanged from the PR base. - Retain successful audit records only for the dedicated workload marker, preserving ordinary tests' JSON, HTML, and worker-report formats. ## Compatibility and execution boundary - Omitted segment states preserve the existing persistent-state behavior; task and dropped-segment history are subject to existing retention/GC, not a fixed response-size or record-count limit. - These workloads are not added to per-PR E2E or ordinary Nightly pipelines. - The dedicated job must own the deployment/config namespace exclusively, install a compatible PyMilvus build, and provide connection options plus a unique `CI_LOG_PATH`. - The entry enforces `-n 0`, the L3 workload marker, and explicit opt-in, while forwarding connection options, filters, and pytest's exit status. - Insert/Import workloads use three rounds by default; insert load units are batches, not a promise of a physical segment count, and dense vector dimensions are 16. ## Observability semantics and follow-ups - **Segment listing is not an atomic snapshot.** The Proxy first calls `GetSegmentsByStates`, then `GetSegmentInfo`; returned details are filtered again against the requested states (or the legacy default: Flushing, Flushed, Sealed). A segment whose fetched state no longer matches is omitted, and a segment becoming eligible after ID enumeration may be absent. Consumers must not treat a single response as a consistent lifecycle snapshot. - **Existing lookup errors are preserved.** `IncludeUnHealthy` is enabled only when Dropped is explicitly requested. In the default healthy-only path, a selected source becoming Dropped before detail lookup still returns `SegmentNotFound`, as before; that error is not converted into a successful partial list. A history segment being GC'd between the two RPCs can also fail the lookup. - **Aggregate `Completed` means no running tasks, not universal success.** This preserves the existing aggregation semantics: all-failed/all-timeout retained tasks, or an empty task set, can report Completed. Inspect each task's `state` and `failure_reason` for its outcome; proving an actual data transition additionally requires lineage, active-set adoption, and serving-set evidence. This PR does not redefine task retry or cleanup behavior. - **History capacity is a follow-up.** Dropped-segment queries enumerate the retained history and fetch its details, while collection-scoped compaction queries return retained tasks across triggers. Neither path currently provides pagination or a fixed count/size cap, so scan, copy, and response costs grow with the retained record count despite retention/GC. Pagination and capacity controls for both paths are deferred; they must preserve the ability to enumerate complete retained history rather than silently truncate lineage or task evidence. ## Previously recorded verification The results below are earlier recorded runs, including the original six-workload suite before the V2 DDL workload was added; the documentation/review-response update did not rerun tests. - 22 server-independent CI boundary tests passed, covering ordinary/special audit output, worker report merging, scoped execution guards, and the dedicated entry's argument/exit-code behavior. - 20 differential checks against the PR-base logging plugin produced byte-identical ordinary worker JSON, report JSON, and HTML across all five result outcomes. - 14 existing DataIntegrity and etcd-helper unit tests passed. - The three original L1 DataIntegrity parameter instances passed with `-n 2 --dist loadgroup` in 59.39 seconds; their merged successful JSON records retained the original metadata-only shape. - Ruff lint/format, shell syntax, and scoped PR-base whitespace checks passed; the shared runner is byte-identical to the PR base. - The new dedicated entry ran all six workloads serially against the existing local cluster: **6 passed in 3373.52 seconds**, with three rounds for each Insert/Import workload and the storage-transition workload, plus five DDL checkpoints over 25,000 rows. - All six successful reports retained their audit evidence (403 structured events), including every expected round/DDL checkpoint, full segment/task snapshots, serving frontiers, and matching expected/retrieved PK digests. ## Design and job entry - [Data-integrity design contract](https://github.com/MrPresent-Han/milvus/blob/codex/compaction-e2e-observability/docs/design-docs/design_docs/compaction_data_integrity_e2e_design.md) - [Dedicated serial entry](https://github.com/MrPresent-Han/milvus/blob/codex/compaction-e2e-observability/tests/scripts/ci_compaction_integrity.sh) Signed-off-by: MrPresent-Han <[email protected]> Co-authored-by: MrPresent-Han <[email protected]> |
||
|
junjiejiangjjj
|
bf02c32d35 |
enhance: chain supports dynamic field (#53493)
https://github.com/milvus-io/milvus/issues/46565 Signed-off-by: junjie.jiang <[email protected]> |
||
|
wei liu
|
f887463b25 |
fix: Serialize snapshot restores targeting the same collection (#53547)
issue: #53513 Concurrent internal restores can both pass Phase 0 and reuse the same target collection through idempotent CreateCollection, producing duplicate copy jobs. Share the existing per-target external restore lock with internal restores and validate target absence under the Phase 0 DDL lock before pinning the source. Hold the target lock through collection creation, job submission and synchronous cleanup, while releasing the DDL lock before nested DDL calls. Normalize an empty database name to `default` so both spellings share the same lock. An existing restore target is now rejected during admission, before the source snapshot is pinned. Its error message changes from "create duplicate collection with different parameters" to "target collection ... already exists in database ..."; the error code remains 1100. Update both existing Python rejection tests to check the new message and explicitly assert the code, since the shared error checker only validates message substrings. Add regression tests for internal/external contention, independent targets, admission failures, rollback retry and registered-broadcast ACK timeouts. Add an L2 Python e2e that starts two restores to the same target and requires one success, one already-exists error, one completed job and matching restored data. ### Validation - Targeted restore Go tests passed with `-tags dynamic,test`, `-gcflags="all=-N -l"` and `-race`. Changed restore functions have 100% statement coverage; this is not whole-package coverage. - `TestMilvusClientSnapshotConcurrency::test_concurrent_restore_same_target` passed with `--count=3`: 3 passed in 33.08s. Each run checks all 256 primary keys, scalar fields and vectors. Test collections were cleaned up. - After rebuilding Go on base `f5475123f2`, the two existing-target rejection tests (`test_snapshot_restore_collection_exist` and `test_snapshot_restore_failure_no_resource_leak`) and the concurrent-target regression passed together: 3 passed in 35.42s. The existing-target tests explicitly verify error code 1100 as well as the admission error message. Independent cleanup verification found no remaining collections. - Ruff lint/format checks and `git diff --check` passed. - Live testing used this worktree's binary in isolated standalone mode with embedded etcd, RocksMQ and local storage. Mixed internal/external contention is unit-tested; the original cluster/S3 topology was not exercised. The lock serializes restore requests within the snapshot manager. It does not add fencing against unrelated CreateCollection/DropCollection operations or coordinator failover. --------- Signed-off-by: Wei Liu <[email protected]> |
||
|
wei liu
|
2cbfc53b81 |
feat: preserve primary keys in full AutoID upsert (#53158)
issue: #53157 ## What changed - Full AutoID Upsert shares preparation with Partial Upsert and classifies supplied lookup keys with one PK-only Strong Retrieve using standard Query visibility and TTL handling. It adds no Full-specific fixed MVCC or guarantee-timestamp mechanism. - Existing rows preserve their primary keys, so their delete and insert route to the same VChannel transaction. - Missing rows always receive allocator-owned IDs. No Delete is emitted for a missing lookup key. No insert-on-not-found configuration or new not-found error code is introduced. - Partial Upsert retains master's Strong Query and actual per-channel CAS snapshots. AutoID Partial preserves existing keys and allocates new IDs for missing rows with complete insert fields, retaining them across internal CAS retries. Full Upsert does not collect or bind CAS proofs. - The Go row client retains AutoID lookup keys for Upsert, and the response returns final IDs in request order. - Shared `prepareUpsert` performs Retrieve, PK classification and missing AutoID allocation. Full Upsert returns before Partial-only field merging and CAS processing, preserving the complete request payload and row order. - Missing business PKs are allocated during preparation; internal RowIDs remain initialized in `insertPreExecute`. One PK helper validates keys and rewrites only missing positions in a copy of the PK field. Response IDs also drive Insert routing, without separate routing state. ## Compatibility and scope This preserves the historical create-on-not-found outcome while fixing identity replacement for existing rows. Full AutoID Upsert now depends on the standard Query/load-state path. Retrying a missing lookup key after an ambiguous outcome may create another generated identity; this is not an idempotency mechanism or a cross-VChannel request transaction. No RootCoord behavior, collection property, public protobuf, WAL format, or replication protocol is added. Deployments must use homogeneous Proxy versions during the semantic transition, as described in the design. Design: [Preserve Primary Keys in Full AutoID Upsert](https://github.com/weiliu1031/milvus/blob/refine-autoid-upsert/docs/design-docs/design_docs/20260803-autoid-upsert-primary-key-preservation.md) ## Validation Latest publication (`24e2ae1974`, 2026-09-15): - Rebased onto master `fee4ed4316` without conflicts. Both branch-only commits are equivalent in range-diff; the aggregate patch ID and changed paths are unchanged. - Post-rebase `SKIP_3RDPARTY=1 make milvus` passed in the designated worktree, rebuilding Milvus C++ and Go code while reusing third-party dependencies already built there by the preceding full `make milvus`. - 82 targeted Proxy tests passed after rebase with `-tags dynamic,test`, `-gcflags='all=-N -l'`, `-count=1` and local macOS linker options. Coverage includes shared Full/Partial preparation, PK validation, missing-ID allocation, CAS retries, routing and Query visibility. These mocked tests required no shared-service reset. - Full Proxy golangci-lint 2.11.3 passed with 0 issues. `gofmt` checks and `git diff --check` passed; the worktree was clean before publication. - At the user's direction, full-suite tests, the 99% coverage gate and repository-wide verifiers were skipped. No live E2E or fresh coverage result is claimed for this HEAD. New-head CI remains required. Previous publication (`e6000d8b67`, 2026-09-15; historical): - Rebased onto master `423b8e483d`. Resolved query/upsert conflicts by preserving master's Partial Strong-read/CAS protocol and AutoID fallback, while keeping Full classification on its ordinary visibility path. - The mmap counter collision fix is already upstream and was dropped as a duplicate. No new feature paths were added; the design and tests were aligned with the authorized Partial behavior. - `make build-cpp` passed in the designated worktree. - 56 targeted Proxy tests passed with `-tags dynamic,test`, `-gcflags='all=-N -l'`, `-count=1` and local macOS linker options, covering Full/Partial AutoID, CAS retries, snapshot binding, routing and Query visibility. These mocked tests required no shared-service reset. - Full Proxy golangci-lint 2.11.3 passed with 0 issues; formatter and `git diff --check` passed. - Full-suite tests, coverage and live E2E were not rerun. New-head CI remains required; earlier-head green results do not validate this head. Previous publication (`ffa01a4d1d`, 2026-09-10; historical): - Rebased onto master `13cd0e99f9` without conflicts. All seven commits are equivalent in range-diff; aggregate patch ID and paths are unchanged. - Full merr package tests and the Proxy paramtable subtest passed. - Formatter and git diff checks passed. - Post-rebase Proxy tests and full Proxy lint were attempted but blocked by stale local C++ headers missing master's new initcore C interfaces. Rebuilding this worktree with `make build-cpp` then failed at `LoadAdmissionController.cpp:87,91,131`: returning `future` calls the deleted `folly::coro::Future<LoadAdmissionLease>` copy constructor. That source file is identical to master and is not changed by this PR. No unrelated C++ repair was included. Post-rebase integration package compilation was not reached; Proxy validation remains incomplete. - Mock tests did not require starting or resetting shared services. - Full-suite tests, coverage and live E2E were not run for this publication. New-head CI must still validate the complete branch. Historical validation, not fresh evidence for this HEAD: - Before rebase, the switch-removal candidate passed 19 targeted Proxy tests, full merr tests, the Proxy paramtable subtest, full Proxy lint 2.11.3, formatter checks and HelloMilvus integration package compilation. - Earlier revisions ran targeted CAS/retry/routing tests, Go SDK `TestUpsertAutoIDRows` and `TestUpsertAutoIDMixedBatch`, REST v2 `test_upsert_vector_pk_auto_id` for Int64/VarChar, builds and Python syntax checks. - The prior QueryNode `TestFillOutputFieldsOrdered_NoCMemoryLeak` CI failure was not repaired by this change; its implementation and thresholds remain untouched. New-head CI is needed to establish status. --------- Signed-off-by: Wei Liu <[email protected]> |
||
|
cai.zhangandClaude Opus 4.8
|
bc5edc1e01 |
fix: bump github.com/klauspost/compress to v1.18.7 for GO-2026-5841 (#53503)
issue: #53001 ## What Bump `github.com/klauspost/compress` from v1.18.2 to v1.18.7 in the three modules that declare it: root, `pkg`, and `tests/go_client` (indirect). `client/` does not require it and is unchanged. v1.18.2 is affected by [GO-2026-5841](https://pkg.go.dev/vuln/GO-2026-5841) / [GHSA-259r-337f-4rfw](https://github.com/klauspost/compress/security/advisories/GHSA-259r-337f-4rfw) (CVE-2026-63209, affected <= v1.18.6, fixed in v1.18.7): a signed integer overflow in `s2.NewDict` that lets a crafted dictionary bypass the repeat-index check and crash in `Dict.Encode`. Requested in #53001; not covered by the recent dependency sync PRs #53456 / #53454 / #53377. ## Scope - Only the pin lines and the new go.sum hashes change (6 files, +9/-3). `go mod tidy` was not run, so the old v1.18.2 go.sum lines remain. - The v1.18.6 -> v1.18.7 delta is only the `s2.NewDict` fix, but moving from v1.18.2 also picks up v1.18.3-v1.18.6 (including the downstream CVE-2025-61728 zip fix and internal zstd/huff0/s2 changes). - Milvus imports only `klauspost/compress/zstd` (EncodeAll/DecodeAll and stream reader/writer). `s2` is linked in through `minio-go` (`s2.NewWriter` in the snowball upload path); no package in the compile set calls `s2.NewDict` or uses s2 dictionaries. So this mainly clears the advisory for scanners; no exploitable path in Milvus was demonstrated. ## Verification - `go mod verify` and `go list -mod=readonly -m all` in root, pkg, client, tests/go_client. Compared with the base, the only selection change is klauspost/compress in root/pkg/tests/go_client; client is identical. - go.sum hashes match sum.golang.org. - `pkg`: `go build ./...` passes; `go test -race -cover ./util/compressor/...` passes. - Root: `go test -race -cover ./internal/metastore/kv/querycoord/...` passes (zstd catalog compression); `go build ./internal/storage/compress/` passes. - `tests/go_client`: `go vet ./...` passes. Not verified locally: root packages that depend on the cgo storage layer (e.g. `internal/util/grpcclient`, `internal/storage`), because the local prebuilt C++ core does not match master's milvus-storage pin. CI covers these. No image scan was run. This PR does not touch 3.0 (still v1.18.0) or 2.6 (v1.18.0); backports are left for a separate decision. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: cai.zhang <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
zhenshan.caoandClaude Opus 5
|
955072942c |
test: fix data race on the mini cluster node maps during teardown (#53534)
`clearRedundantNodes` passes `lo.MapToSlice(c.mixcoord)`, `lo.MapToSlice(c.querynode)` and the three others as arguments to `go clearNodes(...)`. Arguments are evaluated in the calling goroutine, and `clearNodes` takes `c.mu` only inside its own body, so those five map reads happen with no lock held. A Milvus process that exits on its own runs the stop callback installed in `getOptions`, which calls `clearProcess` and deletes from the same maps while holding `c.mu`. A node stopping while `MiniClusterSuite.TearDownTest` runs `Reset` is therefore an unsynchronized map read against a concurrent delete: ``` cluster.go:391 clearRedundantNodes -> lo.MapToSlice(c.querynode) read, no lock cluster.go:739 clearProcess -> delete(c.querynode, nodeID) write, holds c.mu ``` ### Fix Take the snapshot of all five maps under `c.mu` before starting the goroutines. `clearNodes` still takes the lock itself, so the snapshot is released before any of them run and the locking discipline of the rest of the function is unchanged. Reading a node that `clearProcess` removes between the snapshot and the loop is harmless: `clearNodes` only calls `IsWorking`, `GetNodeID` and `Stop` on the pointer, which is what the code already did. ### Verification Reproduced with the race detector on an integration suite that adds a QueryNode and lets it stop before teardown. The race fires 2 out of 2 runs before this change and 0 out of 1 after, with the case passing. Twelve integration test files call `AddQueryNode`, so any of them can hit the same window; whether a given suite hits it depends on whether a node happens to be stopping when `Reset` runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: zhenshan.cao <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
|
10a6bd999d |
enhance: make configuration projections safe by default (#52579)
issue: #49846 Design: [Safe Configuration Projections](https://github.com/liliu-z/milvus/blob/enhance/secure-config-projections/docs/design-docs/design_docs/20260826-safe-configuration-projections.md) Configuration diagnostics and management endpoints previously shared raw manager views with runtime consumers. Those views can expose credentials, connection topology, process-environment entries, and deletion tombstones. This change adds declared scalar/group metadata and safe projections while preserving the management configuration API's existing write contract. ## Behavior - Safe configuration views omit undeclared entries and mask sensitive values. Management point reads return `*****` for sensitive keys and reject undeclared keys. Registration and sensitivity decisions account for aliases and dynamic group namespaces. - `ParamItem.Sensitivity` uses `Auto` (zero value), `Sensitive`, and `NonSensitive`; explicit states override inference. Sensitivity controls projections and logging only. Generic management SET/DELETE retains the existing immutable, `mq.type`, and `common.security.adminAuthEnabled` restrictions, original request formats, caller-key handling, duplicate validation, and etcd transaction behavior. Sensitive, unregistered, and security-related settings remain writable unless an existing restriction applies. A successful write does not make its raw value visible through GET. - Built-in file/etcd publication and safe reads share a snapshot lock, keeping source values and their spelling classification together. Collisions and first-time spelling changes retain ambiguity across deferred ownership events and removal. Initial local file spellings are established before environment overrides. Raw manager getters retain their previous locking behavior and source priority. - Effective group reads omit tombstones; deleting or resetting an overlay through an alias addresses equivalent stored overlay spellings. - Sensitivity metadata is installed before scalar/group declarations become visible to concurrent refreshes, including fallback keys and the hook namespace. Positive inventories check actual declarations for reviewed credentials, connection targets, transport, authentication, trust, and routing settings, including the Pulsar tenant and namespace consumed by topic construction. Registration classification stays internal; audit tests use public reads and projections. - WAL configuration maps, management mutations, and dynamic config events omit request names and values from logs. Configuration parser, remote-source TLS initialization, and opaque plugin/callback diagnostics avoid echoing sensitive input. Immutable startup writes mask composite keys containing the protected root path. - Native plugin initialization reports the failed stage without the configured path, plugin-provided name, or opaque exception text. Sanitization precedes native exception observers and Go startup logs while retaining typed codes, allocation-failure classification, and untyped-exception counting. Go plugin wrappers preserve returned errors while suppressing their diagnostic text. - Kafka diagnostic strings mask credentials. `FieldBroadcastID` uses the native `uint64` type and preserves its full range through RPC metadata. Projections remain uncached O(N) diagnostic/management operations. Management authentication, authorization, and additional write restrictions are separate work; endpoint authentication is supplied by #52580, now included through master. This PR does not encrypt stored configuration or audit application-wide backend diagnostics and logs emitted inside out-of-tree plugins. ## Validation ### Merge with current master Validated merge commit `24a8c123fd` with master `2e0677c1d2`. All Go tests used `-tags dynamic,test -gcflags="all=-N -l" -count=1`. - Full `pkg/config` passed with an isolated embedded etcd server. - Focused ParamTable sensitivity, declaration ordering, projection, and lifecycle tests passed. - Config race tests passed for all normalized key spellings, cache entry/byte/concurrency bounds, owned request strings, sensitivity aliases, `knowhere` identity, and refresh consistency. - The actual config GET/ALTER handlers, extracted into an isolated adapter, passed retained coordinator regressions against real embedded etcd. Coverage includes ordinary and sensitive writes, redacted GET, admin-auth SET/DELETE rejection under five spellings, no partial writes in rejected batches, `mq.type` aliases, transaction failures, and omission of payload names/values from logs. - Full `internal/http` and `internal/util/adminauth` passed. `go vet` passed for config, ParamTable, and the extracted handler adapter; `git diff --check` passed. The real coordinator package could not compile locally because RocksDB and `milvus_core` native dependencies are unavailable. The extracted-handler checks do not replace native coordinator CI. ### Feature validation before the master merge Prior local validation used `-tags dynamic,test -gcflags="all=-N -l" -count=1`: - Full `pkg/config` suite passed with an isolated embedded etcd server. - Focused ParamTable tests passed for the sensitivity enum, scalar/fallback overrides, Pulsar resource selectors under dotted/slash/uppercase-underscore/collapsed spellings, raw runtime values, projections, callback logs, declaration audits, and credential/immutability rules. The Pulsar point-read regression failed on the prior head and passed after the fix. - Focused ParamTable `-race` tests passed for enum behavior, Pulsar resource visibility, and sensitivity registration before declaration visibility. - The retained coordinator regressions passed in an isolated adapter that extracts the actual configuration HTTP handlers and connects them to real embedded etcd. The same write-behavior checks passed against the pre-PR write handler. Coverage includes sensitive/unregistered/security settings, Pulsar tenant/namespace SET and DELETE across four key spellings, single and batch requests, empty values, omitted/null resets, raw storage values, redacted GET responses, original key spellings, immutable/WAL rejection, duplicate error behavior, and transaction atomicity. Revised-handler logs omit request canaries on successful writes and transaction failure. - `go vet -tags dynamic,test` passed for config, ParamTable, and the extracted current-handler adapter; `git diff --check` passed. Full ParamTable testing was attempted but stopped because existing tests require creating `/var/lib/milvus/data`, which this host cannot write. Full coordinator tests were attempted but could not compile on this host because RocksDB and `milvus_core` native libraries are unavailable. The extracted-handler checks do not replace the full coordinator/native CI run. Broader backend RPC error descriptions, third-party network-client diagnostics, and logs emitted inside external plugins remain outside the verified log-sanitization coverage. --------- Signed-off-by: Li Liu <[email protected]> Co-authored-by: OpenAI Codex <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
junjiejiangjjj
|
c24624e280 |
feat: HybridSearch support l0/l1/l2 rerank chains (#52884)
https://github.com/milvus-io/milvus/issues/46565 doc: docs/design-docs/design_docs/20260818-hybrid-search-function-chain.md Signed-off-by: junjie.jiang <[email protected]> |
||
|
wei liu
|
2e0677c1d2 |
fix: support custom S3 endpoints for external collections (#52811)
issue: #52378 ## What changed - Accept `extfs.endpoint_url` for S3-compatible external collections, including Ceph RGW and MinIO with standard `scheme://bucket/key` sources. - Require an HTTP/HTTPS authority-only endpoint. Reject URL credentials, queries, fragments (including an empty fragment), non-root paths, invalid ports, and conflicting `bucket_name` / `use_ssl` overrides. In explicit mode, the source host must be a bucket without a port. - Map the endpoint to the collection filesystem address and TLS setting, and take the bucket from the source URI. Do not forward `endpoint_url` as a native storage property or inherit the internal storage provider. - Keep source interpretation private to metadata ingestion. The public external file reader and downstream packed readers consume resolved paths through one shared implementation, without a persisted path flag or another public reading API. - Preserve repeated bucket-like object-key prefixes and the bucket/key separator at the bucket root, including when endpoint host == bucket. - Keep validation errors free of the supplied endpoint URL, so malformed credentials cannot leak through error messages. - Add Go/C++ regression coverage and an explicit-endpoint Iceberg E2E case that reuses the legacy case's test workflow. ## Why External collections could not resolve standard S3 URIs from Iceberg metadata through an unrecognized private S3-compatible endpoint. Explicit endpoint configuration separates the physical address from the bucket without changing legacy URI interpretation. Source and resolved URI strings can also be identical when the endpoint host matches the bucket. Tracking interpretation at source ingestion prevents a downstream read from duplicating the bucket or dropping an object-key prefix. ## Compatibility - Explicit endpoint mode is selected only by the presence of `extfs.endpoint_url`. Legacy endpoint/provider resolution is unchanged when the field is absent. - The field is persisted in `external_spec`. Upgrade all participating components before creating collections that use it; mixed-version operation with older components is not claimed. ## Validation - Pre-rebase: full `packed` and `externalspec` package tests, plus seven targeted DataNode Milvus-table tests, with `-tags dynamic,test` and `-gcflags="all=-N -l"`. - Changed path normalization/resolution/reader functions and explicit endpoint validation functions: 100% statement coverage. - Rebased onto master at `b49b1e900b507cab6661f4b77b230141ee424dc7`. The exact feature path set and context-free patch-id are unchanged; range-diff shows only upstream snapshot-property comment context. - Post-rebase: `make milvus` passed using this worktree's freshly built native libraries; macOS product RPATH was inspected. - Post-rebase: full `packed` and `externalspec` package tests and seven targeted DataNode Milvus-table tests passed with the tags/flags above. - Post-rebase: a native local-filesystem alias probe confirmed the empty bucket root requires a bucket/key separator and the canonical root resolves successfully, both with and without endpoint host == bucket. This probe does not claim object-storage E2E coverage. - Changed-line C++ formatting check passed. - `git diff --check` and exact reviewed-candidate/committed-patch comparison passed. - Historical validation before this follow-up: Ceph RGW v19.2.3 custom-endpoint Iceberg E2E, including multipart upload, range GET, refresh/load/search/query. It was not rerun for this follow-up. --------- Signed-off-by: Wei Liu <[email protected]> |
||
|
Li Liu
|
2e36c6ff01 |
enhance: bump google.golang.org/grpc to v1.82.2 to fix CVE-2026-84445 (#53377)
## Summary Bump `google.golang.org/grpc` from v1.82.1 to v1.82.2 to fix the HIGH CVE reported by the master-image scan. ## CVE fixed | CVE | Package | From | To | Severity | | --- | --- | --- | --- | --- | | CVE-2026-84445 | google.golang.org/grpc | v1.82.1 | v1.82.2 | HIGH | ## Change scope `go.mod` / `go.sum` (root module) and `pkg/go.mod` / `pkg/go.sum`. Since `go.mod` is shared across every runtime image variant (ubuntu22.04 / ubuntu20.04 / amazonlinux2023 / rockylinux9, plus gpu/ubuntu22.04 / gpu/ubuntu20.04), a single bump covers all of them — no per-Dockerfile change needed. ## Not addressed (blocked) CVE-2026-84304 (grpc, Uncontrolled Resource Consumption, CVSS 8.7) requires grpc v1.83.1+. Bumping to v1.83.x transitively forces `otelgrpc` v0.61.0, which drops the `otelgrpc.UnaryServerInterceptor` / `UnaryClientInterceptor` APIs that are still referenced by: - `go.etcd.io/etcd` v3.5.x (including v3.5.33) — `etcdserver/api/v3rpc/grpc.go` - `github.com/zilliztech/woodpecker` v0.1.x (including v0.1.42) — `server/service.go` Neither etcd nor woodpecker has released a version migrated off the deprecated API, so the v1.83.x bump cannot build until those upstreams migrate. Tracked separately. Auto-generated CVE fix. Signed-off-by: Li Liu <[email protected]> issue: #53001 Signed-off-by: Li Liu <[email protected]> |
||
|
Li Liu
|
0f15a6dd3c |
fix: synchronize dependency upgrades across Go modules (#53456)
issue: #53001 pr: #53165 #53205 The root dependency upgrades in #53165 and #53205 left existing declarations in the independent `pkg` and `tests/go_client` modules at older versions. Complete those five declarations: | Module | Dependency | Version | | --- | --- | --- | | `pkg` | `golang.org/x/crypto` | `v0.55.0` → `v0.56.0` | | `pkg` | `github.com/gorilla/websocket` | `v1.5.0` → `v1.5.3` | | `pkg` | `go.mongodb.org/mongo-driver` | `v1.13.1` → `v1.17.7` | | `tests/go_client` | `golang.org/x/crypto` | `v0.55.0` → `v0.56.0` | | `tests/go_client` | `github.com/apache/thrift` | `v0.23.0` → `v0.24.0` | Audited the dependency additions/upgrades from #53377, #53259, #53205 and #53165 against all four `go.mod` files, retaining versions already above the source PR targets. #53377 covers gRPC in all four modules; #53259 covers etcd in the two modules that declare it. Those upgrades remain in their source PRs. Modules without an existing declaration receive no additional dependency pins. Ran `go mod tidy` separately in root, `pkg`, `client`, and `tests/go_client`. The root/SDK changes are stale checksum cleanup; pkg tidy also removes its unused x/tools requirement. The root and SDK `go.mod` files need no change for this follow-up. Validation on this branch: - All four modules: `go mod tidy -diff`, `go mod verify`, and `go list -mod=readonly -m all` pass; resolved versions match the applicable upgrades above. - Full SDK unit suite and pkg crypto/etcd/logging tests pass. - Woodpecker WAL implementation builds; all `tests/go_client` test packages compile using `go test -c`. - Tests use `-tags dynamic,test -gcflags="all=-N -l" -count=1`. `git diff --check` passes. Full native builds, cluster E2E, vulnerability reproduction, and container-image scanning were not run. The same missing declarations are being included in the 3.0 backport #53454. Signed-off-by: Li Liu <[email protected]> |
||
|
XuanYang-cn
|
05980c9e2f |
test: extend Storage V2 CMEK coverage and fix integration baselines (#53168)
## What problem does this PR solve? Storage V2 CMEK integration tests did not independently inspect encrypted raw data and serialized vector indexes through cold loading and searching. Running the integration subpackages exposed additional baseline failures and missing coverage hidden by the CI package-selection issue in #53250. Related issue: #53250 ## What is changed and how does it work? - Add independent encrypted Parquet and vector-index inspectors. Cover raw scalar, vector, and struct-array fields, small segments without physical vector indexes, and every current sealed segment/index lineage before release, reload, and search. - Fix expression suite discovery, struct-array vector `max_capacity`, partition-key Query expectations, and literal-backslash escaping in JSON LIKE patterns. Preserve the result-count and Search rejection assertions. - Carry forward the applicable #53068 fixture fixes: valid fixed-size-list Parquet fields, database-scoped RBAC setup, actual MiniCluster replication IDs/channels/WAL start positions, and observing a query-node outage before testing recovery. - Refresh an already-loaded collection after 2PC import commit, and re-enable the obsolete skipped commit-timestamp assignment check. - Encode struct-array Float16/BFloat16/Binary/Int8 vectors as numeric JSON/CSV values. Add actual-parser round-trip tests and name all 15 format/vector import combinations independently. - Give the 23 ordinary format/vector combinations a separate MiniCluster suite. The expanded BulkInsert suite otherwise exhausted even the CI 20-minute shared context and prevented its later cases from running. - Correct snapshot restore source/target collection parameters and snapshot deletion scope. Fail promptly on invalid RPCs, and exercise both Storage V2 and V3 with assertions on the actual segment version and V3 manifest. - Include the already-merged #53281 test fix as a cherry-pick with its original author and source attribution. - Remove the three obsolete independent L0 import cases and their delete-only setup. Keep ordinary no-delete binlog restore and verify that independent L0 import is rejected by default. These cases predate import two-phase commit; the new behavior in #51225 is outside this change. - Skip `TestVectorClusteringCompaction` pending owner investigation of the Storage V2 analyze path. Keep its original fixture and storage scope. - Re-read FlushAll segment observations only for `SegmentNotFound`, up to five attempts. Sort compaction can replace an ID between listing segments and fetching details. Preserve the single-segment, Flushed-state, and 100-row assertions; fail immediately on other errors. - Use the required `github.com/cockroachdb/errors` import in FlushAll to fix the Code-Check depguard failure. Add 42 inspector rejection subcases covering incomplete metadata and damaged encryption envelopes. ## Verification Latest follow-up on September 9 (`78173dd83a`, after merging master `1759fb0961`): - Full resource-guarded C++/Go build passed (no `--skip-3rdparty`). - Race-enabled inspector tests passed, including all 42 new rejection subcases. Local statement coverage is 96.6% for `raw_data_v2.go` and 97.2% for `vector_index.go`; these are file-level local measurements, not the Codecov patch percentage. - Race-enabled FlushAll passed in 33.291 seconds. RawData V2, VectorIndex V2, and their flush/index metadata helper regressions passed in 33.714 seconds. - Source-scoped static-check passed with 0 issues in core, pkg, client, and tests/go_client using the repository configurations and resource guard. The unscoped invocation was blocked by an existing unreadable etcd volume under deployments/docker/dev; the scoped invocation included all Go source directories and the root package. - Repository formatters and `git diff --check` passed. - The initial combined FlushAll/CMEK invocation ran packages concurrently and hit a MiniCluster port collision (`22125`); the isolated CMEK rerun passed. Orphaned processes from that failed invocation were cleaned up. The full 44-package run below predates this follow-up and master merge. New-head CI and Codecov results remain pending. Enumerated and ran all **44 Go integration packages** locally without fail-fast on September 9. The initial full pass took 75.1 minutes: 43 packages passed and FlushAll encountered the observation race described above. After the test-only retry fix, the complete FlushAll package passed in 73.364 seconds. All 44 packages therefore have passing results; the seven skips listed below remain excluded from executed coverage. The guarded runs used 4 jobs, `-race -tags dynamic,test -gcflags='all=-N -l' -count=1 -parallel=1`, with a 20-minute suite context. The local production/native baseline is `3431d027f6`; this evidence does not cover subsequent master changes. - All 95 declared top-level test entries started in the full run; all 376 started test/subtest entries had matching completion records. Counts include parent suites and are not independent-case counts. - The complete import package passed in 1654.3 seconds, including 2PC, commit timestamps, idempotency, ordinary formats, all 15 vector-array format combinations, BulkInsert, and zero-row cases. - All other packages, including CMEK, snapshot, compaction's enabled cases, replication, and commit_timestamp, passed in the full run. - A temporary client fault-injection probe exercised the real FlushAll suite: current and legacy SegmentNotFound responses recovered and all 100 collections were verified; persistent SegmentNotFound failed after five calls; an unrelated internal error failed after one call. The probe was removed before the final full-package run. - `gofmt` and `git diff --check` passed. Earlier static-check passed; it was not rerun for the final four test-file changes. ## Excluded coverage Six existing skips remain: `TestChannelExclusiveBalance`, `TestCompaction/TestMixCompaction`, `TestCoordSwitch`, `TestGetVector`, `TestRefreshConfig`, and `TestTarget`. `TestVectorClusteringCompaction` is additionally skipped as agreed, pending owner investigation. The original 30,000-row fixture computes 15 centroids, below the minimum 16. A temporary test configuration lowering the minimum to 8 reached the Storage V2 analyze path and observed zero input bytes instead of 15,360,000. The diagnostic configuration was reverted. No production implementation is added here. The removed L0 tests relied on the obsolete independent delete-only import flow. Ordinary binlog restore and rejection of disabled L0 import remain covered. --------- Signed-off-by: yangxuan <[email protected]> |
||
|
wei liu
|
10beedd721 |
enhance: Avoid TimeTick waits in partial update reads (#53336)
issue: #53335 Serial partial updates currently use a fixed MVCC read that can wait for the next TimeTick. Use a fresh Strong query for the first attempt and each safe CAS retry, then bind QueryNode's actual per-channel snapshots to the CAS proof. StreamingNode rejects proofs older than its current WAL tracking lifecycle, including a reopened WAL with the same channel term. Require a nonzero snapshot for every write channel before merging or writing. Existing conflict, retention, and incomplete-transaction guards remain active. All participating Proxy, QueryNode, and StreamingNode instances must implement this protocol; mixed-version rollout is outside this change. Design: [Partial Update Optimistic Lock](docs/design-docs/design_docs/20260601-partial-update-optimistic-lock-design.md). AutoID partial updates now preserve existing PKs and allocate a new AutoID for each missing row with complete insert fields. Each attempt resolves all possible destination terms before one Strong query, then selects the final write channels from its actual snapshots after allocation. Freshly allocated IDs are not queried again. Destination IDs remain stable within internal CAS retries; original fields stay immutable and returned IDs retain input order. Missing source PKs generate no Delete, and successful results return destination IDs in input order. Cross-vchannel transactions remain independent, and this does not add cross-request deduplication. Missing-PK insert failures name the PK and missing required field, without a row index, while retaining the non-retriable parameter error. The design and Python e2e error assertions are synchronized. Earlier validation (before the latest refactor): - Targeted Proxy query/upsert regression tests, QueryNode executed-snapshot tests, and StreamingNode lifecycle-history tests passed locally. - Earlier Strong-read Release build and serial A/B/B/A benchmark (before the AutoID fallback update): batch 16 / 128 / 512 mean latency changed from 199.771 / 199.774 / 199.731 ms to 1.293 / 1.838 / 4.658 ms. Each version/batch has 300 measured requests; 2,160 total requests including warmups succeeded, with full scalar-field and sampled vector-preservation checks. - Benchmark scope: Apple M5, Standalone, RocksMQ, 2 shards, 8,192 rows, 128-dimensional vectors, scalar REPLACE. Both versions used identical local native libraries, including a separate local compiler compatibility adjustment. This is not a distributed or concurrent-load benchmark. - `git diff --check` passed. Distributed WAL failover/fault injection and the full repository test suite were not run. - AutoID fallback: targeted Proxy tests cover Int64/VarChar IDs, absent/existing rows, real message packing with mocked mixed-channel success/CAS failure, stable retry IDs, result order, and missing-field error classification. QueryNode snapshot and StreamingNode lifecycle tests were also rerun on this branch. - Python error assertions were updated and syntax-checked; Python e2e and distributed fault injection were not run for this update. Latest refactor validation: - Updated the design and tests for one query per attempt, stable AutoIDs, and shared PK handling. Added RPC-to-CAS packing cases for distinct channel snapshots, empty destinations, missing/zero snapshots, and fixed namespace routing. - Formatting and diff checks passed. No tests or local builds were run for this update, as requested; earlier test and benchmark results do not validate this refactor. --------- Signed-off-by: Wei Liu <[email protected]> |
||
|
lorenzozane
|
2ad9913d41 |
fix: align XGBoost corrupted model assertion with segcore diagnostics (#53401)
issue: #53398 ## Summary The corrupted-XGBoost-UBJ e2e test asserted that `segcoreCode=` never appears in the error message. That contradicts the current error contract: `merr` keeps the original segcore code in the diagnostic, and the code travels the wire directly. Update the assertion to require `segcoreCode=2042` instead of rejecting the decoration. The test still verifies the explicit parse failure, wire code 2042, input-error classification, non-retriable behavior, and successful reranking with the valid model. ## Test Plan - `python3 -m py_compile tests/python_client/milvus_client/test_milvus_client_function_chain.py` - `ruff check --config tests/ruff.toml tests/python_client/milvus_client/test_milvus_client_function_chain.py` - CI on this PR: `ci-v2/e2e-default`, `ci-v2/go-sdk`, and `Ruff (changed files only)` pass. Fixes #53398 Signed-off-by: lorenzozanee <[email protected]> |
||
|
tinswzyandClaude Opus 5
|
3217194869 |
enhance: bump woodpecker to v0.1.42 to pick up empty-segment recovery and append-path fixes (#53294)
issue: #43638 ### What Routine woodpecker version tracking: bumps `github.com/zilliztech/woodpecker` from v0.1.40 to v0.1.42 in the root and `pkg` modules, and moves the woodpecker image tag in the three service-mode e2e helm values to match. No Milvus code change is required and none is included. `v0.1.41..v0.1.42` is five commits: ``` 966a5d4 enhance(test): cover empty-segment recovery on every backend and scope the shared footer validator (#311) a7de1f1 fix(ci): pass the rerun comment body through the environment instead of the script (#312) d52c322 fix(disk): accept a finalized empty segment on recovery; share footer index parsing across local backends (#307) 9f39e25 fix(ci): resolve the PR from open pull requests when the head commit is not local (#310) 281ab4d fix(ci): move the ci-passed label and rerun hints out of the test workflows (#309) ``` `v0.1.40..v0.1.41` is ten commits: ``` bec06a2 fix(client): stop FastSuccess from cancelling a replica that is still answering (#301) 3c44fc5 fix(chaos): read the node's container runtime instead of assuming minikube's default (#305) 52bc09b fix(test): stamp the "created after task start" file's mtime instead of relying on creation order (#303) e1293bc fix(meta): stop one provider mutex from serializing every log's metadata writes (#300) 1e7fa51 fix(log): marshal the message before taking the writable segment handle (#296) ec128bc enhance(metrics): expose the reader frontier of each log (#299) a6bff23 fix(metrics): drop the per-reader label from the reader bytes counter (#295) bbda63d enhance(metrics): measure the writable-segment handoff every append passes through (#297) 9a12d79 fix(topology): close the two gaps #294 left, and hold its boundary in code (#298) 4843ad9 fix(topology): say when placement is unset, and never label it "" (#294) ``` ### What matters for Milvus **#307 / #311 — a WAL that never reopens, on the local-file backend.** Finalizing a segment with zero blocks writes a complete file whose footer carries `IndexOffset = header size` and `IndexLength = 0`. woodpecker's disk backend rejected that footer as corrupt (`invalid footer record: IndexOffset=25, IndexLength=0`). Since `segmentProcessor.Fence` always reopens the writer in recovery mode, an empty segment that had been finalized but was still `Active` in metadata could never be fenced again, and every `OpenLogWriter` on that log failed. From Milvus that reads as a WAL that never reopens after a restart: the balancer loops in backoff, the channel stays `ASSIGNING`, DDL blocks, and healthz still reports OK. #307 moves footer validation and index-section parsing into `codec` so "a finalized empty segment is valid" is defined once for both local-file backends; #311 mirrors the missing coverage onto the other backends and pins the object-storage footer out of that shared validator. Scope, so this is not read as broader than it is: the defect was in the **disk** (local-file) backend only. The staged-storage backend already accepted the empty footer, and the object-storage backend keeps its index in a separate `footer.blk` and never consulted `IndexOffset`. Milvus defaults to `woodpecker.storage.type: minio`, so this is reachable on deployments that set `woodpecker.storage.type: local`. Three commits from the v0.1.41 range also sit on paths Milvus exercises: - **#300** removes the provider-wide `sync.Mutex` in `metadataProviderEtcd` that serialized every log's metadata write behind one lock across an etcd round trip. Per-log `Store` / `Update` / `Delete SegmentMetadata` now rely on etcd's own CAS transactions; only `CreateLog` and `ClearMeta` keep an instance mutex. A Milvus process drives one woodpecker client across all of its pchannels, so this is the commit with the widest reach in that range. - **#296** moves `MarshalMessage` out of the window between acquiring a writable segment handle and using it. That handle is perishable — another writer can roll the segment in the window, and the append is then rejected with `ErrSegmentHandleSegmentRolling` — and marshalling a multi-MB payload inside it stretched a window that should be one function call into milliseconds. - **#301** stops `FastSuccess` from closing the result channel of a replica that is still answering, which was aborting in-flight gRPC streams and emitting `rpc error: code = Canceled` warnings at high volume. Service mode only (`remote_result_channel`). It also wraps ACK read timeouts in `ErrAppendOpTimeout` so a late replica response is classified retryable rather than terminal. The rest are metrics, topology labelling and woodpecker's own CI. ### Compatibility - **API**: unchanged for every symbol Milvus uses. `pkg/streaming/walimpls/impls/wp` consumes `NewClient` / `NewEmbedClient` / `StopEmbedLogStore` / `LogExists` / `CreateLog` / `OpenLog` / `LogReader` and the message-id helpers; no signature moved and nothing was removed from the `woodpecker.Client` interface. The v0.1.41..v0.1.42 range touches only `server/storage/**` and `.github/workflows/**` — no client-facing file at all. - **Config**: woodpecker's config schema is untouched across both ranges, so `configs/milvus.yaml` and `setCustomWpConfig` need no change. - **Transitive deps**: woodpecker's own `go.mod` is identical at v0.1.40, v0.1.41 and v0.1.42, so nothing else in Milvus's module graph moves — the go.sum change is the two woodpecker lines and nothing else. - **On-disk format**: #307 widens what recovery accepts; it does not change what is written. A file finalized by v0.1.40 stays readable, and a v0.1.42 writer produces the same bytes, so rollback is safe except for the empty-segment case v0.1.40 could not read in the first place. - **Metrics**, two changes from the v0.1.41 range worth knowing about. Neither breaks anything in this repo: no dashboard under `deployments/` references a `woodpecker_*` series. - `woodpecker_client_reader_bytes_read` loses its `reader_name` label and now accumulates per log, so an external dashboard grouping by it needs updating. - `write_frontier_*` / `compaction_frontier_*` / `truncation_frontier_*` are no longer forced monotonic in-process and can now step backwards. - New series: `read_frontier_segment` / `read_frontier_entry` (labelled by `reader_name`, deleted on reader close) and `log_handle_operation_latency` samples for the writable-segment handoff. - **New startup warning**: #294 / #298 make both client constructors report placement once at startup. Milvus does not set `REGION` / `AVAILABILITY_ZONE`, so every process that builds the woodpecker WAL opener now emits one `topology placement is not configured ...` warning. One line per process at opener build, and placement is genuinely optional — called out here so the new line is not read as a regression. ### Test A dependency bump with no Milvus code change, so the coverage is woodpecker's own tests for the commits above plus the existing Milvus e2e suites. #307 carries a codec unit suite, a disk finalize/reopen/fence test and a local-storage integration test that reproduces the exact production error on the unfixed code; #311 mirrors it onto object-storage and staged-storage. Verified locally that both modules resolve to v0.1.42 and that `pkg/streaming/walimpls/impls/wp` builds and vets clean against it. No Milvus-side behavioural claim is made beyond the API / config / dependency-graph audit above: the effects of #307, #300, #296 and #301 are woodpecker-internal and have not been reproduced or measured on a Milvus cluster in this PR. **Prerequisite:** `harbor.milvus.io/woodpecker/woodpecker:v0.1.42` must be published before this merges, or the three service-mode e2e jobs will fail to pull the image. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: tinswzy <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
|
82bcce8867 |
enhance: classify segcore errors across producers and enforce classification end-to-end (#50768)
## What Consume the producer-owned error classification at the segcore boundary and make the whole C++→Go classification drift-proof, so a segcore error is classified as **input** (caller's fault, non-retriable), **transient** (retriable) or **permanent** (non-retriable) instead of flattening to `UnexpectedError(2001)` or carrying the wrong retry default. Design + tracking: #50903. ## Changes - **T1** — register the storage fallback pair in `pkg/util/merr/segcore.go`: `StorageError(2044)` non-retriable, `StorageTransientError(2045)` retriable. - **T2** — `KnowhereStatusToErrorCode` → a switch with **no `default` + `-Werror=switch`** over the full `knowhere::Status`; add build-path variant `KnowhereBuildStatusToErrorCode` so a build-time OOM / disk read stays **retriable** instead of collapsing into a permanent `IndexBuildError`. - **T3/T4** — `ArrowStatusToErrorCode` delegates to the producer's `milvus_storage::ToSegcoreError` (retires milvus's duplicate mapper); audited and routed **25 storage arrow-status sites** that were collapsing to `2001` through the single mapper (extracted to `storage/StatusToErrorCode.h`), always preserving the arrow sub-code in the message. - **T5** — unmapped-code observability: `UnmappedSegcoreCodeTotal{code}` counter + rate-limited WARN via an observer hook (merr is a leaf package); registered on QueryNode and DataNode. Unknown code degrades to non-retriable, never panics. - **T6** — codegen + compile-time enforcement: a generated `SegcoreCode` type (from milvus-common's `EasyAssert.h`) + an exhaustive `classForCode` switch marked `//exhaustive:enforce`, with the `exhaustive` golangci-lint enabled opt-in — a new C++ code that is not classified fails lint (the C++→Go analog of `-Werror=switch`). - **§3 B-tier** — classify `marisa` and `simdjson` errors (build/load/parse) instead of collapsing to `2001`, sub-code in the message; simdjson optional-access (`NO_SUCH_FIELD`/`INCORRECT_TYPE`) stays a benign skip; the `loon_ffi` FFI boundary is untouched. - **Boundary hardening (adversarial self-review of this PR's own diff)** — closed the escapes that would defeat the mapping above: a `throw e;` slicing rethrow in `LoadWithStrategy` that destroyed the very codes the columnar-read mapping attaches (bare `throw;` now), the same slice in `MinioChunkManager::PreCheck`; `GetCoreMetrics` / `EstimateLoadIndexResource` / init-and-config entry points that could let an exception cross the C ABI and terminate the process; and every remaining extern-C entry that caught only `std::exception` now ends in `catch(...)` via the shared `CGoCatch.h` macros. - **Pin + semantics** — bump `milvus-storage_VERSION` to `11f8a36` (the milvus-io/milvus-storage#574 merge, which also contains #575) and align the no-detail `IOError` expectation with the settled semantics: the producer tags every known-transient failure with a retryable `ExtendStatusDetail`, so a bare `IOError` with no detail is unclassified and deliberately falls back to permanent `StorageError(2044)` — a stripped-detail NotFound now degrades to non-retriable (safe) instead of retriable (retry storm on a permanent 404). - **Wire pass-through (client-visible)** — a segcore error now reaches the client with its ORIGINAL code (2009 stays 2009, 2024 stays 2024) instead of collapsing to the `ErrSegcore(2000)` umbrella with the real code buried in the message. Family identity for `errors.Is` is preserved via inner/Unwrap; input/system/retriable classification unchanged. Guardrails: only in-band (2000-2099) codes pass through (garbage still collapses to 2000); cross-family mappings (2046 → wire 110) keep their sentinel's code. `ErrSegcoreUnsupported`/`ErrSegcorePretendFinished` move to the C++ values they represent (2001→2003, 2002→2033) — their old numbers squatted on C++ UnexpectedError/NotImplemented and would false-match under code-based `errors.Is`. Verified end-to-end on a live standalone (ef<k reaches the client as 2042, unsupported tokenizer as 2001); the three e2e assertions pinning the old 2000 updated. - **Remaining code-destroying sites** — the three classes that still swallowed a producer's classification before the cgo boundary are now gone from `internal/core/src` and `internal/core/thirdparty`: status-consuming `AssertInfo` (104 → 0, incl. ~47 arrow builder paths whose commonest failure is OOM, now retriable `MemAllocateFailed` instead of a permanent 2001), bare `throw std::runtime_error/logic_error/bad_alloc` (68 → 0 — these were not `SegcoreError`, so they collapsed to 2001 *and* falsely fired the untyped-exception observer), and `throw fmt::format(...)` (12 → 0 — it throws a `std::string`, which `catch (std::exception&)` cannot see at all). tantivy's 73 `AssertInfo(res.result_->success, ...)` (plus 10 raw-`RustResult` stragglers found later) now classify the rust error — originally by its Display prefix, since replaced by a proper `#[repr(i32)]` discriminant carried in `RustResult.error_code` (see the Aug-10 update below). Typed `ThrowInfo` sites: 894 → 1081. The ~1500 genuine invariant asserts are untouched — 2001 is correct for them. The long-standing FIXME about `err_code` not surviving the nested LOON FFI boundary is also resolved, delegating to `milvus_storage::ToSegcoreErrorCode` rather than duplicating its table. ## Verification **Verified in this PR:** - **Mapping correctness (unit-tested, in-process):** `test_knowhere_status_mapping.cpp` / `test_storage_error_code.cpp` / `test_exec.cpp` cover every mapper branch (knowhere Status incl. the build variant, arrow/extend status incl. `AwsErrorNotFound→ObjectNotExist(2017)`, permanent-S3 vs transient), plus `FailureCStatus` code preservation and both observer hooks firing. - **Code projection to Go (one hop, unit-tested):** `segcore_test.go` pins `classForCode` for every generated code and asserts `merr.Status(err).GetRetriable()` for transient codes; the T6 generator is idempotent and the `exhaustive` lint fails on an unclassified code. - **Full C++ suite:** 8213/8223 unit tests pass locally (10 skipped; Azure connectivity tests excluded), 8648 in CI, rebased on current master (one pre-existing, unrelated concurrency test excluded: `GrowingConcurrentReopenTest` deadlocks deterministically on current master with or without this PR — rwlock writer starvation in growing-segment reopen code this PR does not touch; reported separately). - **Static audit (grep-verifiable):** every storage arrow-status consumption site on the read path routes through `ArrowStatusToErrorCode`, and every extern-C boundary ends in a `catch(...)` tail. **Explicitly NOT verified here (follow-up):** - **Runtime fault injection.** No S3 throttle / 404 / OOM / corrupt-file failure has been triggered end-to-end in a running cluster. Transient codes reach Go with `retriable=true` (unit-tested projection), but the downstream consumption — `lb_policy` replica reroute on `merr.IsRetryableErr`, index/analyze scheduler retry — is pre-existing logic from #50221 and has **not** been driven by a real segcore transient error in this PR. This PR preserves classification for observability and correct retry defaults; the retry behavior itself is exercised only by its own pre-existing tests. ## Dependencies - ~~milvus-common `StorageTransientError(2045)` — zilliztech/milvus-common#102~~ **merged**. - ~~milvus-storage `ToSegcoreError` / packed `ExtendStatusCode` — milvus-io/milvus-storage#575 + #574~~ **merged; pin bumped in-tree to `11f8a36`**. - ~~knowhere three-way classification — zilliztech/knowhere#1704~~ **merged** (the milvus-side `KnowhereStatusToErrorCode` → thin delegate to knowhere's own `ToSegcoreErrorCode` is a follow-up, gated on a knowhere version bump). - ~~milvus-common untyped-cgo-exception observer — zilliztech/milvus-common#112~~ **merged and released as `1.0.0-1fd1160`; the pin now points at the published package.** All dependencies are in. ## Update (Aug 10) — full-population audit, LOON path, runtime observability The originally deferred FFI/LOON path is now **done on the milvus side**, and the audit was extended from the three grep-able classes to the *entire* 2001-producing population: - **Every remaining 2001 site read.** All 1,517 `AssertInfo` (four sweeps: errno fingerprint, failure-keyword messages, condition morphology, and finally **data provenance** — does the guarded value come from disk/network?) and all 198 explicit `ThrowInfo(UnexpectedError)` sites. ~290 were externally-triggerable and now carry typed codes: file/remote IO -> `FileOpen/Create/Read/WriteFailed` (retriable), mmap/allocation -> `MmapError`/`MemAllocateFailed` (retriable), persisted-format damage (CRC/magic/parquet meta/index-meta keys) -> `DataFormatBroken`, deployment config -> `ConfigInvalid`, request content -> `InvalidParameter`, a cancel-race -> `FollyCancel`. The ~1,400 kept sites are genuine invariants or cgo contracts where 2001 is the correct report. - **Two infinite-retry bugs.** Statically-impossible conditions (index_type x metric blacklist, per-type metric allowlists, json/geometry index gates) threw 2001 -> generic retry -> the build task spun forever; they now throw `Unsupported`, which `getStateFromError` maps to a terminal `JobStateFailed`. Missing `index_type`/`metric_type`/`min_gram`/`max_gram` keys in persisted index meta had the same loop on the load path; they are `DataFormatBroken` now. - **knowhere `expected<>` bypasses closed** (8 sites in `QueryResult.h`/`CachedSearchIterator`): iterator failures went through `AssertInfo` and discarded the Status knowhere had already classified; they now route through `KnowhereStatusToErrorCode`, so an OOM/disk failure during search iteration stays retriable. Preflight rewraps in `segment_c`/`boost_score` similarly preserved the original `SegcoreError` code instead of flattening to 2001+string. - **tantivy discriminant over the FFI.** `RustResult` now carries `error_code` (`#[repr(i32)] TantivyBindingErrorCode`, cbindgen-exported); the C++ mapper switches on the enum instead of parsing the Display text, and the inner `tantivy::TantivyError` is discriminated too (`IoError/Open*Error` -> Io/retriable, `DataCorruption/IncompatibleIndex` -> DataCorruption). Wording changes on the rust side can no longer silently degrade classification. - **LOON / FFI path (the deferred item), milvus side complete.** The Go funnel `HandleLoonFFIResult` dropped `err_code` entirely and wrapped every failure as `ErrLoonTransient` — a 404/access-denied/corrupt-data retried as transient. It now classifies by the producer's own `loon_ffi_is_retryable_errcode`; permanent failures carry the new `ErrLoonPermanent` and terminate retry loops (`pack_writer_v3` via `retry.Unrecoverable`; the external-refresh manager guard extended so behavior does not invert). On the C++ side `LoonErrCodeToErrorCode` is the single classification entry (low band -> hand table, extend band -> producer's `ToSegcoreErrorCode`, unknown -> producer's retryable probe), unifying the two previously-divergent `ThrowIfFFIError` helpers — `LOON_FILE_NOT_FOUND(12)` now converges to `ObjectNotExist(2017)` on both integration paths. Remaining LOON items (e.g. promoting FileNotFound into `ExtendStatusCode`) live in the milvus-storage repo. - **Regression guards.** `scripts/check_segcore_error_boundaries.sh` wired into `make static-check`: every `throw` in `internal/core/src` must carry a milvus ErrorCode (zero-tolerance; currently 0 violations); vendored `fmindex::` is confined to its boundary files; knowhere/arrow/milvus_storage/tantivy are ratcheted by a checked-in file-set baseline (new consumer files fail the check; shrinking is free). - **Runtime observability for what is left.** `milvus_cgo_unexpected_segcore_origin_total{origin="<file>:<line>"}` counts every 2001 crossing the cgo boundary by its C++ source location (parsed from the ` at file:line` suffix `AssertInfo` already emits, build paths collapsed to repo-relative). A site that fires in production names itself — reclassification becomes evidence-driven instead of re-reading ~1,400 asserts. Site count for the 2001 family: 1,955 on master -> 1,525 on this branch; the delta is reclassification into actionable codes, not deletion of checks. ## Deferred - milvus-storage-side LOON improvements: promote `LOON_FILE_NOT_FOUND` into `ExtendStatusCode`, category byte (design §4.7) — tracked in the storage repo. - knowhere-side: thin-delegate `KnowhereStatusToErrorCode` to knowhere's own `ToSegcoreErrorCode`, gated on a knowhere version bump. issue: #50903 --------- Signed-off-by: Zack <[email protected]> Co-authored-by: Zack <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]> Co-authored-by: xiaofanluan <[email protected]> |
||
|
|
48a5f578c0 |
test: update client cases for storage and sdk changes (#53305)
issue: #51544 Reopens #51755, whose author has left the project. The branch is rebased onto current master and the conflicts are resolved. ## What changed - Derive the default Python client test namespace from `MILVUS_NS` or `MILVUS_HELM_NAMESPACE` before falling back to `chaos-testing`. - Add an optional external-source MinIO endpoint override (`MILVUS_EXTERNAL_SOURCE_MINIO_HOST`) for external table tests, so the client upload endpoint and the server-side `external_source` endpoint can be configured independently. - Resolve the bulk insert MinIO endpoint from the existing `minio_host` / `minio_bucket` fixtures instead of looking up the MinIO pod IP through the k8s API, so these cases no longer require in-cluster access. - Extend TEXT LOB layout validation to read Vortex segment data files in addition to the existing parquet layout. The Vortex files are decoded with `vortex-data`; when the installed reader cannot decode Milvus StorageV3's custom encoding the case skips rather than asserting on synthesized references. ## Dropped from the original PR Two changes from #51755 have since landed on master independently and are not included here: - The multi-primary-key invalid schema assertion — master already validates at `add_field` time, via the framework's `CheckTasks.err_res` helper rather than `pytest.raises`. - The `Date` / `Time` / `Decimal` entries in the external table add-field matrix. ## Verification - `test_text_lob_bulk_import_json_csv_parquet` - `test_text_lob_inline_vs_lob_object_layout` - `test_milvus_client_external_table_add_all_supported_field_types` - `test_milvus_client_xgboost_supports_all_numeric_feature_types` Signed-off-by: zhenshan.cao <[email protected]> Co-authored-by: nico <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
yanliang567
|
76b1e3a307 |
test: pass IVF nprobe through ANN params (#53302)
## What changed
- Pass `nprobe=64` through `CustomAnnParam`/`WithAnnParam` in
`TestCreateIndexVanillaFaissGeneric`.
- Correct expected/actual ordering in `CheckSearchResult` assertions.
## Why
`WithSearchParam("nprobe", "64")` adds a top-level search request key,
while the proxy reads algorithm-specific search parameters from the
`params` JSON. As a result, the intended nprobe value was ignored and
the FAISS IVF search could return fewer than topK results when a probed
list contained fewer candidates.
Using `WithAnnParam` serializes `nprobe` into the ANN params JSON.
Setting nprobe equal to the IVF nlist scans all 64 lists and makes the
topK assertion deterministic.
## Verification
- `GO_CLIENT_SKIP_EXTERNAL_TABLE_DEPS_INSTALL=1 go test -c -o
/tmp/milvus-testcases-pr-50392.test ./testcases`
- Target E2E passed 10/10 against standalone
`master-20260908-bf65725be6`: `go test ./testcases -run
"^TestCreateIndexVanillaFaissGeneric$" -count=10`
- go_client golangci-lint: `0 issues`
- Full `make static-check` was attempted but its environment setup
requires LLVM 14-18, which is not installed locally.
Fixes #50392
Signed-off-by: Yanliang Qiao <[email protected]>
|
||
|
yihao.daiandClaude Opus 5
|
d6179da4f7 |
test: fix commit_timestamp integration tests never being able to pass (#53281)
issue: #53280 ## Summary `tests/integration/commit_timestamp` has never been able to pass. Its shared helper `setCommitTimestamp()` reads `SegmentInfo` records straight out of etcd and filters with `len(seg.GetBinlogs()) == 0` as the "segment has data" predicate — but that record never carries binlog arrays. `CloneSegmentWithExcludeBinlogs` and `resetBinlogFields` strip them before serialization; they are persisted under the separate `datacoord-meta/binlog/...` prefix (V2) or resolved from the LOON manifest (V3), and `applyBinlogInfo` re-attaches them only on load. Every segment was skipped, the helper tripped its own `no flushed segments were modified`, and with `-failfast` the remaining eight cases never ran. This surfaced only recently because the CI integration stage re-ran the root `tests/integration` package for every sub-package, so the stage was green without ever executing these tests. Four unrelated PRs (#53163, #53256, #53257, #53258) all failed on the same line once the sub-packages were actually run. ## Changes - Filter on `NumOfRows`, which is present on the persisted record for both storage versions. - Restart the mixcoord after writing into etcd. DataCoord serves segment meta from its in-memory `SegmentsInfo` loaded at startup, so the raw write was invisible to it: compaction plans carried `commit_timestamp: 0` and `overwriteRecordTimestamps` was a no-op, failing the "output binlog timestamps must be >= commitTs" assertion. Same `DefaultMixCoord().Stop()` + `AddMixCoord()` pattern as `tests/integration/datanode/compaction_test.go`. - Add `waitUntilTsVisible()`. A `guarantee_ts` in the future is not waited for — the read is served at the read path's current MVCC timestamp — so the cases that set `commit_ts = now + 10s` and immediately read at it saw no rows. The helper waits until the guarded rows are actually readable, then the existing assertions run. No production code is touched. ## Test Plan Full suite against a local MiniClusterV3 (etcd + minio + `./bin/milvus`): ``` --- PASS: TestCommitTimestampSuite/TestCompaction_NormalizesCommitTs --- PASS: TestCommitTimestampSuite/TestDelete_AfterCommitTs --- PASS: TestCommitTimestampSuite/TestDelete_BeforeCommitTs --- PASS: TestCommitTimestampSuite/TestGC_ImportSegmentNotPrematurelyDropped --- PASS: TestCommitTimestampSuite/TestMVCC_StrongConsistency_CommitTsInPast --- PASS: TestCommitTimestampSuite/TestMVCC_Visibility --- PASS: TestCommitTimestampSuite/TestSearch_WithGuaranteeTs --- PASS: TestCommitTimestampSuite/TestUpsert_AfterCommitTs --- PASS: TestCommitTimestampSuite/TestUpsert_BeforeCommitTs ``` - [x] 9/9 over five consecutive full-suite runs (0/9 before this change) - [x] `make static-check` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019yaajpE35SPdRXweyMnGWL Signed-off-by: bigsheeper <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
zhuwenxing
|
dd75544f37 |
test: fix external refresh and oversized insert expectations (#53148)
issue: #53133 issue: #52648 ## What was changed - Handle both valid outcomes of two back-to-back external collection refresh requests: - reject or reuse the request while the first job is active; - allow a new job only after the first job reaches a terminal state. - Update the oversized insert test to verify the current application-level `maxInsertSize` rejection. - Assert error code `1102` and the stable `exceeds maxInsertSize` message instead of the obsolete gRPC `message larger than max` error. ## Why The external refresh test assumed that the first job would always remain active until the second request arrived. On faster environments, the first job can complete before the second request, making a new refresh job valid. The oversized insert test assumed a 64 MiB gRPC receive limit. The Proxy receive limit is now 128 MiB, while oversized materialized inserts are rejected by the 64 MiB `quotaAndLimits.limits.maxInsertSize` guard. ## Verification - `ruff check` passed for both changed files. - `ruff format --check` passed for both changed files. - Python compilation checks passed. - External refresh regression E2E: passed. - Oversized insert regression E2E: passed with error code `1102`. --------- Signed-off-by: zhuwenxing <[email protected]> |
||
|
yanliang567
|
3067e2542d |
test: add Go, PyMilvus, and REST e2e for FMINDEX and membership_match (#52629)
## Summary Add end-to-end coverage for FMINDEX and the unified `membership_match` expression across the Go SDK, PyMilvus, and RESTful v2. Related issues: #51375, #52778, #53036 ## Coverage ### FMINDEX - Build parameter defaults, boundaries, combined parameters, and invalid types or ranges. - VARCHAR-only validation, including JSON, scalar, and struct-array sub-fields. - Prefix, infix, suffix, general LIKE recheck, regex fallback, escaped wildcards, NULL, and UTF-8 behavior. - Long-text low-hit fixtures and sealed segments above `minSegmentNumRowsToEnableIndex`, preventing fake-finished index coverage. - Exact indexed-field versus unindexed twin-field PK comparison. - Growing plus sealed visibility, mutation, upsert, named-index drop, and rebuild lifecycle checks. - Query and Search coverage with exact projected fields, distance ordering, index identity, type, and row-progress assertions. - VARCHAR AUTOINDEX resolution is verified against the configured scalar-index policy without hard-coding a specific index type. ### membership_match - Bloom and Roaring wire compatibility through a shared static manifest while each SDK keeps independent collection setup. - Bloom zero-false-negative checks plus a statistically bounded false-positive check tied to the configured FPR; Roaring uses exact PK ground truth. - Query and Search must return the same complete Bloom-filtered PK set when topK covers the fixture. - Integer widths, negative values, INT64 boundaries, VARCHAR, JSON strict typing, nested paths, dynamic fields, NULL semantics, and unsupported top-level BOOL/FLOAT fields. - Positive, negated, empty, malformed, domain-mismatch, delete, growing plus sealed, and struct-array rejection cases. - Explicit `STL_SORT`, `INVERTED`, `BITMAP`, and `TRIE` scalar-index paths. - Internal-only `HYBRID` paths are exercised through the supported `AUTOINDEX` request and verified against DataCoord's resolved build parameters for primitive and JSON-path indexes. - Mutation tests use isolated collections; shared read-only fixtures register cleanup before setup begins. ### RESTful v2 - FMINDEX anchored LIKE Query and Search compared with exact PK ground truth and an unindexed twin field. - Index identity, type, persisted parameters, row progress, completion, and collection load state are verified before assertions. - Search validates exact projected values by primary key. - JSON string and missing membership template parameters are verified as server-side validation failures. - Python `bytes` are verified to fail local JSON serialization before HTTP. A positive REST binary-template case remains blocked by #53036. ## Validation - `go test -tags dynamic,test -gcflags='all=-N -l' -c ./testcases` - `go vet -tags dynamic,test ./testcases` - `go test ./membership/sbbf -count=1` - Ruff check and format check for all changed Python and REST files - Python byte-compilation for all changed Python and REST files - Pytest collection: 68 PyMilvus/FMINDEX tests and 7 REST tests - Cross-SDK membership wire-manifest verification - `git diff --check` The Proxy parameter-resolution unit test and Bloom integration package cannot execute on the host because it lacks the native `milvus_core`, `rdkafka`, and `rocksdb` pkg-config packages. A targeted PyMilvus FMINDEX run reached index creation but timed out after 120 seconds before assertions; its temporary collection was cleaned up. Full server-backed E2E was not completed. Signed-off-by: Yanliang Qiao <[email protected]> |
||
|
yihao.daiandClaude Opus 5
|
0581ea34f3 |
enhance: Support idempotent broadcast, with BulkImport as its first user (#52544)
issue: #50954 Adds an idempotency mechanism to the **broadcast** path, and makes BulkImport its first user. An orchestrator (Airflow, Temporal) that crashes after Milvus accepted an import but before it persisted the returned jobID currently retries the same files and gets a **second import job** — duplicated data and bloated segments. A client-supplied idempotency key now makes that retry resolve to the **original jobID**. The mechanism is deliberately not import-shaped. Mirroring #50007, which made WAL append generically idempotent and implemented Insert on top of it, this makes *broadcast* generically idempotent: any broadcast type becomes idempotent by carrying the key, with no proto field and no transport plumbing of its own. ## How it works ``` client ──(gRPC metadata idempotency-key / REST Idempotency-Key)──> proxy │ client interceptor ▼ copies it onward coordinator │ .WithIdempotencyKey(NewCollectionScopedIdempotencyKey(collectionID, key)) ▼ broadcast message property `_ik` │ StreamingCoord broadcaster: (messageType, scopedKey) → broadcastID ``` **Transport is generic.** A shared unary client interceptor (`pkg/util/interceptor`) copies the key from the incoming context onto outgoing coordinator RPCs, the way `cluster` already travels. No request proto carries it. **The scope is an identity the caller declares, and the type system makes it non-optional.** A client key only deduplicates within `(messageType, the scope it was bound to)`. `messageType` is added by the broadcaster, because CreateIndex and DropIndex on one collection would otherwise share a scope and the second would be silently swallowed. The scope comes from the caller, through `message.New{Cluster,Database,Collection}ScopedIdempotencyKey` — `WithIdempotencyKey` takes an `IdempotencyKey`, and no constructor produces one from a bare string, so a caller cannot ship a key that silently deduplicates cluster-wide by omission. Choosing cluster scope is a decision that reads as one. **The scope carries an ID, never a name.** That is what closes both halves of the name-vs-identity problem: - **Rename.** `RenameCollection` keeps the collectionID and changes the name. The scope is the ID, so a retry arriving after a rename still resolves to its original job. Under a name-based scope this was a silent miss that imported the same files twice. - **Drop and recreate under the same name.** The recreated collection has a new ID, so the scope differs, the lookup misses, and a fresh job is created — the correct outcome, since the two requests target different collections. No domain-specific rejection is needed for this; import's `collectionID` comparison stays only as an invariant check against an encoding or scoping bug, and is now a `ServiceInternal` error rather than a client-facing one. The encoding needs no framing tricks: `<domain>:<scopeID>:<clientKey>` where the first two fields are decimal digits and the client key is the unbounded tail, so a crafted client key is inert no matter what it contains. `messageType` prefixes it the same way. **A duplicate is transparent.** `broadcasterWithRK.Broadcast` consults the index BEFORE consuming its resource-key lock guards, and on a hit returns the original broadcast's message *and* its per-vchannel append results, rebuilt from the persisted checkpoints. Callers that only read append results cannot tell a deduplicated broadcast from a fresh one. **The broadcaster adds no admission check of its own.** Everything the caller validates runs before the dedup lookup, because the lookup cannot happen until the resource keys are held. A caller limit that the original request is itself still counted against therefore rejects that request's retry — see the retry edge under Known limitations. The one bound that does sit below the lookup is the broadcaster's own key-length check, which is taken over the **client portion** of the key so the scope this process prepends cannot push a legal key over the advertised limit. **Why the lookup lives inside `Broadcast`:** `broadcasterWithRK` only exists once the resource keys are held, so "look up under the lock" is enforced by the type. Without that ordering two concurrent same-key requests would both miss and create two jobs. Adopting it from a new subsystem is one line — `.WithIdempotencyKey(message.NewCollectionScopedIdempotencyKey(id, interceptor.IdempotencyKeyFromContext(ctx)))` — plus whatever decoding that subsystem needs to recover its own response. Import's remaining domain-specific code is exactly that: decode the jobID from its own message body. ## Idempotency window The index lives and dies with the broadcast task, so **the window equals the broadcast-task tombstone retention**: `streaming.walBroadcaster.tombstone.maxLifetime` (24h) or `maxCount` (8192 tasks), **whichever comes first**. The count bound is hard — a cluster with heavy DDL or import churn can evict tombstones well before 24h. Retries outside the window create a new job; that is contract, not defect. Any subsystem that hands a client an ID derived from a deduplicated broadcast should keep that ID resolvable at least as long as the window. For import that means `dataCoord.import.taskRetention` should exceed `maxLifetime` with margin; its default moves 10800 → 172800, and the coupling is stated in that config's own doc. ## Verified end to end - `_ik` survives every property-mutating transform in the repo — replication, per-vchannel split, the broadcast-task persist/restore round trip, clones, `MarkIgnore`. Two round trips proven by execution, not by reading. - **A rename between a request and its retry no longer ends the window.** Proven at the scope level and end to end through `Broadcast`: a retry holding the renamed collection's resource keys still resolves to the original broadcastID. Mutation-tested — folding the resource keys back into the scope turns both assertions red. - **Concurrent same-key requests through the real entrypoint** (`TestConcurrentSameKeyBroadcastsCreateExactlyOneTask`): eight goroutines through `broadcastTaskManager.WithResourceKeys`, exactly one task created, all eight answered with the same broadcastID, one index entry. This is the invariant import's dedup correctness rests on and it was previously only asserted against a hand-built `broadcasterWithRK`; it is mutation-tested (disabling the lookup makes it red). - **The zero-key trap is pinned**: an empty client key must produce the zero key rather than a bare scope prefix, or every keyless broadcast of one message type would deduplicate against every other. No success-path test reaches that branch, so it is asserted directly. - Crash after PENDING persist: recovery rebuilds the index across **all** task states, tombstones included, and re-acquires the resource keys. - Tombstone evicted by `maxCount`: the entry goes from memory **and** etcd; a restart cannot resurrect it. - Job GC'd while its tombstone survives: the original jobID is still returned. `GetImportProgress` reports the missing job on its own, so DataCoord does not check it a second time on the way out. - Oversized key: rejected once, at the broadcaster — the single point every entry path passes through. A limit of `0` fails closed; a keyless request is unaffected at any limit. A key sitting exactly on the advertised limit still deduplicates, which is what proves the bound is taken over the client portion and not over the encoded value. - Scope encoding: the client key is the unbounded tail of the encoding, so a key crafted out of the encoding's own separators cannot reach another scope's entry — asserted in both directions. - A retry sent **while the job limit is full** is rejected before the lookup, and rejected cleanly: no broadcast, no second job. A request that is not a duplicate is rejected under the same limit. Both directions mutation-tested. - The REST `Idempotency-Key` header is in the CORS preflight allowlist, so a cross-origin browser client can actually send it. - **The mechanism is not import-specific:** a CreateCollection broadcast — a type with no idempotency proto field and no coordinator that reads a key — deduplicates through the same machinery purely by carrying the key. - `make static-check`: four stages, 0 issues. Every new test in this PR was mutation-tested: the mechanism it guards was removed and the test confirmed to go red. ## NOT verified - **The integration test has not been re-run since the scope redesign.** `tests/integration/import/idempotency_test.go` was green on MiniClusterV3 before it (a client sends the key as real gRPC metadata, retries, gets the original jobID, and a Strong-consistency `count(*)` shows the data was written once; with deduplication disabled the retry produces a different jobID and doubles the row count). It drives the key purely as gRPC metadata and touches nothing this redesign changed, but that is reasoning, not a run. - **The ctx-cancel/timeout branch of `Broadcast`** is traced by reading, not fault-injected. Note the behavior there is *more* idempotent than a naive reading suggests: the task and its index entry are created before the scheduler can fail, so a retry resolves to the original. Do not read this as "no index entry is left on error". - **The secondary / promoted-cluster path** is traced by reading only. Post-failover behavior — a promoted secondary honoring pre-failover keys, and whether two clusters' `broadcastID` spaces can collide — is unverified. - **Real crash-restart recovery** is not exercised end to end; the recovery test feeds hand-built protos to `newBroadcastTaskManager`. - `internal/proxy`'s full suite needs a live Pulsar broker and was not run. ## Known limitations - **The window can end early.** `maxCount` is a hard bound, so 24h is an upper bound, not a guarantee. - **The request payload is not validated against the original.** A duplicate returns the original jobID whatever the retry carries. Keeping the key unique per logical request is the client's contract. This is deliberate: enforcing it server-side means inventing an equality predicate over request content, and the failure directions are not symmetric — a *false* mismatch would reject a legitimate retry and tell the caller to mint a new key, importing everything twice. - **The scope axis and the lock axis are no longer the same thing.** The serialization of two concurrent same-key requests holds only when the caller's exclusive lock covers the object the key is scoped to. Import satisfies this (collection scope, `ExclusiveCollectionName` on that collection). An adopter that scopes to one object while locking another, or that holds only a shared lock, gets no serialization: both requests can miss, and the index keeps the first `broadcastID` while the second has already reached the WAL. This is stated in the code and in the broadcaster doc; it is not enforced. - **A retry can be rejected while the job limit is saturated.** Everything DataCoord validates — `dataCoord.import.maxImportJobNum`, the backup-file listing, the replication topology — runs before the broadcaster's idempotency lookup, which cannot happen until the resource keys are held. So a retry sent while the limit is full, counting the original job among others, is rejected rather than resolving to that job. Nothing is imported twice by the rejection itself: no broadcast happens, and retrying the same key once a slot frees up resolves normally. A client that answers the rejection by minting a fresh key is what imports twice, which is why the contract says not to. - **A failed original poisons the key for the rest of the window.** The duplicate branch resolves a key without consulting the original job's state, so a key whose original ended `Failed` keeps returning that failed jobID. That is ordinary idempotency semantics, and it is the one case where minting a fresh key is the correct move. `ImportV2` logs the original job's state on every hit so an operator can tell this apart from a key waiting on a healthy job. - **The rolling-upgrade compat path is not idempotent.** `forwardImportToDataCoord` exists for old proxies, which send no key. Pre-existing behavior, unchanged. - **The key is read on every endpoint, and validated there.** A gin middleware installed ahead of both the `/v1` and `/v2/vectordb` route groups carries it on every REST route, and the shared client interceptor carries it on every coordinator RPC. An endpoint that does not support idempotency **ignores a well-formed key** rather than rejecting it — inventing an error for a capability that endpoint does not claim would be worse than carrying a few extra bytes. A *malformed* key (over the length bound, or holding a byte gRPC metadata cannot carry) is rejected on any route, because forwarding it would turn one request into a transport-level `codes.Internal` and repeated teardowns of a shared connection. - **Lowering `streaming.idempotency.maxKeyLength` mid-window rejects in-window retries.** The broadcaster checks the bound *after* the lookup, so it adds no second rejection once a key is admitted — but both doors validate the same refreshable parameter *before* the request reaches the broadcaster, so a lowered limit does reject a retry of an already-indexed key at the edge. Lower it only when no window that matters is open. - **The retention coupling is documented, not enforced.** Nothing warns when `taskRetention` is configured below the tombstone lifetime; a retry inside the advertised window then resolves to a jobID whose job has already been collected, and the client sees that when it queries progress. Matching the two exactly is not enough either: `tombstoneScheduler.Initialize` re-stamps every recovered tombstone with `time.Now()`, so a StreamingCoord restart delays that tombstone's GC by up to another `maxLifetime` while `taskRetention` keeps counting from the job's own completion. The window is therefore a lower bound, and both configs now say to leave margin. Persisting the tombstone's real creation time is a metastore change and is not in this PR. - **`dataCoord.import.taskRetention` default increases 16×**, affecting every cluster including those that never use an idempotency key. - **The key is stored verbatim** in the WAL (one copy per vchannel), in the broadcast task in etcd, and is replicated to secondaries. Only logs carry a fingerprint instead, on the grounds that logs travel further than the data plane. Callers should not put secrets in the key. - **A result computed after the broadcast is not covered.** Import's jobID is in the request message, so it round-trips for free. A future type whose response is generated in the ack callback would need an opaque result slot on the broadcast task — the analogue of #50007's `IdempotentResult`. Designed for, not built. --------- Signed-off-by: bigsheeper <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> |
||
|
|
48b649a616 |
feat: [RLS1] add row-level security metadata foundation (#52072)
relate: #50263 design doc: docs/design-docs/design_docs/20250610-rls_design.md design doc PR: #53173 ## Summary Adds the collection RLS switch, management APIs, privileges, validation, and persistence. --------- Signed-off-by: aoiasd <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Codex <[email protected]> |