Skip to content

enhance: reduce idle compaction allocations - #23

Merged
sunby merged 1 commit into
codex/load-1m-segments-pr-stack-rebased-qv-workfrom
codex/compaction-check-allocation-20260909
Sep 9, 2026
Merged

enhance: reduce idle compaction allocations#23
sunby merged 1 commit into
codex/load-1m-segments-pr-stack-rebased-qv-workfrom
codex/compaction-check-allocation-20260909

Conversation

@sunby

@sunby sunby commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Summary

Reduce temporary allocations in periodic schema-bump and storage-version/format compaction checks, especially when collections do not need new tasks.

  • Select stale eligible segments before cloning the collection schema. Keep the same published schema reference for selection and cloning, and share one frozen clone across a collection's views.
  • Allocate a storage-compaction trigger ID only when eligible candidates and task budget remain.
  • Remove the unconditional per-collection logger construction while preserving the external-collection log and collection ID field.
  • Check exhausted storage-compaction budget before materializing the collection list; preserve rate-window replenishment.

No new configuration, trigger intervals, eligibility rules, protocol or scheduler changes. Full scans remain; event-driven discovery is not part of this PR.

Base and reference

Base: sunby/milvus:codex/load-1m-segments-pr-stack-rebased-qv-work at b18065d489bcdc559e94b00b4bbc05ea758813e1 (includes the merged quota PR #22). This PR contains one compaction-only commit and five files.

Related scalability context: milvus-io#48867. This does not claim to resolve that QueryNode recovery issue.

Verification

Rerun on the submission base using -tags dynamic,test -gcflags="all=-N -l" and the existing local native libraries. These tests use in-memory metadata/mocks and do not require a running external etcd.

  • Schema/storage policy suites, schema-bump task suite and compaction trigger-manager suite passed across three repetitions, with TestCompactionTriggerManagerSuite/TestGetExpectedSegmentSize excluded as described below.
  • Both policy suites passed with -race.
  • Regression coverage verifies no schema clone/ID allocation without candidates, one frozen schema and trigger ID per collection, cache replacement during selection, unchanged candidate filters, TEXT downgrade protection, ID-allocation failure followed by successful retry, and exhausted/replenished task budget.
  • Ownership audit checked collection-cache publication and schema-alteration paths; they publish replacement collection objects. The task-submission and request-building paths continue to use the captured schema.
  • Running the new allocation/budget regressions against the unchanged baseline policy files reproduced their expected failures.
  • Changed-code golangci-lint, including the new benchmark file in the committed diff, passed: 0 issues. git diff --check passed.

Mandatory pre-push commands were run in a disposable validation worktree:

  • make lint-fix completed gofumpt/gci and ran root-module lint, then failed at internal/metastore/kv/querycoord/kv_catalog_test.go:372:15: undefined: mocks. The file has the identical blob in base and head. The pkg/client lint stages were not reached.
  • internal/core/run_clang_format.sh internal/core completed with clang-format 15.0.7. None of the five PR files needed formatter changes; unrelated formatter changes are excluded.

The existing local TestGetExpectedSegmentSize/all_DISKANN assertion fails both with and without the policy changes (expected 200 MiB, observed 100 MiB). It is not fixed in this PR.

Allocation benchmark

Apple M5, darwin/arm64, Go 1.26.5; the same 10,000-collection workload on the submission base, with baseline policies supplied through a Go overlay. Each collection has one up-to-date flushed V3 segment and a 16-field schema. Median of five runs, -benchtime=3x -benchmem -tags dynamic,test -gcflags="all=-N -l":

Policy check Before B/op After B/op Reduction
Schema bump 40,400,352 1,759,941 95.6%
Storage version 15,713,880 1,777,973 88.7%

The allocator is a local stub, excluding the production ID-allocation call path. These are synthetic allocation measurements, not production CPU/GC or search-latency improvements.

Full make test-go, production-build end-to-end acceptance and a same-load deployed MixCoord CPU/GC/latency comparison remain unverified.

Signed-off-by: sunby <sunbingyi1992@gmail.com>
@sunby
sunby merged commit 2fe7373 into codex/load-1m-segments-pr-stack-rebased-qv-work Sep 9, 2026
10 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant