mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 02:05:41 +00:00
master
25145
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
7b05f993f3 |
fix: avoid DataNode panic when a binlog is missing during sort compaction (#51121)
## What this fixes When sort compaction reads a segment whose binlog references an object that is missing in object storage, the DataNode panics with a nil-pointer dereference and enters `CrashLoopBackOff`, instead of failing the compaction task cleanly. ## Root cause `IterativeRecordReader.Next()` assigned `ir.cur` **before** checking the error returned by `iterate()`. When opening the next binlog chunk fails, `newPackedRecordReader` returns a nil `*packedRecordReader` boxed into a non-nil `RecordReader` interface (a typed-nil). The deferred `rr.Close()` in `sortSegment` then called `Close()` on a nil receiver and panicked — the existing `if pr.reader != nil` / `if ir.cur != nil` guards don't help because the receiver itself is nil. ## Fix - Only publish the reader once `iterate()` succeeds (assign to a temp, check the error, then set `ir.cur`). - Make `packedRecordReader.Close` / `ffiPackedRecordReader.Close` nil-receiver safe as defense-in-depth. - The missing-object error is still surfaced from `Next()` (it is **not** converted to `io.EOF`), so the sort task fails cleanly and is retried rather than silently dropping data. ## Test Added `TestIterativeRecordReader_MissingChunkDoesNotPanic`, which reproduces the missing-object chunk: it asserts the error is surfaced from `Next()` and that the deferred `Close()` does not panic. issue: #50927 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: bigsheeper <yihao.dai@zilliz.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
25ac6b4c38 |
fix: correct the CDC replication lag metric (#51049)
## Summary The documented CDC lag PromQL subtracted two independently scraped time-tick gauges, which caused idle oscillation and misleading restart behavior. This PR fixes the query and hardens the lag metric lifecycle without introducing a new metric. ### #50633 — idle oscillation Subtract each gauge timestamp before calculating the difference, canceling the cross-target scrape-phase offset: ```promql clamp_min( max by (channel_name) (milvus_wal_last_confirmed_time_tick - timestamp(milvus_wal_last_confirmed_time_tick)) - min by (channel_name) (milvus_cdc_last_replicated_time_tick - timestamp(milvus_cdc_last_replicated_time_tick)), 0) ``` ### #51048 — restart and switchover lag lifecycle - Seed `milvus_cdc_last_replicated_time_tick` from `InitializedCheckpoint` before target-client initialization, so target-unavailable startup exposes a conservative lag instead of an absent series. - Overwrite the seed with the target-confirmed checkpoint after `GetReplicateInfo` succeeds. - Ignore nil and zero checkpoints to avoid creating an epoch-valued series. - Remove lag-series deletion from generic stream `OnClose`. - Delete the lag series only from `ReplicateManager.RemoveReplicator`, the genuine etcd DELETE path. Outdated revision cleanup therefore cannot delete the live revisions shared label pair. issue: #50633 issue: #51048 ### Test Plan - [x] Added unit-test coverage for initialized-checkpoint seeding and nil/zero guards. - [x] Added regression coverage for genuine removal versus outdated revision cleanup. - [ ] Local Go tests and static check. - [ ] Confirm the lag lifecycle on a live switchover environment. --------- Signed-off-by: Yihao Dai <yihao.dai@zilliz.com> Signed-off-by: bigsheeper <yihao.dai@zilliz.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Jeng.GwanandGitHub
|
9ffd97c0f0 |
fix: update getFailMap to include status for failed get operations (#51099)
issue: #51100 ## Summary The C++ persistent-storage `get` failure counter was registered without the `status="fail"` label, unlike the other operation failure counters. The counter was still incremented only on failed `get` operations, but Prometheus exported it as a no-status `get` series, so status-based dashboards and queries could not identify it as a failure series. This PR adds `status="fail"` to `getFailMap`, making `internal_storage_op_count{persistent_data_op_type="get", status="fail"}` consistent with `put`, `stat`, `list`, and `remove` failures. ## Note This changes the Prometheus time series shape: the old no-status `get` failure series will stop, and the new `status="fail"` series will start from zero. Signed-off-by: xaxys <tpnnghd@163.com> |
||
|
jiaqizhoandGitHub
|
b847349e39 |
enhance: bump milvus-storage to e658197 (#51406)
issue: #50915 for errorcode and lance upgrade to 7.0.0 Signed-off-by: jiaqizho <jiaqi.zhou@zilliz.com> |
||
|
Li LiuandGitHub
|
c819a2c5e1 |
build(deps): upgrade go toolchain to 1.26.5 (#51271)
## What - Upgrade the root, pkg, client, test, and telemetry example modules from Go 1.26.4 to Go 1.26.5. - Upgrade CPU/GPU builder Dockerfiles, macOS CI Go setup, KRTE build arg, rpm setup, meta-migration builder, and go-client test image to Go 1.26.5. - Point CPU and GPU builder consumption in `.env` to the successful build-env tag `20260714-c135601`. ## Why - Latest direct Trivy scan of `milvusdb/milvus:master-20260711-1993feae-amd64` reports CVE-2026-39822 in Go stdlib 1.26.4, fixed in 1.26.5. - The normal daily image scan is currently blocked through Jenkins `milvus_scan_image_daily` #540 with `exec format error`; last successful scan is #508 from 2026-06-10, so this was verified by direct image scan. ## Image coverage - Go toolchain fixes are shared by all Milvus runtime images; no per-OS runtime Dockerfile package change is required. - Builder definitions are updated for CPU ubuntu22.04, ubuntu20.04, ubuntu24.04, amazonlinux2023, rockylinux9 and GPU ubuntu22.04, ubuntu20.04. - No runtime image variant is deliberately skipped; this is not an OS-package CVE. ## Verification - `go.dev/dl/?mode=json&include=all` confirms go1.26.5 is stable and linux amd64/arm64 tarballs exist. - `go env GOTOOLCHAIN GOVERSION` selects `auto` / `go1.26.5` after the module update. - `go list -m` succeeds for root, pkg, client, tests/go_client, and both telemetry examples. - `cd pkg && go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./util/typeutil` passes after rebasing onto the latest upstream master. - Jenkins build-env #40 succeeded and published builder tag `20260714-c135601`; `.env` now selects that tag for CPU and GPU CI builds. - A repo-wide audit found no remaining Go 1.26.4 toolchain pins. Note: full root `go mod tidy` is not included because this checkout has a root-owned `deployments/docker/dev/volumes/etcd/member` directory that makes `go mod tidy` fail with permission errors; `go mod tidy -e` subsequently OOMed locally. No go.sum changes were produced by the successful module tidies. Generated by autonomous CVE maintenance. --------- Signed-off-by: Li Liu <li.liu@zilliz.com> |
||
|
sparknackandGitHub
|
6bd2418df7 |
enhance: Load JsonStats shredding through manifest translator (#51116)
issue: #51114 JsonStats shredding now loads through `ManifestGroupTranslator`/`ChunkReader`, but JsonStats files do not have the normal sealed-segment manifest metadata. The load path reconstructs the needed column-group view from the existing parquet footer/schema: column names, Arrow field ids, row counts, and file ordering. This keeps the current `meta.json` contract unchanged; parquet key-value metadata is only a fallback for old files that do not have the separate meta file. The main follow-up after switching to the manifest translator was projection behavior. Lazy loading now builds one projected reader per shredding column, so a predicate on one JSON path does not pull the whole group; when warmup is enabled it still uses the full column group so warmup can preload everything together. The added tests cover parquet files without packed field-list metadata, multi-file ordering, lazy single-column projection, and warmup full-group projection. --------- Signed-off-by: Shawn Wang <shawn.wang@zilliz.com> |
||
|
|
6ca0350944 |
fix: type the zero-hit chunk's $id array after the other chunks in MergeOp (#51374)
issue: #51372 ### Problem `MergeOp.buildResultArrays` builds a zero-hit chunk's `$id` array with a hardcoded Int64 builder (`operator_merge.go:713-721` before this change), while chunks that do have hits are typed from the first actual ID (`buildInt64Results` / `buildStringResults`). All per-query chunks then go into `AddColumnFromChunks`, which builds one `arrow.NewChunked` column out of them. For a **VarChar-PK collection** where **one query of an nq >= 2 batch returns zero hits** while another returns hits, those chunks mix `utf8` and `int64` and arrow panics: ``` panic: invalid: arrow/array: mismatch data type int64 vs utf8 ``` `MergeOp` sits at chain index 0, so every request that goes through a function rerank chain — plain search with a rerank function as well as hybrid search — can hit it. The proxy has no recovery interceptor on its gRPC chains (`internal/distributed/proxy/service.go:295-307`, `416-424`) and no `recover()` in `internal/proxy/`, so the panic takes the proxy process down. The input side was never affected: `collectRRFScores` / `collectWeightedScores` read IDs row by row, so an empty chunk is simply iterated zero times. Only the output side was broken. ### Fix Resolve the `$id` arrow type once per merge, from the first input that actually carries IDs (`resolveIDType`), and build the zero-hit chunk with that type. Zero-row inputs are deliberately skipped when resolving: an empty result carries no ID type of its own and is materialized as an empty Int64 column regardless of the collection's PK type (`importEmptyIDs`, converter.go). When *every* input is empty, Int64 is kept — every chunk is then empty as well, so the column is self-consistent and the rerank operator's `allEmpty` early-return short-circuits before it anyway. An unexpected arrow ID type now returns a `merr` error instead of silently producing an Int64 array. ### Testing - `TestMergeWithEmptyChunkVarCharIDs` — single input, VarChar IDs, `topks=[2,0]`; asserts the merged `$id` column stays `utf8` with an empty second chunk. Written first and observed panicking with `mismatch data type int64 vs utf8` on unmodified master. - `TestMergeMultiInputEmptyLegVarCharIDs` — hybrid-search shape: one leg entirely empty, the other with a zero-hit query. - The existing `TestMergeWithEmptyChunk` (Int64) still passes, pinning the unchanged Int64 path. - `./internal/util/function/...` green, including `-race`; `gofmt` clean. ### Note Found while reviewing #51156 (fix for #50969). This panic is **not** introduced by that PR — `operator_merge.go` is byte-identical between master and its head, and the repro above does not touch the lines it changes. #51156 does widen the reachable surface, though: before it, a single-shard zero-hit result failed earlier in the converter with `unsupported ID type` and never reached `MergeOp`; after it, that input builds a DataFrame and walks into this panic. Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
41eacb5dc4 |
test: run heavy snapshot / external-table go-sdk cases serially (#51370)
## What Drop `t.Parallel()` from the heavy go_client cases in `snapshot_test.go` (10 cases) and `external_table_refresh_test.go` (12 cases), and mark each with a comment explaining why it must stay serial. ## Why `t.Parallel()` was blanket-added to every go_client testcase in #49138. That is fine for the light cases, but the snapshot/restore and external-table refresh cases are heavy: they insert tens of thousands of vectors and then wait minutes for index build / snapshot / restore. Running them alongside the large parallel test swarm starves the shared standalone cluster, and they flake on their own index-build / restore timeouts, e.g.: - `TestSnapshotRestoreWithMultiSegment` — timeout waiting for indexes to be built - `TestSnapshotRestoreWithMultiFields` — timeout waiting for restore to complete Go runs the non-parallel tests first (sequentially, each with the cluster to itself), then the parallel suite. Keeping these heavy cases serial gives them the resources they need while leaving the light parallel tests untouched. The trade is a little extra wall-clock for stability. ## Changes - `tests/go_client/testcases/snapshot_test.go` — remove `t.Parallel()` from 10 heavy cases; add marker comment. - `tests/go_client/testcases/external_table_refresh_test.go` — remove `t.Parallel()` from 12 heavy cases; add marker comment. No test logic changed; `gofmt` clean, `go vet ./testcases/` passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
7ef2d32c49 |
test: cover woodpecker WAL truncate/retention read semantics (#50880)
## What this PR does Adds an integration test (`TestWpRetentionTruncateRead`) for the Woodpecker (wp) WAL implementation pinning the read semantics around truncation that Milvus relies on but had no explicit coverage for. Milvus truncates the wp WAL aggressively right after a flush, but truncation only moves a metadata watermark — it does **not** delete data. Data at/under the watermark stays readable until the retention-TTL GC physically removes it. The test pins this with three sub-cases: 1. **TruncatedDataStillReadable** — after truncating to the latest id, an explicit `StartFrom` at any position in `(earliest, latest]` (spanning segment boundaries) still returns every message, including the watermark id itself. As a contrast, a fresh `DeliverPolicy_All` reader is parked past the watermark and returns nothing — proving the watermark really moved while the data underneath is intact. 2. **SeekForwardWithinSegment** / **SeekForwardCrossSegment** — a reader asking for data older than the earliest deliverable position (`DeliverPolicy_All` after truncation) is moved forward to the first available position instead of erroring or stalling. The two sub-cases exercise both branches of `adjustPendingReadPointIfTruncated`: a truncation point inside segment 0 (within-segment) vs. in a later segment (cross-segment id jump). ## Notes - **Deterministic by design**: large retention (`72h`) so GC never fires, a small `segmentRollingPolicy.maxSize` so writes span several segments, single-threaded writes so ids are monotonic, and all assertions are message-id based. Run logs confirm each sub-case hits its intended adjustment branch (`truncatedSegmentId` 0 vs >0). - **Scope**: covers the open-time read-position adjustment after truncation. It does NOT exercise the physical retention-TTL GC + runtime skip-forward path — that needs waiting for the async GC sweep and is intentionally left out to keep the test deterministic. - Runs against local fs storage; the truncate/read logic under test lives in the woodpecker log handle/reader and is storage-backend agnostic. issue: #43638 --------- Signed-off-by: tinswzy <zhenyuan.wei@zilliz.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
5eebaa9ad4 |
enhance: add WAL trace propagation (#50796)
issue: #47404 - message trace context: add trace context serialization and restore/inject helpers for WAL messages and msgstream conversion - WAL append trace: normalize WAL spans for autocommit, txn, broadcast, append, appendimpl, and broadcast callback paths - trace propagation: restore message trace context in producer, broadcast retry, replicate primary/secondary, recovery, flusher, and query/data flowgraph consumers --------- Signed-off-by: chyezh <chyezh@outlook.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
fudongyingandGitHub
|
5cee9b607e |
fix: Return actual db name and db id in cached DescribeCollection queried by collection id (#51254)
issue: #51253 ### What this PR does When `proxy.enableCachedServiceProvider` is on and `DescribeCollection` is called with only a collection id (e.g. the HTTP management API on port 9091), the cached provider echoed the request db name — empty or default — instead of the database the collection actually belongs to, and never filled `db_id`. The remote path does not have this problem because the coordinator resolves the database by DBID and returns it in the top-level `DbName`/`DbId` fields. The proxy meta cache already stores the actual db name of each collection; this PR also stores the db id, and prefers the cached `dbName`/`dbID` when assembling the cached DescribeCollection response. ### Verification - New unit test `TestCachedProxyServiceProvider_DescribeCollection_ByIDReturnsActualDbName` (request carries only `collectionID`, asserts real `db_name`/`db_id` are returned); existing tests in the file still pass. - Verified end-to-end on a standalone build of this branch: created `db1`/`coll1`, queried `GET :9091/api/v1/collection` with only `collectionID` — response now returns `"db_name": "db1"` and the `db_id` matching `databases/describe`, where it previously returned neither. - The same logic was also verified on a 2.5-based deployment (real cluster) before porting to master. Happy to backport to 2.5/2.6 if needed. Signed-off-by: fudongying <fudongying@bytedance.com> |
||
|
wei liuandGitHub
|
47e64ff08e |
fix: Avoid external collection DML catchup on cold load (#51089)
issue: #45881 ## What changed - Mark read-only external collection delegators as not catching up DML streaming data during cold load. - Skip WAL tSafe waits for external Search, Query, QueryStream, and GetStatistics reads. - Always use `MaxTimestamp` as the MVCC timestamp for external reads, including partial searches and requests carrying an explicit finite `MvccTimestamp`, so loaded deltalog deletes remain visible. - Prevent external reads from updating or exposing a required WAL MVCC timetick. - Preserve the existing streaming catch-up and MVCC behavior for normal collections. ## Why External collections are read-only and query-visible data comes from the loaded external snapshot, not DML WAL replay. Starting from an old channel checkpoint can otherwise keep QueryNode unavailable while it catches up WAL history that cannot change the external snapshot. Using the full MVCC range also prevents low guarantee placeholders or explicit finite MVCC timestamps from hiding rows or deltalog deletes already loaded as part of that snapshot. This PR does not change external channel checkpoints or broadcast refresh messages to collection vchannels. ## Validation - `make clean && make milvus` - `git diff --check` - Targeted QueryNode delegator tests: ```bash go test -tags dynamic,test -gcflags="all=-N -l" \ github.com/milvus-io/milvus/internal/querynodev2/delegator \ -run 'Test(ExternalCollectionDelegatorDoesNotCatchUpStreamingData|'\ 'ExternalCollectionWaitTSafeUsesFullSnapshotTimestamp|'\ 'ExternalCollectionPartialSearchUsesFullSnapshotTimestamp|'\ 'ExternalCollectionQueryUsesFullSnapshotTimestamp|'\ 'ExternalCollectionQueryStreamUsesFullSnapshotTimestamp|'\ 'ExternalCollectionStrongConsistencyDoesNotRequestWALMVCC|'\ 'NormalCollectionDelegatorCatchesUpStreamingData)$' -count=1 ``` Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
|
5bce29fed1 |
fix: prepare release handoff before manual flush (#51267)
issue: #50425 Signed-off-by: luzhang <luzhang@zilliz.com> Co-authored-by: luzhang <luzhang@zilliz.com> |
||
|
congqixiaandGitHub
|
57b314cb48 |
enhance: move JSON indexes into sealed segment runtime state (#51323)
Related to #51068 - manage JSON index load, replace, and drop through reopen COW snapshots - remove LoadScope_Index and the legacy QueryCoord-to-segcore update path - drop JSON indexes by field and nested path to preserve sibling indexes - regenerate query proto and QueryNode mocks --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
Li LiuandGitHub
|
7ca8693af4 |
Update maintainers in OWNERS_ALIASES (#51356)
Signed-off-by: Li Liu <li.liu@zilliz.com> |
||
|
|
a50765f1c9 |
enhance: reduce coordinator metadata lock contention (#51255)
issue: #51333 ## Summary - serialize DataCoord checkpoint mutations per channel so unrelated channel catalog writes can proceed concurrently - keep overlapping single, batch, mark, and drop operations serialized with deterministic multi-channel lock ordering - use a read lock for the read-only RootCoord `DescribeAlias` path - exclude all newly added metrics and profiling instrumentation --------- Signed-off-by: sunby <sunbingyi1992@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Spade AandGitHub
|
9e11bdae4e |
fix: fix ngram index UTF-8 literal length checks (#51238)
issue: https://github.com/milvus-io/milvus/issues/42053 This PR fixes ngram index eligibility checks for UTF-8 literals by using character count instead of byte length in the C++ gate and phase1 validation. It also adds regression coverage for short multibyte literals to ensure they correctly fall back instead of being sent to Tantivy ngram queries. Signed-off-by: SpadeA <tangchenjie1210@gmail.com> |
||
|
JeremiahandGitHub
|
69da6f0f66 |
fix: link milvus_conan_deps to gcp-native-storage (#51326)
issue: #36214 issue: #51324 ## Summary gcp-native-storage transitively depends on `boost/align/aligned_allocator.hpp` and `google/cloud/storage/retry_policy.h` headers but did not declare milvus_conan_deps. As a result, building with ENABLE_GCP_NATIVE=ON failed because the required Conan include directories were not propagated to the object target during compilation. ## Changes Link milvus_conan_deps directly to gcp-native-storage in `internal/core/src/storage/gcp-native-storage/CMakeLists.txt` so its usage requirements are available when compiling the target. ## Verification Verified locally. ### Reproduce ```bash ENABLE_GCP_NATIVE=ON make build-cpp ``` Before this change, compilation failed with: ```text fatal error: boost/align/aligned_allocator.hpp: No such file or directory fatal error: google/cloud/storage/retry_policy.h: No such file or directory ``` After this change: - `ENABLE_GCP_NATIVE=ON make build-cpp` succeeds. - `make verifiers` passes, including: - build-cpp - getdeps - cppcheck - rustcheck - fmt - static-check Signed-off-by: Jeremiah Xing <dev.jrmh@gmail.com> |
||
|
sijie-ni-0214andGitHub
|
1acbe20c57 |
enhance: Reduce QueryNode segment memory usage (#51053)
## Summary issue: #51052 This PR reduces QueryNode memory pressure by avoiding duplicated or long-lived segment metadata after sealed segment load/reopen and by caching frequently reused schema conversion state. ## Storage Mode Scope - For manifest-backed Storage V3 segments, both Go-side runtime load info and segcore-side runtime load info are compacted after load/reopen. - For Storage V2 binlog-mode segments, Go-side sealed runtime load info is compacted, while segcore keeps the full load info because binlog diff still depends on binlog/index metadata. - The optimization primarily targets Storage V3, which is the forward path. ## Commit Breakdown 1. `9d4dbdf065 enhance: cache text LOB loon arrow schema` - Cache the Loon Arrow schema generated for the TEXT LOB binary projection used by Storage V3 load paths. - Invalidate the cache when schema fields change and clear mutable cache state on schema assignment. 2. `4f0bb58ef6 enhance: avoid duplicate segment index load info` - Remove duplicated converted index load info storage. - Reuse the existing field-index cache for current index identity and diff/drop decisions. 3. `89e8a15b42 enhance: compact runtime segment load info` - Compact Go QueryNode sealed segment load info after load/reopen. - Keep runtime-required lightweight fields and cache related data size before dropping large metadata. 4. `8a38b2fdbf enhance: skip real count scan without deletes` - Return row count directly when a segment has no deleted rows. - Avoid scanning the deleted bitmap on the real-count fast path. 5. `d041837d33 enhance: compact segcore runtime load info` - Compact manifest-backed segcore runtime load info after load/reopen. - Preserve lightweight diff identity and local runtime state needed by schema-only reopen and later diffs. ## Test Plan - [x] `git diff --check upstream/master...HEAD` - [x] `gofumpt -l internal/querynodev2/segments/segment.go internal/querynodev2/segments/segment_loader.go internal/querynodev2/segments/segment_test.go internal/querynodev2/segments/utils.go internal/querynodev2/segments/utils_test.go` - [x] `/opt/homebrew/opt/llvm@18/bin/clang-format --dry-run --Werror --style=file <changed C++ files>` - [x] `ninja -C cmake_build all_tests` - [x] `./cmake_build/unittest/all_tests --gtest_filter='SchemaTest.ConvertToLoonArrowSchemaCachesTextLobBinarySchema:SegmentLoadInfoTest.CompactRuntimeInfoForManifest:SegmentLoadInfoTest.CompactRuntimeInfoForManifestKeepsIndexDiffIdentity:SegmentLoadInfoTest.CompactRuntimeInfoForManifestSchemaCopyKeepsIndexIdentity:SegmentLoadInfoTest.ReplaceSchemaForReopenPrunesDroppedFieldRuntimeState:SealedSegmentLoad.LoadPublishesDefaultFilledStateAfterCompact:SealedSegmentReopen.SchemaOnlyReopenPublishesDefaultFilledState:SealedSegmentReopen.SchemaAwareReopenDiscardsOlderSchema:SealedSegmentReopen.SchemaOnlyReopenPreservesCreatedTextIndexState:SealedSegmentReopen.TextIndexCreatedWipedByReopen'` - [x] `go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" -ldflags="-extldflags \"-Wl,-rpath,${MILVUS_WORK_DIR}/cmake_build/lib -Wl,-rpath,${MILVUS_WORK_DIR}/internal/core/output/lib -Wl,-rpath,${MILVUS_WORK_DIR}/cmake_build/thirdparty/boost_ext\"" ./internal/querynodev2/segments -run "Test(CompactSegmentLoadInfoForRuntime|GetSegmentRelatedDataSize)$" -timeout 300s` - [x] `go test -v -count=1 -tags dynamic,test -gcflags="all=-N -l" -ldflags="-extldflags \"-Wl,-rpath,${MILVUS_WORK_DIR}/cmake_build/lib -Wl,-rpath,${MILVUS_WORK_DIR}/internal/core/output/lib -Wl,-rpath,${MILVUS_WORK_DIR}/cmake_build/thirdparty/boost_ext\"" ./internal/querynodev2/segments -timeout 300s` timed out locally after 303.862s while still running MinIO-backed retrieve tests (`TestRetrieveWithFilter`). --------- Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> |
||
|
|
4dbaba0042 |
fix: skip dropped-field column groups when loading StorageV3 segments (#51275) (#51277)
issue: #51275 ### Problem Under StorageV3 growing-source flush, a manifest legally keeps column groups whose fields were later dropped: drop does not rewrite flushed data, and compaction removes the leftover lazily. Loaders however projected such groups against the **current** schema and treated the mismatch as an error: - **Growing recovery** (`SegmentGrowingImpl::LoadColumnsGroups`) submitted every manifest group with a full-schema projection; a group whose fields were all dropped has an empty projection, which milvus-storage rejects by contract (`ChunkReaderImpl::open`: "No needed columns found in column group"). The channel-recovery level retried the deterministic failure every second — observed locally: **4140 identical failures over ~3 minutes**, the channel unavailable (`no available shard leaders`) the whole time, until a compaction rewrite happened to remove the group. - **Sealed load** (`ChunkedSegmentSealedImpl::LoadColumnGroups`) resolved every manifest column to a field id without a membership check and tripped the schema-lookup assert on any group still carrying a dropped column — mixed groups included. ### Fix Treat a dropped-field column as a legal leftover at the loaders: - Growing: skip a column group before submitting its load task when none of its columns exist in the segment schema (logged). - Sealed: filter resolved field ids absent from the schema snapshot while building `cg_field_ids` (logged); a group filtered to empty produces no load task. Mixed groups keep loading their live columns via projection, unchanged. ### Verification - Local A/B with a schema-evolution chaos workload (concurrent add/drop ordinary + BM25 function field, DML/query/search, `useLoonFFI` + growing-source flush enabled, StreamingNode SIGKILL/restart injections): before the fix, recovery wedged in the retry loop (4140 errors / ~3 min / final query 503); after the fix the same scenario logs 2 group skips, the channel recovers in seconds, and the full serial consistency validation passes. - New UT `LoadGrowingSegmentSkipsDroppedFieldColumnGroup`: flushes a manifest with a field-exclusive column group (writer pattern `"0|1|100,101"`), reloads the segment under a schema without that field, and asserts the load succeeds with the group skipped (plus a guard assertion that the manifest really contains the exclusive group). - The sealed-path filter is exercised by code inspection and mirrors the growing-side semantics; on current master the external-table load path derives its projection from schema-owned column names and is not exposed to this class. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
3c4aed6e0c |
enhance: upgrade knowhere to remove num_build_thread upper-bound validation (#51259)
## Summary - Upgrade Knowhere dependency to 49507ef3 to pick up the fix that removes the num_build_thread upper-bound validation. issue: #42937 Signed-off-by: xianliang.li <xianliang.li@zilliz.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
c322c84c22 |
test: cover add-field default values on growing segments (#51303)
## What this PR does - Remove the `xfail` marker from the existing drop-and-readd analyzer-field regression for #50484. - Add L1 E2E coverage for the live QueryNode growing-segment reopen path when an `enable_match` VARCHAR field with `default_value` is added to loaded growing data. - Verify the default value is immediately searchable on pre-existing unflushed rows, eventually searchable on a subsequent defaulted insert after the growing text index catches up, and remains searchable after flush/reload. Related fix: #51201 issue: #50484 ## Scope note This PR validates the live reopen path: ```text loaded collection -> insert rows that stay in a QueryNode growing segment -> add an `enable_match` VARCHAR field with `default_value` -> query triggers LazyCheckSchema/Reopen -> default values for pre-existing growing rows are indexed and searchable immediately ``` It does not cover the QueryNode recovery / LoadGrowing path. That path requires forcing a QueryNode restart, replacement, or channel rewatch while the data is still growing: the new QueryNode can create the segment with the latest schema and then load old binlogs written before AddField. Since the segment schema is already current in that path, LazyCheckSchema/Reopen is not the mechanism being tested here. Recovery coverage should be handled by a focused stability test. ## Test results - `test_milvus_client_add_match_field_with_default_value_on_growing_data`: 3/3 passed - `test_drop_then_add_same_name_analyzer_field`: passed - Ruff lint and format checks: passed - `git diff --check`: passed Signed-off-by: lyyyuna <yiyang.li@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
wei liuandGitHub
|
10adefb7d4 |
fix: Load StorageV3 child deletes during compaction fallback (#51208)
issue: #49706 issue: #50663 ## What This PR fixes StorageV3 compaction fallback loading when delete records are stored in compact-to child manifests instead of legacy deltalogs. Changes: - Add `child_manifest_paths` to DataCoord `SegmentInfo` and QueryCoord `SegmentLoadInfo` as a load-time delete overlay. - Make DataCoord recursively collect compact-to descendant delete sources for cloned unhealthy segment responses. - Keep legacy child delete logs in `Deltalogs`, and keep StorageV3 child delete logs as `ChildManifestPaths`. - Pass `ChildManifestPaths` through QueryCoord to QueryNode. - Make QueryNode load parent delete data and child manifest delete data into one `DeltaData` before calling `LoadDeltaData`. ## Why Fallback loading keeps the parent segment identity, but StorageV3 compact-to children may keep newer delete records only in their manifests. The old fallback path only appended legacy child deltalogs, so it could miss V3 child deletes and expose rows that should be filtered. The V2 path is kept for compatibility. Mixed chains such as parent V2 to child V3, and parent V3 to child V2, are handled by keeping each child delete source in its native representation. ## Validation - `gofmt -w internal/datacoord/services.go internal/querycoordv2/utils/types.go internal/querynodev2/segments/segment_loader.go` - `git diff --check` - `source ~/.profile && source scripts/setenv.sh && make generated-proto-without-cpp` - `git diff --exit-code -- pkg/proto/data_coord.proto pkg/proto/query_coord.proto pkg/proto/datapb/data_coord.pb.go pkg/proto/querypb/query_coord.pb.go` Blocked: - `make check-proto-product`: conan could not resolve `milvus01.jfrog.io`. - Go tests were not rerun after review cleanup because `reset-milvus` is blocked by missing `bin/milvus`; an earlier attempt was blocked by missing `milvus_core` pkg-config. Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
zhuwenxingandGitHub
|
cdd9136fd7 |
test: enable FileResource regression cases for closed issues (#51239)
## What changed - Remove the stale `xfail` marker from `test_file_with_bom_and_crlf`, which covers the UTF-8 BOM regression tracked by [#49684](https://github.com/milvus-io/milvus/issues/49684) and fixed by [#49691](https://github.com/milvus-io/milvus/pull/49691). - Remove the stale `skip` marker from `test_drop_collection_then_remove_remote_resources_no_panic`, which covers the FileResource lifecycle panic tracked by [#49279](https://github.com/milvus-io/milvus/issues/49279) and fixed by [#49412](https://github.com/milvus-io/milvus/pull/49412). Both issues are closed. This change makes the existing cases active CI regressions; it does not change their fixtures, test bodies, assertions, or server behavior. ## User impact CI will now detect regressions where stop-word files with a UTF-8 BOM are parsed incorrectly, or where dropping a collection and removing its remote analyzer resources can panic the server. ## Validation The existing cases were previously run against the latest `upstream/master` commit `03762320e8537f855c18c28133834bf292b45361` using image `harbor.milvus.io/manta/milvus:master-20260710-0376232`. The `-p no:skipping` option only bypassed pytest's skip/xfail marker handling; no case code, fixture, or assertion was changed: ```bash python -W ignore -m pytest -v -p no:skipping \ milvus_client/test_milvus_client_file_resource.py::TestMilvusClientFileResourceContent::test_file_with_bom_and_crlf \ milvus_client/test_milvus_client_file_resource.py::TestMilvusClientFileResourceLifecycleAdvanced::test_drop_collection_then_remove_remote_resources_no_panic \ --host <temporary-milvus-host> --port 19530 --token 'root:Milvus' \ --minio_host <temporary-minio-host> --minio_bucket fr-closed-issues-master \ --tb=short ``` Result: `2 passed in 2.16s`. The #49279 case was then repeated ten times with the same marker bypass: ```bash python -W ignore -m pytest -q -p no:skipping \ milvus_client/test_milvus_client_file_resource.py::TestMilvusClientFileResourceLifecycleAdvanced::test_drop_collection_then_remove_remote_resources_no_panic \ --count=10 \ --host <temporary-milvus-host> --port 19530 --token 'root:Milvus' \ --minio_host <temporary-minio-host> --minio_bucket fr-closed-issues-master \ --tb=short ``` Result: `10 passed in 14.59s`. Milvus remained healthy, pod restart count was zero, and logs contained no panic, `tokenizer.h:31`, fatal error, or segmentation fault. The temporary validation instance has been cleaned up. The two node IDs also collect successfully after removing the markers: ```bash python -W ignore -m pytest --collect-only -q \ milvus_client/test_milvus_client_file_resource.py::TestMilvusClientFileResourceContent::test_file_with_bom_and_crlf \ milvus_client/test_milvus_client_file_resource.py::TestMilvusClientFileResourceLifecycleAdvanced::test_drop_collection_then_remove_remote_resources_no_panic ``` Result: `2 tests collected in 0.09s`. Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com> |
||
|
zhuwenxingandGitHub
|
408204099e |
test: add struct array partial update coverage (#51166)
## What - Enable StructArray coverage in `PartialUpdateChecker` and verify StructArray-only partial update rows. - Add MilvusClient E2E coverage for StructArray partial update over sealed and growing data, including HNSW and DISKANN embedding-list search validation. - Bump Python client test dependency to `pymilvus==3.1.0rc62`. ## Verification - `python3 -m py_compile tests/python_client/chaos/checker.py tests/python_client/milvus_client/test_milvus_client_partial_update.py` - `ruff check tests/python_client/chaos/checker.py` - `ruff check --select E9,F63,F7,F82 tests/python_client/milvus_client/test_milvus_client_partial_update.py` - `git diff --check` - Targeted live run against 2.6-latest: `test_milvus_client_partial_update_struct_array_sealed_growing` passed for HNSW and DISKANN. --------- Signed-off-by: zhuwenxing <wenxing.zhu@zilliz.com> |
||
|
|
ee943a1bf5 |
enhance: allow updating inactive field analyzer params (#50492)
issue: #50961 ## Summary Allow AlterCollectionField to set or delete analyzer configuration on string fields before they are used by text match or BM25. Keep analyzer params immutable once text match is enabled or a BM25 function depends on the field. When analyzer params change, RootCoord revalidates the new analyzer config, reserves newly referenced file resources before broadcasting, and lets MetaTable.AlterCollection reconcile file resource refCnt for request, replay, replicated, and recovery paths. ## Verification - `git diff --check` - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./util/typeutil -run TestIsBm25FunctionInputField` from `pkg/` - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./internal/rootcoord -run 'TestMetaTableAlterCollectionFileResourceRefCnt|TestDDLCallbacksAlterCollectionFieldAnalyzerValidation'` blocked locally: missing `milvus_core.pc` - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./internal/proxy -run TestAlterCollectionField` blocked locally: missing `milvus_core.pc` and `milvus-storage.pc` Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Buqian ZhengandGitHub
|
6ceb3ed540 |
enhance: cache JSON flat validity bitmaps (#51291)
## Summary - Reuse the existing `ExprResCacheManager/ExprCacheHelper` from the expression layer. - Cache JSON-flat exact-path validity bitmaps by segment + field/full JSON pointer + `Any|Numeric|String|Bool` canonical key. - Exclude literal/operator from the key so validity can be reused across predicates. - Do not add a `JsonFlatIndex`-owned LRU or change the index format. This is based on the latest `master`, including merged PR #51235. ## Verification Verification is pending and will continue after the PR is created. issue: #51234 Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com> |
||
|
|
944b614a8c |
enhance: pick least-loaded DataNode in globalScheduler instead of first-fit (#50912)
## What The datacoord `globalScheduler.pickNode` used **first-fit**: it returned the first node in map-iteration order whose available slots could satisfy the task. Because Go map iteration order is non-deterministic and the search stops at the first fit, scheduled tasks (import / compaction / index / stats) were not spread evenly across DataNodes — a cluster could land most tasks on one pod while others stayed idle. This PR maintains a per-round **max-heap** of nodes keyed by available slots and always assigns each task to the **most-available (least-loaded)** node (water-filling on available slots). The heap is built once per `schedule()` round and reused across all picks, so later picks observe the decremented slots. The previous fallback (drain the most-available node when no node can fully satisfy `taskSlot`) and the empty-cluster / no-slot behaviors are **preserved**. ## Why Related discussion: https://github.com/milvus-io/milvus/discussions/50872 — bulk import on a multi-DataNode cluster spent ~90% of wall-clock on a single pod. One root cause is on the **scheduling layer**: first-fit selection did not balance tasks across nodes. This PR addresses that layer. > Note: the import-specific regrouping in `RegroupImportFiles` (which can merge multiple files into a single task) is a separate, complementary issue and is **not** addressed here. ## Test - Reworked `TestGlobalScheduler_pickNode` (tie / least-loaded / fallback / single-node decrement / all-empty / empty-cluster). - Added `TestGlobalScheduler_pickNode_Balancing`: 3 nodes × 100 slots, 30 tasks × 10 slots → each node receives exactly 10 and is drained to 0. - `go vet` clean; full `internal/datacoord/task` package passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) issue: #50914 --------- Signed-off-by: cai.zhang <cai.zhang@zilliz.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
9fe38b1f09 |
doc: add design document for online shard split of namespace collections (#50465)
Add the design document for online shard split of namespace (multi-tenant) collections. The design covers: - Range routing over `big_endian(hash(namespace)) || namespace` with a versioned routing table derived from the collection meta. - Write switching via a `SplitShard` WAL fence message (`T_switch`) with reject-and-refetch on the proxy; target vchannels are initialized with a `BarrierTimeTick` so every message of the new WALs is strictly greater than `T_switch`. - In-place child delegators fronted by the source delegator during the transition window (sealed segments stay single-loaded, deletes and timeticks are forwarded back). - Multi-round metadata-only relabel redistribution by DataCoord, with the release-safety analysis (frozen targets, merged recovery view, register-then-release). - One-shot QueryCoord adoption with in-place delegator handoff, consistency guarantees, engineering constraints, configuration, and failure handling. issue: #50463 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: cai.zhang <cai.zhang@zilliz.com> Signed-off-by: Cai Zhang <cai.zhang@zilliz.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
congqixiaandGitHub
|
158b1dc38a |
enhance: migrate sealed text indexes into runtime state (#51280)
Related to #51068 Store sealed-segment text index holders and cache slots in the published runtime resource snapshot. Stage create, load, and reopen updates against cloned state before publishing them atomically. Keep text index resources alive through captured snapshots, prune indexes removed by schema reopen, and preserve created-index markers only for fields present in the target schema. Remove the obsolete direct LoadTextIndex path, legacy batch overloads, and marker wrappers. Add coverage for staged visibility and old snapshot resource lifetime. Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
|
a3a5ac8d44 |
fix: skip non-materialized function output field on growing segment flush (#51117) (#51266)
issue: #51117 Supersedes #51207 per team decision to ship the skipped-field tolerance for 3.0.0; the era-consistent replay consume path (#51146) remains a follow-up. ### Problem A growing segment created while a field was absent from the schema never materializes that column: the InsertRecord ctor only allocates fields present at creation, Reopen fills later-added ordinary fields but skips function outputs by design, and a field dropped before segment creation is skipped at consume. Flushing such a segment with a schema snapshot that still carries the field hit `Assert "data_.find(field_id) != data_.end()"` in the growing-source flush path and crash-looped the StreamingNode (exit 134 / CrashLoopBackOff in the schema-evolution L3 runs). ### Design Principle: the flush writes exactly `flushSchema ∩ materialized`. A non-materialized column is legally absent — a field dropped from the segment's own schema, or a function output recomputed by bump-schema compaction backfill. Real schema/data inconsistency is segcore's own concern and is verified inside the flush. - **Go (layout source of truth)**: trim the column groups to the source segment's materialized field ids (plus system fields) right after layout derivation — `Fields` and the parallel `Columns` array in lockstep — so the writer pattern, binlog meta and metacache current split all describe the same layout. An empty materialized report is refused. On a committed-flush ack retry the layout is trimmed to the committed binlogs (`ChildFields` union), the persisted truth. - **C++ (segment-internal judgement)**: a column missing from the insert record (`HasFieldData` semantics: allocated-but-empty counts as missing) is skipped when the field is gone from the segment's own schema (dropped) or is a function output; a regular field still present in the segment schema is materialized by ctor/Reopen by construction, so that case hard-errors instead of asserting. - Skipped columns are backfilled by bump-schema compaction via `RecordMaterializer`. ### Review responses (@liliu-z) - stale `Columns` / phantom column in writer pattern → `filterColumnGroupFields` trims `Fields`+`Columns` in lockstep at all three trim sites - committed ack-retry currentSplit divergence → layout trimmed to committed binlogs' `ChildFields` union - allocated-but-empty function-output column wedging the flush → materialized report and the flush skip both gate on `HasFieldData` semantics (empty = not materialized) - silent skip of a missing regular field → three-way judgement: dropped from segment schema → skip; function output → skip; present-in-segment-schema regular field → hard error (`has no field data in growing segment ... but is present in the segment schema`) - retryability routing of the missing-field error: the legally-absent cases no longer produce an error at all; the remaining hard error indicates real data loss and is construction-unreachable via public APIs (the consume path asserts regular fields carry data) ### Verification - Go UT (`growing_source_test.go`): trim semantics — dropped ordinary field trimmed, `Columns` kept in lockstep, empty materialized report refused, committed-retry trim by `ChildFields` - C++ UT (`test_storage.cpp`): flush skips a non-materialized function output; skips a dropped field carried by a staler flush schema; skips an allocated-but-empty function-output column - Local schema-evolution chaos runs (concurrent add/drop field + BM25 function field + DML/query/search with `useLoonFFI` + growing-source flush enabled, incl. StreamingNode force-kill/restart injections): no panic, no crash loop, final serial consistency validation passes. The exact original assert requires an era-mismatch replay not reproduced locally; that path is covered by the unit tests above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Buqian ZhengandGitHub
|
c8848b1226 |
fix: preserve JSON flat contains three-valued validity (#51235)
## Summary - add `json_subpaths` support to Tantivy `json_exist_query` - expose exact-path existence through `ExistsQuery(..., false)` - use a contains-specific `JsonExactPath` validity mode for actual JSON-flat executors - keep contains hit lookup through `In()` while using exact-path non-null-leaf validity ## Semantics - NULL, missing paths, JSON null, and object-only paths remain UNKNOWN - `NOT UNKNOWN` remains UNKNOWN - scalar values and non-empty arrays preserve current flattened contains behavior - empty arrays remain the documented limitation ## Verification - `JsonFlatIndex*`: 25/25 passed - `*JsonContains*`: 597/597 passed - `cargo fmt --check` - clang-format dry-run on changed non-generated C++ files - `git diff --check` ## Benchmark For scalar-benchmark `json_array_operations/base/A01_contains_flat_array/json_flat` with 1M rows and 2 matches: - first query: 24.945 ms - average: 24.625 ms - P50/P99: 24.525/24.607 ms - current local master: approximately 1114/1119 ms P50/P99 issue: #51234 --------- Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com> |
||
|
congqixiaandGitHub
|
3e000f2442 |
enhance: move sealed vector index state into runtime resource (#51224)
Related to #51068 Migrate sealed segment vector index runtime state from live members into RuntimeResourceState, including vector index entries and binlog vector index configs. Update vector search, vector retrieval, raw-data checks, refine, prefetch, load/drop/clear, and staged reopen paths to read and publish through runtime snapshots. Also update SearchOnSealedIndex to consume a pinned vector index entry directly, and make vector index config accessors const-compatible for snapshot usage. --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
|
1993feaeea |
fix: build text index for newly added enable_match field on growing segment reopen (#50484) (#51201)
## Summary After schema evolution adds an `enable_match` field (e.g. `drop_collection_field` + `add_collection_field` of a same-name text field), `text_match` could fail with `text index not found for field <id>` on a growing segment even though `describe_index` reported `Finished`. ## Root cause A growing segment builds its per-segment text indexes exactly once, in the constructor (`CreateTextIndexes`), against the construction-time schema — `text_indexes_` has a single writer, reachable only from that path. When a query carrying the new schema drives `LazyCheckSchema -> Reopen(SchemaPtr)`, Reopen only backfilled empty column data (`fill_empty_field`) and never built the text index for the newly added field — unlike the sealed path, where both load and schema-only reopen go through `ComputeDiff -> text_indexes_to_create -> CreateTextIndex`. So an old growing segment upgraded by Reopen had no `text_indexes_[new_field]`, and `text_match` hit `GetTextIndex()` and threw `TextIndexNotFound`. The window closes once the segment is flushed/sealed, loaded (sealed load rebuilds the index) and released from the delegator, which is why the failure is intermittent and self-healing but hard, non-retryable while it lasts. ## Fix In `SegmentGrowingImpl::Reopen(SchemaPtr)`, for newly added `IsStringDataType && enable_match` fields, **before publishing `schema_`** (LazyCheckSchema readers don't take `sch_mutex_`, so a concurrent query observing the new schema skips Reopen and queries immediately — the index must already exist by then): 1. Build the text index via the new `BuildTextIndexForMeta(const FieldMeta&)` (the field is not yet in the published `schema_`, so the meta is passed in; mirrors sealed's `CreateTextIndexWithSchema`). The index is **staged as a local object** — it is not visible in `text_indexes_` yet. 2. **Index the pre-existing rows with the same default-value-or-nulls fill the column received** (constructed directly — the content is knowable without reading the column back, since the exclusive `sch_mutex_` blocks concurrent inserts). This keeps the index consistent with the column, matching sealed's create-from-raw behavior (`BulkRawStringAt -> AddTextSealed`): when the added field carries a `default_value`, old rows must match text queries against the default text and report not-null. Backfilling every row (nulls included) also keeps text-index doc offsets dense and aligned with `insert_record_` rows, which query-side bitmap sizing relies on. 3. Force `Commit()+Reload()` on each staged index; after **every** new field succeeds, publish all staged indexes into `text_indexes_` together (an O(1) map insert per field under one `mutex_` acquisition). Publish-after-success makes the failure path sound across fields as well as within one: a present `text_indexes_` entry is always a fully backfilled, committed index; a throw anywhere during any field's build/backfill/commit publishes nothing and leaves `schema_` un-advanced, so the next query rebuilds everything from scratch (no per-field partial publication that a retry's presence guard would skip over). It also shrinks the `mutex_` hold during Reopen from the O(N) backfill to the final map insert, so concurrent `GetTextIndex` readers (text_match on existing fields) are not stalled by the backfill. Function-output fields (e.g. BM25 sparse output) remain skipped: they are non-string and never carry a text index. The related-but-distinct `field index meta not found` symptom for BM25 function-output fields (follow-up of #50783: `index_meta_` is not updated on growing reopen) is a separate fix and not covered here. ## Test `test_schema_reopen.cpp`: - `ReopenBuildsTextIndexForNewEnableMatchField`: growing segment on schema v1, old rows inserted, Reopen to v2 adding a nullable `enable_match` VARCHAR (no default) — `GetTextIndex` throws before Reopen; after Reopen it returns the index, `MatchQuery` finds nothing and `IsNotNull` reports all rows null, consistent with the column. - `ReopenTextIndexIndexesDefaultValueForOldRows`: same flow with a `default_value` on the added field — after Reopen `MatchQuery("default")` matches all N pre-existing rows and `IsNotNull` reports them non-null, matching sealed semantics. - `ReopenBuildsTextIndexesForMultipleNewFields`: one Reopen adds two enable_match fields (one nullable-null, one with default) — both indexes come out complete through the batched staged publish. All tests query immediately after Reopen with no explicit `Commit()/Reload()`, asserting the forced-visibility behavior. ## Known limitation (pre-existing, follow-up) `EnsureArrayOffsetsForStructField` still runs after `schema_` is published, so a throw there can leave a schema-advanced segment without array offsets — pre-existing behavior, unchanged by this PR; reworking Reopen's publication order for non-text state is left as a follow-up. (The text-index half of this concern is resolved: the index is now built before publication and a build failure leaves the reopen retryable.) issue: #50484 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Bingyi SunandGitHub
|
88f357a345 |
enhance: add switch for query driver prefetch (#51081)
issue: #51252 ## What - Add `common.enableDriverPrefetch` with default `false`. - Wire the config into segcore via initcore and a refresh callback. - Skip `Driver::PrefetchAsync()` when the switch is disabled. ## Why This provides a runtime mitigation for regressions caused by query driver operator prefetch on short scalar queries, while keeping prefetch configurable for follow-up validation. ## Notes When disabled, expression execution path determination still runs lazily on the query thread; this only disables driver-level async prefetch submission/waiting. ## Tests - `make generate-yaml` - `CLANG_FORMAT=/opt/homebrew/opt/llvm@15/bin/clang-format internal/core/run_clang_format.sh internal/core` - `make lint-fix` Signed-off-by: sunby <sunbingyi1992@gmail.com> |
||
|
Li YiyangandGitHub
|
f8130ad8d9 |
test: add REST e2e coverage for regex and array updates (#51189)
## Summary - Add REST v2 e2e coverage for partial update `ARRAY_APPEND` and `ARRAY_REMOVE` field operations on Array fields. - Add REST v2 e2e coverage for regex filters across query, search, delete, JSON paths, Array<Varchar> elements, nullable VarChar fields, negation, boolean expressions, and invalid expressions. ## Test Plan - [x] `PYTHONPYCACHEPREFIX=/private/tmp/milvus_pycache /Users/yiyang.li/gitup/milvus/tests/python_client/.venv/bin/python -m py_compile tests/restful_client_v2/testcases/test_partial_update_array_op.py tests/restful_client_v2/testcases/test_regex_filter.py` - [x] `git diff --cached --check` --------- Signed-off-by: lyyyuna <yiyang.li@zilliz.com> |
||
|
junjiejiangjjjandGitHub
|
694c2e6dba |
feat: support xgboost function chain expr (#51195)
issue: #51192 design doc: docs/design-docs/design_docs/20260708-xgboost-function-chain.md Add native xgboost FunctionChain expression support for L0 rerank with FileResource-backed UBJ models. This change includes: - xgboost FunctionChain expression registration, parameter validation, and execution - FileResource-based UBJ model discovery and local path resolution - lazy model loading with singleflight, lease/refcount lifecycle protection, and stale eviction on FileResource sync - cgo bridge for Arrow C Data based batch prediction - native C++ UBJ model parser and predictor for supported tree models - runtime-disabled stub for builds without cgo and with_xgboost - validation for unsupported params, output modes, feature count mismatch, invalid models, unsupported objectives, unsupported boosters, multiclass models, multi-target leaf vectors, and unsupported input column types - C++ unit tests, Go tests, native parity tests, and Python client L0 E2E tests - xgboost FunctionChain design document L2 rerank support is intentionally deferred because Proxy does not yet support FileResource sync and local resolution. Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com> |
||
|
sijie-ni-0214andGitHub
|
ce9ebdf318 |
fix: Avoid structured binding capture in OpenMP build (#51232)
## Summary This PR avoids directly capturing a structured binding in `ChunkedSegmentSealedImpl::LoadBatchJsonKeyIndexes`. Clang rejects direct structured-binding captures when compiling this code path with OpenMP enabled. The lambda now uses a generalized init-capture for `field_id`, preserving the existing value-capture behavior while avoiding the unsupported capture form. issue: #51231 ## Test Plan - [x] `git diff --check` - [x] `/tmp/milvus-clang-format-15-bin/clang-format-15 --dry-run --Werror --style=file internal/core/src/segcore/ChunkedSegmentSealedImpl.cpp` - [x] `make SKIP_3RDPARTY=1` - [x] `PATH="/tmp/milvus-clang-format-15-bin:$PATH" make verifiers` Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> |
||
|
Zhen YeandGitHub
|
03762320e8 |
enhance: trigger streaming balance on primary rg change (#51145)
issue: #51123 - streamingcoord balancer: wake the balance loop when `streaming.primaryResourceGroup` changes and keep convergence on the existing balance path - streamingcoord tests: cover primary resource group runtime config changes without relying on the periodic balance timer Signed-off-by: chyezh <chyezh@outlook.com> |
||
|
XuanYang-cnandGitHub
|
df36780667 |
enhance: use tsoutil helper for segment expiration (#51190)
Use tsoutil.AddPhysicalDurationOnTs when calculating DataCoord segment assignment expiration timestamps, instead of manually parsing and composing the physical and logical parts. This keeps expiration arithmetic on the shared hybrid timestamp helper and avoids converting large valid physical timestamps through time.Time.UnixNano(). Signed-off-by: yangxuan <xuan.yang@zilliz.com> |
||
|
|
6257132c56 |
fix: restore conjunct AND early-exit at null-rejecting consumers (#51182)
issue: #51181 The 3VL null-semantics rework correctly made conjunctions keep UNKNOWN rows active (under `NOT`, `UNKNOWN AND FALSE = FALSE` must stay reachable), but this disabled the AND early-exit everywhere — including at the top-level filter, which folds UNKNOWN into the excluded set exactly like FALSE. For filters like `json["a"] == 1 and heavy_expr and ...` over data where most rows lack the key, every following conjunct ran over rows whose outcome was already decided. ## Changes **Null-rejecting propagation (compile-time, zero hot-path cost)** - New virtual `Expr::MarkNullRejecting()`: no-op default stops the propagation at any non-conjunct node (in particular `NOT`, where FALSE ≠ UNKNOWN); `PhyConjunctFilterExpr` overrides it to set `null_rejecting_` and recurse into its inputs. The property legally crosses nested AND/OR: their combination cannot turn an operand's UNKNOWN into TRUE when the root folds UNKNOWN into FALSE. - `ExprSet` gains a `null_rejecting` ctor flag; `PhyFilterBitsNode` (folds UNKNOWN in `ConvertPredicateToFilteredBitset`) and `PhyIterativeFilterNode` (includes rows on data bits) pass true. Element-level filter operators keep the default and merely miss the optimization for now. - A null-rejecting AND uses `data & valid` as its active set: the zero-TRUE batch early exit is restored, and UNKNOWN rows are dropped from the next expression's row-level bitmap input. OR is untouched (`UNKNOWN OR TRUE = TRUE` can still flip a row in). Included row set is provably unchanged — pinned by `NullRejectingMatchesDefaultIncludedSet`; the existing NOT-context test keeps guarding strict 3VL behavior in unmarked conjuncts. **Single active-bitmap build per input per batch** - Previously the identical `data | ~valid` bitmap was built twice per non-first input (once in `UpdateResult` only to be counted and discarded, once in `SetNextExprBitmapInput`), plus once more for the final input where `ClearBitmapInput` immediately discarded it. Now `BuildActiveBitmap` runs once: `none()` (early-terminating, no full popcount) decides the batch-level exit and the same bitmap is `std::move`d into the eval context. The build is skipped for the last actually-evaluated position (`last_eval_pos`), so trailing batch-ngram entries don't force a useless build. **Pre-existing OOB crash fixed at its root** - `ReorderConjunctExpr` reserves `input_order_` slot `inputs_.size()` for a runtime `PhyLikeConjunctExpr`, but the block that materializes or erases it was gated on `!has_input_offset`. On `PhyIterativeFilterNode`'s native offset path (reachable: LIKE supports offset input) the reserved slot never got a backing expression, and both the Eval loop and `SkipFollowingExprs` indexed `inputs_[inputs_.size()]` — reproduced as a SIGSEGV under gtest with the production-reachable state. The init block now decides materialize-vs-erase in every mode (the batch-ngram path cannot be used with an offset input anyway, and an instance's offset mode is fixed for its lifetime), restoring the invariant "every `input_order_` entry has a backing expression" at its single owner rather than guarding each consumer. **IterativeFilterNode: null-rejecting by construction** - Both consumption paths now fold `data &= valid` explicitly instead of relying on the unenforced convention that UNKNOWN rows carry `data=0` (the fallback path collected `valid_bitset` but used it only in asserts). Note: `ColumnVector::AllTrue/AllFalse` lose their last production caller here but are kept as general-purpose public API with existing tests. ## Verification - Unit tests (Linux, `make build-cpp-with-unittest`): `ConjunctExprTest` 7/7 (6 new: null-rejecting early exit / TRUE rows still evaluated / included-set equivalence marked-vs-unmarked / OR unaffected / propagation stops at non-conjunct nodes / offset-input reserved-slot erasure incl. the previously-crashing early-exit path), `*Conjunct*` 9/9, `*Iterative*` 4/4, `TaskTest*` + `FilterOnlySearchTest*` 37/37, `LikeConjunctExpr` pass. - Two rounds of multi-agent adversarial review; all confirmed findings addressed. - clang-format (19) clean on all changed lines. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
0f485f28c1 |
doc: add vector-anchored join design doc (#51186)
## What
Adds the design doc for the **vector-anchored join** feature:
`docs/design-docs/design_docs/20260708-vector-anchored-join.md`. Doc
only — no code.
## Why
Milvus has no first-class way to join a vector collection against a
*separate, frequently-updated* scalar table keyed by a shared identifier
("vectors are stable, their business metadata mutates constantly").
Today users must denormalize the mutable fields into the vector
collection (re-writing the vector on every metadata change) or join in
the application layer (losing all pushdown). This design puts the join
back inside the engine without coupling mutable data to the immutable
vector-segment lifecycle.
## Scope
Three join shapes, all keeping vector search on the critical path:
| # | Capability | SQL semantics | Physical operator |
|---|---|---|---|
| 1 | Vector-driven enrichment | `INNER` / `LEFT OUTER` | index
nested-loop; vector top-K drives, probe side by key |
| 2 | External-predicate pre-filter | `SEMI` | foreign filter → key-set
→ bloom/bitmap inline into ANN |
| 3 | Lateral vector search (driver-side) | `LATERAL` | scalar table
drives; one batched ANN (`nq = N`) per driver row |
Built on a first-class **scalar-only (non-vector) table**, a **Global KV
Index** (mutable, LSM, decoupled from vector segments), and
**generalized bloom-filter pushdown**. Delivery is **baseline-first**
and not gated on the KV index (Phase 0 is doable on existing access
paths).
Governing principle: *Milvus does vector-anchored joins, not general
joins.* Two scalar-only tables joined together, unbounded all-pairs KNN,
non-equi/range joins, and `FULL`/`RIGHT OUTER`/`CROSS` are explicitly
out of scope.
issue: #51185
Signed-off-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
wei liuandGitHub
|
536ece6737 |
fix: Check external fields against loaded manifest (#50773)
issue: https://github.com/milvus-io/milvus/issues/50416 ## What This PR moves external field checks into the C++ Search/Retrieve path after `LazyCheckSchema` has refreshed the segment schema and load info. Instead of probing `HasFieldData` or `FieldAccessible`, the guard checks whether every referenced external field exists in the currently loaded external manifest: - `CheckExternalFieldsInLoadedSnapshot` is renamed to `CheckExternalFieldsInLoadedManifest` to match the real readiness boundary. - Plan `access_entries_` now includes expression/execution fields and requested output fields, so Search and Retrieve use one field-reference list for external manifest checks. - Search filter-only skips the vector field and output target fields because that path does not access them. - Non-manifest segments keep the existing behavior. The loaded manifest column-group cache is also preserved across schema-only reopen when the manifest path is unchanged, so the query path can check manifest membership without parsing remote manifests or falsely reporting missing columns after schema refresh. ## Why For external collections, a newly added field can appear in the Go collection schema before the loaded C++ segment has refreshed to a manifest that contains the corresponding external column. If Search/Retrieve touches that field against the stale loaded manifest, segcore can surface an internal field-state failure instead of a clear external-field-not-ready error. The real readiness condition here is whether the loaded manifest contains the external column, not whether the field is loaded into memory. ## Validation - `ninja -C cmake_build milvus_core all_tests` - `DYLD_LIBRARY_PATH=cmake_build/src cmake_build/unittest/all_tests --gtest_filter='PlanProto.SearchPlanCollectsFieldAccessInfo:PlanProto.RetrievePlanCollectsFieldAccessInfo:SegmentLoadInfoTest.HasManifestColumnUsesManifestColumnNames:SegmentLoadInfoTest.CachedManifestColumnsCanBeInherited'` - `/opt/homebrew/opt/llvm@15/bin/clang-format --dry-run -Werror internal/core/src/query/PlanImpl.h internal/core/src/query/PlanProto.cpp internal/core/src/query/PlanProtoTest.cpp internal/core/src/segcore/ChunkedSegmentSealedImpl.cpp internal/core/src/segcore/ChunkedSegmentSealedImpl.h internal/core/src/segcore/SegmentInterface.h internal/core/src/segcore/SegmentLoadInfo.cpp internal/core/src/segcore/SegmentLoadInfo.h internal/core/src/segcore/SegmentLoadInfoTest.cpp internal/core/src/segcore/segment_c.cpp` - `CLANG_FORMAT=/opt/homebrew/opt/llvm@15/bin/clang-format make cppcheck` - `git diff --check milvus/master...HEAD` --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
|
f244642ba9 |
enhance: support query order-by in RESTful v2 API and Go SDK (#51170) (#51173)
issue: #51170 Expose the existing server-side query ORDER BY capability (query param key `order_by_fields`, already consumed by proxy `task_query.go`) through the two remaining client surfaces: - **Go SDK**: `queryOption.WithOrderByFields(fields ...string)` — each spec is `"field"`, `"field:asc"` or `"field:desc"`, joined into the `order_by_fields` query param. - **RESTful v2**: `QueryReqV2` gains `orderByFields` (`[]string`, same spec format), forwarded by the query handler as the `order_by_fields` query param. Validation (sortable type, explicit-limit requirement, iterator exclusion) stays server-side in the proxy, consistent with how limit/offset/group-by are handled at these layers. Interface shape mirrors pymilvus (`order_by=["price:desc"]`). Tests: Go SDK option unit test, RESTful v2 handler unit test (asserts the KV pair is forwarded, and absent when not requested), and a go_client e2e case (desc / asc-default / no-limit rejection). Follow-up (out of scope here): search-side `order_by` first-class parity for RESTful v2 / Go SDK (currently reachable via raw search params only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
yanliang567andGitHub
|
b542464f30 |
test: Add JSON filtering e2e coverage (#51171)
## What does this PR do? Adds Python e2e coverage for JSON filtering UNKNOWN semantics in `tests/python_client/milvus_client/expressions/test_milvus_client_json_filtering.py`. This PR covers: - raw filtering paths on growing and sealed segments - JSON arithmetic/filtering UNKNOWN behavior for missing, null, and type-mismatch values - JSON array predicate behavior for missing and non-array values - ARRAY subscript UNKNOWN behavior for null, empty, and out-of-range values - contradiction rewrite cases, including outer `not (...)`, to ensure UNKNOWN is not rewritten into TRUE - JSON path index filtering with `json_cast_type=DOUBLE` - JSON path index filtering with `json_cast_type=VARCHAR` - JSON flat index coverage for currently passing UNKNOWN cases - query/search filter consistency for representative JSON filters, including indexed collections - L2 coverage for boolean JSON values, fractional double values, additional comparison/logical operators, JSON path array subscripts, mixed arrays, and `ARRAY_DOUBLE` JSON path index The known JSON flat index array-vs-scalar comparison mismatch is documented in #51193 and covered by a skipped regression case so the coverage can land first without taking server-side fixes in this PR. issue: #50976 issue: #51193 Related #50699 #50698 ## Test Plan - [x] `git diff --check` - result: passed - [x] `uvx ruff check --config tests/ruff.toml tests/python_client/milvus_client/expressions/test_milvus_client_json_filtering.py` - result: passed - [x] `uvx ruff format --check --config tests/ruff.toml tests/python_client/milvus_client/expressions/test_milvus_client_json_filtering.py` - result: passed - [x] `/Users/yanliang.qiao/fork/milvus/tests/.venv/bin/python -W ignore -m pytest milvus_client/expressions/test_milvus_client_json_filtering.py --collect-only -q` - result: `29 tests collected` - [x] `milvus-dev-cli deploy create harbor.milvus.io/manta/milvus:master-20260709-9a0d7f2 --name json-filtering-l2-9a0d7f2 --namespace chaos-testing --no-auth --wait-timeout 1800 --wait-interval 10` - result: Healthy - endpoint: `lb-6d57ndgl-sw4giqjhodl3bn15.clb.bj-tencentclb.net:19530` - [x] `/Users/yanliang.qiao/fork/milvus/tests/.venv/bin/python -W ignore -m pytest milvus_client/expressions/test_milvus_client_json_filtering.py --uri http://lb-6d57ndgl-sw4giqjhodl3bn15.clb.bj-tencentclb.net:19530 --token "" -s -v --tb=short` - result: `28 passed, 1 skipped in 215.82s` ## Notes The skipped case is `test_json_flat_index_array_scalar_comparison_unknown_semantics_query`, linked to #51193. The skip can be removed after the server-side JSON flat index UNKNOWN semantics issue is fixed. Signed-off-by: Yanliang Qiao <yanliang.qiao@zilliz.com> |
||
|
congqixiaandGitHub
|
3c0b6a968d |
enhance: track JSON stats in runtime state (#51172)
Related to #51068 Keep sealed JSON stats in RuntimeResourceState so published runtime snapshots carry them through clone and freeze transitions. Load and replace JSON stats through the staged reopen committer, and drop them by mutating the staged runtime state. This removes the old direct LoadJsonKeyIndex CGO path and the unused RemoveJsonStats interface. --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com> |
||
|
wei liuandGitHub
|
760407a17e |
enhance: support external JSON stats for external tables (#51008)
issue: #45881 ## What changed This PR enables JSON key stats / JSON shredding support for external StorageV3 segments that already have a committed manifest. The main behavior changes are: - Allow DataCoord to schedule and reload `JsonKeyIndexJob` for external StorageV3 manifest-backed segments. - Add `StatsResult.base_manifest` so DataCoord can reject stale Text and JSON stats results when an external refresh has already moved the segment to a newer manifest. - Keep stale-result CAS and task placeholder release in the common stats task path, so a rejected result does not finish the old task forever. - Best-effort cleanup rejected Text/JSON stats files only for external collections; internal collections keep the existing GC ownership rules. - Clear old `TextStatsLogs` and `JsonKeyStats` metadata during external refresh. - Gate manifest JSON stats exposure on DataCoord metadata placeholders, preventing stale manifest stats from being loaded after refresh. - Keep external BM25 inspector jobs skipped. - Update external table design docs to match the new support boundary. ## Why External table refresh can replace the manifest of the same segment while an older JSON stats task is still running. Without a base-manifest check, a late stats result built from the old manifest could overwrite the refreshed manifest or expose stale JSON stats. External collection segments are patched only when the source changes. If the source stays stable, the segment can remain active forever, so rejected external stats files should be cleaned immediately instead of waiting for dropped-segment GC. ## Validation - `make generated-proto-without-cpp` - `gofmt -w` on touched Go files - `git diff --check` - `git diff --cached --check` - `git diff --check milvus/master..HEAD` - staged mockery addition scan Targeted Go test attempted locally: ```bash source ~/.profile && source scripts/setenv.sh && \ go test -tags dynamic,test -gcflags="all=-N -l" -count=1 \ github.com/milvus-io/milvus/internal/datacoord \ -run 'Test_statsTaskSuite/TestQueryTaskOnWorkerDiscardsStaleStatsResult' \ -v ``` It is blocked by the local C++ link environment: `_SegcoreSetPrefetchThreadPoolNum` is missing from the local linked segcore library. `reset-milvus` also cannot fully restart standalone in this worktree because `bin/milvus` is missing. Signed-off-by: Wei Liu <wei.liu@zilliz.com> |
||
|
|
aea5d701bd |
enhance: bind index meta to add function field DDL (#51105)
issue: #51104 companion SDK PR (merge first): milvus-io/pymilvus#3670 ## Summary `add_function_field` previously accepted a vector output field without any index meta; bump-schema-version compaction then stalled forever (no segment index task can be created for a field with no index defined, and `GetQueryVChanPositions` never hands off unindexed compacted-to segments — the fail-closed chain is silent). This PR makes the index meta a mandatory, atomically-bound part of the DDL: - **Mandatory at every layer**: SDK requires `index_params`; proxy and rootcoord reject vector function-output fields without an explicit `index_type` strictly before the broadcast (no silent AUTOINDEX). - **create_index-aligned materialization**: proxy normalizes the params with the same logic as `create_index` (name rules via `validateIndexName`, checker existence, `CheckTrain` incl. dimension filling, function-type defaults such as `bm25_k1/bm25_b/bm25_avgdl`) and writes them back into the request; rootcoord prepare allocates index id/name, rejects name conflicts and unknown index types, and serializes a complete `indexpb.FieldIndex` into the new `AlterCollectionMessageUpdates.bound_field_indexes` WAL field. - **Atomic apply**: the alter-collection ack callback commits the schema and then applies the bound index by dispatching a synthetic `CreateIndexMessage` to datacoord's existing `createIndexV2AckCallback` (same pattern as `cascadeDropFieldIndexesInline`). The callback is replayed until success across crashes and is fully idempotent (all ids come from the message body), so `DDL success ⇒ index meta exists` holds with no partial terminal state. - **Reuse promotion**: `ValidateIndexParams`/`CheckDuplidateKey` moved from datacoord to `internal/util/indexparamcheck`; shared `ExpandIndexParams` / `FillFunctionOutputIndexParams` / `PrepareFunctionOutputIndexParams` helpers now back both the create_index path and this DDL. - **Dead fan-out removed**: the ordinary `CreateIndex` broadcast is reverted to control-channel-only; the vchannel fan-out had no consumer and wrote one dead WAL entry per vchannel per index creation. - **No milvus-proto change**: the request already carried `FieldInfo.index_name`/`extra_params` (previously unused); this feature activates them. - e2e suites updated to the new semantics: `test_add_function_field_feature.py` rewritten (explicit `index_params` everywhere, post-DDL `create_index` blocks removed since the bound index conflicts with a second distinct index per existing semantics, new negative cases for the mandatory checks); the external-table negative case updated as well. ## Verification - New/updated unit tests: rootcoord DDL callbacks (mandatory/AUTOINDEX/unknown-index-type rejections, bound-index materialization + ack-callback application via a recording fake), proxy task validation (invalid index name, normalization write-back), shared helper tests. - End-to-end on a live cluster (StorageV3 + bump compaction enabled): request without params rejected; index meta visible with create_index-aligned params immediately after the DDL returns; bump compaction rewrote 2000-row sealed segments and the bound index built on the results (the exact chain that previously stalled); BM25 search over pre-DDL rows succeeds once the compacted segments are loaded. ## Limitations (follow-ups) - Live propagation of the new index meta to already-loaded QueryNode delegators is intentionally out of scope; a loaded collection observes the bound index through the segment load path (e.g. reload / compacted-segment load). Tracked as a separate follow-up PR. - The python e2e suites require the companion pymilvus change: milvus-io/pymilvus#3670 is merged and `tests/python_client/requirements.txt` is bumped to `pymilvus==3.1.0rc61` in this PR (all 22 rewritten cases + the external-table case verified locally against the released rc61). - During a rolling-upgrade window an old coordinator replaying the new message ignores `bound_field_indexes` (proto3 unknown field), i.e. pre-existing behavior; avoid the new argument in mixed-version clusters. - `AddCollectionFunction` on an existing field and plain add-vector-field keep their current behavior (the WAL carrier is generic for future extension). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
75526b2463 |
fix: drain thread pool futures before unwinding on error paths (#51180)
issue: #51179 `ThreadPool::Submit` returns `std::packaged_task` futures, which do not block on destruction. Sites that submit tasks capturing stack state by reference and wait with a bare `future.get()` loop let the first exception unwind the frame while remaining tasks are still queued/running — turning a recoverable I/O error into use-after-scope / use-after-free. Same defect class as the `IndexEntryReader` fix (#46958 / 2.6's #50937); an audit found four remaining sites. ## Changes - **`storage/Util.h`**: add noexcept `DrainFutures` / `DrainFuture` cleanup helpers (wait for every still-valid future, swallow its exception), complementing the existing `WaitAllFutures`. For use in catch blocks / scope guards where another exception is already propagating. - **`DiskFileManagerImpl::AddBatchIndexFiles`**: capture `local_chunk_manager` by value instead of `[&]`; collect slice buffers via the existing `WaitAllFutures`, which drains all tasks before rethrowing the first error. - **`GetFieldDatasFromStorageV2`** (`storage/Util.cpp`): wrap the channel-consumption loop; on exception, drain the channel first (the producer may be blocked pushing into the bounded channel), then wait for the load task, then rethrow — the pattern `GroupChunkTranslator` already uses. - **`SegmentGrowingImpl::load_column_group_data_internal`**: same treatment via a `folly::makeGuard` drain guard (dismissed on the success path). - **`IndexEntryEncryptedLocalWriter::WriteEntry` / `EncryptAndWriteSlices`**: drain the sliding window of pending encryption tasks (which capture `this`) with a scope guard before unwinding, so no task outlives the writer. ## Semantics The primary exception always propagates to the caller unchanged (bare `throw;` / `WaitAllFutures` rethrows the first task error after all tasks complete). Only secondary background-task errors during cleanup are swallowed — C++ permits a single in-flight exception, and letting another escape from unwind-time cleanup would `std::terminate`. ## Verification - All five touched files compile cleanly (clang 19 `-fsyntax-only` with the project's exact compile flags), including a collision check against the file-local `DrainFutures(futures, first_error)` in `IndexEntryReader.cpp`. - Changed lines pass clang-format 19 with the repo `.clang-format` (zero diffs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
b16c821b36 |
enhance: carry TEXT LOB refs through schema-bump full-rewrite compaction (#51125)
### What & why `bump_schema_version`'s full-rewrite path (`runFullSchemaRewrite`, reached only for a **drop-field** schema bump) builds a from-scratch output manifest via `NewBinlogRecordWriter` and did **not** carry the source segment's TEXT LOB references — leaving dangling refs for an out-of-line (`>=64KB`) TEXT column. This was the `TODO(#50021)`. ### What this does Wire **REUSE_ALL** LOB handling into `runFullSchemaRewrite`, mirroring sort compaction. Both are `1->1` with a single output manifest, so REUSE_ALL is always correct: a schema bump never changes the existing TEXT LOB data — only its references are carried. - `internal/compaction/lob_compaction.go`: `GetForcedStrategy` forces REUSE_ALL for `BumpSchemaVersionCompaction`. - `internal/datanode/compactor/bump_schema_version_compactor.go`: collect the source LOB files into a `LOBCompactionContext`, pass `storage.WithTextRefsAsBinary()`, update per-LOB-file `valid_rows` (`SetSegmentRowStats`), and merge the LOB references into the output manifest (`applyLOBCompaction`) **before** building the text-match index — `createTextIndex` reads the TEXT column through this manifest, so the carried LOB files must already be referenced (matches sort/mix ordering). Removed the `TODO(#50021)`. - Unit tests: forced strategy, init/apply LOB wiring, the applyLOBCompaction-before-createTextIndex ordering (`TestFullRewriteMergesLOBRefsBeforeBuildingTextIndex`), and a real add-nullable-TEXT + drop-field full-rewrite regression driving the actual packed writer (`TestFullRewriteFillsMissingNullableTextAsBinary`). ### Also: guard add_function_field behind StorageV3 (#51167) Adding a function to an existing collection must backfill the new output field into **pre-existing** sealed segments; that backfill runs through `bump_schema_version` compaction, which only works on a StorageV3 segment. A pre-existing V2 segment is only backfilled after the storage-version upgrade compaction rewrites it to V3, and that upgrade runs only when **both** `common.storage.useLoonFFI` and `dataCoord.compaction.storageVersion.enabled` are on. The proxy now rejects add-function unless both are enabled (`validateAddFunctionRequiresStorageV3`, wired into `addCollectionFunctionTask` and `alterCollectionSchemaTask`). `create_collection` with a function is unaffected. Full reasoning in #51167. ### Notes - Reuses the LOB machinery from **#50784**. - The `GenerateEmptyArrayFromSchema` binary-TEXT production fix landed independently on master via **#51124**; this PR keeps the regression test guarding the bump path. issue: #50021, #51167 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: MrPresent-Han <chun.han@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |