959 Commits

Include renames

Author	SHA1	Message	Date
Zhen Yeandsijie-ni-0214	0b0ad3ee8d	feat: extract QueryView coordinator components (#54547) ... issue: #40451 - Load and placement management: extract immutable load snapshots, configuration recovery, replica allocation, and manual placement. Restrict assignments and balancing to loaded partitions, with a full-partition fast path, and preserve missing-segment error precedence in manual balance. - Scheduling and cost: reconcile background work in collection batches using cache scanners so scoped events can run between batches. Compute immutable LogicalSize weights in DataView using initialization-fixed rowCount or insertBytes mode, a configurable 64KB unit, row-count fallback, and segmentBaseCost=16; consume the weights throughout placement, fanout, scoring, recovery, and movement. Keep physical rows separate and use one LostStats flag. - Schema and view lifecycle: persist the certified read schema in each DataView, support direct logical transitions or waiting for the current complete materialized projection, and bind new QueryViews to the selected snapshot. Preserve old published-schema recovery, worker synchronization, collection-scoped final-removal subscriptions, and inactivity tracking based on actual progress. See the [schema contract](https://github.com/chyezh/milvus/blob/ce23b02e1c0facfeede0073b22365e1607756d48/docs/design-docs/design_docs/qviews/data_view_schema.md). - Readiness and protocols: count expected replicas and shards, support resource-group-scoped loading and refresh progress with configuration/schema/data-version fences, and provide node clients and simplified loading protocols. Remove compatibility paths for unreleased QueryView formats while retaining existing QueryCoord metadata recovery. - Integration boundary: keep production runtime activation, Load/Release routing, W/A producers, DDL/schema-materialization wiring, and missing dependency implementations in the follow-up integration PR. Provide the required LoadInfoCatalog and PrepareDataViewSchema interfaces here; the follow-up also removes direct RPC load-config writes and retains serialized WAL ACK retries and partition-readiness semantics. --------- Signed-off-by: chyezh <chyezh@outlook.com> Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> Co-authored-by: sijie-ni-0214 <sijie.ni@zilliz.com>	2026-10-27 01:41:55 +08:00
somfornot	61de3a82cc	fix: write manifest index artifacts under the segment _index directory (#54412) ... Conflict refresh (2026-10-26): merged master `f067780bd8` and regenerated the conflicting DataCoord protobuf bindings in the matching development container, preserving both compaction terminal outcomes and segment-rooted index fields. Proto generation, datapb compilation, metautil tests, and protobuf round-trip checks passed. Focused DataCoord/session tests could not compile against the existing local native outputs: `milvus-storage/ffi_talon_c.h` is missing and the installed segcore headers lack `SegcoreSetSparseInterimIndexAlgorithms`. Those tests and the full native build are not claimed as passing; hosted CI must validate the refreshed head. When manifest index publication is enabled, StorageV3 index builds still write outside the segment directory. Their manifest entries consequently contain parent traversal paths, which violate the storage manifest path contract. This change writes new vector/scalar index artifacts to `<manifest base>/_index/<buildID>/<indexVersion>/<fileKey>` and rejects additions or updates outside that directory. - Persist path version 2 when creating a build and pass its complete prefix to native upload. Load, snapshot and GC consumers retain each index's recorded layout; existing version-0/1 metadata remains readable by upgraded components. - Pin the destination layout in copy-task metadata, convert sources in all three layouts, and copy only the snapshot's explicit index file list. Include the new namespace in orphan GC with the existing build, import, age and snapshot protections. - Pin publication mode at task creation: v2 builds commit to the manifest after an on-to-off switch; v0/v1 builds finish in etcd after an off-to-on switch. Coordinator restarts preserve that choice. - Check the selected worker's explicit v2 capability before dispatching a v2 build or copy destination. Unsupported workers fail the task with an upgrade-required reason; transient RPC errors remain retryable. - Reclaim v2 index orphans during the existing insert_log traversal, before the live-V3 skip, without a second recursive listing. Historical v2 artifacts remain eligible for cleanup when the write switch is off. - Exclude `_index/` from snapshot export directory enumeration and collect indexes from the explicit snapshot file list, avoiding dependencies on unreferenced builds that GC may remove. - Resolve v2 load/snapshot paths from the owning manifest base and return explicit errors for missing or invalid metadata. Snapshot generation uses its selected segment metadata. Keep the v1 layout predicate exact and document conservative retention of malformed index orphans. Upgrade DataCoord, index workers and QueryNodes before enabling the switch. Older workers omit the v2 capability and are rejected before dispatch; readers still require the operator upgrade gate. This PR neither repairs already-corrupted manifests nor updates the milvus-storage dependency; storage-side validation and preservation of existing entries are handled separately. Historical noncanonical local namespace orphan-scan gaps remain unchanged. Validation in the matching Milvus development container: - Snapshot index descriptors retain the selected segment ID. The regression assertions failed before the fix (returned test-global ID 500 instead of segment ID 2), then passed after it. Production `make build-go` without the `test` tag also passed. The prior head failed build-ut-cov #1910, E2E dispatcher #22389 and Go SDK #18145 on the same stale `segID` reference; downstream E2E/SDK tests did not run. Hosted CI must rerun on the corrected head. - Review follow-up: metautil and focused DataCoord, importv2 and packed tests passed, including v2-to-v0/v1 copy conversion, missing/invalid metadata errors, snapshot error propagation, and real FFI repeated commits under a legacy local base with and without a trailing slash. Incremental lint passed for the affected root-module and pkg packages. - Native build/install and six focused native tests passed, including upload/read under a segment's `_index`. - Focused metautil, DataCoord, importv2 and packed tests passed, including real local FFI repeated commits, external-path rejection, copy conversion and GC protection. Legacy recovery/read tests passed separately. - Go formatting, staged whitespace checks and incremental lint for the affected root-module and pkg packages passed. GC regression tests passed again after lint cleanup. - Follow-up focused tests passed for creation-time publication across both switch directions and restarts, layout mismatch rejection, manifest recovery, worker capability checks, durable task failure, and the merged GC scan with snapshot/import/build protection. The worker capability advertisement test passed without service registration. - Snapshot file collection, path rewriting/validation and DataCoord snapshot exporter tests passed, together with incremental snapshot storage lint. The regression verifies referenced legacy/v2 index paths and an unchanged collected file set after removing an unreferenced build; live export/GC concurrency was not fault-injected. - The DataNode service suite could not initialize because etcd was unreachable. A broader GC/manifest test run was interrupted and is not counted as passing. The full Go suite, live QueryNode mixed-layout loading, live object storage and mixed-version cluster E2E were not run. Jenkins build-ut-cov #1635 at `787b353b92` passed Build, Code Check, all 22,176 C++ tests and integration tests, but UT-GO failed on generated YAML comment drift and a stale proxy queue mock. The follow-up regenerates YAML without changing configuration values and targets `DqTaskQueue.Enqueue` in the affected Search/function-error and resource-group tests. The original proxy panic was reproduced in the development container; the config consistency test, affected proxy tests and incremental lint pass after the fixes. Hosted CI must rerun on the new head. The PCH-disabled build also required the missing `<optional>` include in `RegexQuery.h`, included here as a build prerequisite. Single-layer stack on `master`: implementation, protocol, tests and design are submitted together. issue: #53047 Design: [Segment-rooted index artifacts](https://github.com/somfornot/milvus/blob/fix/manifest-index-path-roundtrip/docs/design-docs/design_docs/20261019-segment-rooted-index-artifacts.md) --------- Signed-off-by: somfornot <318146256+somfornot@users.noreply.github.com>	2026-10-26 20:59:56 +08:00
Bingyi Sun	07b8121e03	enhance: add an opt-in cap on pending and running compaction tasks (#54309) ... Adds `dataCoord.compaction.maxTaskNum`, an opt-in soft cap (default 0, unlimited) on pending and running compaction tasks, each of which is persisted in etcd. Once it is reached, automatic compaction stops planning and submitting tasks. This includes sort compaction, where the sweep submits the sorts of just-flushed segments before the rest, and target-based compaction. Manual compaction, import, and the sort of clustering outputs are not capped, since a clustering task holds its slot until its outputs are sorted. A value close to the data nodes' compaction slots keeps the queue short, at the cost of slots idling between trigger ticks. issue: #54143 --------- Signed-off-by: sunby <sunbingyi1992@gmail.com>	2026-10-23 14:34:07 +08:00
Jamesandxiaofanluan	c151e75e80	enhance: use sorted indexes for arrays and typed JSON array paths (#54505) ... Ordinary ARRAY HYBRID indexes can select SORT for high cardinality, and ordinary ARRAY fields also accept explicit STL_SORT. Typed JSON paths support ARRAY_BOOL, ARRAY_DOUBLE and ARRAY_VARCHAR under HYBRID or explicit STL_SORT. Projection preserves parent-row postings and separates contains validity from path existence, including NULL, missing, empty and mixed arrays. Numeric ARRAY SORT construction now uses normal vector growth instead of per-row reservations. JSON ARRAY presence checking and extraction reuse the parsed DOM while retaining separate EXISTS and predicate-validity rules. Use the existing VersionGateSwitcher with dataCoord.enableArraySortIndex=auto: activation requires registered online components to reach 3.1.0 for the one-minute stability window. Every ARRAY/JSON build request carries the resolved gate, so new segment builds for existing HYBRID definitions are protected too. Closed or missing build flags keep ordinary high-cardinality ARRAY HYBRID on INVERTED. SORT selection additionally requires engine version 5 on master; nested ARRAY retains its existing rule. New logical entry points fall back for AUTOINDEX or return ServiceNotReady for explicit creation/restore before activation. Scalar engine current/maximum remain 5/5. Packed formats are unchanged, existing indexes retain their physical family, and index definitions previously rewritten to INVERTED require recreation to adopt HYBRID. The software gate is one-shot and does not continuously protect rollback after activation. NaN semantics and float encoding changes are outside this PR. issue: #51055 Related: #53972 (ARRAY support extracted without NaN changes). Design: https://github.com/xiaofan-luan/milvus/blob/13fc1f29805d01713c610016827b26599f522878/docs/design-docs/design_docs/20261021-ordinary-array-hybrid-sort.md Validation: - Current cast-rule cleanup (`13fc1f29`): the full Go indexparamcheck package passed (69 top-level tests, 154 subtests, zero failures/skips; default vet, dynamic/test tags and disabled optimizations/inlining). Four changed production C++ units were freshly compiled and linked; 48 focused C++ tests passed, zero failures/skips. Source hashes match this squashed commit. Other native objects/generated headers/dependencies were reused from the previous validated baseline; this is focused validation rather than a full native rebuild. DataCoord runtime tests were not rerun. New CI for this head is pending. - Review fixes on `33a01442`: both changed production C++ units and five related test units were freshly compiled and linked; 48 focused tests passed, zero failures or skips. Covers ordinary ARRAY and typed JSON ARRAY builds/queries, including missing, NULL, empty and all-uncastable arrays. Other native objects, generated protobuf headers and third-party dependencies were reused from the prior validated baseline; this was not a full native rebuild. - Software-gate tests on the preceding `3560bcce` head passed (3 top-level, 15 nested), and TestYamlFile passed with default vet and `-tags dynamic,test -gcflags='all=-N -l' -count=1`. Scalar engine threshold remains 5. - Earlier native feature validation rebuilt the core and focused C++ tests: 89 passed, no failures/skips. Covers all eight ordinary ARRAY types, three JSON ARRAY casts, gate enabled/disabled/missing, JSON HYBRID engine versions 3/4/5, packed heap/mmap and actual ANY/ALL/NOT and equality predicates. - Earlier Go feature validation: Create/Restore gates 3 top-level / 90 subcases; per-build gate suite 9 leaf cases; full indexparamcheck 68 top-level tests. Includes metadata preservation, parameter-override prevention and cast-function rejection. - clang-format 15, gofumpt, source/artifact consistency checks and git diff --check passed. - Earlier CI regression checks passed with default vet enabled: Proxy ARRAY admission tests, TestYamlFile and per-build gate tests. SDK testcases compile; their ARRAY assertions now handle the precise closed-gate error and retain Await/Describe validation when enabled. Full SDK runtime coverage awaits CI. - Focused native/unit verification; no complete cluster upgrade, stock old-reader/rollback E2E, NaN or performance claim. Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com>	2026-10-23 14:11:54 +08:00
sparknack	5f527bbb25	enhance: isolate search fanout from middle pool (#54523) ... issue: #54521 Search PK filling, distance refinement, and ordered field materialization share MIDDLE with field and index loading. Give search fanout a separate SEARCH_FANOUT queue, and keep small resident scalar work inline to avoid scheduling overhead. Fanout requires multiple participating segments. The additional conditions are: | Stage | Fanout condition | | --- | --- | | PK filling | Eviction enabled **OR** VARCHAR PK **OR** actual candidate count `>= 16384` | | Ordered field materialization | Eviction enabled **OR** a requested field is a vector/non-fixed-size type **OR** actual `total_rows >= 16384` **OR** a requested field uses mmap in a participating segment | | Distance refinement | No additional condition | Use the existing `IsFixedSizeType` classification and explicitly include vectors (FP16/BF16/INT8 vector types can be fixed-size). Inspect only requested fields. `total_rows` counts selected output rows across queries and segments; it is not per-query topk. PK candidates are counted from actual `seg_offsets_.size()` after invalid-row filtering and optional global-refine truncation, before cross-segment reduction. PK and field materialization reuse one 16K constant; counting stops once the threshold is reached. Both protobuf and Arrow materialization use the shared gate. Mmap uses the existing per-segment `is_mmap_field()` runtime marker, scoped to requested fields in participating segments. The mmap scan runs only when all earlier conditions are false and stops at the first match. Sealed output paths already hold a read lease that prevents online resource replacement. No new state or interface is added. Serial paths visit every segment. Mirror MIDDLE sizing, thread caps, lazy creation, and runtime resizing. Add separate pool metrics and a master-only TODO for global coroutines. Inner loaders and load-worker accounting stay unchanged. No new configuration key or public API is introduced. The thread cap applies to each pool, so combined worker capacity can double; CPU and storage bandwidth remain shared. Install PK and refinement cleanup guards before submitting tasks. If a later submission throws, wait for already accepted tasks before unwinding caller-owned state, while preserving the original exception. Field materialization already follows this ordering. ### Validation - 447 focused routing/cleanup checks passed with eviction configured off/on in separate processes. The harness extracts PK, refinement and field-fill scheduling blocks directly from source and uses production ThreadPools, Manager, cancellation, and type classification; segment callbacks are stubbed. Covers 16383/16384/16385 rows, INT64/VARCHAR PKs, one million PK candidates, uneven/empty per-segment counts, scalar/vector/non-fixed outputs, unrequested vector fields, zero/single/multiple segments, serial completeness, callback exceptions, and cancellation. Schema and segment/refinement callbacks are fixtures; this does not exercise the refinement kernel. Extracted blocks match 3.0 exactly. This does not exercise storage, merge, Arrow conversion, or the C API. - Added 24 mmap routing cases across eviction off/on. The previous source failed the requested-mmap fanout assertion; the updated source passes first/last-segment matches, non-requested fields, ordinary memory fields, single-segment execution, and short-circuit checks for eviction/type/16K, plus exceptions, cancellation and invalid segment references. Mmap status and materialization are fixtures; cold-page I/O performance is unmeasured. Audited the production runtime marker updates for load, replacement, default-value fields and drop, and the read-lease/publication ordering. Static sub-agent review of the mmap change found no actionable issue. - Submission-failure injection reproduced the old PK behavior: after one accepted task, the next submission could throw and return before the first task finished. The fix passed 36 cases across PK/refine and eviction off/on, failing before the first, second, or fourth submission, with accepted callbacks succeeding, throwing, or observing cancellation. Accepted work uses the real pool; a fixture injects `bad_alloc` before enqueue. These cases verify task completion before the original submission exception propagates. - Syntax-only compilation of both modified callers passed using the exact milvus-common dependency. Changed-line clang-format and `git diff --check` passed. - Earlier pool verification: 11 focused tests passed for lazy/live resizing, MIDDLE queue isolation, metrics, exceptions, cancellation, and concurrent creation/resize. Static sub-agent review identified an existing submission-failure cleanup gap. PK and refine guards now cover the submission loop; a second static review found no remaining actionable issue in the fix. - Repeated the resident INT64 gather microbenchmark after the saturated-host run: 144 balanced cases and 48 skewed cases, seven paired rounds each, with output correctness checks. It uses production column/cache/pool code with synthetic resident input. The balanced run's host CPU usage averaged 20.72% (maximum 35.37%). At eight segments/eight concurrent requests/one output field, 8K rows were approximately tied (81.94 vs 81.82 us), while 16K rows measured 206.30 us serial vs 127.49 us fanout. - Resident INT64 PK component benchmark: 144 balanced and 24 skewed cases, each repeated in 21 paired rounds (three complete passes), with output checks. It exercises production compressed PK lookup, cache pinning, protobuf allocation/conversion and pool scheduling, not complete FillPrimaryKeys/search. Under current background load, 79/80 balanced cases at or above 16K improved median latency in all three passes; 25/80 had worse batch-p99 medians. At 100 segments/one million candidates/one request, one layout measured 81.34 ms serial versus 10.78 ms fanout. The shared 16K threshold favors a single simple bound on inline work; it is not an optimal crossover or a tail-latency guarantee. Skew and concurrency still produce regressions. The benchmark used master sources; equivalent 3.0 PK kernels were compared, not separately benchmarked. 16K is a heuristic, not a universal crossover: 35/36 balanced cases at or above 16K improved, but a two-segment case regressed, and skewed outputs also regressed. The skew run had higher background activity and is only a qualitative stress check. Full Milvus build, `make verifiers`, end-to-end searches, and cold-I/O/mixed-load validation were not run. No end-to-end latency or throughput improvement is claimed. --------- Signed-off-by: Shawn Wang <shawn.wang@zilliz.com>	2026-10-23 13:21:52 +08:00
congqixia	c49d62d874	enhance: add segment change readiness and publish (#54198) ... Add the shared write-time readiness predicate for segment change staging (data / index / sort / stats / schema, with the no-index behavior fix, pure-scalar degradation and C23 schema fence) and the group lifecycle layer on top of the merged model (#53322) and DataView publish primitives (#53892): - publishGroup atomically commits a READY group in one catalog txn: members flipped visible, superseded parents retired, group COMMITTED, and a DataView snapshot (compact_version +1) on dataview-enabled DataCoords (the dataview-less write is the pure SegmentMeta composite). - segmentChangeInspector periodically evaluates STAGED groups to READY, drives publish, aborts on staging timeout, and fails unconvergeable groups (externally dropped members, C23 schema fence fail-fast before any wait). - R10: legacy (non-dataview) view / seek paths exclude ALIVE-group members via aliveStagedMembers; this deliberately filters group membership, not blanket IsInvisible, to preserve sort-enabled awaiting-sort segments' DML contribution (design O1-gated). - READY->STAGED regression (design R5) added to the group state machine so an index/schema regression between READY and publish is re-evaluated instead of retried forever. - config: dataCoord.segmentChange.stagingTimeout / maxMembersPerGroup. No producer stages groups yet, so the inspector no-ops and system behavior is unchanged. All staged tests pass. issue: #53321 --------- Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>	2026-10-23 09:53:53 +08:00
Zhen Ye	6f4ba71d17	feat: add remote TransformLog subscriptions and MVCC barriers (#54344) ... issue: #40451 - TransformLog subscriptions: add SubscribeTransform RPCs with local/remote owner routing, creation/close handshakes, ordered Entry/SyncUp delivery, and resumable cursors. Align recovery with WAL Consumer, filter replay before data-only handlers, back off failed attempts, suspend reconnects without subscriptions, and reclaim idle physical streams. - MVCC barriers: preserve BarrierUntil and active TimeTick synchronization; publish scoped progress in WAL order without advancing durable checkpoints or GC. Queue volatile MVCCBarriers until RecoveryStorage observes the complete prefix and dispatches query progress, allowing Inspector to discard published scopes while preserving query and Delete/L0 behavior. - WAB cache and backpressure: preserve capacity/TTL eviction with RecoveryStorage consumption as an additional constraint. Retain the latest durable confirmation independently for catchup handoff without pinning consumed volatile barriers. Replace scanner mode-based pressure with cache-occupancy-based admission control; use internal, live configuration with a default slowdown multiple of 1.5 (minimum 1) and a rejection multiple of 2 relative to the slowdown threshold, while preserving independent RecoveryTail pressure. - WALSummary reads: reuse verified chunk footers across payload eviction while validating reloaded framing/checksums, prevent retired entries from being repopulated, and share chunk retries with independent reader cancellation and GC snapshot protection. Design: [TransformLog subscription adaptor](https://github.com/chyezh/milvus/blob/qv_transformlog_subscribe/docs/design-docs/design_docs/wal/transform_log.md). --------- Signed-off-by: chyezh <chyezh@outlook.com>	2026-10-22 21:09:53 +08:00
Zhen Ye	cf0e5b68a0	fix: prepare analyzer file resources through streaming client (#54302) ... issue: #54102 - Resource loading: add an internal batch FetchFileResources RPC by bound resource IDs or analyzer resource names. AnalyzerResourceManager owns metadata fetching, per-ID download flights and local file references, with transient failures retried locally and shared downloads canceled only after the last waiter leaves. Retain unused files for common.fileResource.cacheTtl (default 15m), with safe reacquisition, expiration and shutdown cleanup. - Analyzer validation: route coordinator validation through the streaming client, check remote resource names and filenames against fetched metadata, and return validation/preparation failures through business Status. Preserve resource reservation around DDL publication and distinguish invalid input from system failures, including generic Rust IoNotFound handling at the C++ validation boundary. - Runner lifecycle and WAL: pin exact resource bindings through native runner lifetime, include schema resource IDs in runner identity, and initialize resource-dependent runners on demand. Prepare unavailable runners in the redo interceptor after inner locks and admission scopes unwind, then retry materialization while preserving the outermost idempotency guard. - Integration: retain public RPCs, legacy resource distribution, mode selection and the WAL assignment readiness gate as groundwork for qv. The new loading path follows on-demand execution and Milvus's supported upgrade order, with MixCoord upgraded before worker nodes. --------- Signed-off-by: chyezh <chyezh@outlook.com>	2026-10-22 14:17:53 +08:00
aoiasd	56e27337e3	enhance: remove RLS implementation while retaining proto APIs (#54352) ... relate: #50263 ## Summary Remove RLS metadata, caches, query/write/import enforcement, SDK helpers, and configuration while retaining proto definitions and unsupported API responses. Existing RLS-enabled collections use ordinary collection behavior; persisted RLS records are left untouched and retired WAL message types remain decodable. Master build and focused tests passed; cluster integration and CDC validation were not run. --------- Signed-off-by: aoiasd <zhicheng.yue@zilliz.com>	2026-10-22 10:29:53 +08:00
zhenshan.caoandClaude Opus 5.5	24ba250f4d	feat: report feature usage on demand via MixCoord and the Proxy management port (#53311) ... issue: #51149 design doc: docs/design-docs/design_docs/20260902-feature-usage-reporting.md (#53136) Milvus has no way to answer "which features does this instance use, and how many objects use each one". The question comes up when deciding whether a feature can be deprecated, when measuring adoption of something newly shipped, and in support triage. Today the only sources are ad-hoc `DescribeCollection` scripts, Prometheus counters keyed by RPC name, and access logs, none of which can say how many collections declare a partition key or how many search requests last week used grouping search. This adds an on-demand, pull-only report: `GET /management/feature_usage` on the Proxy management port, gated by `common.security.featureUsageEnabled` (default off) and authenticated as root. ### What it reports **Static groups**, recomputed from coordinator metadata on every read, never cached: field types, index and metric types, functions and their providers, declared predicates, collection and database properties, field and index params, object counts, fixed distribution buckets, segment traits, and what QueryCoord currently has loaded. **Request counters**, a fixed-size array of atomics whose id set is a compile-time constant (174 in this build), each slot carrying a value and the second it was last hit: - Proxy, what the request asked for: search and query options, ranking and highlighting, the expression features (one walk over the parsed `planpb.Expr` per request, hung off the parser's output because `planparserv2` caches by expression string), query aggregation and ordering, upsert and delete modes, and the search shape — the `ef`, `nprobe`, `limit` and `nq` a client sends and the retrieval kind, as fixed buckets counted once per ANN subrequest, plus the hybrid search shape once per request. - Proxy, what the QueryNodes actually did: which path each filter took (`scalar_index`, `pk_index`, `text_match_index`, `json_shredding`, `ngram_index`, `brute_force`), which scalar index kind served it, an index that was declined, expression cache hits, the interim index, the strict grouping second phase, cold reads, and the delegator's own decisions (two-stage search, segment pruning). The Proxy asks for these only on the requests it counts (`PlanOption.collect_feature_bits`); segcore and the delegator record them into a bit set shared by every segment of the plan, the QueryNodes return it as `feature_bits` on the results, and the Proxy counts each feature once per user request. - DataCoord: import file types and compaction types, counted where the job is created. Counting them on the DataNode that executes the task would be wrong wherever DataNodes are pooled across instances, and would count retries twice. - QueryNode: a `config` group naming the twenty boolean `queryNode.*` switches in effect on that node (a non-refreshable switch at the value the node started with). `RunAnalyzer` is counted at the Proxy, once per client request. Every counted request moves a counter at most once, however many times the Proxy parses or re-runs it: the search task marks into a set flushed once, only the first task built for a user request counts, and the queries the Proxy synthesizes for itself are marked and skipped. MixCoord merges its own metadata statistics with a fan-out to every Proxy and QueryNode. An unreachable node is reported with its error, never omitted, because an omitted node reads as "this feature is unused". Report names follow the Milvus and Zilliz documentation: the documented feature name where there is one, otherwise the documented parameter key, with documented literal values after `=`. ### Cost and safety Counting is gated process-wide by `common.featureUsage.countersEnabled`, read once at component start. A request that is not counted pays one branch per hook, and its plan never asks segcore to record anything. Measured on the counted path: flushing a request's marks costs about 300 ns with no allocation, the per-subrequest search shape about 310 ns, and the expression walk about 7 ns per term; segcore pays one relaxed atomic load per expression per segment. Nothing is persisted, no timer runs, no log line is added, nothing is keyed by collection or user, and counters are never reset. Every string that reaches an output entry comes from a code-defined set: an enum name, an official key from the allowlist in `pkg/common`, a known provider, a bucket label or a counter name. Anything else folds into `_custom` or `_other`. A test parses `common.go` and fails when a dotted key constant is added without being allowlisted, and another keeps the scalar index parameters of `indexparamcheck` on the list. ### Tests - Unit tests per package for the computed entries, the counter semantics, the bit plumbing and the fan-out behaviour, and C++ unit tests for the segcore recorder. - `internal/featureusage/testdata/report_surface.golden` lists every group and counter with its role. Renaming or dropping one is a breaking change for the report consumer that nothing else in the build would catch, so a test compares the code against the file and prints the added and removed lines. A Go test also parses segcore's `FeatureBit` enum so the bit layout cannot drift between C++ and Go. - `tests/integration/featureusage` drives one request per counter against a real cluster and asserts that **exactly** the expected counters moved, by exactly that amount. It ends with a coverage gate that reads the same golden file and fails if any counter was never exercised: all 174 are driven end to end except six, each listed with its reason (`auth_method=password` is driven in the sibling authentication suite). 25 test methods, about 207 seconds on one cluster. - A local run on a standalone instance with a pymilvus workload covering every group, recorded in the design doc's Local Verification section. Two of the undrivable entries, `compaction=partition_key_sort` and `compaction=clustering_partition_key_sort`, look like an incomplete cleanup in DataCoord rather than anything to do with this report; filed separately as #53310. They are kept in the catalog with a note in the consumer contract, because a counter name is part of the consumer's schema and dropping one to re-add it later would be two breaking changes instead of none. ### Fourth review round and path audit The five findings of the last review were each one feature with a second realization path the hook did not sit on. Rather than patch the five, every counter was traced to each path a client, a retry or a recovery can take it through (parser rewriter, function chains, two-stage search, requery, cache adapter, QueryCoord recovery, server-written properties, RLS, REST v1, iterators), and the hooks were moved to where those paths converge: expression features are counted on the parser's output **before the rewriter** through a hook on `ParserVisitorArgs` that also sees boost scorer filters; `function_chains` counts under the same `reranker=*` slots as `function_score`, with `rerank_provider=*` for model rerankers; `two_stage_search` and `segment_prune` travel as execution feature bits and `run_analyzer` is counted at the Proxy, so no retry or internal re-run counts; the `loaded` and `segment` groups resolve collection IDs against the RootCoord snapshot (`CollectionContext`); server-written properties and fields are excluded. The commit message lists every change; the definition-level findings that remain are in the design doc as known limitations. ### Compatibility In-repo protos only. `milvus-proto` and the SDKs are untouched. The endpoint is off by default, so nothing changes for an existing deployment until an operator turns it on. The new `feature_bits` fields are append-only bit sets, so a QueryNode and a Proxy of different builds agree during a rolling upgrade. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VELxn8Gx83pqx1s3FWXy6K Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>	2026-10-21 12:09:52 +08:00
Zhen Ye	267103584a	fix: gate broadcaster tombstone GC on durable WAL checkpoints (#54432) ... issue: #54425 - Broadcaster GC: require every participating PChannel's fresh persisted WAL checkpoint TimeTick to strictly pass its local broadcast copy. Collect expired eligible tombstones first, then evict the smallest BroadcastIDs until the remaining eligible count meets maxCount; count eviction may precede TTL expiry. Skip catalog access when neither age nor total count can trigger GC. - Scheduling: use unbounded MultipartQueue inboxes for ACK callbacks and tombstones, with short locks only around enqueue and queue exchange. Coalesce ACK wakeups without blocking, preserve conflicting callback order, and keep GC scanning and catalog I/O outside the inbox locks. - WAL recovery: filter the completed prefix after scanner ordering and transaction assembly while preserving the startup barrier and live continuation. Legacy migration keeps the safe physical start and publishes a zero global TimeTick so inclusive boundary messages remain replayable across restarts. - ACK handling and retention: suppress duplicate ACKs for TOMBSTONE and DONE tasks, retain tasks with unknown legacy ACK positions, and document checkpoint-gated age/count retention and its bounded protection against delayed duplicate RPCs. --------- Signed-off-by: chyezh <chyezh@outlook.com>	2026-10-21 11:41:51 +08:00
wei liu	42d337e774	feat: Support precomputed sparse vectors in external collections (#53893) ... issue: #53827 External collections reject precomputed sparse vector fields before reading the source. Allow these fields and normalize Arrow Map/Struct columns to Milvus sparse rows at the shared index, load, Take and sampling boundary. Preserve null versus empty rows, reject malformed indices and weights, and normalize source Binary rows, reusing already ordered Binary arrays. Reuse sparse binary validation in the native row copier while retaining the target branch's length checks and error classification. Add regression cases for Parquet, Lance and Vortex, and update the existing external-table design. Preflight conversion buffers against a startup-only budget of 256 MiB. Encode ordered rows directly, reserve scratch only for unordered rows, and handle BinaryView/LargeBinary before generic canonicalization can allocate. Accept two null Struct child lists as an empty vector. Fixed-budget rejection terminates index tasks through Unsupported; allocation failures retain their existing retry classification. Design: [External table design](https://github.com/weiliu1031/milvus/blob/support-external-sparse/docs/design-docs/design_docs/20260105-external_table.md) Validation (current rebased candidate): - Rebased onto master `4f0f748f1056446b6c84366a7295ff6cd0597685`. Preserved the upstream array-validity parameter; the prior fixture-fix commit was already present upstream and was dropped. - 10 isolated C++ tests passed using the extracted production entry, dispatcher, canonicalizer and sparse conversion with real Arrow. Metadata/error scaffolding and unrelated branches were simplified; this is not a full-core integration run. - The isolated production index scheduler classification test passed with 100% statement coverage. The paramtable common-config test passed. Go tests used `-tags dynamic,test -gcflags="all=-N -l"`. - `git diff --check` passed; source equivalence was checked with `git range-diff`. - Full index package and C++ test compilation remain blocked by stale local native C ABI and generated protobuf dependencies. Full-package and end-to-end runtime validation remain outstanding. --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com>	2026-10-20 09:37:51 +08:00
Jeng.Gwan	82a1dd7714	feat: implement Import V3 reshard (#53334) ... issue: #53333 ## Summary Implements the Import V3 reshard path: source files are parsed and sorted once at first read, fragments are planned into final segments across files and tasks, and the Import V3 merge produces segments that already satisfy the sort-compaction ordering contract, so the post-import SortCompaction pass is no longer run for jobs on this path (#50593). Rollout gate: `dataCoord.import.enableImportV3` (default `false`). The execution version is selected once at job create, persisted in the import WAL (`msgpb.ImportMsg.version`) and the job catalog, and never re-selected on retry, restart or config change. L0 imports and in-flight/legacy ImportTaskV2 jobs keep the old path. ## Mechanism - **DataCoord** - Import V3 job state machine: `Pending -> [PreImporting -> AssigningIDRange] -> Resharding -> Planning -> Importing -> IndexBuilding -> Uncommitted -> Committing -> Completed` (backup/binlog imports skip the count-only preimport and ID-range stages). - Planning aggregates reshard fragments by `(vchannel, partition)` bucket across all reshard tasks, packs them into segment plans in canonical order (sequential next-fit against the expected segment size) and preallocates the formal segment IDs. Reshard file grouping is a deterministic best-fit-decreasing packing capped by `dataCoord.import.maxSizeInMBPerImportTask`. - Per-file literal `IDRange` allocation reuses the two-phase mechanism added in #53544; files that already carry a range (replicated/forwarded WAL bodies) are authoritative and are not re-allocated. - Result acceptance applies worker results to the preallocated segments with storage-version self-healing on retry (a retry converges onto the current storage version instead of failing on a stale version fence). - Orphan reconciliation in the garbage collector is extended to the Import V3 intermediate paths, plus terminal-job GC derived from durable facts. - **DataNode** - `ReshardTask` parses each source file exactly once: field normalization, PK/RowID materialization, bucket routing and per-batch sort under the shared sort order (`importutilv2.SortFieldIDs`), written as temporary fragments under `{root}/import_v3/{job}/reshard/{task}/`. - `ImportTaskV3` merges fragments into final segments with a one-head k-way merge (direct fan-in `min(dataCoord.import.fragmentMergeFanIn, len(fragments))`, clamped to `[2,1024]`, default **128**; hierarchical merge when the plan has more inputs), bounded by a working-set slot estimate and DataNode slot admission. - Count-only preimport reads parquet/numpy row counts from file metadata (bounded by `dataCoord.import.parquetFooterMaxSize`) instead of scanning. - Merge intermediates go to node-local Arrow IPC / local disk, so the object store sees each fragment exactly once. - Transient IO and local-storage errors stay retryable via the shared checker denylist; non-transient run errors fail the task. ## Behavior boundaries - No user-facing API change: job creation, CommitImport/RollbackImport, auto-commit and visibility behavior are preserved; only the internal execution pipeline is split into `ReshardTask`/`ImportTaskV3` with per-run `run_id` isolation for late results. - L0 imports, V2 backup/binlog restore jobs and legacy in-flight jobs are unchanged. - **Changes observable even with the gate off** (reviewer awareness): - `dataCoord.import.maxSizeInMBPerImportTask` default 16384 -> 8192. It is consumed by the legacy V2 file regrouping (`RegroupImportFiles`), so V2 imports on collections where `segmentMaxSize x P x V` lands in (8 GiB, 16 GiB] now produce roughly twice as many, half-sized file groups/tasks. Set the knob back explicitly to keep the old grouping. - CSV import no longer silently truncates on a mid-file read/parse error: the reader used to drop every non-EOF error (`csv/reader.go`); it now fails the job. - CSV header / parquet file-reader failures move from `ErrImportSysFailed` (2101) to `ErrImportFailed` (2100, Input) or the preserved IO code; failure reason strings change accordingly. Retry classification is unchanged. - Existing datacoord metrics gained labels: `ImportJobLatency`/`ImportJobs` (`job_version`), `ImportTaskLatency` (`task_type`). - The V2 collection-drop check no longer tries to fail `Committing` jobs. The job state outcome is unchanged (`UnfailableJobStates` already refuses `Committing -> Failed`); this only removes one futile catalog write plus a warning log per GC tick. ## CMEK / TEXT columns Column groups are encrypted with the target collection's CMEK zone; fragment readers use the read/source plugin context. **TEXT columns are deliberately written plaintext.** The TEXT read chain has no key retriever wiring yet: segcore reads V3 column groups through milvus_storage's `api::Reader` without a key retriever, and the Go TEXT-decode segment reader has no FFI entry point for one, so an encrypted TEXT segment would be written but never read back (both query and the compaction TEXT rewrite). The plugin context is still threaded all the way down into the TEXT-aware writer; only its two call sites (`internal/flushcommon/syncmgr/pack_writer_v3.go`, `internal/storage/binlog_record_writer.go`) intentionally pass `nil`, with a `TODO:` to re-enable encryption once the read chain supports decryption. This matches the design doc's Stage 3 note, and only matters when `common.storage.useLoonFFI=true` and the collection has an encryption zone. ## Design design doc: `docs/design-docs/design_docs/20260921-import-v3-reshard-design.md` ## Validation - [x] Compiles with `-tags dynamic,test` (`internal/datacoord`, `internal/datanode/importv3`, `internal/storage`, `internal/flushcommon/syncmgr`, `internal/util/initcore`). - [x] `golangci-lint --build-tags dynamic,test` clean on the touched root packages and on `pkg/util/paramtable`. - [x] Unit tests pass: `internal/datanode/importv3`, `pkg/util/paramtable`, `cmd/tools/config -run TestYamlFile`. - [x] Integration test included: `tests/integration/import/import_v3_test.go`. - [ ] E2E: to be run on CI. --- # Import V3 (Reshard) vs Import V2 — Core Performance Metrics > The only difference between V3 and V2 is `enableImportV3`; everything else is identical (both use milvus-storage V3). ## TL;DR On the same dataset of 111 GB / 37.92M rows: **end-to-end time is 14%–79% faster** (the more shards/partitions, the bigger the gain), **the write chain is 41%–89% faster**, and **S3 request counts drop by 70%–99%**; moreover, **only V3 completes at 128 / 1024 partitions** (V2 fails because of the etcd 2 MB metadata limit). ## Test setup | Item | Value | |---|---| | Environment | 10 × 8C 32G datanodes | | Data | 11 jobs / **1100 parquet files / 111.31 GB / 37,924,512 rows** | | Schema | 13 fields; `dense_embedding` = **FLOAT16_VECTOR dim=512**, FLAT+IP index; **partition key = `pk_id`** (99.65% unique) | | Tiers | 10 tiers: 1x1 / 1x2 / 1x4 / 2x4 / 2x8 / 2x16 / 2x32 / 2x64 / 2x128 / 2x1024 | | Tier naming | **NxM = num_shards × number of partitions** (e.g. `2x64` = 2 vchannels / 64 partitions) | | Parameter | Value | | --- | --- | | `dataCoord.import.fragmentSizeInMB` | 128 | | `dataCoord.import.maxSizeInMBPerImportTask` | 8192 | | `dataCoord.import.reshardResidentBucketCap` | 16 | | `dataCoord.import.reshardFlushConcurrency` | 2 | | `dataCoord.import.reshardMemoryExpansionFactor` | 1.5 | | `dataCoord.import.fragmentMergeFanIn` | 128 | | `dataCoord.import.enableImportV3` | true | | `common.storage.useLoonFFI` | true | | `dataNode.import.readBufferSizeInMB` | 16 | | `dataNode.import.reshardSpillMaxStreams` | 128 | ## Core metrics ### ① Elapsed time  **Per-stage elapsed time (seconds; V3 and V2 have different stage sets; "–" = not applicable)** | Tier | Version | create | preimport | reshard | planning | importV3\* | importV2 | sort\* | index\* | wall | |---|---|---|---|---|---|---|---|---|---|---| | 1x1 | V2 | 1 | 50 | – | – | – | 112 | 127 | 480 | 835 | | 1x1 | V3 | 4 | 9 | 75 | ~0 | 43 | – | – | 497 | 643 | | 1x2 | V2 | 1 | 50 | – | – | – | 135 | 132 | 499 | 829 | | 1x2 | V3 | 4 | 9 | 73 | ~0 | 45 | – | – | 485 | 629 | | 1x4 | V2 | 1 | 46 | – | – | – | 100 | 139 | 488 | 780 | | 1x4 | V3 | 3 | 10 | 108 | ~0 | 50 | – | – | 489 | 669 | | 2x4 | V2 | 1 | 54 | – | – | – | 134 | 228 | 419 | 848 | | 2x4 | V3 | 4 | 9 | 132 | ~0 | 49 | – | – | 491 | 689 | | 2x8 | V2 | 1 | 52 | – | – | – | 308 | 341 | 336 | 1043 | | 2x8 | V3 | 3 | 10 | 233 | ~0 | 45 | – | – | 472 | 769 | | 2x16 | V2 | 1 | 50 | – | – | – | 602 | 294 | 277 | 1238 | | 2x16 | V3 | 4 | 10 | 446 | ~0 | 50 | – | – | 480 | 1004 | | 2x32 | V2 | 1 | 50 | – | – | – | 958 | 337 | 322 | 1681 | | 2x32 | V3 | 2 | 11 | 486 | ~0 | 64 | – | – | 464 | 1032 | | 2x64 | V2 | 1 | 48 | – | – | – | 4302 | 667 | 26 | 5058 | | 2x64 | V3 | 3 | 10 | 510 | ~0 | 53 | – | – | 483 | 1069 | | 2x128 | V2 | 1 | 50 | – | – | – | ≈7000 | ≈1330 | ≈25 | ≈8400 | | 2x128 | V3 | 2 | 11 | 542 | ~0 | 61 | – | – | 450 | 1075 | | 2x1024 | V2 | 1 | 50 | – | – | – | ≈105000 | ≈10700 | ≈25 | ≈116000 | | 2x1024 | V3 | 2 | 8 | 625 | ~0 | 115 | – | – | 275 | 1036 | V3 and V2 have different stage sets: V3 = preimport → reshard → importV3; V2 = preimport → importV2 → sort. Because `index` is dispatched as soon as its segments are ready: - V3's `index` overlaps `import` - V2's `index` overlaps `sort` Therefore: (1) the stages **cannot simply be summed** — the sum does not equal `wall` either, the gap being mainly index building; (2) the starred columns are stretched by the index tail (which is why V3's `import` is inflated). The last two V2 tiers actually **Failed** (etcd 2 MB metadata limit); their importV2 / sort / wall are **estimates derived from the source code** (segment count = T·V·P, see ③), not measurements. ### ② S3 access   | Tier | V2 read (ops/GB) | V3 read (ops/GB) | read drop | V2 write (ops/GB) | V3 write (ops/GB) | write drop | |---|---|---|---|---|---|---| | 1x1 | 115,195 / 129.3 | 17,758 / 130.8 | ↓84.6% | 76,535 / 215.6 | 22,686 / 218.5 | ↓70.4% | | 1x4 | 137,522 / 129.4 | 17,167 / 130.8 | ↓87.5% | 91,785 / 214.2 | 22,293 / 218.5 | ↓75.7% | | 2x8 | 255,808 / 130.7 | 17,263 / 130.7 | ↓93.3% | 170,905 / 214.8 | 21,912 / 214.7 | ↓87.2% | | 2x16 | 331,359 / 131.6 | 17,342 / 130.6 | ↓94.8% | 224,947 / 216.5 | 22,213 / 216.9 | ↓90.1% | | 2x32 | 654,440 / 131.2 | 18,695 / 130.5 | ↓97.1% | 440,787 / 219.8 | 22,087 / 215.5 | ↓95.0% | | 2x64 | 1.30M / 132.9 | 20,919 / 130.7 | ↓98.4% | 870,798 / 222.9 | 22,437 / 214.8 | ↓97.4% | | 2x128 | ≈2.61M / — | 28,786 / 130.7 | ↓98.9% | ≈1.75M / — | 22,708 / 215.4 | ↓98.7% | | 2x1024 | ≈20.86M / — | 104,040 / 130.0 | ↓99.5% | ≈14.02M / — | 59,070 / 216.2 | ↓99.6% | The **bytes** transferred are nearly identical on both sides and **barely change across tiers** (read ≈130 GB, write ≈215 GB, both ≈ the dataset size); **the entire difference is in the number of requests** — V2's sort compaction re-reads and re-writes the full dataset, so its request count is 1–2 orders of magnitude higher, whereas V3 has no sort and keeps request counts low and flat. ### ③ Segment shape   | Tier | V2 final segments | V3 final segments | V2 segment size p50 | V3 segment size p50 | V2 sort compactions | |---|---|---|---|---|---| | 1x1 | 197 | 172 | 846 MB | 965 MB | 197 | | 1x4 | 172 | 172 | 988 MB | 978 MB | 172 | | 2x8 | 176 | 176 | 1022 MB | 978 MB | 176 | | 2x16 | 352 | 192 | 511 MB | 976 MB | 352 | | 2x32 | 1344 | 192 | 128 MB | 985 MB | 1344 | | 2x64 | 2000 | 256 | 64 MB | 656 MB | 2000 | | 2x128 | ≈5,376 | 256 | ≈32 MB | 658 MB | ≈5,376 | | 2x1024 | ≈43,008 | 2048 | ≈4 MB | 82 MB | ≈43,008 | **V3 runs 0 compactions** and directly produces well-formed, large segments; V2 runs one sort compaction per segment after import (count = number of segments), so as partitions grow the data shatters into a large number of tiny segments (e.g. 2x64: 2000 segments × 64 MB). Fewer, larger segments are friendlier to subsequent query/index. ### ④ Memory (per datanode)  | Tier | V2 per-datanode peak | V3 per-datanode peak | |---|---|---| | 1x1 | 19.1 GB | 29.8 GB | | 1x2 | 22.9 GB | 28.5 GB | | 1x4 | 21.5 GB | 29.4 GB | | 2x4 | 26.2 GB | 29.9 GB | | 2x8 | 18.1 GB | 29.6 GB | | 2x16 | 15.1 GB | 28.6 GB | | 2x32 | 10.6 GB | 26.1 GB | | 2x64 | 11.6 GB | 25.8 GB | | 2x128 | — | 25.8 GB | | 2x1024 | — | 27.5 GB |  **Notes:** - **V3 peaks at 26–30 GB per datanode, slightly above V2 (10–26 GB)** — this is the cost of reshard holding bucket data resident in memory; in exchange there is no sort rewrite and none of V2's flood of tiny segments at high partition counts. - V3's peak is **stable and slightly decreasing across tiers** (≈26–30 GB) and **does not grow with partition count (up to 1024)**; the per-datanode usage curves (taking the busiest datanode) almost coincide across tiers — i.e. V3's reshard memory model keeps usage independent of the shard/partition count. - V2's peak actually *drops* at high partition counts (2x64: only 11.6 GB), but that is because the cost is pushed into sort/IO (see ② ③), not because it is more memory-efficient. Signed-off-by: xaxys <tpnnghd@163.com>	2026-10-19 15:11:51 +08:00
wei liu	b24da989eb	enhance: import StorageV3 snapshots with L0 and external sources (#52932) ... issue: #52930 ## What this PR does Add snapshot-sourced StorageV3 backup import through the existing Import pipeline. Accept referenced and self-contained snapshot layouts, including relocated export bundles, using an exact snapshot metadata URI: ```json { "files": [["s3://bucket/root/snapshots/collection/metadata/snapshot.json"]], "options": { "backup": "true", "source_type": "snapshot" } } ``` - Expand captured StorageV3 data manifests and rewrite surviving rows into an existing target collection. Fold applicable L0 deletes together with segment-local manifest deltas; preserve source commit timestamps and validate the row/commit timestamp invariant before filtering in both Import phases. - Read all source partitions. Without `partition_mapping`, use ordinary target routing: a specified/default partition for non-partition-key collections, or hashing by the target partition key. Source partition names are not automatically matched to target partition names. - Support `partition_mapping` as a JSON-object string in options, mapping every source partition name to an existing target partition name. Many-to-one mappings are allowed. Partial mappings, combining it with `partition_name`, and partition-key target collections are rejected. Source partition IDs therefore participate in both explicit target mapping and L0 matching. - Follow backup Import's physical FieldID contract, not name-based mapping or schema equality. Reuse field selection and default/nullable filling, with consistent PreImport statistics and Import routing. - Capture top-level snapshot metadata through WAL/ACK, then expand and persist the exact source plan in bounded background Pending preparation. DataNode PreImport performs physical data/L0/LOB reads and validation; failures are reported through job state. Typed descriptors survive task regrouping and retries, with empty legacy paths so unsupported consumers fail closed. - Accept `external_spec` for request-scoped source storage and credentials; validate source URI, bucket and endpoint without fallback to target storage. DataCoord import logs and REST Import trace logs redact sensitive options. A task-scoped ReaderFactory reuses resolved source storage and encryption dependencies across data readers and L0 preparation. Terminal job records remove external credentials; historical WAL entries follow WAL retention. - Reuse manifest/PackedReader and logical SegmentReader paths, including plaintext TEXT/LOB materialization. Skip LOB listing when the physical-field projection contains no TEXT fields; exclude dropped fields' LOBs from validation and size accounting. - Add Go SDK cases for both layouts, L0 folding, self-contained cross-bucket imports, manual commit, physical-field backup schema behavior, partition mapping, and ignoring an unnecessary EZK on a plaintext source. ## Behavior boundaries at the current HEAD - Bare metadata keys, direct segment manifests, `source_partition_name`, and explicit `storage_version` are rejected by the public snapshot contract. - Snapshot import does not restore collection properties, indexes or physical identities. Ordinary file import, StorageV1/V2 backup import, legacy `l0_import`, and snapshot restoration through `CopySegment` retain separate entry-point contracts. - Plaintext snapshots ignore and remove a supplied EZK, even if malformed, before WAL/job persistence. Encrypted sources require an EZK matching the source encryption zone. Encrypted TEXT/LOB snapshots are rejected. Non-TEXT CMEK source reads reuse the existing source-key path, but end-to-end CMEK support is not validated; dedicated CMEK E2E is outside this PR. - Historical snapshots with omitted commit timestamps are accepted. Omitted values remain zero, so deletes are compared against raw row timestamps. If the producer lost a real commit-time override, this can incorrectly remove rows that should survive. This is an explicit historical snapshot limitation; manifest probing or re-export cannot reconstruct that value. No timestamp-completeness marker or compatibility manifest probe is used. - The expanded typed source plan remains capped at 256 KiB. L0 inventories are stored by source channel/partition instead of copied into every data segment. PreImport and Import group shared-L0 tasks within those scopes. - The public protobuf Import API is unchanged. Internal snapshot/task/WAL descriptors carry source timestamps, preparation state and shared inventories. Development-version persisted tasks with per-segment inline L0 are explicitly rejected and must be resubmitted. ### Execution and resource bounds - CreateImport reads and captures top-level metadata, but performs no remote calls per segment. Pending preparation is limited to four concurrent jobs per checker and persists the complete expanded plan before task creation. Workers use exact captured manifest versions, never resolve latest. - A reader-independent DeleteMerger handles decoding, timestamp-window filtering, PK-to-max-timestamp aggregation and memory accounting. Shared-L0 tasks stream bounded delete batches into per-segment physical-row bitmaps; segment-local deletes update only their own masks. Output readers start only after preparation completes. No unbounded whole-channel delete map is retained. - Each phase builds its own masks. A shared L0 stream is consumed once per task attempt in the normal path, not once per segment; storage retries and legacy decoder probing can still issue additional reads. Separate tasks, phases and retries do not share cached deletes. Each batch rescans the affected segments' PK/timestamp columns, trading extra reads for bounded delete-map memory. - Reuse `dataNode.import.readDeleteBufferSizeInMB`, now defaulting to 128 MiB; no second snapshot-specific capacity setting is added. This also changes the existing legacy L0 slot estimate. Shared-L0 tasks do not fail merely because the total delete set exceeds one batch, but oversized keys, retained path inventories or bitmap exhaustion still fail explicitly. Typed readers without shared L0 retain their whole-map budget limit. - Shared-L0 task admission reserves `B + R * (rowBuffer + B)`: one delete batch plus admitted row buffers and a bounded bitmap pool. DataCoord includes these reservations in both phases' slot estimates and rounds memory slots up; DataNode controls actual concurrency. At the default budget, even one shared-L0 reader needs an Import allowance greater than 256 MiB. Smaller nodes must lower the delete budget or increase that allowance. - These are accounted memory budgets, not strict process-RSS caps for Arrow, decoders or the Go allocator. The existing V1 decoder may materialize a complete individual delete file. No spill-to-disk or cross-task cache is added. ### Shared StorageV3 delta validation This PR also tightens delta metadata validation in the shared StorageV3 manifest readers, not only the snapshot Import entry point: - Negative `entries_num`, or a positive count without a nonblank delta path, now returns `ErrDataIntegrity` instead of allowing malformed entries to be silently skipped or passed downstream. - Declared delta entries with missing path/count arrays are classified as `ErrDataIntegrity` rather than an internal-service error. - Legitimate zero-entry markers remain supported; the readable-path helper still skips them. Valid delta metadata keeps its existing read behavior. This is fail-closed behavior: missing delete metadata must not be interpreted as an empty delete set, which could make deleted rows visible again. The change also applies to existing callers of these helpers, including QueryNode StorageV3 segment loading, compaction, external Milvus-table delta reads, and shared manifest column-group reads. Operations encountering the malformed metadata above can now fail where an earlier path could continue. This does not add the same validation to ordinary file or StorageV1/V2 backup Import. The affected call paths and error propagation were inspected in source. This description update adds no new runtime-validation claim; malformed manifest fault injection across those existing workflows was not run as part of this update. ## Design [Snapshot-Sourced StorageV3 Backup Import](https://github.com/weiliu1031/milvus/blob/suport-v3-import/docs/design-docs/design_docs/20260902-snapshot-sourced-storagev3-backup-import.md) The design remains proposed. Approver selection and design-review approval are still pending; opening/updating this PR does not imply merge readiness. ## Revision and validation - Description synchronized with PR HEAD `c2590162fc7c8eb957a26dd42069532b14529e3e`. - Target branch: `milvus-io/milvus:master`; merge-base at this update: `19f5933370c44866e2b6ffbcd3b227e488d2dbf3`. - Local publication validation: full `make milvus` passed before the final REST-only base advance; `make build-go` and repository verifiers passed again afterward. Native build evidence was reused with unchanged native inputs. Proto generation checks and formatting checks passed without file drift. - Eleven related Go packages passed with `dynamic,test` and `-gcflags="all=-N -l"`. The affected HTTP package was rerun after the final base advance; other unchanged package results were reused. Two user-approved local DataCoord exclusions were retained: the macOS DISKANN size subcase and the symlinked internal-directory path case. Neither exclusion exempts CI. - Go SDK testcases compiled with `-mod=readonly -tags test,L0,L1,L2`. Live E2E was not rerun for this revision; compilation is not an E2E result. Complete live CDC/backend, restore-compatibility and malformed-manifest fault-injection coverage remains unverified. CMEK E2E is outside this PR. - This is not a strict 99%-coverage check-commit pass. Measured changed-function coverage gaps remain in `proxy.importTask.PreExecute` (98.8764%), `packed.readColumnGroupsFromManifest` (83.5616%) and `packed.GetDeltaLogsFromManifestWithExtfs` (92%). - GitHub Checks and Commit Statuses inspected on 2026-09-22 for this exact SHA: Summary, DCO and Ruff succeeded; build/UT and the main CI pipeline were running, and E2E/Go SDK were queued. Pending CI is not a passing result. - This update publishes review replies and edits the PR description only; no local tests, rebase or code push were performed in this metadata update. Design approval and merge gates remain separate from validation results. --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com>	2026-10-16 18:49:49 +08:00
Zhen Yeandsijie-ni-0214	3f5e2439df	feat: add streaming node query runtime and resource preparation (#53819) ... issue: #40451 - SN query execution: add two-phase search/query RPCs, Growing/Transform MVCC waits, DataVersion-based segment selection, monotonic runtime admission and renewable Up-view leases (default 60s). Reuse QueryNode execution and boost scoring through query-owned growing handles, including the master Arrow retrieve path. - Query resources: atomically capture growing snapshots and register live delivery, replay historical Deletes and fence commit/sealed notifications before Ready. Add minimal Coord resource/load-info RPCs; maintain one shared SN BM25 aggregate, retain growing statistics in memory and read sealed statistics from object storage when needed. - View lifecycle: persist Up topology before reporting, recover views in version order, drain canceled builds before recovery-resource teardown and preserve retryable lifecycle/resource errors. Keep growing and shared IDF versions monotonic across replicas and expose top-level queryView configuration. - WALSummary and message consumption: introduce a manifest-aligned chunk cache populated by reads and writes, coalesce cold loads and scope live notifications while preserving coverage barriers. Retry transient TransformLog reads with cancellable backoff without advancing delivery cursors on failure. Retain every committed Insert summary, resume old subscriptions at the retained lower bound and share immutable Body decoding through singleflight and bounded LRU caching. Review scope: the foundational change titled `enhance: split walsummary, transformlog and vchannel view modules from WAL recovery` belongs to another PR and is excluded from review. Interactions introduced by SN query integration remain in scope. Integration boundaries: new Proxy orchestration, complete Coord scheduling/history, remote QN TransformLog subscriptions, Requery execution and DDL MVCC visibility remain outside this extraction. The minimal load-info RPC reads current metadata; LoadInfoVersion is advisory. Reconciliation of schema, index and load-scope changes when reusing a runtime remains follow-up work. Ready fences a captured event prefix; execution still waits for the request's MVCC. Follow-ups include preparation failure recovery and dependency lifetime across superseded Preparing views, slow-consumer buffering/backpressure, NodeScheduler task priorities and reliable WALSummary GC across restart, remote consumers and owners. Lower-bound resume does not reconstruct skipped Deletes; existing SN retention pins remain active. --------- Signed-off-by: chyezh <chyezh@outlook.com> Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com> Co-authored-by: sijie-ni-0214 <sijie.ni@zilliz.com>	2026-10-16 09:43:50 +08:00
somfornot	865c85105a	enhance: use caller-owned async manifest IO in DataCoord (#54044) ... Startup manifest recovery currently waits for fixed batches, so one slow read prevents replenishing the other slots. This change uses the caller-owned async Open/Commit API from merged milvus-storage PR #698 and continuously replenishes a bounded recovery window. - Recovery owns one scan-scoped executor for native load work and callbacks. `manifestIndexReader` handles completion ordering, per-segment retries and draining; metadata installation stays in the recovery caller. - Commits use an independent long-lived component. `newMeta` resolves `dataCoord.manifestCommitConcurrency` once (default 16, clamped to [1, 256]); changes require a DataCoord restart. The obsolete `dataCoord.compaction.levelzero.manifestUpdatePoolSize` is ignored and is not a fallback. Acquisitions take only the request context; reader/executor helpers do not look up configuration. - Native-task workers grow on demand up to executor capacity. Idle workers remain available for reuse until context shutdown; there is no idle-time shrink. Dropped-segment and unused-index GC each reuse one read context per sweep and close it on every exit. Remove the redundant saved concurrency field and unused path-only async commit wrapper. - Batch commits submit open/mutation/commit chains directly to that executor, without an extra worker pool or per-segment waiting goroutines. The packed adapter returns the index-marker change from the already-loaded input manifest and staged mutations after successful commit. Single and batch publication no longer reopen the committed manifest or run a marker-read phase; failures cancel peers and drain accepted callbacks before unlocking. This uses the existing OVERWRITE contract and does not change milvus-storage or its pinned ABI. - Legacy L0 batches also submit directly, with a caller-driven completion queue preserving per-segment revision order. Copy/restore source prefetch and copied-index verification each share one batch-owned read executor. All three redundant pools around async manifest calls are removed; cancellation drains accepted callbacks before publication, cleanup or executor release. - The cgo bridge owns callback tokens and operation handles, drains accepted work on cancellation/shutdown, and preserves COMMITTED / NOT_COMMITTED / UNKNOWN outcomes without replaying consumed transactions. Segment serialization, CAS and catalog publication remain in place. - Pin milvus-storage to `c40babbb83ff34e837ccf61345e589f16a1b21a1` and migrate synchronous call sites to `loon_transaction_open`. Includes the `milvus::ErrorCode` qualification needed to build the existing GCP backend. No RPC or on-disk format changes. The native deadline bounds queued work; synchronous I/O already executing still depends on backend timeouts. No transport nonblocking or measured throughput improvement is claimed. Validation of the worker/GC/configuration changes in the Milvus development container: - All 14 packed async tests passed three repetitions under `-race`, including worker growth, capacity, reuse and shutdown draining. - Focused DataCoord executor, reader, GC context-lifecycle, L0 batch, single/batch commit and shared read-budget tests passed three repetitions under `-race`. - Related dropped-segment, unused-index, recovery and rejected-copy cleanup regressions passed. - Focused `golangci-lint 2.11.3` checks reported zero issues in `internal/datacoord/...`, `internal/storagev2/packed/...` and `pkg/util/paramtable/...`; the repository-wide `make static-check` was not run. - The generated YAML passed `cmd/tools/config.TestYamlFile`; the DataCoord parameter tests passed, covering the new key, ignored old key and capacity clamps. - Go tests used `-tags dynamic,test -gcflags='all=-N -l'`. Full-suite, cloud-backend, E2E and performance testing were not run. CI and follow-up validation: - [Jenkins #34045](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-build-ut-ciloop-pipeline/34045/pipeline-overview/) tested `1f52ea306c`: Build, Code-Check, UT-CPP-Cov (9,738 tests) and UT-Integration (439 tests) passed; UT-GO failed. - The DataCoord failure was a stale mock signature after adding the GC IO-context parameter. It was reproduced in the development container and corrected in `aa3e53461b`; 18 related dropped-segment GC tests then passed with `-race -count=3`. - The same CI run also reported two QueryNode fixture failures because MinIO bucket `a-bucket` did not exist. Those failures have not been reproduced locally or resolved by this test-only fix. Full CI on the corrected head remains pending. Earlier validation at `83eb32a02d` (based on `master` at `cab9d65256`): - Native build/install passed with the existing feature configuration. - DataCoord: 217 top-level tests, 804 including subtests. 18 top-level tests passed three repetitions under `-race` (270 including subtests), covering L0 submission chains, inline completion and rejection, UNKNOWN outcomes, partial-success caching, callback draining, copy/restore concurrency and ordering, cleanup exclusion and real single-worker native I/O. Includes real single/batch marker publication with assertions that no post-commit read occurs, catalog failure atomicity, and failure draining before segment unlock. GC regressions also assert reduced manifest-read counts. - Packed: 22 top-level tests, 45 including subtests; all 13 async tests passed three repetitions under `-race`. The marker tests compare projected results with native persisted manifests for partial/all/absent drops, drop-plus-add, stale build rejection, file-append invalidation, column-group additions, unchanged sections and version drift. Covers real local-storage FFI, queued cancellation/deadlines, admission rejection, shutdown draining, single-worker callback progress and native UNKNOWN commit outcomes. - `cmd/tools/config.TestYamlFile` passed against an unchanged copy of the committed YAML; a test-only `LOCAL_STORAGE_SIZE` override avoids probing the read-only container default path. - Go tests used `-tags dynamic,test -gcflags='all=-N -l'`. Full-suite, cloud-backend, E2E and performance testing were not run. issue: #53047 (follow-up; does not close the entire issue) Design: [DataCoord asynchronous manifest loading and commit](https://github.com/somfornot/milvus/blob/coord-manifest/async-io/docs/design-docs/design_docs/20260921-datacoord-async-manifest.md) Dependencies: [milvus-storage #698](https://github.com/milvus-io/milvus-storage/pull/698) and Milvus #53048 are merged. This is the sole layer of `master <- coord-manifest/async-io`. --------- Signed-off-by: somfornot <318146256+somfornot@users.noreply.github.com>	2026-10-14 11:19:45 +08:00
junjiejiangjjj	06161a3334	feat: Support pyudf (#52001) ... doc: docs/design-docs/design_docs/20260624-function-chain-api.md issue: https://github.com/milvus-io/milvus/issues/52319 https://github.com/milvus-io/milvus/issues/46565 Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com>	2026-10-12 15:25:45 +08:00
tinswzy	023e356a8b	enhance: bump woodpecker to v0.1.46 and expose client operational bounds (#54092) ... issue: #43638 Bump Woodpecker from v0.1.45 to v0.1.46 in both Go modules and all three service-mode e2e image tags, following #53468/#53470. Expose the ten new client settings through Milvus paramtable and the existing Woodpecker configuration builder. Milvus constructs the library configuration in code, so adding keys only to YAML would not make them tunable. All defaults match Woodpecker's effective defaults and all new parameters require restart. | Key under `woodpecker.client` | Default | Operational purpose | | --- | --- | --- | | `segmentAppend.sendTimeout` | 2s | Bound opening a replica append stream and receiving buffered responses; not the durability-ack or total append budget. | | `quorum.selectNodesTimeout` | 2s | Bound one selection RPC to one seed. | | `grpc.dialTimeout` | 1s | Configure gRPC's minimum connection attempt timeout for Woodpecker logstores. | | `grpc.connectBackoff.baseDelay`, `multiplier`, `jitter`, `maxDelay` | 100ms, 1.6, 0.2, 3s | Pace reconnect attempts independently of Milvus internal gRPC clients and RPC retry backoff. | | `segmentRead.activeTimeout`, `settledTimeout` | 3s, 20s | Per-replica quorum-read budgets; Active service-mode reads can retry with the longer budget. Embedded reads use the settled budget. Direct object-storage reads bypass these budgets. | | `skipRangeRefreshInterval` | 10s | Control background refresh traffic when stalled readers consult operator-declared skip ranges; reporting ticks also affect propagation. This is not a maximum cache age or an automatic loss declaration. | Non-positive/sub-unit bounds retain library defaults. Read bounds are applied together only when active <= settled; connection backoff is applied together only when its bounds, multiplier and jitter are valid. Malformed jitter and NaN/Inf cannot produce an invalid backoff. Durations use Milvus's existing unit-suffixed convention, with milliseconds for RPC bounds and whole seconds for skip-range refresh. CLI-only options such as `--node-admin-url` / per-node admin endpoint mappings stay in wp CLI configuration: Milvus's WAL client does not use that admin HTTP discovery path. No additional logstore server configuration keys were introduced in v0.1.46. Woodpecker's go.mod is unchanged, including its checksum. Validation: - Root and pkg `go mod verify` passed; both modules resolve v0.1.46. - Complete `pkg/util/paramtable` suite passed on an independent rerun. - Existing configuration-builder tests and new default/custom/invalid-bound tests passed, using `-tags dynamic,test -gcflags='all=-N -l' -count=1`. - The mqwrapper/wp package compiled (no test files). - Generated the Woodpecker YAML section with the repository config tool; preserved unrelated YAML examples/defaults. Net YAML change is 13 added lines. - Audited consumers in NewClient, the remote client/pool, quorum discovery, segment read passes, metadata skip-range cache and reader reporting. These establish where configuration is consumed; no claim of measured Milvus failover latency or end-to-end skip-range recovery is made. No Milvus fault-injection cluster was run. Integration validation limitation: the existing TestWAL run encountered local disk-watermark write backpressure at ~87% disk usage (above the soft threshold), then panicked in the shared test framework's `sortByMessageID.Less` when unsuccessful appends left nil messages. Full WAL integration coverage is therefore not green locally; CI with suitable dependencies/disk capacity is still needed. The unmodified upstream/master baseline with v0.1.45 reproduced the same disk-pressure errors and nil-message sorting panic in `TestWAL/LocalFsStorage`; this failure is not unique to the upgrade. --------- Signed-off-by: tinswzy <zhenyuan.wei@zilliz.com>	2026-10-12 12:17:42 +08:00
Jamesandxiaofanluan	f0f197ec04	enhance: add fastpb runtime switch and optimize decoding (#54017) ... issue: #50742 Add `common.enableFastPB` (default `true`) to select fastPB or official protobuf decoding on the supported RPC and direct search-result paths. The selected value is cached atomically so the RPC hot path avoids the config-manager read lock. Codec use before parameter initialization preserves the default enabled behavior without initializing configuration itself. The switch covers releaseCodec's RetrieveResults/InsertRequest/UpsertRequest dispatch and the direct SearchResultData decode sites in Proxy, QueryNode, LocalWorker, and late output-field materialization. The pure fastpb package remains configuration-independent. WAL encoding/repacking and codec buffer pooling are separate. Client Insert/Upsert strings retain proto3 UTF-8 validation. Enabled internal-result fast string paths assume trusted Milvus/segcore output and skip validation; disabling fastPB uses the official decoder and restores its UTF-8 validation. Packed scalar arrays (`LongArray`, `IntArray`, `BoolArray`) use protobuf-go's specialized decoder, avoiding the allocation and CPU regression from per-element growth. Raw packed fields still preallocate once. The shared wire primitive now uses `protowire.ConsumeVarint`, which has unrolled normal-value paths and rejects an overflowing varint after at most ten bytes. This covers long `HashKeys` and sealed-segment IDs as well as tags, length prefixes, and scalar values. Serialization and peer compatibility are unchanged. Validation for the latest update: - `go test ./util/fastpb ./util/resource -count=1` passed in the nested `pkg` module on master, 3.0, and 2.6. - 48 differential boundary cases cover max uint64, nonminimal ten-byte zero, truncation, overflow, and a 1 MiB continuation sequence through scalar, packed, length-prefix, and unknown-field paths. - Added reproducible `BenchmarkInsert_HashKeys` and `BenchmarkRetrieve_SealedSegmentIDs` cases. - Paired local master benchmarks (Go 1.26.6, Apple M4 Pro, `GOMAXPROCS=1`, 11 alternating rounds) reduced 1000 five-byte HashKeys decoding from 5.377 to 4.561 us and 1000 nine-byte sealed IDs from 8.465 to 7.157 us. Allocations were unchanged, and both cases were near the official decoder. These are synthetic decode benchmarks, not end-to-end QPS results. - For a malformed NumRows value, increasing a continuation sequence from ten bytes to 1 MiB no longer increases rejection time. Packed fields still scan their payload to count elements before decoding. Native internal integration paths and end-to-end QPS/P99 were not rerun locally. CI results must be assessed on the latest PR head. Runtime configuration refresh now registers an exact-key handler through the config dispatcher, which matches normalized aliases including etcd's separator-free key. CREATE, UPDATE, and DELETE events re-read the uncached effective value, preserving runtime overrides and restoring the default after deletion or Reset. The refresh read and atomic publication are serialized on the update path; RPC decoding still only loads the atomic snapshot. Callback regression validation: - The normalized CREATE test failed before the fix: the effective ParamItem was false while the atomic switch remained true. - The new source-event test covers normalized CREATE/UPDATE/DELETE, a prewarmed typed cache, runtime overrides, Reset to the default, and runtime key aliases. It checks actual gRPC RetrieveResults and direct SearchResultData decoder selection without mocking. - `go test ./util/fastpb ./util/resource -count=1` passed on master, 3.0, and 2.6 after the callback fix. - `go test -race ./util/resource -run '^TestFastPBConfigSourceEvents$' -count=3` passed on all three branches. - Focused `go test ./util/paramtable -run FastPB -count=1` passed on master and 3.0. The pre-existing 2.6 paramtable test-package compilation issue involving `log.CaptureGlobalLogs` was not changed; the production paramtable package compiled through the resource tests. ```yaml common: enableFastPB: true ``` Related decoder backports: #54002 (2.6), #54003 (3.0). --------- Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com>	2026-10-10 23:23:43 +08:00
jiaqizho	cab9d65256	enhance: configure Talon read routing for remote filesystems (#53817) ... issue: #53875 Add common.storage.talon settings and propagate them through the existing StorageConfig and filesystem properties pipeline. Support routing all reads or only small reads through Talon, with a default small-read threshold of 512 KiB. Talon remains disabled by default; External Tables require enableForExternalTable to opt in. Also bump storage to e5ac4d8. --------- Signed-off-by: jiaqizho <jiaqi.zhou@zilliz.com>	2026-10-08 17:59:38 +08:00
aoiasdandClaude Opus 4.6	79a0f6b62b	feat: support RLS array tags and NOT expressions (#53944) ... relate: #50263 design doc: docs/design-docs/design_docs/20250610-rls_design.md design doc PR: #53173 ## Summary - Support homogeneous one-dimensional array tags, promoting mixed integer/double arrays to double, and unary NOT over the existing simple-predicate subset. - Keep missing/incompatible tags and null/type-invalid data fail-closed; bound tag metadata and materialized templates and account for aggregate RLS HybridSearch plan size. - Capture a locally coherent policy/tag snapshot per request, shared by upsert USING/CHECK, and retry acquisition if policies change during tag loading. - Reuse RLS expression-length and field-dependency helpers, move test-only helpers out of production, and fail fast on duplicate policy names during recovery. ## Verification - Static call-path and invalidation-interleaving review; gofumpt and git diff --check passed. - Added numeric promotion, type/null integrity, resource-bound, recovery, and policy-change-during-tag-load regression tests. - Local build/tests have not been run for this revision; PR CI will provide execution results. ## Scope Authorization remains request-snapshot-based, not mutation-CAS or distributed-linearizable. Previously deferred dynamic enablement, process-wide cache budgeting, and pagination are unchanged. --------- Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-10-08 11:23:37 +08:00
wei liu	62569cb333	fix: Use multipart copy above a configurable 1 GiB threshold (#53787) ... ## Problem issue: #53786 An approximately 2 GB cross-bucket OSS snapshot copy currently uses one CopyObject request and can fail with `net/http: timeout awaiting response headers`. ## Change Use the existing server-side multipart copy path above 1 GiB by default. Add the refreshable `minio.multipartCopyThreshold` setting (bytes, version 3.0.3), read for each subsequent copy and exported in the default YAML. Positive int64 values are accepted; invalid values fall back to 1073741824. The effective single-copy limit remains capped at 5 GiB. Explicit and endpoint-inferred GCP configurations retain the single-copy path through shared provider resolution without mutating caller configuration. The separate Azure implementation is retained. ## Validation On the rebased master commit, the targeted storage and paramtable tests pass with `-tags dynamic,test -gcflags='all=-N -l' -count=1`. The real SDK HTTP test verifies that a 2,000,000,000-byte object emits four server-side part-copy requests and completes, and that a rejected second part returns the storage permission error without completing the upload. Routing tests cover threshold boundaries, runtime changes, the protocol cap, explicit and endpoint-inferred GCP configurations (including IAM), provider precedence, and caller configuration immutability. Configuration tests cover valid and invalid values; the generated YAML consistency test also passes. ```sh source ~/.profile && source scripts/setenv.sh go test -tags dynamic,test -gcflags='all=-N -l' -count=1 -timeout=5m \ -ldflags="-r ${RPATH}" \ github.com/milvus-io/milvus/internal/storage \ github.com/milvus-io/milvus/pkg/v3/util/paramtable \ -run '^(TestMinioObjectStorageCopyObjectCrossBucket.*|TestMinioConfig_MultipartCopyThreshold)$' -v ``` `git diff --check` passes. No live OSS timeout reproduction was run. ## Known limitations - minio-go v7.0.73 still uses a background context for its internal source HEAD and does not abort an incomplete multipart upload on failure. Those existing SDK limitations remain out of scope. ## CI follow-up The multipart HTTP fixture now generates ETags from its internal part counter instead of reflecting a query parameter, resolving gosec G705. The unrelated hookutil test-isolation change has been removed from the final diff. Its pre-existing race is outside this PR. Provider-resolution and inferred-client tests pass in `pkg/objectstorage`. No live GCS integration test was run. --------- Signed-off-by: Wei Liu <wei.liu@zilliz.com>	2026-09-29 15:29:35 +08:00
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 <zhenyuan.wei@zilliz.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-29 09:21:35 +08:00
sijie-ni-0214	24db48a7e6	enhance: add QueryCoord autoscale load resource precheck (#51218) ... ## Summary issue: #51217 Load requests introduce resource demand before QueryNode waterlines reflect the new data. This PR adds optional QueryCoord resource prechecks before LoadCollection/LoadPartitions configuration broadcasts and publishes cumulative demand counters for external autoscaling decisions. Design: [Autoscale-Aware Load Resource Admission in QueryCoord](https://github.com/sijie-ni-0214/milvus/blob/querycoord-autoscale-kernel/docs/design-docs/design_docs/20260721-querycoord-autoscale-load-resource-precheck.md). ## Admission behavior - Estimate incremental final sealed-segment memory and local-disk demand per resource group from the requested and current load configurations. - With tiered eviction disabled, compare demand with memory availability and each Worker's configured `LOCAL_STORAGE_SIZE` quota minus cached local-directory usage after admission thresholds. Optionally admit shortages within configured global autoscale upper bounds. - With tiered eviction enabled, retain demand estimation and skip QN capacity sampling, node availability checks, and global autoscale-limit checks. Existing evictable caches make physical usage unsuitable for this admission decision; Worker QN retains its loading-stage guard. - Fail open on metadata, estimation, and capacity-metrics failures. Capacity-metrics failures preserve successfully estimated demand; metadata or estimation failures leave no demand to publish. - Return a resource-insufficient status with `suggested_expand_percent` when a complete capacity evaluation rejects the request. Proxy preserves the status and ExtraInfo. - Admission remains best-effort, without reservations or accounting for concurrent accepted requests. QueryCoord does not subtract Worker in-flight load reservations; the Worker guard remains authoritative. The existing replica node-count prerequisite remains; node provisioning and scale-from-zero are outside this PR. ## Load demand counters Expose cumulative counters labeled by resource group: - `milvus_querycoord_load_demand_memory_bytes` - `milvus_querycoord_load_demand_disk_bytes` Counters increase only when demand estimation succeeds, the request proceeds, and its configuration broadcast succeeds. This includes tiered loads and loads admitted through capacity-metrics fail-open. Disabled prechecks, unchanged configurations, zero incremental demand, estimation failures, explicit rejections, and failed broadcasts do not increase counters. Load completion and release do not decrement them. Series start at zero for existing resource groups and are removed on RG deletion. The dashboard displays cumulative values in MiB; the control plane computes interval increments, handles counter resets, and combines demand with QN waterlines and provisioning state. These counters are observations, not durable reservations. Internal handoff and balancing tasks do not contribute to them. ## Resource estimation and sampling - Share segment/index estimation in `internal/util/segcore/loadresource`, retaining separate final-resource and loading-peak APIs. Worker QN uses its CGo pool; QueryCoord estimates sequentially within each request. - Fetch recovery metadata, full segment metadata, and batched index metadata. Support Storage V3 resource summaries and dropped-field metadata. Filter indexes by loaded fields, retaining multiple JSON-path indexes on one field. - Apply the load-request collection mmap inheritance logic to cloned expected/current schemas while preserving field overrides. The final estimator applies cache ratios only with tiered eviction enabled. Delegator-side Bloom/BM25 resources are outside the Worker estimate. - Deduplicate QN resource sampling by node ID, use at most 16 concurrent requests and one attempt per node, and bound the entire sampling round to ten seconds. Reuse the snapshot for RG and global calculations. Exclude embedded SQNs from global capacity and sampling; empty eligible-node sets are unevaluable and fail open. - Worker QNs report `LOCAL_STORAGE_SIZE` and cached local-directory usage through `system_info`. QueryCoord uses these byte values for disk availability, RG suggestions, and the global disk-capacity sum; filesystem-wide disk metrics remain for monitoring. Shared filesystems require per-QN quotas consistent with the shared capacity. ## Configuration - `queryCoord.autoscale.precheckEnabled`: enable demand estimation and precheck; default `false`. - `queryCoord.autoscale.enabled`: allow admission within global autoscale bounds when current capacity is insufficient; default `false`, effective only with precheck enabled and tiered eviction disabled. - `queryCoord.autoscale.maxMemoryLimit`: maximum global Worker QN physical memory capacity after scaling, in GiB; default `0` means no additional memory autoscale capacity. - `queryCoord.autoscale.maxDiskLimit`: maximum sum of Worker QN `LOCAL_STORAGE_SIZE` quotas after scaling, in GiB; default `0` means no additional disk autoscale capacity. Both limits use `1 GiB = 1024^3 bytes`. For non-tiered deployments, configure QueryCoord's memory high-water level no higher than the Worker admission threshold. This relationship is a rollout prerequisite and is not enforced by this PR. ## Validation Local autoscale estimator and targeted QueryCoord tests passed, including multiple-index filtering, collection mmap overrides, the ten-second sampling timeout, empty-node fail-open, embedded-SQN exclusion, tiered demand preservation, capacity-metrics failure behavior, and local-storage quota accounting for QueryNodes sharing a filesystem. Both load callbacks are tested for precheck rejection, broadcast failure, successful counter updates, tiered loads, and capacity-metrics failures followed by successful or failed broadcasts. Tests assert counters remain unchanged during Broadcast and increment only after it succeeds. These are local unit/callback checks, not end-to-end external autoscaler validation. Local `make static-check` passed across core, pkg, client, and go_client with zero issues, and segcore error-boundary checks passed. Targeted QueryCoord and QueryNode tests, YAML consistency, and `make build-go` passed. External autoscaler behavior remains subject to CI and rollout validation. --------- Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com>	2026-09-29 01:47:36 +08:00
ChenLiqingandCLiqing	d2c77fd9e2	fix: complete strict groups with per-group filtered search (#53471) ... ## Summary Fixes #53277 Complete strict groups with ordinary per-group filtered Search. Only single-query strict grouping with group_size > 1 is eligible; preserve visibility, null semantics and typed backend errors. ## Configuration / migration - `queryNode.groupBy.strictGroupStrategy`: `per_group` or `original`. - `queryNode.groupBy.strictGroupPhase1CandidateWeight`: nonnegative integer; default 0 disables truncation. Effective phase-one consumer Next limit is `topk * group_size * weight`, saturating at INT64_MAX on overflow. It never limits completion of already locked groups. Truncation may reduce recall. - Weight 50 with gs=3 gives limits 1500 at topK=10 and 7500 at topK=50. - `queryNode.groupBy.strictGroupSkipRefine`: default false; `queryNode.groupBy.strictGroupDebug`: default false. ## Local validation - Config YAML consistency, optimizer/config snapshots, and ParamTable refresh/reset tests passed; core, all_tests and the dynamic Go executable built successfully. - 60 targeted C++ regressions passed, including HNSW ef handling, weighted cutoff/completion, overflow, unsupported iterator errors, nullable/growing and legacy group-by. - Isolated Cohere 20K HNSW/IVF_FLAT: 24 searches and 4 single-field aggregation requests passed; original has no per-group batches, per_group does. Includes ef=2 and ef="2", topK=2, gs=5. - 6 deterministic IVF_FLAT E2E requests: topK=3, gs=5, weight=1 stops discovery at 15 and still completes the two locked groups to 5 rows each. Weight 0/2 yields all three groups; both strategies verified. - A real IVF_PQ group-by request preserves the expected Unsupported / "doesn't support" compatibility. --------- Signed-off-by: CLiqing <23721160+CLiqing@users.noreply.github.com> Co-authored-by: CLiqing <23721160+CLiqing@users.noreply.github.com>	2026-09-28 21:41:35 +08:00
zhagnluandluzhang	16ed2f4076	enhance: support expression cache for brute-force search (#52992) ... issue: #53741 https://zilliverse.feishu.cn/docx/QokhdG445o3JUmxjG41cBJ1rn6d Signed-off-by: luzhang <luzhang@zilliz.com> Co-authored-by: luzhang <luzhang@zilliz.com>	2026-09-28 10:41:35 +08:00
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 <chyezh@outlook.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-09-26 01:09:36 +08:00
aoiasdandClaude Opus 4.6	15fdc95998	feat: [RLS3] evaluate row-level security policies (#52074) ... relate: #50263 design doc: docs/design-docs/design_docs/20250610-rls_design.md design doc PR: #53173 ## Summary Adds policy combination, template compilation, and a three-valued local evaluator for write checks. Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-09-25 00:50:23 +08:00
zhagnluandluzhang	48faae3b3a	enhance: parallelize json stats build (#52120) ... issue: #53765 JSON stats build benchmark — 500K rows per segment Concurrent segments || Non-parallel || 8 workers || Speedup 1 || 6.810 s || 1.641 s || 4.15× faster 4 || 7.140 s || 4.806 s || 1.49× faster 8 || 10.878 s || 9.658 s || 1.13× faster Setup: Release build, 12 CPUs, local filesystem, one warm-up followed by five measured runs; median values are reported. Data generation is excluded from the timing. Note: “Non-parallel” means each segment is built serially internally. Multiple segment builds are still launched concurrently in 8-segment cases. Signed-off-by: luzhang <luzhang@zilliz.com> Co-authored-by: luzhang <luzhang@zilliz.com>	2026-09-24 19:07:34 +08:00
sijie-ni-0214	1ae8642b8b	enhance: defer Storage V3 column group materialization (#53375) ... ## Summary issue: #53374 Fixes #53374 Add opt-in deferred materialization for Storage V3 column-group tasks. With effective data warmup disabled, eligible internal and externally mapped fields can retain a lightweight `ChunkedColumnGroup` and create the projected reader, translator, and cache slot on first access. - Add the hot-reloadable `queryNode.segcore.tieredStorage.lazyColumnGroupEnabled` setting, disabled by default. - Keep `ProxyChunkColumn` as the column interface. Serialize first initialization per task, publish the completed slot atomically, retain the factory on failure, and release its captures after success. - Share column-size estimates across tasks for the same physical column group within a load batch and cache the task's total estimated size. - Preserve batched asynchronous reader preparation for regular tasks. Lazy factories capture the async-load mode and defer reader opening until materialization. - Preserve regular PK/system-field loading, warmup decisions, interim-index preparation, and mmap average-size behavior. Nullable vectors use the common eligibility checks. - Skip output-size estimation for zero-result retrieves so unmatched segments do not materialize lazy output column groups. - Cover deferred reads, concurrent initialization, failure recovery, reopen, external column mappings, mmap, zero-result retrieves, and configuration updates. ## Behavior boundaries - A materialization unit is a load task, not necessarily an entire physical column group. Tasks retain their load-generation reader and projection. - Warmup and interim-index preparation may still materialize a task during loading. A task that needs a shared Parquet file can read its footer even if sibling tasks remain unmaterialized; this is not a universal zero-footer guarantee. - Metadata/layout access can initiate materialization without a request context. Existing data-access cancellation behavior is retained; this does not promise zero metadata I/O for every canceled request. - Segment `MemSize` accounting is not unified with cache residency by this change. No quantified memory or load-latency improvement is claimed. ## Verification - Rebased onto upstream master `17d017a56b`. Before the final code-review cleanups, the rebased snapshot passed the C++ unit-test build and Go executable build. The local macOS build used a deployment-target override of 11.0. - On that rebased pre-cleanup snapshot, the targeted C++ selection ran 153 cases: 152 passed and one skipped because `/dev/full` is unavailable on macOS. The 40-case Lazy/group/manifest preparation selection passed all 10 shuffled rounds. - The rebased pre-cleanup snapshot also passed the complete relevant Go `initcore` and `paramtable` packages, including configuration readback and hot-update coverage. - Final review cleanups adjusted function placement and naming, restored declaration annotations and the empty-task early return, retained the original async-branch structure, and replaced the task-index vector with ordered reader assignment. The six reported clang-format 15 diffs were applied, and the zero-result Retrieve guard and regression case were added. These final changes passed `git diff --check`; compilation and runtime tests were not rerun after them, per request. - Earlier, pre-rebase local streaming standalone E2E covered internal and mmap queries, external Parquet column mappings, both take and CG output paths, concurrent cold reads, release/load, delete visibility, external refresh, and runtime flag toggling. Values were checked, not only request success. - That earlier MinIO request tracing confirmed that eligible internal CG files were untouched at load and their footers were read on first query; disabling Lazy moved those reads back to load. A missing cold CG file allowed load, failed on first access, and a fresh request succeeded after restoring the file. ## Outstanding checks - Full `make verifiers`, compilation, and runtime tests have not been rerun on the final commit. - Nullable-vector Lazy-specific E2E has not been rerun after removing the exclusion; the prior external E2E still used the regular path for its nullable vector. - The broader earlier C++ regression run had 192 passes, 9 failures, and 1 skip out of 202 cases. The failures reproduced with Lazy disabled and were investigated as macOS cross-dylib `ColumnarArrayChunk`/`ColumnVector` vtable issues; a clean-base build comparison was not run. They are not claimed as passing here. --------- Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com> Signed-off-by: sijie-ni-0214 <sijie.ni@zilliz.com>	2026-09-24 08:25:33 +08:00
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 <105927039+liliu-z@users.noreply.github.com> Signed-off-by: Li Liu <li.liu@zilliz.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>	2026-09-23 14:23:33 +08:00
3 people	78367081ef	enhance: support multiple gRPC compression algorithms and configurable zstd codec (#53548) ... ## What this PR does Extends Milvus's internal gRPC compression from zstd-only to three algorithms, makes the zstd codec configurable, and reworks both codec paths around bounded pools. - Registers `snappy` and `s2` alongside `zstd`. No dependency change: both are subpackages of `klauspost/compress`, which is already required. - Makes the zstd codec configurable through `grpc.client.compressionAlgorithm`, `compressionLevel`, `compressionCRC` and `compressionConcurrency`. - **Changes the zstd decode path from `Decoder.DecodeAll` to a pooled streaming decoder.** `DecodeAll` sizes its destination from the frame header and materialises the whole decoded message *before* gRPC's size check can fire, which is the shape of CVE-2024-36129. Streaming decodes under the `io.LimitReader` gRPC wraps a decompressor in. - **Caps the match window a peer may declare at 8MB.** The decoder's history buffer is sized from that window and allocated before any output exists, so the `io.LimitReader` does not reach it — a few hundred bytes of hostile frame can otherwise ask for the library's own 512MB ceiling. This is reachable without credentials: the compressor registry is process-global, so the Proxy's external port accepts these encodings, and gRPC decompresses in `recvAndDecompress` before dispatching through the interceptor chain. 8MB covers every encoder that can reach a Milvus node. - **Every pool is a bounded channel rather than a `sync.Pool`**, sized `compressionConcurrency × GOMAXPROCS`, because a `sync.Pool` parks whatever a burst hands it until the next GC and gRPC caps concurrent streams at `MaxUint32` by default. The zstd encoder state pool is additionally held to a 256MB budget, clamped with a warning. ## Config | Param | Values | Default | |---|---|---| | `grpc.client.compressionEnabled` | bool | `false` | | `grpc.client.compressionAlgorithm` | `zstd` / `snappy` / `s2` | `zstd` | | `grpc.client.compressionLevel` | `fastest` / `default` / `better` / `best` | `default` | | `grpc.client.compressionCRC` | bool | `false` | | `grpc.client.compressionConcurrency` | 1–8, a multiplier on `GOMAXPROCS` | `2` | All four are read once at dial time and are `refreshable:"false"`. `compressionConcurrency` sizes the zstd **encoder** state pool and the per-codec free lists; decoding does not draw on it. `compressionCRC` defaults to `false`: the checksum costs ~9% of encode time on a 4KB message and ~27% at 64KB and above, while TCP already checksums the link and zstd's entropy decoding rejects a damaged frame rather than returning garbage. Frames a peer wrote *with* a checksum are still verified. **The streaming clients are pinned to zstd** and ignore `compressionAlgorithm`. They dial through `GetDialOptionsFromConfig` rather than `grpcclient.ClientBase`, so they cannot detect a peer that fails to decode snappy/s2 and retry on a downgraded connection. ## Compatibility Rolling the code out is safe in any order: the defaults are `compressionEnabled: false` and `zstd`, and zstd stays registered in both directions. The two wire-visible changes — a 1MB encoder window and CRC off — are optional fields in the zstd frame header and decode on any version. Changing `compressionAlgorithm` to snappy/s2 **does** have an ordering constraint, because the receiver of a request needs the decompressor and nothing falls back to zstd automatically: - Every node that receives requests from a switched node must be upgraded first. - Deployments that use Hummer additionally need zilliztech/hummer#210 before any Milvus instance talking to it switches. - Responses follow the request's algorithm, so they are not affected. - Revert the setting to zstd before rolling a binary back. ## Measurements Decode, through gRPC's own consumption path (`mem.ReadAll(io.LimitReader(...))`), streaming vs `DecodeAll`, benchstat n=6: | message | sec/op | B/op | |---|---|---| | 4KB | -8.2% | -96.8% | | 64KB | -8.5% | -99.7% | | 1MB | -8.3% | -99.9% | | 8MB | +8.6% | -99.9% | Ratio and throughput per codec on protobuf-ish metadata at 64KB: zstd 6.79x at 663 MB/s encode, snappy 4.38x at 1564 MB/s, s2 4.38x at 1467 MB/s. s2 only pulls ahead of snappy at 1MB blocks (4.77x), which is why it keeps s2's default block size rather than a smaller one. issue: #53546 Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-20 17:14:43 +08:00
aoiasdandClaude Opus 4.6	580e797e1e	feat: [RLS2] synchronize row-level security metadata (#52073) ... relate: #50263 design doc: docs/design-docs/design_docs/20250610-rls_design.md design doc PR: #53173 ## Summary Adds bulk RLS metadata loading, versioned Proxy snapshots, notifications, TTL reconciliation, and fail-closed refresh. Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-09-20 10:34:51 +08:00
yihao.daiandClaude Opus 5	0e892e719f	fix: fence stale pulsar writer on pchannel takeover (#53449) ... issue: #53368 ### Breaking change The default Pulsar WAL producer access mode changes from shared to exclusive. Existing Pulsar deployments get the new behavior after upgrading, even without adding the new configuration. Suggested release note: > **Breaking change — Pulsar WAL producer access:** StreamingNode WAL producers now default to exclusive access instead of shared access. A new writer must wait for the previous producer registration to be released, which can delay channel takeover and affect rolling upgrades involving older StreamingNodes. Exclusive access requires Pulsar broker 2.8.0 or later for enforcement. Operators can explicitly set `pulsar.producerAccessMode: shared` to retain the previous access behavior, but this disables the new writer-exclusion protection against stale writes during takeover. A known pulsar-client-go close/reconnect race can leave an orphaned exclusive producer that keeps a PChannel unwritable until the old connection or StreamingNode process is closed; it is tracked in #53492. ### What problem does this PR solve? During a pchannel takeover, a streaming node that lost its etcd session (for example under CPU starvation) could stay alive for a while with its pulsar producer still connected. The coord skipped the `Remove` of its wal because the node was gone from the session view and assigned the pchannel to another node right away. The old node then appended messages whose time ticks were allocated before the new owner's first time tick, but persisted after it. Consumers dropped them as time tick violations, and a transaction could be delivered without some of its body messages, which is the 708-row gap in #53368. Nothing stopped the second writer: the pulsar producer used the shared access mode, the coord treated a vanished node as removed, and the old process only exits when its own keepalive goroutine runs. ### What is changed and how does it work? **Pulsar WAL** (`pkg/streaming/walimpls/impls/pulsar`) - New config `pulsar.producerAccessMode` (`shared` or `exclusive`, default `exclusive`; an unknown value falls back to `exclusive`). In exclusive mode the broker rejects a second producer on a wal topic while the previous one is connected. `WaitForExclusive` is not used: the go client waits for the broker grant without a timeout, which would make the wal open uninterruptible. - New `walProducer` owns the producer of a read-write wal together with a pulsar client dedicated to it, and replaces the producer future and notifier in `walImpl`. Closing the wal closes the producer and then that client. pulsar-client-go v0.19.0 (also v0.21.0 and master) can register a producer again on a new connection when a reconnection races with `Producer.Close`, and never closes it there; in exclusive mode that leftover producer would block the topic. Closing the dedicated client closes its connections, so the broker drops it. - The producer creation retries with backoff until it succeeds, the context of `Open` is done, or the new `pulsar.producerCreateTimeout` (default `1m`, `0` disables it) elapses; `Open` then returns the error. Before, the retry loop inside `Open` could only end by success. - `walProducer.Send` marks the error `walimpls.ErrFenced` when the pulsar client has closed the producer by itself (fenced, closed, topic terminated or not found). The wal adaptor then fences the wal and stops retrying; the existing client path reports the assignment error and the coord reassigns the channel (traced in code, not exercised). **Streaming node** (`internal/streamingnode/server/walmanager`) - `managerImpl.Close` cancels in-progress wal opens before it waits for in-flight calls. Only opens are canceled; the wal lifetime goroutines keep serving `Remove` during shutdown. - `walLifetime.Remove` returns an error only when its own caller gives up, so a remove after an open that failed with a context error succeeds. This is also what lets the balancer's retry round converge after an assign timed out. **Streaming coord** (`internal/streamingcoord/server/balancer`, `internal/streamingnode/client/manager`) - `ManagerClient.Remove` returns `types.ErrNotAlive` when the streaming node is not in the session view, instead of reporting the wal as removed. - The balancer treats `ErrNotAlive` as removed and, for a read-write pchannel, waits `streaming.walBalancer.nodeLostGracePeriod` (default `5s`, `0` disables it) before assigning it, giving the old process time to exit. This lowers the chance of two writers on one wal; it does not rule it out. ### Verification - `TestPulsarExclusiveProducer`, against a real Pulsar 2.8.2 broker: a second read-write open on the same topic keeps failing while the first producer is connected and succeeds after the first wal is closed. The same test fails with `pulsar.producerAccessMode=shared`. - `walProducer` with a fake pulsar client: retries until created with the configured access mode, stops when the context is done and at `pulsar.producerCreateTimeout` and closes its client in both cases, closes the producer and then the client, and marks an unusable producer `ErrFenced`; also under `-race`. - `TestManagerCloseCancelsOpen`, `TestWALLifetimeCloseCancelsOpen` and `TestWALLifetimeRemoveAfterOpenCanceled` with a mock opener, also under `-race`. - `TestBalancer_WaitGracePeriodWhenNodeNotAlive`, `TestBalancer_NoGracePeriodWhenRemoveSucceeds`, `TestBalancer_NoGracePeriodWhenDisabled`, and the `Remove` contract cases in `TestManager`. - Package tests pass for `pkg/util/paramtable`, `pkg/streaming/walimpls/impls/pulsar`, `internal/streamingnode/client/manager`, `internal/streamingnode/server/walmanager`, `internal/streamingnode/server/wal/adaptor`, `internal/streamingnode/server/service`, `internal/streamingcoord/server/balancer` and `cmd/tools/config`. - `make static-check` passed before the review follow-up; the follow-up only touches `pkg`, where golangci-lint with the repo config reports 0 issues. ### Not verified and trade-offs - The original incident was not reproduced end to end, for example by freezing the owning streaming node with `SIGSTOP` in a two-node cluster. - A real broker returning `ProducerFenced` to a reconnecting producer, the broker dropping the connection of a frozen producer, and the reconnection race with `Producer.Close` were not exercised; the fenced mapping and the client close are covered with fakes only. - The dedicated client narrows the race above but does not remove it: a reconnection that has finished its lookup can still open a new connection right after the client is closed, because pulsar-client-go's connection pool does not reject new connections after `Close`. A complete fix needs pulsar-client-go to close the producer on the connection it registered after `Close`. - Each read-write pchannel on a streaming node uses one more pulsar client and broker connection. - With `pulsar.enableClientMetrics` on (the default), `pulsar_client_sending_buffers_count` no longer reflects producer send buffers. pulsar-client-go v0.19.0 registers that gauge without the `AlreadyRegisteredError` fallback its other metrics use, and all clients share the same labels. Only the first client's gauge is exported, which is the shared reader client that never sends. The SDK fix is tracked in #53492. - A new owner waiting for exclusive access blocks the whole balance round, now for up to `pulsar.producerCreateTimeout` (1m by default); the assign then fails and the balancer retries the pchannel in a later round. A topic whose backlog quota is exceeded now fails the wal open every `pulsar.producerCreateTimeout` instead of blocking it. - Availability cost of exclusive mode: the broker returns `ProducerFenced` to any producer while an exclusive producer is still registered on the topic (apache/pulsar `AbstractTopic.java`). A rightful owner that reconnects before the broker drops its old connection is fenced, its pchannel is reassigned once, and the reopen waits until the broker drops that stale connection. `pulsar.producerAccessMode: shared` restores the previous behavior. - While a producer creation is rejected, the pulsar client itself logs errors on every attempt, about two lines per second per waiting pchannel. - Shutdown cancellation of a wal open is not instant: a single producer creation attempt or a TSO sync does not observe the context. - The grace wait adds `nodeLostGracePeriod` of write unavailability to the read-write pchannels of every non-graceful streaming node loss, and to some graceful stops, including on backends that fence writers themselves such as Woodpecker. It is paid once per balance round: if the round fails after the wait and retries, it waits again. Balancer requests queued during the wait are delayed by up to one grace. The wait does not help when a live node is missing from a lagging etcd read at coord cold start, and 5s is shorter than the worst-case self-exit of a streaming node (about 15s). - One local run of the pulsar package tests panicked with a nil pointer dereference while the machine was under load; the stack was not captured, and it did not reproduce in 16 later runs, including 3 under `-race`. ### Compatibility (reasoned from code, not tested) - A new streaming node (exclusive) opening a topic whose producer is still held by an old streaming node (shared): the broker rejects the new producer, and the new node retries until the old producer disconnects or `pulsar.producerCreateTimeout` elapses. - An old streaming node (shared) opening a topic whose exclusive producer is still registered: the broker rejects it, and the old node's producer creation retry cannot be interrupted, so its wal open, the assign RPC and its shutdown wait until the exclusive producer is gone. - Upgrading a cluster that has not enabled streaming yet: the legacy rootcoord msgstream producers on the same dml topics are released asynchronously after streaming is enabled, and the first read-write opens wait for that release. - Pulsar brokers older than 2.8.0 do not enforce exclusive access; the producer is expected to be created without fencing, which is not verified. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: bigsheeper <yihao.dai@zilliz.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 10:27:34 +08:00
Jamesandxiaofanluan	37f3a829e7	fix: make local storage keys complete paths at every layer (#53145) ... issue: #53051, #53052, #53152 ## Summary Align local storage around one path contract: a key is a complete absolute filesystem path, and the local loon/Arrow filesystem is rooted at `/`. Previously, some writers used `minio.rootPath` while others supplied complete local paths to an already-prefixed filesystem, placing files in different physical locations from those expected by readers and GC. Remote keys remain complete bucket-relative object keys; their filesystem-root convention is unchanged. ## Changes - Unify Go/C++ local filesystem roots and StorageV3 manifest-base construction across writers. Preserve the existing base/version pair when continuing a growing-source manifest. - Fix local JSON/TEXT stats reads, rejected-stats cleanup, and explore temporary paths. Exercise resolved V3 stats paths through the actual `LocalChunkManager.MultiRead` interface. - Resolve legacy local manifest bases in loaded metadata and locally referenced snapshots, without eager catalog or manifest-file writes. Preserve manifest CAS compatibility and pathless V3 summaries; copy/restore writes use the target storage root. - Require an absolute, nonempty `localStorage.path` at initialization, including on remote deployments where it is a cache directory. Freeze forbidden parameters against configuration-source refresh while retaining explicit runtime/test overrides, and derive mmap, pprof, and disk-capacity defaults from the same canonical startup value. - Make segment deletion prefixes segment-boundary aware, retain root/identity and snapshot checks, and add BM25 orphan scanning. These shared GC changes also apply to remote storage; this PR does not claim every remote code path is untouched. - Remove the local test-only root override and unused local-only property initialization chain; use the production storage-config initialization path. ## Automatic local upgrade Let `R = localStorage.path` and `M = minio.rootPath`. Before standalone components start, local deployments check the known legacy locations: | Existing data | Upgrade handling | | --- | --- | | Canonical files under `R/<layout>` | Leave in place. | | Double-root files under `R/<R-without-leading-slash>/<layout>` | Move into `R/<layout>` for `insert_log`, `text_log`, `json_stats`, `index_files`, and `index_v1`. | | Recognized late-2.6 unified packed indexes under the process CWD | Move into the corresponding directory under `R`, if visible to the new process. | | Legacy manifest base `M/insert_log/<c>/<p>/<s>`, with files under `R/M/insert_log/<c>/<p>/<s>` | **Do not move the files.** Resolve the loaded manifest base to that existing absolute location. New segment layouts use the canonical root; updates to an existing legacy manifest can retain its base. | Migration prefers directory/subtree rename and recursively merges overlapping directories. Cross-filesystem moves fall back to copy-then-remove. Existing same-name files cause a conflict rather than content comparison or overwrite, except a published copy recognized as this migration's own interrupted work. The copy path synchronizes the file and target directory chain before deleting the source. Remaining sources are rediscovered on retry; empty legacy directories are pruned. Once migrated, restarts probe the known old locations without rescanning canonical data. The initial pass walks legacy file metadata but does not hash file contents. There is no configured migration timeout. SIGTERM/SIGINT cancels the migration and prevents etcd or components from starting; directory walks and cross-filesystem copies observe cancellation between filesystem reads. A canceled partial copy is never published and is recopied from the intact source on the next start. No user migration script, offline command, reverse migration, journal, or backup is provided. Successfully moved sources are removed. ## Compatibility and limits - Preserve the absolute local root, data mounts, and legacy `minio.rootPath` when upgrading. Canonical 2.6 data stays in place; misplaced late-2.6 indexes and affected 3.0.0/3.0.1 layouts are handled as above. Relative roots from older versions are unsupported. - Automatic migration is standalone/local-only. If old CWD indexes disappeared with an old container, they cannot be discovered: preserve/make those files visible or rebuild the derived indexes. Cross-filesystem copy has no configured timeout and can extend the first startup, but a shutdown signal cancels it and the next start resumes from the remaining source. - Downgrade to the affected old layout is unsupported after migration. Symlink layouts require manual resolution. - Known legacy LOB/orphan GC gaps under `R/M/insert_log`, pending old external-refresh paths, and rare interrupted-copy publication residue remain outside scope. This is not a claim that every historical file is recovered or reclaimable. ## Validation At head `898dba4b41`, `ci-v2/build-ut-cov` passed Build, Code Check, Integration UT, Go UT, and C++ UT (9,145/9,145). Diff coverage was 100% for C++ and 81.2% for Go; total patch coverage was 82.8%. Focused local tests for canonical/frozen `localStorage.path` derivation and migration cancellation/deadline/copy recovery passed with `-tags dynamic,test -gcflags='all=-N -l' -count=1`, and `git diff --check` passed before publication. Full paramtable execution remains locally blocked by tests attempting to create `/var/lib/milvus/data`; roles/native-dependent Go tests remain build-blocked by local C++ header/library mismatches. A real 3.0.0/3.0.1 version-to-version upgrade was not executed locally. `ci-v2/e2e-default` and `ci-v2/go-sdk` are still pending for this head. Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com>	2026-09-17 09:43:08 +08:00
3 people	fee4ed4316	enhance: force one row group per cell for the stats skip index (storage v2) (#51441) ... ## What & why `common.parquetStatsSkipIndex.enabled` builds chunk-level skip metrics from Parquet footer statistics for Storage V2 sealed segments. The previous implementation was unsafe and did not reliably save IO: - Parquet statistics were positional per row group, while one cache cell could contain multiple row groups. Applying one row-group metric to a differently packed cell could produce false-negative pruning and silently drop matching rows. - VARCHAR min/max values could outlive the Parquet metadata that backed their non-owning views. - Missing, incompatible, or stale metrics could be associated with the wrong cell after a column replacement. - Some scan and prefetch paths still pinned or materialized cells after the skip index had already ruled them out. This PR makes the Storage V2 footer-statistics skip index conservative and generation-safe, moves pruning ahead of cell IO on the execution paths that queries use, retires the Storage V1 data-derived skip index that this seam supersedes, and enables the feature by default. Issue: #51440 Design doc: `docs/design-docs/design_docs/20260903-skip-filter-interface.md` ## Removed: the Storage V1 chunk skip index This PR also **removes** the Storage V1 data-derived chunk skip index. It is the change with the widest blast radius, so it is stated here rather than left to be inferred from the diff. The rationale and the full consequence analysis are in the design doc, section *Storage V1: no chunk pruning under this interface*. Before this PR, `load_field_data_common` bound every non-proxy scalar/string sealed column to a `FieldChunkMetricsTranslator` that derived per-chunk min/max lazily from the chunk data. That branch ran unconditionally, independent of `common.parquetStatsSkipIndex.enabled`. After this PR the only producer of real metrics is `LoadGroupChunkMetadata`, which requires the flag **and** Storage V2. Metrics are consumed through the column itself: every column is a `FieldChunkMetricsProvider`, `FieldSkipMetricsView::FromProvider(column)` resolves the view, and ordinary V1 columns expose a null metrics list, so they fail open and never prune. There is no segment-level field-to-metrics map to keep in sync with column replacements. | Segment | Before | After | | --- | --- | --- | | Storage V1 sealed | data-derived pruning, always on | no pruning | | Storage V2 sealed, flag off | no pruning | no pruning | | Storage V2 sealed, flag on (now the default) | footer-stats pruning, unsafe as described above | footer-stats pruning, conservative and generation-safe | | Storage V3 / manifest | no pruning | no pruning | The removal is deliberate: Storage V1 is being phased out and no longer warrants a second, separately-shaped skip-metrics lifecycle whose bookkeeping the new seam cannot keep in sync. Queries stay correct — the skip index only ever removed work — but clusters still holding V1 segments read more chunks for selective range and `IN` predicates. Storage-version compaction (`dataCoord.compaction.storageVersion.enabled`, on by default) converts V1 segments to V2, so that exposure shrinks over time. Restoring V1 pruning through the same `FieldChunkMetricsProvider` seam is a follow-up, kept out of this PR so its memory and load-time cost can be measured on their own. Two consequences worth flagging for review: - The V1-only regressions that drove pruning through the lazy-compute path (`Sealed.SkipIndexSkip{Unary,Binary}Range{,Nullable}`, `Sealed.SkipIndexSkipStringRange`, `TaskTest.SkipIndexWithBitmapInputAlignment`, and `OffsetsEvalProductionRegressionTest.ConjunctOffsetInputKeepsMatchAfterRealSkipIndexPruning`) are re-expressed against the Storage V2 footer path. - The arrow/`RecordBatch` half of `SkipIndexStatsBuilder` (`LoadMetrics`, `WithNullState`, the bloom-filter and n-gram construction, `BooleanFieldChunkMetrics`) no longer has a production caller. It is retained for the Storage V3 producer tracked in Follow-ups; if that slips it should be deleted rather than left as dead weight. ## Correctness - Force exactly one Parquet row group per cache cell for **every** column group while the flag is on. Packing is a function of the startup flag alone — never of schema content or footer content — so a segment's cache layout is deterministic and identical for vector-only, pk/ts, and scalar groups alike. Skip metrics are still only collected for the footer-statistics-skippable types INT8, INT16, INT32, INT64, FLOAT, DOUBLE, and VARCHAR; groups and fields without usable statistics simply get no filter. This gives skip metrics and cache cells a 1:1 positional mapping wherever metrics exist. - Build VARCHAR metrics while the Parquet reader and metadata are alive, then retain deep-owning immutable bounds. - Preserve one metric entry per row group, across all files of the group in load order. Missing or unusable statistics become `NoneFieldChunkMetrics`, which never prune, so holes cannot shift later metrics onto another cell. - Validate each field's metric list twice: `PlanGroupChunkPacking` drops a field whose list length differs from the total row-group count or that has neither usable bounds nor a known all-null chunk (such a list can never prune; dropping it avoids binding a useless filter), and `GroupCTMeta::InstallSkipMetrics` re-checks list length against the loaded cell count as a producer-independent backstop. Any mismatch disables footer pruning for that field and reads the data. - A `FieldSkipMetricsView` retains a `shared_ptr` to the column generation it resolved from. Replacing or dropping a field updates `runtime.fields` under copy-on-write publish; a view resolved before the replacement keeps the old column (and its metrics) alive, a view resolved after sees the new generation. A Storage V1/V2 remap cannot reuse stale metrics. - Fix conservative behavior for BOOL `IN`, empty-string VARCHAR `IN`, missing or one-sided min/max values, and Parquet physical-type mismatches. - Disable skip-index pruning for all arithmetic predicates. Arithmetic expressions still execute normally, but they cannot introduce overflow, truncation, rounding, or comparator-boundary false negatives through the skip index. - Preserve SQL three-valued logic by gating the filter itself (`CanUseSkipFilter`): an expression binds a skip filter only for a non-nullable field or a null-rejecting consumer. Null-rejection propagates through `AND`/`OR` and stops at `NOT`; every top-level filter consumer (FilterBits, iterative filter, rescore) is null-rejecting, so a standalone predicate on a nullable field still prunes. A nullable field whose consumer distinguishes UNKNOWN from FALSE — anywhere below a `NOT` — binds no filter and evaluates the data normally, so `NOT` never turns a skipped NULL into a match by construction. Unsupported or invalid statistics always fail open: they may lose a pruning opportunity, but they do not remove matching rows. ## IO avoidance Pruning now happens before a sealed cell is pinned or materialized on both sequential scans and offset-input or iterative-filter scans. The scan path, driver prefetch, and fallback prefetch share the same skip decision. A pruned cell is therefore not fetched later by another stage. VARCHAR `IN` uses an owned `std::string` value list on both scan and prefetch paths, avoiding the previous `std::string` and `std::string_view` mismatch. Known boundary: an `IN` list whose literals all fall outside the column type's range (`int8_col IN (300, 400)`) is now treated like a genuinely empty `IN` — `CanSkipIn` fails open on an empty value set, so the column is scanned and the (correct) empty result is computed from the data. The earlier always-prune shortcut for this case was dropped together with the `CanUseSkipFilter` simplification; it saved IO only for provably-empty `IN` lists and carried the subtlest machinery in the change (an element-level carve-out and a `type_info` guard). Restoring it is a small, separable follow-up. ## Reopen consistency The current base includes #51623, which provides request-level `SegmentReadLease` protection for sealed segments: - Production Search, Retrieve, and RetrieveByOffsets acquire the lease before expression execution. - Search retains the lease in `SearchResult` through reduce, export, and output-field materialization. Retrieve holds it through result serialization. - `PublishStateOnline` waits for active readers to drain before atomically publishing a Reopen state. - Reopen stages changes in cloned runtime state instead of mutating the currently published snapshot. The skip view is resolved from the same runtime snapshot as the data it describes: `GetFieldSkipMetrics` and `GetDataScanResources` read `runtime.fields` once and return the column and its metrics view together, and the bound filter retains that generation for the cursor's lifetime. Consequently, Reopen cannot publish runtime B between skip-metric resolution and chunk-layout or data access in the same production request. Direct internal C++ callers that combine skip metrics with chunk layout or data access must hold `SegmentReadLease`, as documented in `SegmentInterface.h`; all production Search and Retrieve entry points do so. ## Configuration and scope - `common.parquetStatsSkipIndex.enabled` is **enabled by default**. - The setting is startup-only and not refreshable because it determines row-group-to-cell packing. Changing it requires a restart and segment reload. Enforcement is structural: the value is pushed to the core once during QueryNode/StreamingNode init and no runtime update path exists. - Footer-statistics pruning in this PR is Storage V2 only. Manifest-based Storage V3 segments do not build this skip index. - The nullable-vector footer-statistics collection added in #49168 is fully retired, not merely narrowed. `CollectSkipStatsFields` gathers only types `SkipIndexStatsBuilder::Build` produces real metrics for, and vector types fell to its `default` arm yielding `NoneFieldChunkMetrics`, so those statistics could never prune. The `null_count`-driven offset mapping they fed has been superseded by `BuildValidRowIds()`, which reads real validity. A backport should not reinstate the dependency. - Enabling it has a real cache cost, now borne by every column group: cells are as fine as one row group, so a group of 64 KiB row groups against the 4 MiB default `cellTargetSizeBytes` goes from roughly one cell to roughly 64, multiplying cache-slot metadata, pin/unpin calls and eviction bookkeeping. Two mitigating factors: finer cells reduce the bytes a point lookup or `Take` must pull in, and the per-cell overhead is proportional to row-group count, which the writer already controls. One known accounting artifact: `GroupChunkTranslator::cells_storage_bytes` floors each cell at 1 MiB for scanned-bytes and cache-byte metrics, so segments with small row groups will report inflated scanned bytes; keying that floor to actual cell size is a follow-up. The footer metrics themselves live in `GroupCTMeta` for the column group's lifetime and are not yet counted in the tiered-cache budget (one `FieldChunkMetrics` per row group per skippable field; VARCHAR entries own two `std::string` bounds) — bounding that is also a follow-up. Disabling the flag restores target-size packing everywhere and disables footer pruning. ## Tests Storage V2 query correctness and executed-path coverage: - `SkipIndexPr51441.StorageV2SkipQueryResultsCorrect` - `SkipIndexPr51441.StorageV2CellPruneByFlag` - `SkipIndexPr51441.StorageV2PackingFollowsFlagRegardlessOfFooterMetrics` — packing follows the flag for every group, including groups with statistics disabled or all-null columns; fields without usable bounds and without all-null chunks get no filter - `SkipIndexPr51441.StorageV2VarcharInPrunesOnExecutedPath` - `SkipIndexPr51441.StorageV2OneSidedVarcharFooterStatsFailOpen` - `SkipIndexPr51441.PrunedCellsAreNotPrefetchedOrPinned` - `SkipIndexPr51441.ArithmeticPredicatesDoNotUseSkipIndex` - `SkipIndexPr51441.NullableSkippedCellsPreserveNotSemantics` — `NOT` over a nullable field never turns a NULL into a match - `SkipIndexPr51441.OutOfRangeBinaryRangePrefetchMatchesScan` `SkipIndexStatsBuilderTest` also covers missing statistics, physical-type mismatches, BOOL and empty-string `IN`, integral bounds, and VARCHAR bound ownership. `ConjunctExprTest.SkipFilterEligibility` pins the `CanUseSkipFilter` truth table; `FieldSkipMetricsViewTest` pins the provider tri-state (list / null list / per-chunk fallback) and generation retention across rebind and erase. ## Review follow-ups The branch is squashed to one commit; these are the review fixes folded into it: - `PhyUnaryRangeFilterExpr::PrefetchRawData` read its literal with `GetValueFromProto`, which truncates an out-of-range value to `T{}` without reporting the overflow, so `int8_col < 300` pruned every cell whose minimum was `>= 0`. It now checks `query::out_of_range` and prefetches exactly what the scan will touch — `PreCheckOverflow` answers such a predicate from the literal alone and reaches the column only to materialize validity, which `ApplyFieldValidData` skips for a non-nullable field. `PhyBinaryRangeFilterExpr` already had this guard; the unary sibling did not. - `GetSkipIndexValues` used `GetValueFromProtoWithOverflow` for float/double, which asserts `kFloatVal`, while the `GetValueWithCastNumber` it replaced accepted an int64 literal — the assert would have surfaced as an opaque query failure from inside the prefetch future. - The prepared-value cache hangs off a single `std::once_flag` but is built from a `T`-dependent `ValueType`. It records the type and fails open if the `PrefetchRawData` and `Eval` dispatch switches ever disagree, instead of reinterpreting the cached variants. The `type_info` comparison is by value, not address. - `ProcessDataChunksForChunkedSegment` called `GetSkipIndex()` inside the per-chunk scan loop, and `GetSkipIndexSnapshot` allocated a fresh empty `SkipIndex` whenever the segment had none. The snapshot is hoisted, the empty instance is shared, and the scan reuses the prefetch loop's verdicts so a Term `IN` hull is not recomputed per cell. `ProcessDataChunksForElementLevel` and `ProcessDataChunksForSingleChunk` had the identical pattern — the latter is the growing-segment path, where the view is always empty and a throwaway was allocated per chunk per batch. All five call sites now capture the view once, which also keeps every chunk of a batch judged against a single resolved generation. - `PhyBinaryRangeFilterExpr::PrefetchRawData` abandoned pruning whenever a literal did not fit the column's physical type and fell back to the whole-field prefetch, while the scan's `PreCheckOverflow<T>` resolves the same literals — a bound past the far end of `T` makes the range provably empty, a bound past the near end constrains nothing — and keeps pruning with the clamped bounds. With driver prefetch on, `3000000000 <= int32_col <= 3000000001` matched nothing and the scan read no cell, yet the prefetch pulled in the whole column (83 MB in the test fixture). The prefetch now mirrors `PreCheckOverflow`. Over-fetch only, but it broke the shared-skip-decision property claimed above. - The metrics/cell alignment rule (a field's metric list must have exactly one entry per cache cell, or be dropped) lives in `GroupCTMeta::InstallSkipMetrics` with `skip_metrics_by_field_` private behind it, so every producer — including the test fixture — inherits the check by construction. `FindSkipMetric` absorbs the bounds guard `ChunkedColumnGroup` used to open-code. `PlanGroupChunkPacking` adds the prune-worthiness cleanup on top (misaligned lists and lists with neither usable bounds nor all-null chunks are dropped before install). - `PhyTermFilterExpr::CanSkipSegment` value-initialises the `min`/`max` hull (an empty `IN` on a primary key previously reached `CanSkipBinaryRange` with indeterminate bounds) and fails open on an empty literal list. - The per-cell skip check used to perform two field lookups (`SkipIndex`'s field → provider map, then the column group's field → list map) before indexing by `chunk_id`, and that cost scaled with the row-group count once cells are 1:1 with row groups. The segment-level map is now gone entirely: columns are their own `FieldChunkMetricsProvider`, `FieldSkipMetricsView::FromProvider` resolves the optional list once, and every scan and prefetch path indexes the view by `chunk_id`. The view owns a reference to the column generation it resolved, so a rebind or erase cannot change what an in-flight expression sees (`FieldSkipMetricsViewTest.RetainsResolvedGenerationAcrossRebindAndErase`). `skip_func` is `bool(const FieldSkipMetricsView&, int64_t)`, which also removes the `int` narrowing of chunk ids. `GetSkipMetricsList()` is tri-state so a producer that implements only the original per-chunk contract keeps pruning through a per-cell callback, while list-capable providers with no list for the field — Storage V1 columns included — never pay a per-cell call. The generic `SkipIndex` helper remains only for the `JsonKeyStats` callback interface and standalone tests. - The nullable-validity handling was simplified from *skip the cell but still fetch its validity bitmap* (`SkippedChunkNeedsValidity`) to *bind no skip filter at all* (`CanUseSkipFilter`) for a nullable field with a validity-observing consumer. The old design required every scan, take, prefetch, and element-level path to thread a validity-only read for pruned cells, and one such path (the element-level skip branch, which hands `evaluate_batch` an empty `ValidityView`) was a review-found `NOT`-semantics bug. The new design makes the incorrect configuration unrepresentable, at the cost of losing pruning for nullable fields below `NOT`. - `HasMetrics()` no longer reports a bound view for a known null or empty metrics list, so such fields bind no `ColumnFilter` at all instead of paying a per-cell callback that always fails open. - `PlanGroupChunkPacking()` collects the force-one-row-group decision and its rationale in one place instead of copy-pasting it between the two storage-v2 column-group load overloads. The decision is the startup flag alone; it no longer depends on whether the group contains a skippable-typed field. ## Follow-ups - Storage V3 footer-statistics skipping remains a follow-up on milvus-storage#583. - Restoring chunk pruning for Storage V1 is a follow-up that plugs into the same `FieldChunkMetricsProvider` seam rather than reviving the removed path, kept separate so its memory and load-time cost can be measured on their own. - Arithmetic skip-index pruning can be reconsidered only with an exact proof for overflow, conversion, rounding, and comparator boundaries. - The element-level filter `ExprSet`s consume only data bits and could mark their roots null-rejecting, which would restore chunk pruning for nullable array fields. - Restoring the always-prune verdict for `IN` lists whose literals all truncate out of the column type's range (dropped with the `CanUseSkipFilter` simplification; see *IO avoidance*). - Footer metrics memory accounting, and keying the `cells_storage_bytes` 1 MiB floor to actual cell size (see *Configuration and scope*). Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>	2026-09-15 18:29:36 +08:00
Li LiuandClaude Opus 5	e008a8429d	enhance: add optional root authentication for the management plane (#52580) ... issue: #49846 design doc: docs/design-docs/design_docs/20260912-management-plane-authentication.md Merge dependency: #52579 supplies safe configuration projections. Both changes are needed for the full configuration-redaction requirement in #49846. ## Problem and behavior Port 9091 exposes process shutdown, coordinator management, profiling, event logs, the web console, and legacy REST data operations. Add `common.security.adminAuthEnabled`, default **false**, to authenticate this surface independently of the main service port's data-plane authorization. When enabled: | Surface | Policy | | --- | --- | | `/management/*` except readiness; `/debug/pprof/*`; `/log/level`; `/eventlog`; `/webui/`; `/telemetry`; `/api/v1/_*` | Root HTTP Basic authentication | | Legacy REST data operations under `/api/v1` | Existing valid-user/API-key authentication if `authorizationEnabled=true`; otherwise root Basic authentication | | `/healthz`, `/livez`, `/metrics`, `/metrics_default`, `/management/check/ready` | Open | | `/api/v1/health` | Exempt from the new gate; existing data-plane authentication still applies | After the request-origin checks below, missing/incorrect credentials return 401; non-root users are rejected before password lookup with 403; unavailable credential storage and saturated verification return 503. Dependency details and stack traces remain out of unauthenticated response bodies. HTTP and Gin share one decision and carry a verified root principal through a typed request context for in-process telemetry calls. ## Browser and client requirements The gate rejects requests identified as cross-site or same-site. An unknown Fetch Metadata value and `Origin: null` are rejected as well. Top-level GET document navigation is allowed only on document surfaces, not operator actions or console APIs. **Credential-bearing requests without Origin or Fetch Metadata must include `X-Milvus-Admin-Request: true`.** This is required because missing metadata does not establish that a request is from a non-browser client; ordinary remote HTTP is outside the Fetch Metadata trustworthy-URL guarantee. An explicit header does not override a reported cross-site origin. The metrics-port CORS preflight must not grant this header to cross-origin callers. Browser console access requires HTTPS with Fetch Metadata preserved by the reverse proxy. A document request without sufficient request context is refused without a Basic challenge. Non-browser callers add the explicit header, for example: ```sh curl --user root --header 'X-Milvus-Admin-Request: true' \ 'https://milvus-admin.example/management/querycoord/balance/status' ``` The flag remains opt-in. Existing clients with the flag disabled keep their prior behavior. ## Credential lifecycle and resource bounds Verifier ownership is deterministic in standalone: Proxy, then MixCoord, then worker. Workers create a MixCoord client lazily. Each lookup caller has its own five-second wait bound, so a service-discovery call that ignores the backend context cannot block the HTTP request indefinitely. Its accounting slot remains occupied until the shared backend result completes, preserving the resource bound even in that case. Root hashes are cached for 10 seconds, reused for up to 10 minutes after their last successful fetch if refresh fails, and lookup failures are cached for 2 seconds. The stale window preserves management access during a coordinator outage but can also retain acceptance of a password rotated during that outage. Concurrent hash refreshes share work and admit at most 32 callers, including canceled callers whose singleflight result channels remain retained. Worker client construction uses a single shared completion signal; repeated lookup timeout windows no longer append a retained channel to a stuck constructor. Construction failure can be retried, and a client completed after shutdown is closed instead of published. bcrypt concurrency is bounded to a quarter of GOMAXPROCS, floored at two, with a bounded queue. This is a concurrency cap, not an attempt rate limit; on small pods sustained incorrect passwords can still consume substantial CPU. ## Registration, configuration, and event logs - `http.Register` refuses an unmarked operator route, including method- and host-qualified ServeMux patterns. Assembled-router tests and a console-prefix test cover Gin registration. - Milvus-owned handlers use a private mux. Flag-off compatibility with more-specific legacy DefaultServeMux routes is retained when pprof is enabled; enabling the gate removes that fallback. - `/management/config/alter` always rejects this flag, using the same `config.FormatKey` identity as config storage, so an anonymous caller cannot plant a disabling etcd value before the operator enables the gate. - Key-normalization caching admits at most 4096 entries and only keys whose input and normalized forms fit 1024 bytes, using owned string storage so short substrings cannot pin large request buffers. Larger keys still normalize correctly without retained memo entries. This bounds cache retention, not individual HTTP body size. - The eventlog gRPC stream switches to loopback with authentication enabled. A failed security upgrade closes the wildcard listener. Event-log shutdown uses a separate close signal and stops the gRPC server, preserving safe concurrent recording. - `milvus_admin_auth_total{endpoint,result}` records root-gate and origin decisions using registered route patterns; failure logs are rate limited. - The standard Go runner now includes HTTP, adminauth, coordinator, eventlog, and metrics regressions. It restores fail-fast behavior after sourcing the environment so a failed test command returns a failure from the runner. ## Compatibility and deployment Port 9091 remains plaintext. Use a trusted network or TLS-terminating proxy, rotate root using `UpdateCredential` / `db.update_password`, and update graceful-shutdown and profiling clients before enabling. Editing `defaultRootPassword` does not rotate an existing credential. Official compose deployments and embedded standalone scripts publish 9091, so review that exposure. Browser access needs the HTTPS deployment described above. Regardless of the flag, downstream operator routes without `AdminAuth` now fail registration; the admin-auth key cannot be altered through the management API; configuration normalization is bounded; pprof registration is explicit; and the data-plane stored-password comparison shares the credential-format helper. ## Verification and limits CI observed on head `c8992fc7b4d3fcf9a28bf73ad4700283922367f2` at **2026-09-12 14:30 UTC**: `ci-passed` and `dco-passed` are present. | Check | Result | | --- | --- | | [Build / unit / integration / code checks](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-build-ut-ciloop-pipeline/30883/pipeline-overview/) | SUCCESS; status reports build, code checks, integration, Go, and 9,106 C++ tests passed | | [Go SDK E2E](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-go-sdk-pipeline/15021/pipeline-overview/) | SUCCESS | | [Default E2E](https://jenkins-milvus-ci.milvus.io/job/MILVUS-CI-V2-PR-PIPELINES/job/milvus-e2e-pipeline-dispatcher/19796/pipeline-overview/) | SUCCESS | | Main CI trigger / DCO | SUCCESS | The non-gating Tencent Pulsar gray run reports `ERROR` because its downstream job could not be triggered. The previous PR head had the identical start failure. No Pulsar test result is claimed, and the precise trigger failure is unknown because Jenkins logs are inaccessible here. The TC-US shadow build and legacy Jenkins status still show pending; they are outside the master `ci-passed` requirements. No label was added manually. Local verification and remaining limits: All Go tests use `-tags dynamic,test -gcflags="all=-N -l"`. - Full `internal/util/adminauth`, including race detection, passed. A blocked-constructor regression retained 20,011 extra heap objects after 10,000 canceled waits before the fix and no retained growth after it. Cancellation, retry, panic, and a client completing after close are covered. Four blocked-backend scenarios cover cold cache, expired stale state, and correct/incorrect stale passwords; race runs repeated three times passed, including retained slots and recovery after release. - Full `internal/http` and its race run passed with the explicit-request-header changes, including 34 new net/http/Gin cases. `pkg/eventlog`, `pkg/metrics`, and `pkg/util/merr` also passed. - Configuration-normalization tests passed, including oversized inputs, Unicode expansion, and request-buffer ownership. The full config race suite passed excluding `TestEtcdSource`, whose two tests require a reachable external etcd and failed against this host's deliberately unavailable test endpoint. The config comment and ParamItem documentation match exactly. - The newly scheduled HTTP, adminauth, eventlog, and metrics commands passed with the runner's race and coverage flags. A dispatch harness checked all five package additions through the full and selected-group entry points. Ten injected command failures were swallowed before restoring fail-fast and now stop the runner immediately with the original nonzero status. Coordinator execution was unavailable locally; the subsequent native CI aggregate reports Go success. - Legacy Basic challenges are preserved on the main port, gate-off metrics routes, and the health exemption. A regression using the actual extracted parser with real Gin failed before the repair and passed after it; assembled-route and dynamic-gate compatibility tests were added to the native package suites. The subsequent CI Go check reports success; individual native test logs were not accessible here. - `internal/proxy`, `internal/distributed/proxy`, `internal/coordinator`, and `cmd/roles` cannot compile on this host because `milvus_core.pc`, `milvus-storage.pc`, and `rocksdb.pc` are unavailable. The subsequent native CI aggregate reports build and Go success; this does not change the local compilation limitation. - Full `make test-go` was attempted; native dependency construction failed in the host SDK linker (`library 'System' not found`) before the Go suite. This is not recorded as a passing full test run. - golangci-lint v2.11.3 built with Go 1.26.6 reported zero new issues in `internal/http`, `internal/util/adminauth`, and the changed config/eventlog/metrics/paramtable packages. Native-linked packages could not be linted locally; the subsequent CI code-check status reports success. - A real coordinator failover and a real browser console flow remain unverified. Unit and source-level failure tracing do not establish full browser credential replay or transport behavior. --------- Signed-off-by: Li Liu <li.liu@zilliz.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-15 14:27:33 +08:00
Ted Xu	423b8e483d	enhance: reduce mix compaction write amplification (#53197) ... See #53198 Replace the three-pass greedy bin-packing compaction planner with a two-tier composition algorithm to reduce write amplification: - Full-tier packs compactable segments toward idealSize with an 85% fill gate, tolerating small residuals instead of rewriting near-full segments repeatedly. - Fragment-tier activates when fragment count exceeds maxFragmentsPerGroup (default 8), packing fragments toward middleSize (idealSize/4). - Force-triggered segments each get their own single-segment bucket instead of being merged together. Deprecates SegmentCompactableProportion, SegmentSmallProportion, MinSegmentToMerge, and SegmentExpansionRate in favor of hardcoded constants. Adds MaxFragmentsPerGroup config (default 8). --------- Signed-off-by: Ted Xu <frankxus@gmail.com> Signed-off-by: Ted Xu <ted.xu@zilliz.com>	2026-09-15 10:19:34 +08:00
3 people	240b9c0a91	feat: publish StorageV3 index artifacts through segment manifests (#53048) ... Publishes completed StorageV3 index artifacts through the segment manifest while keeping build-task state durable until publication. Builds on the segment-scoped manifest commit framework from #52621. ## What changes **Publication and placement.** `dataCoord.index.writeSegmentIndexToManifest` (default `false`) controls new completed artifacts. When enabled, a Finished StorageV3 build with files publishes its manifest entry and advances the segment pointer while deleting its etcd task row in one catalog transaction; its index record remains available in memory. Unissued, InProgress, Failed, fake-Finished, and StorageV1/V2 records continue to persist in etcd. Disabling the switch directs new completions to etcd without hiding already published manifest indexes. **Recovery.** The per-segment `manifest_has_index` marker determines which retained healthy or Dropped non-L0 StorageV3 manifests are read, independently of the current write mode. Existing etcd records win BuildID conflicts. Recovered records also retain indexes belonging to dropped definitions so GC can find their artifacts. Dropped compaction parents recover their indexes for query fallback and orphan-GC protection. Unreadable or invalid marked manifests fail startup, except when a separate successful existence check proves a Dropped segment's manifest was already removed by GC. That exception leaves the marker intact for pending GC. Confirmed empty sections normalize historical true markers against the exact published pointer. Manifest reads share one metadata-owner admission budget across recovery, copy assembly, copy verification and GC. `segmentIndexManifestLoadConcurrency` defaults to 64, is clamped to 1–256, and is capped by positive `minio.maxConnections` values. Startup uses fixed-size batches and per-segment retries; an exhausted read does not replay the entire scan through the metastore retry loop. **Retirement and synchronization.** GC deletes unused index bytes before removing their manifest entries and records, retaining the metadata needed to retry failed deletion. It refreshes segment placement after admitting terminal index tasks so a concurrently completed publication is not classified using an older marker or revision. The commit lock order is `segmentManifestLocks -> indexMeta.keyLock -> segMu`. BuildID locks span the initial snapshot, manifest I/O and publication; `segMu` is released during object-storage I/O. OVERWRITE applies updates to the explicitly selected input revision; higher stored revisions affect version allocation, not the input contents. **Copy and restore.** Dispatch persists the selected index placement and sends target index identities keyed by name. Capable workers receive shared request-level definitions; older workers receive the per-target encoding. The worker removes inherited source entries and republishes the copied target artifacts. DataCoord validates artifact-bearing manifests before publishing any target, including identity, parameters, path layout and file keys. V1/V2 target indexes remain in etcd even when the task selected manifest placement for its V3 siblings. Coordinator-derived cleanup prefixes are persisted before dispatch for every storage version, including each retry's allocated index directories. Rejected completed results retain durable cleanup intent. A separate cleanup worker waits for ordinary segment GC to retire every target, preserving its snapshot, loaded-reader, pause and retention protections, then removes leftover task-owned prefixes. A per-task lock excludes cleanup from result installation. Failed tasks with cleanup plans reject late completions; failed admission writes and restart retain polling until completion or confirmed worker loss. Worker loss after completed-result admission fails the task and preserves cleanup rather than redispatching potentially partly published targets. Missing local prefix roots are idempotent no-ops; other deletion failures remain retryable. ## Validation Latest base synchronization: merged `master` at `b49b1e900b`. The only textual conflict was generated `data_coord.pb.go`; it was regenerated in the development container with `make generated-proto-without-cpp`. The other PR-added/deleted content is unchanged relative to the new base. Protobuf packages compile, and checks of field numbers and wire round trips preserve both upstream compaction `fail_status` and the PR's manifest/copy fields. Targeted DataCoord/session tests on this merged revision could not compile against the existing local native output: it predates upstream `create_token_stream`, `ReadNext`, and `AddCColumnSplit` C API changes. These tests require a refreshed native build; no passing DataCoord runtime result is claimed for this merge. The focused runtime and lint results below apply to the pre-merge revision. Focused tests passed in the Milvus development container `milvusdb/milvus-env:ubuntu22.04-20260714-c135601`, with the Conan cache mounted and `-tags dynamic,test -gcflags="all=-N -l" -count=1`: - DataCoord copy task and cleanup lifecycle, Dropped recovery, and publication/GC interleaving regressions: passed (13.570s). - Existing copy inspector/checker/meta, manifest/reload and affected dropped/index GC regressions: passed (16.084s). - DataCoord session capability/compatibility and DataNode importv2 copy/manifest tests: passed (5.547s and 5.672s). - Read-only Go formatting check and `git diff --check`: passed. - Incremental repository-configured lint over the publication diff in DataCoord, DataNode and packed storage packages: passed (0 issues). These are focused unit and deterministic failure/interleaving tests. Full CI, real-cluster snapshot restore/QueryNode recovery and the race detector were not run for this update. A result installation already in progress can still race an external task-failure transition; the cleanup lock does not serialize that older state transition. The cleanup plan cannot recover unrecorded BuildIDs from tasks dispatched before the plan was introduced. Partition-level LOB cleanup remains owned by the existing orphan collector. Turning publication off is supported, but it does not make manifest-resident metadata compatible with pre-feature DataCoord binaries. All potential DataCoord leaders must support manifest recovery and GC before activation. ## Known concurrency limitations (not fixed by this PR) The segment manifest commit framework serializes its participating writers, including L0 batches. Two producers still generate revisions outside that boundary. Their limitations are tracked separately: **Spark backfill on an ordinary StorageV3 collection — #53328.** Spark can write v11 = v10 + a backfilled column group while DataCoord still points to v10. If an index completion then publishes v12 = v10 + its index entry, the new column group is absent from v12. When backfill pre-validation passed before that index commit, the later `BatchUpdateManifest` ACK skips v11 as an older version and `CommitBackfillResult` can nevertheless count the segment as committed. The issue tracks the exact pre-validation-to-ACK interleaving, final manifest contents and per-segment result correctness. **External collection refresh patching an existing segment — #53329.** Consider unchanged external Parquet rows: refresh adds the mapping for `score`, while an index on existing field `id` completes. The DataNode patches the manifest version dispatched to it. If the index publishes v11 first, a refresh v12 built from old v10 can contain `score` but omit the index; DataCoord accepts v12 without checking that input base. In the opposite order, an unadopted refresh v11 can be overtaken by index v12 lacking `score`, and the job treats v11 as already applied merely because v12 is newer. The issue covers both index recovery metadata loss and a successful refresh whose new column mapping is absent. This path applies refresh job results directly; it does not use `CommitBackfillResult` or the batch ACK. Until these issues are fixed, neither Spark backfill generation/adoption nor an existing-segment external refresh patch should overlap manifest commits on that same segment. The restriction covers base selection and revision generation through catalog publication; locking only the final adoption does not close it. These are source-derived failure sequences; deterministic concurrency tests and restart verification remain part of the respective issues' acceptance criteria. issue: #53047 design: docs/design-docs/design_docs/20260811-storagev3-manifest-index-publication.md design: docs/design-docs/design_docs/20260817-datacoord-segment-manifest-commit.md --------- Signed-off-by: somfornot <318146256+somfornot@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Codex <noreply@openai.com>	2026-09-15 01:57:35 +08:00
3 people	92b0b56d61	fix: honor explicit pulsar.webaddress and keep the derived web address a valid url (#53193) ... issue: #53192 ## What this PR does `pulsar.webaddress` is the Pulsar admin REST endpoint Milvus uses to clean up subscriptions (`pkg/mq/msgstream`). Today it is always derived from `pulsar.address` as `http://<hostname>:<pulsar.webport>`, and the formatter in `pkg/util/paramtable/service_param.go` discards any value configured under the key itself. Two problems follow: 1. **There is no way to point the admin API anywhere else.** Deployments that expose the admin API through a proxy / load balancer, under a different hostname, or over `https://` cannot express that. This PR honors an explicitly configured `pulsar.webaddress` as-is. When it is not set, the value is derived exactly as before. 2. **The derivation can produce an unparsable url.** `url.Hostname()` mangles a multi-host service url (`pulsar://h1:6650,h2:6650` -> `http://h1:6650,h2:80`, while the pulsar client accepts that form and the `pulsar.address` formatter passes it through) and drops IPv6 brackets (`pulsar://[::1]:6650` -> `http://::1:80`). The web address is now built from the first host with `net.JoinHostPort`, so it is always a valid url (`http://h1:80`, `http://[::1]:80`). Single-host derivation is byte-for-byte unchanged (`pulsar://localhost:6650` still gives `http://localhost:<webport>`); only inputs that previously produced unusable values change. ## Test `go test ./util/paramtable/ -run TestServiceParam` in `pkg/`. Added a table of derivation cases (hostname, IP, `pulsar+ssl`, no port, no scheme, userinfo, IPv6 with and without port, multi-host with and without scheme) and the explicit override checked against single-host, multi-host and empty `pulsar.address`. --------- Signed-off-by: wkd-woo <wkd-woo@users.noreply.github.com> Co-authored-by: wkd-woo <wkd-woo@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-09-14 18:45:34 +08:00
aoiasdandClaude Opus 4.6	5ba20e6395	feat: [Vortex1] add scalar local format data scan foundation (#51504) ... relate: #50304 design doc: docs/design-docs/design_docs/20260305-local_format.md ## Summary Introduce the scalar local-format metadata contract and column-oriented DataScan foundation described in the [local format design document](https://github.com/milvus-io/milvus/blob/buqian/aoiasd/aoiasd-vortex-data-scan/docs/design-docs/design_docs/20260305-local_format.md). This includes write-time grouping, field validation, raw-compatible scan interfaces, and focused coverage for the new contract. Signed-off-by: aoiasd <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-09-10 14:49:58 +08:00
sparknackandClaude Opus 4.6	5f7ddf23d2	enhance: add async storage v3 field data loading (#51246) ... issue: #51245 - Add an async executor and Storage V3 field-data load pipeline for manifest column groups and JSON key stats. - Group requested cells into contiguous, byte-bounded row-group read windows and admit each window through the shared transient-memory budget. - Preserve load priority, requested result order, cancellation, and the first storage or finalization failure across coroutine boundaries. - Route mmap finalization to the optional local-file I/O executor while keeping the legacy synchronous path as the default. - Add the temporary disabled-by-default rollout switch `queryNode.segcore.storageV2.enableAsyncLoad`; `storageV2` is retained only as the existing internal configuration name. --------- Signed-off-by: Shawn Wang <shawn.wang@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-09-09 14:35:28 +08:00
ChenLiqingandCLiqing	3189d349ca	fix: optimize strict group-by iterator filtering (#53246) ... Fixes #53277 ## Summary Reduce pathological strict group-by iterator work by passing the locked, unfinished groups as an additional dense invalid-bit filter to a fresh iterator. Keep algorithm/index selection in the backend. ## Current rule and server configuration - Eligible only for strict_group_size=true, group_size>1, nq=1 and supported row-level single-field grouping (master multi-field/element-level guards preserved). - After locking topK groups, probe at most T additional consumer candidates. A counts rows actually accepted into unfinished quotas, not all locked-group hits or graph visits. - Finish immediately if groups fill or the original iterator exhausts. Otherwise recreate once only when A / actual probe count < threshold; equality continues the original iterator. - Refreshable server configuration: `queryNode.groupBy.strictGroupAcceptanceThreshold` (default 0.1, finite [0,1], zero completely disables optimization) and `queryNode.groupBy.strictGroupProbeCandidates` (default 100, positive int64). --------- Signed-off-by: CLiqing <23721160+CLiqing@users.noreply.github.com> Co-authored-by: CLiqing <23721160+CLiqing@users.noreply.github.com>	2026-09-09 10:01:27 +08:00
Jamesandxiaofanluan	11ed73e3fb	fix: harden membership filter planning (#53100) ... ## What this fixes - Keep unsupported membership-filter leaves conservative during clustering-key scalar pruning by cloning the all-true bitmap before mutable logical operations. This prevents false-negative segment pruning when an unsupported leaf is reused under nested AND/OR expressions. - Move each Bloom blob out of its uniquely owned parsed `PlanNode` / `ScoreFunction` into an immutable shared string. Serialized search, retrieve, and standalone scorer paths no longer create a third owning copy of a large MBF1 blob, and they do not retain unrelated protobuf fields; legacy const-reference entry points retain safe-copy behavior. - Reject bytes template values before arithmetic operand type inference, returning a parameter error instead of dereferencing a null internal value expression. - Parse each JSON membership value and resolve its path once before dispatching to the string or int64 Bloom domain. Double and out-of-range uint64 values remain valid non-members; missing/null/bool/object/array values remain unknown. - Avoid widening narrow signed integer slices into a full `[]int64` before Roaring construction. Ordered inputs allocate no full-length conversion buffer; unordered inputs allocate only the required `[]uint64` sorting buffer, while `Build([]int64)` remains source-compatible. No syntax or wire-format changes are introduced. The JSON Bloom scheduling-cost follow-up remains intentionally deferred. Related feature PR: #52778 Original 3.0 feature backport: #53017 3.0 unified follow-up: #53019 Design: https://github.com/milvus-io/milvus/blob/master/docs/design-docs/design_docs/20260822-membership-match-expression.md ## Tests - `LOCALSTORAGEPATH=/tmp/milvus-membership-master-parser go test -tags dynamic ./internal/parser/planparserv2 -count=1` - `go test -tags test ./internal/querynodev2/delegator -run TestUnsupportedBloomLeavesStayConservative -count=1` - Remote C++ build/link against `3d14599542` - `all_tests --gtest_filter=PlanProto.BloomBlobMovesFromOwnedSearchPlanNode:PlanProto.BloomBlobMovesFromOwnedRetrievePlanNode:PlanProto.BloomBlobMovesFromOwnedScoreFunction:BloomFilterExprTest.ProtoParserDispatchAndTypeCheck` (4/4 passed) - Remote C++ build/link against `3445f06a37` - `test_json_uint64 --gtest_filter=JsonNumericTest.AtStringOrInt64DispatchesRuntimeTypes:JsonNumericTest.AtStringOrInt64SupportsRootPointer` (2/2 passed) - `all_tests --gtest_filter=BloomFilterExprEvalTest.JsonPathGrowingAndSealed:BloomFilterExprEvalTest.JsonPathStringBlobKeepsTypesDistinct:BloomFilterExprEvalTest.JsonPathOffsetInput` (3/3 passed, including escaped-string membership and offset-path NULL/uint64 coverage) - `go test -tags dynamic,test -gcflags='all=-N -l' -count=1 ./membership/roaringfilter ./milvusclient` - Added signed-width, named-type, byte-equivalence, non-mutation, and ordered/shuffled allocation-parity regression coverage for Roaring construction. - Final style-only head `c1e842b42a`: clang-format-15 dry-run passed for all 11 changed C/C++ files Signed-off-by: xiaofanluan <xf@hjjaq.com> Co-authored-by: xiaofanluan <xf@hjjaq.com>	2026-09-07 16:25:42 +08:00
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 <yihao.dai@zilliz.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-07 06:39:30 +08:00
3 people	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 <zhicheng.yue@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Codex <noreply@openai.com>	2026-09-06 18:58:57 +08:00
Gali	bf0c145cb7	doc: clarify tiered storage disk watermark limits (#53082) ... Relates to #53053 ## What changed - Clarify that `diskLowWatermarkRatio` and `diskHighWatermarkRatio` are fractions of the effective local-storage disk capacity, rather than fractions of `queryNode.maxDiskUsagePercentage`. - Document the required ordering: `diskLowWatermarkRatio <= diskHighWatermarkRatio <= queryNode.maxDiskUsagePercentage / 100`. - Replace the stale `diskMaxUsagePercentage` reference with the actual `queryNode.maxDiskUsagePercentage` configuration key, and qualify the related memory threshold as `queryCoord.overloadedMemoryThresholdPercentage`. - Add the same disk-capacity description to the `MaxDiskUsagePercentage` parameter metadata. ## Scope This change updates configuration documentation only. It does not change the validation logic, automatically clamp the watermarks, change QueryNode startup behavior, or change health reporting. Therefore, it does not claim to resolve the crash-loop behavior described in #53053. ## Tests - `gofmt -s -d pkg/util/paramtable/component_param.go` — no diff. - `go test -tags dynamic,test -gcflags="all=-N -l" -count=1 ./util/paramtable/...` from `pkg`, with `GOFLAGS=-mod=readonly -buildvcs=false` — passed. - `git diff --check` — passed. ## AI assistance OpenAI Codex assisted with source analysis, drafting, and running the checks listed above. Signed-off-by: joychan <godczy@gmail.com>	2026-09-04 13:54:19 +08:00
JamesandClaude Opus 4.6	cd675e9a13	enhance: chunk oversized WAL payloads at the storage layer, drop proxy size-based packing (#52775) ... ## Problem Every WAL backend enforces a hard cap on one physical record. Proxy row packing budgets entity data but cannot precisely account for the streaming header, properties, encryption expansion, and broker envelope. When the final record crosses the backend cap, the backend rejects it and the existing retry loop can stall the pchannel indefinitely. Related issue: #52474 ## Solution **Split oversized payloads in the StreamingNode WAL adaptor**, below interceptors and above the backend: - When `streaming.splitChunkSN=true`, the adaptor slices the final opaque payload against the backend message limit minus an effective reserve. Chunk records clone the logical properties and carry reserved `_ci` / `_ct` markers. - The durable scanner reassembles physical chunks before v0 conversion, filtering, reordering, transaction handling, flush, or CDC consumption. Malformed metadata returns `ErrCorruptedChunk` and fail-stops the scanner instead of panicking or silently losing acknowledged data. - The backend message-size limit is normalized to at least 256 KiB. Invalid or undersized `pulsar.messageReserveSize` values fall back to safe positive headroom, so a full-budget chunk cannot recreate the permanent message-too-large retry. - RocksMQ and other backends without a bounded record size retain the legacy single-record path. **Keep Proxy and StreamingNode ownership independently switchable:** - `proxy.splitChunk=true` by default: keep the legacy row-based Proxy packer. - `streaming.splitChunkSN=false` by default: do not write physical WAL chunks. - Target state: enable SN chunking first, confirm it on every potential pchannel owner, and only then disable Proxy packing. - When `proxy.splitChunk=false`, Insert and Upsert use `InsertRequestViewEncoder` to encode selected rows directly into the final protobuf payload without first materializing copied `FieldsData`, `RowIDs`, and `Timestamps` slices. Delete produces one logical message per hashed vchannel. The safe rollout is `(proxy=true, SN=false) -> (true, true) -> (false, true)`. Rollback reverses the transition: restore Proxy packing everywhere before disabling SN chunk creation. Old binaries cannot read already-persisted chunk records, so downgrade across chunk-bearing WAL history remains an operator constraint. Segment assignment is treated as a soft sealing target: an indivisible allocation may cross the target once and the segment is then sealed. Independent Proxy resource limits still bound a fully materialized logical write. Design doc: `docs/design-docs/design_docs/20260822-wal-payload-chunking.md` ## Notable behavior changes - Oversized WAL records no longer enter the backend message-too-large retry loop when SN chunking is enabled; they are stored as physical chunk records and exposed to consumers as one logical message. - **`quotaAndLimits.limits.maxInsertSize` now defaults to 64 MiB instead of `-1`**, applies to Insert and Upsert, and is checked after Proxy-side materialization. A larger write is rejected with `ErrParameterTooLarge`. Set it to `-1` to disable the limit. - **`quotaAndLimits.limits.maxDeleteSize` is new and defaults to 16 MiB.** The check is performed on each materialized per-vchannel delete message. With the default `proxy.splitChunk=true`, legacy WAL-size packing keeps messages below this limit in normal configurations; the limit becomes behaviorally relevant after `proxy.splitChunk=false`, when one logical delete message is built per hashed vchannel. Operators should size it before completing that migration step. Set it to `-1` to disable the limit. - New rollout configs: `proxy.splitChunk=true` and `streaming.splitChunkSN=false`; both are refreshable. - New WAL sizing configs/semantics: `woodpecker.maxMessageSize` defaults to 10 MiB, `pulsar.messageReserveSize` defaults to 64 KiB, and bounded WAL message sizes are normalized to at least 256 KiB. - Produce requests carrying the reserved `_ci` / `_ct` properties are rejected as invalid arguments. - A reassembled logical message remains one flusher/transaction-buffer unit, so its payload is contiguous in StreamingNode memory. ## Test plan - [x] Chunk boundary, round-trip, interleaving, corruption, duplicate, and TimeTick cleanup tests - [x] Durable scanner reassembly before legacy v0 conversion - [x] SN switch and live-refresh tests, including the legacy single-record path - [x] Proxy switch tests for Insert, Upsert/partial-update CAS, and Delete - [x] FastEncoder parity against protobuf materialization across scalar, vector, nullable, nested array, dynamic JSON, and arbitrary row-selection cases - [x] FastEncoder descriptor digest and explicit `msgpb.InsertRequest` field-number ownership checks - [x] Insert/Upsert 64 MiB and Delete 16 MiB resource-limit tests - [x] WAL reserve normalization and 256 KiB minimum tests - [x] Reserved marker ingress rejection and scanner fail-stop tests - [x] Focused Go formatting and package tests - [ ] Full CI build and integration suites Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-09-02 19:46:12 +08:00
3 people	5f078c9779	enhance: reject RESTful DQL requests before decode when queue is full (#52986) ... Add an admission middleware on the v2 RESTful DQL routes (`search`/`query`/`get`/`advanced_search`/`hybrid_search`): while the proxy's DQL task queue is full, the request is rejected with HTTP 429 + `Retry-After` before the body is read or decoded. The scheduler rejects such a request with the same TooManyRequests error anyway, but only after paying the HTTP body read, JSON decode and task-creation CPU — under a REST request flood that pre-scheduler cost is what saturates the proxy. Design: - The queue probe follows the `GetMetaCache` pattern: a narrow interface assertion against the concrete proxy, so `types.ProxyComponent` and its mocks stay unchanged; a component without the probe admits everything (pre-admission behavior). A compile-time assertion pins the probe to `*proxy.Proxy`, so a rename fails the build instead of silently disabling admission. - The signal is the same predicate `Enqueue` uses (`unissued >= proxy.maxTaskNum`), read under RLock. Admission is advisory and `Enqueue` stays the authority, so admit/reject races are benign. - Sampling, not reservation: a vacancy admits every request that probes during the pop→refill window, so some admitted requests still pay decode and are then rejected by `Enqueue`. The window has negative feedback — the more requests are mid-decode, the sooner one refills the vacancy — and in steady state the admitted fraction converges to roughly `√(r/λ)` (r = queue drain rate, λ = arrival rate), so the leak is smallest exactly in deep overload; the floor is the pre-PR always-decode behavior. Capacity reservation was considered and rejected: it must be released on every decode/validation failure, disconnect and panic (a missed path silently shrinks queue capacity), and it double-counts against the non-reserving gRPC path. See the review thread for the full analysis. - Guarded by `proxy.http.dqlAdmissionEnabled` (default true, refreshable) as an operational kill-switch. Scope notes: - v2 routes only (v1 REST API is deprecated); the gRPC path is unchanged. - DDL/DML routes are untouched — only the five DQL routes get the middleware. - Only pre-decode rejections surface as HTTP 429; a request admitted during the refill window is still rejected by `Enqueue` and returns the pre-existing envelope (HTTP 200 + error code in body). A uniform merr → HTTP status projection for the RESTful layer is tracked in #52991. - No dedicated reject counter metric yet; 429s are observable at the gateway and access log. issue: #52985 🤖 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>	2026-09-02 15:45:29 +08:00
foxspyandClaude Opus 5	f7478b65e6	enhance: make growing interim index build thread num configurable (#53030) ... ## Summary `VectorFieldIndexing::get_build_params` hard codes `knowhere::meta::NUM_BUILD_THREAD` to `1`, and that config object is shared by every `BuildWithDataset` / `AddWithDataset` call of the growing interim index, so a growing index build always runs single threaded. This adds `queryNode.segcore.interimIndex.growingBuildThreadRate` to make that thread number configurable. The rate is a fraction of the interim index build thread pool, whose size is already `buildParallelRate * cpu num`: ``` thread num of one growing index build = max(1, round(buildParallelRate * cpuNum * growingBuildThreadRate)) ``` The default `0` resolves to 1 thread, so the default behavior is unchanged. The parameter is refreshable and takes effect on the next build. The sealed segment interim index is deliberately left at a single thread. ### Scope note This bounds a single build only. Growing segments may still build concurrently, each taking one slot of the build thread pool, so the worst case thread number is the pool size multiplied by the resolved per-build thread number, as the config doc states. Bounding the aggregate needs a separate concurrency gate and is left as follow-up. ## Verification - `all_tests --gtest_filter=*GrowingIndexBuildThreadRate*`: 2/2 pass. - `ResolvedAgainstBuildThreadPoolSize` asserts the resolution rule; reverting the production line back to the hard coded `1` makes it fail (`Which is: 1` vs `Which is: 16` on a 32 core box with the default `buildParallelRate: 0.5`), so the test is bound to the change. - `MultiThreadedBuildKeepsSearchCorrect` builds the same data with 1 thread and with the whole build pool and compares the search distances, with an exhaustive nprobe so the comparison does not depend on how the coarse quantizer is trained. It also asserts `SyncDataWithIndex` so the comparison cannot pass vacuously through the brute force fallback. - `all_tests --gtest_filter=*GrowingIndex*`: 65/65 pass (63 pre-existing, no regression). - `all_tests --gtest_filter=*Segcore*:*Interim*:*SegmentGrowing*`: 49/49 pass. - `go test ./pkg/util/paramtable/ -run TestComponentParam`: pass. Not claimed: no throughput benchmark was run, so this PR does not assert an insert throughput improvement. It only removes the hard coded single thread limit and makes the value configurable. issue: #53029 --------- Signed-off-by: xianliang.li <xianliang.li@zilliz.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-01 06:49:28 +08:00