mirror of
https://github.com/milvus-io/milvus.git
synced 2026-10-28 17:05:53 +08:00
yanliang567
f067780bd8
## What this PR does Strengthens compaction coverage in the PR gate and removes false-positive and flaky paths. - preserves terminal success/failure/timeout outcomes after task cleanup and uses `EndTime` (with a legacy `StartTime` fallback) for post-finish GC retention - resolves target-based manual compaction through durable target metadata: an executable ACTIVE target remains `Executing` across reconciliation batches, INACTIVE completes only after every task is classified, inert ACTIVE targets and unknown/GCed IDs remain undefined, and `compact_id=-1` remains the explicit no-op sentinel - keeps unknown task states and invalid cleaned terminal states fail-closed even when their target is runtime-active - refuses successful aggregation when any task state or cleaned terminal outcome is unclassified, and covers INACTIVE targets with still-executing tasks - makes the Python compaction waiter consume structured plan counts with context metadata and one monotonic deadline across raw RPC attempts, reconnect, and backoff - restores channel recovery for `UNAVAILABLE` through the synchronous, timeout-aware SDK `reconnect()` interface without leaving a recovery worker that can mutate the handler after timeout - polls compaction-eligible segments through a single raw `GetPersistentSegmentInfo` RPC per observation, preserving call context and status mapping without the SDK retry decorator reusing a stale timeout - retries transient gRPC and rate-limit failures within the remaining deadline, and recognizes both current `code=600` and legacy `error_code=SegmentNotFound` wire formats - retries recoverable `Status.CONNECT_FAILED` reconnect-handshake failures in segment polling and PK pagination within the existing deadline/backoff; non-recoverable handshake errors still propagate - disables auto compaction before topology/concurrency setup and validates MixCompaction source/target multiplicity, exact topology, row count, PK uniqueness, and exact PK set - proves explicit ForceMerge `target_size` behavior with a fixed `segment.maxSize == 64MB` fixture: equivalent datasets must produce 3 outputs at 64MB and 2 outputs at 128MB - exposes finite ForceMerge QueryNode/DataNode memory factors in DataCoord system metrics; non-finite dynamic values fall back to the default before serialization - reads the unfiltered system topology and skips the target-size comparison when QueryNode/DataNode metrics are erroneous or the observed machine-safe ceiling cannot preserve the larger effective target - directly regression-tests the L3 machine-safe wrapper with healthy typed nodes plus an untyped metrics error, preventing a future return to filtered topology input - verifies every planned ForceMerge source leaves the active topology, every exposed target becomes active, the complete PK set survives, and physical rewrite bytes are conserved - releases and reloads the collection before MixCompaction and ForceMerge PK validation, then verifies the complete loaded segment set, retired-source exclusion, and node assignments so readable old inputs cannot mask missing compaction outputs - submits reload asynchronously through single-attempt SDK calls and polls loading progress with fresh remaining budgets; release, load submission, progress, and topology checks share one monotonic deadline - replaces `QueryIterator` validation with deterministic PK-range pages; every raw query RPC receives a freshly calculated remaining timeout, retains MilvusClient `cluster_id`, and retries transient gRPC/rate-limit failures within the same total deadline - verifies reconnect receives only the remaining pagination deadline, an over-budget reconnect cannot issue another query RPC, ignored gRPC codes and real rc83 non-rate-limit business statuses fail fast, and both current and legacy `(code=0, compatible_code=49)` rate-limit mappings retry within budget - preserves valid ForceMerge QueryNode/DataNode memory factors, while non-finite values fall back and all finite factors below 1—including positive fractional values—clamp to 1 - sustains checked insert/CreateIndex/search workloads across compaction and propagates every worker failure, including the last in-flight request during cleanup; an Executing sample counts as overlap evidence only when the same workload call spans the observation - waits for two matching Flushed/L1/sorted source snapshots before the index workload, then expects unselected sources plus Mix targets; selected sources must retire and every loaded segment must have an index and node assignment - validates final PKs, indexed targets, and search results before marking an unobserved overlap window as insufficient coverage (pytest skip); repeated CreateIndex requests establish request concurrency, not repeated index builds - validates complete binary-search results against exact Jaccard ground truth - adds DataCoord aggregation/target lifecycle UT and compaction stats-loader UT coverage `CompactionMergeInfo` exposes one legacy `target` field although a ForceMerge task may split into multiple result segments. Split-output validation therefore requires each exposed target to be active and separately requires every planned source to be retired; complete active-output multiplicity is verified through the persistent-segment topology. Fixes #53272 ## Test plan - [x] Partial-plan review follow-up (2026-10-22): 82 focused local tests passed with Python 3.12 / PyMilvus rc98, plus Ruff lint/format and diff checks. The regression invokes the actual index E2E case with an 8-of-9 Mix plan: the prior implementation times out, the fix accepts the valid indexed target plus unselected source. Negative cases reject retained selected sources, missing/unexpected/unindexed/unassigned segments, and missing PKs; the ORM adapter exercises sorting convergence and context preservation. - [ ] Live index concurrency rerun is blocked: both Dev CLI deployment-list and `deploy status compaction-pr-ci` requests return HTTP 500. No live-cluster pass is claimed for this follow-up. - [x] Review follow-up on 2026-10-22: Python 3.12 / pinned PyMilvus 3.1.0rc98 focused suite: 72 passed; Ruff lint/format and diff whitespace checks passed; all three revised concurrency E2E cases collect successfully. Fault injection covers fast completion, unobserved overlap, worker failures before and after terminal observation, and cleanup. Real SDK loading regressions fail on the previous implementation (5-second budget returns at t=8) and pass with fresh per-poll budgets. - [ ] Re-run the three sustained-workload cases and release/reload integrity cases against a live cluster; this follow-up has local regression coverage only. - [ ] Full `make verifiers` attempted on 2026-10-22 but stopped in `build-3rdparty`: this macOS host lacks a supported LLVM 14–18 toolchain. - [x] Review follow-up on 2026-10-20: Python 3.12 / pinned PyMilvus 3.1.0rc98 focused suite: 56 passed; Ruff lint/format, Python syntax, and diff whitespace checks passed. Reconnect regressions exercise the actual SDK reconnect/readiness functions; topology tests inject delayed/partial/stuck loading, reload failures, and late responses. - [ ] Re-run live MixCompaction / ForceMerge integrity E2E with the new release/reload validation; the review follow-up was verified locally with fault-injection tests and server-path inspection. - [x] Go formatting, Python 3.12 compile, Ruff check/format, protobuf submodule compile, and `git diff --check` - [x] Local Python 3.12 waiter/raw-adapter/pagination/machine-safe matrix: 38 passed - [x] Local focused paramtable + metrics UT: `go test -tags dynamic,test -gcflags='all=-N -l'` - [x] Dev CLI exact non-finite memory-factor/serialization UT: `go-ut-local-yanliang-test-compaction-pr-5aa7d24-ce47cff0-u3805` - [x] Dev CLI exact full memory-factor preservation/fallback matrix: `go-ut-local-yanliang-test-compaction-pr-fab7e57-ce47cff0-u3805` - [x] Dev CLI exact fractional-boundary memory-factor matrix: `go-ut-local-yanliang-test-compaction-pr-3152b2a-ce47cff0-u3805` - [ ] Dev CLI targeted retry L0 could not start pytest because the runner Pod could not resolve the COS source host: `pysdk-e2e-local-yanliang-test-compaction-d642871-420126f0-u3805`; the same 3-case rc83 test passes locally - [x] Dev CLI current server image build: `build-local-yanliang-qia-test-compac-5aa7d24-bf331245b298-u3805` - [x] Deploy `harbor.tencent-stage.zilliz.cn/manta/milvus:local-u3805-test-compaction-pr-ci-coverage-5aa7d24-bf331245b298` to `compaction-pr-ci`; verify `Healthy` and `updated=true` - [x] Dev CLI current DataCoord + metrics UT: `go-ut-local-yanliang-test-compaction-pr-d3b086b-57710439-u3805` - [x] Dev CLI exact GC-retention suite: `go-ut-local-yanliang-test-compaction-pr-c9996b9-29281152-u3805` - [x] Dev CLI full `internal/compaction` + `internal/datacoord` Go UT for the broader PR changes: `go-ut-local-yanliang-test-compaction-pr-7b125fe-6ba90598-u3805` - [x] Dev CLI real no-op trigger plus unknown-ID E2E (3/3): `pysdk-e2e-local-yanliang-test-compaction-710c3bc-cef493b1-u3805` - [x] Dev CLI exact binary-search validation (1/1): `pysdk-e2e-local-yanliang-test-compaction-0728a4d-714e12a8-u3805` - [ ] Run the final fixed 64/128MB ForceMerge topology/full-PK E2E. The latest local-source runner still fails before pytest because its Pod cannot resolve the COS source host: `pysdk-e2e-local-yanliang-test-compaction-5aa7d24-4e733021-u3805`; the PR-source channel independently cannot resolve GitHub: `pysdk-e2e-pr-53273-5aa7d24-8424db8b-u3805` - [ ] Re-run the final Mix multiplicity assertion and insert/index result-binding cases after the same source-runner DNS incident is resolved ## Notes Upgrade, chaos, performance, and experimental-configuration coverage remain lower-priority follow-up work. Signed-off-by: Yanliang Qiao <yanliang.qiao@zilliz.com>