Skip to content

test: cover IVF remote stream lifecycle regressions - #28672

Draft
VioletQwQ-0 wants to merge 3 commits into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-28378-regressions
Draft

test: cover IVF remote stream lifecycle regressions#28672
VioletQwQ-0 wants to merge 3 commits into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-28378-regressions

Conversation

@VioletQwQ-0

@VioletQwQ-0 VioletQwQ-0 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #28378

What this PR does / why we need it:

Add regression coverage for queued stream cancellation, shuffle receiver retirement, and distributed IVF query cleanup. Production fixes are already present through #28358, the relevant dispatch/remote pipeline changes in #28364, and #28539; this PR changes tests only and does not establish their individual causal contribution to the original TKE incident.

  • Observe serialized MORPC sequence numbers after a queued message is canceled. The identical test compiles on the fix(morpc): avoid client stream sequence gaps #28358 parent and fails with expected sequence 2 versus actual 3.
  • Cover first, middle, last, and consecutive shuffle receiver retirement, checking both subsequent sends and one-time notification. Exercise query/connection cancellation during real receiver credit waiting through the CN handler, preserving the terminal error and releasing lifecycle registration.
  • Add a two-CN IVF integration test with 65,536 128-dimensional vectors, eight concurrent sessions, exact full-probe comparison, approximate-result validity, empty results, and subsequent queries. Test preparation failure through the actual phase runner, transaction rollback, worker exit, and database cleanup. Preparation and query budgets are cumulative but separate; cleanup gets its own context. SQL pre-cancellation is not presented as proof of active remote cancellation.
  • Extend test/distributed/cases/vector/vector_ivf_multicn_search.sql and .result with cosine, raw-vector IP/normalized-query, empty-result, and follow-up checks. The GROUP_CONCAT metadata expectation uses the existing server TEXT mapping and Connector/J 8.0.15 metadata; results were not regenerated blindly.

Validation (final file contents include the follow-up assertion fix; commits 231239efba5eca40bde92ed3932d44aa4ffc3bba and 4e6fe7b3049ba2c6a4808c2f6130dab61851a072, product base 2a6fe7f61358e846a20aafba3b50903a61608682):

  • macOS arm64, Go 1.26.4, repository CGo wrapper, GOMAXPROCS=4, test package parallelism 1.
  • New mechanism tests: ordinary 100 iterations and race 20 iterations passed. MORPC, dispatch, and compile packages passed ordinary/race checks; unchanged evidence was reused.
  • Follow-up P2 validation: TestRemoteNotifyCancellationReleasesCreditWaitAndRegistration passed ordinary 100 iterations and race 20 iterations after asserting that connection cancellation closes proc.Ctx, records a non-nil ErrStreamClosed cause, and preserves it through repeated terminal cleanup.
  • After the follow-up edit, the complete owning ./pkg/sql/compile package passed once under the race detector; no race report was emitted.
  • TestIssue28313EmptyRemoteDispatch and TestIssue28378IVFFlatRemoteLifecycle: one ordinary and one race run passed, including injected preparation failure and the full IVF workload.
  • Formal BVT: CN1 and CN2 JDBC entry points each passed 36/36 comparisons with full metadata checking. go vet, gofmt, and diff checks passed.
  • The MORPC parent counterexample validates the sequence mechanism. The IVF integration test is regression coverage, not a deterministic reproduction of the historical incident. Exact PR-head Linux CI remains pending.

Deployment evidence for the already-fixed product base, independently of this new test commit:

  • Old baseline run 34352473993: HNSW/L2/cosine/normalize errors were 0/1702/1266/588.
  • Fixed-product run 34464342049: all four original million-vector SIFT128 cases completed with 100 terminals, five-minute measurements, and zero client errors. The five data hashes match the failed baseline. Acceptance was checked from artifact counters and error files, not only workflow success.

QA required: yes — distributed timing and deployed-workload acceptance. Automated terminal coverage includes completed SQL assertions, cancellation waiter exit, preserved terminal errors, registration removal, and cleanup. QA should review the target release's inclusion of the existing production fixes and the attached run evidence. First-error query/stream/peer correlation and unique root-cause attribution remain unavailable; this PR must not auto-close #28378.

@VioletQwQ-0

Copy link
Copy Markdown
Collaborator Author

Addressed the P2 connection-cancellation oracle gap in follow-up commit 4e6fe7b3049ba2c6a4808c2f6130dab61851a072.

The connection subtest now asserts, before terminal publication and registry cleanup, that proc.Ctx.Done() is closed, context.Cause(proc.Ctx) is non-nil, and the cause is ErrStreamClosed, in addition to the existing wire-error check. This prevents a handler-only cleanup path from passing while the owning query remains alive.

Validation after the edit:

  • focused test: ordinary 100 iterations passed;
  • focused test: race 20 iterations passed;
  • complete ./pkg/sql/compile package: one race run passed with no race report;
  • gofmt and git diff --check: passed.

The BVT plan-path enhancement remains a non-blocking follow-up; this change addresses the blocking P2 only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/test-ci size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: IVFFlat SIFT128 queries close RPC streams on main

2 participants