Repository navigation
Conversation
2 tasks done
XuPeng-SH
added a commit
to matrixorigin/matrixone
that referenced
this pull request
Nov 12, 2026
## What type of PR is this? - [x] BUG - [x] Test and CI ## Which issue(s) this PR fixes: Fixes #32392 Fixes #32420 Fixes #32424 Fixes #32561 Fixes #32562 Fixes #32563 Refs #32324 Refs #32544 Refs #32570 Refs #32185 ## What this PR does / why we need it: Native build preparation was skipped when GNU Make put a command-line variable assignment first in `MAKEFLAGS`, causing the final provenance record to fail. The preparation recipe now lets Make decide whether it runs in dry-run, touch and question modes, instead of parsing Make flags and assignments. Its cleanup sub-makes can fall back to serial execution and emit a jobserver warning with pipe-based GNU Make jobservers; the subsequent native build retains its existing recursion and parallelism. The real gossip bind-collision test now uses the same wildcard TCP address-family selection as memberlist and asserts that the collision is from the expected gossip TCP host and port. On macOS, the former IPv4-only injection could coexist with memberlist’s IPv6 listener, so the intended retry was never tested. The new native Make target regression also runs on the existing macOS native-contract job without adding another job or expanding the workflow’s path trigger. Several integration tests spend one deadline across independent SQL scenarios or teardown. A late scenario then starts with an expired context, and dependent scenarios can continue on a canceled session. Give the independent subscription, temporary-table, branch-diff and prepared-parameter scenarios bounded lifetimes, stop dependent work after a failure, and keep cleanup with the owner that can perform it. The watermark test's final database drop also gets its own bounded cleanup context. The intentional 2.5-second cancellation check no longer leaves that limit on the following spill query. The full-text fixture pauses its single-CN CDC executor while its stage is deliberately invalid, restores the stage before resuming, and retains the same-job completion, watermark and MATCH assertions. The invalid-stage interval no longer exercises a live worker's recovery after a permanent job error. The prepared-parameter test keeps its complete protocol/read/write matrix. Session-bound PREPARE and wire-rewrite tests now pin the connection; numeric negative tests check the actual server error, and a failed fixture does not start competing table or database drops. Each matrix case sets its text-protocol variables once, including when only the write leaf is selected. Test work is reduced by combining paired rollback checks, right-sizing small admission inputs, sharing spill result checks, and sharing one 8,193-row aggregate/sort fixture. All six aggregate/sort transitions and per-row checks remain. This replaces the previous 16,384-row aggregate input and 32,768-row sort input; the sort no longer covers that incidental four-block scale. The linked refs are deliberately not closed by this PR. In #32324 the reported line is the final DROP, whose prior shared context had nearly expired; the reason for the slow server operation and its unknown commit outcome remain unproven. In #32544 the branch-diff deadline cascade is addressed, but a separate prepared-cache flush timeout remains. #32570 also tracks five other timeout roots whose causes are not established by this parameter-test fix. ## Validation - Gossip collision: on macOS/arm64 Go 1.27.1 the original real-service normal and race tests failed at the reported retry assertion; the candidate passed in both modes and logged EADDRINUSE on the exact gossip TCP port, followed by rollback and successful retry. A real Raft-port collision made the old assertion falsely pass but is rejected by the new source/port assertion. Linux final targeted normal and race tests passed, as did the owning package normal run before that assertion-only addition. - Native build: the complete production Makefile target passed isolated Linux tests with GNU Make 3.81 and 4.4.1, including real short/long options, assignment values, recursive Make, clean ordering and injected clean failures. With real provenance and the same prebuilt native artifacts in source-archive copies, GNU Make 3.81 failed before the change at the missing completion marker and succeeded afterward with the same `make cgo` assignment. The GNU 3.81 before/after control was an incremental Linux build; a separate GNU 4.4.1 Linux run rebuilt the CGo and third-party libraries from cleaned source using a checksum-verified prebuilt ONNX library. Both ran in source-archive copies, whose artifacts are correctly marked non-reusable. macOS/arm64 and GPU have not been tested. Real rebuild-all cleanup was also checked under 3.81; the candidate produced two jobserver-unavailable warnings and completed all cleanup. The CI Node suite passed 50 tests; the targeted Make regression also passed on macOS/arm64 with Node 22 and system GNU Make 3.81. - Incremental SCA: repository `static-check-selected` with pinned `molint` and `golangci-lint` for `pkg/embed`, `pkg/iscp`, `pkg/logservice`, `pkg/tests/dml`, `pkg/tests/issues`, `pkg/tests/sqlintegration` and `pkg/tests/upgrade`; all passed. - Before the Makefile-only addition, Linux/amd64, Go 1.27.1, CGO, `GOEXPERIMENT=simd`, `-short -tags matrixone_test`: all six owning packages passed in normal mode; the Go source and tests covered by those runs had not changed at that point. Complete `pkg/embed` and `pkg/tests/sqlintegration` packages passed under `-race`; the changed ISCP, DML, upgrade and issues roots also passed under `-race`. The complete prepared-parameter root passed three times, with all 320 named test events present on each run. - Controlled source overlays exercised expired phase contexts, stage/permission restoration failures, owner-session loss and real SQL timeouts. They verified failure propagation and fixture quarantine. In particular, a canceled context and an ODBC write failure made the old negative/protocol tests pass incorrectly; the corrected tests rejected both. Injected-failure runs intentionally exited nonzero. - The aggregate/sort result checks now read 49,158 rather than 147,456 rows across the six transitions. Separate aligned `EXPLAIN ANALYZE` controls still observed both aggregate and sort spilling at the low threshold; those controls are separate queries, not timings of the original six stages. Short before/after samples do not establish a reliable CI wall-time improvement. - Design and complete-diff review of those earlier changes: `gpt-6.1-terra` with max reasoning; review findings were corrected and revalidated. ### Additional #32185 range-scan panic fix The rebased branch also handles tombstone objects whose object-level rowid zonemap is uninitialized. Such metadata cannot prove that the target rowid is absent, so the scan now delegates to the existing owner-aware tombstone collector. Initialized summaries keep the existing O(1) negative-prune path. If collection fails after an earlier candidate contributed rows, the partial result batch is closed before the error returns. The tests cover a real empty tombstone flush/replay, a non-empty in-memory tombstone with unknown object stats, valid-summary pruning, persisted-object differential behavior, missing object data, and partial-result cleanup. The old-code overlay reproduces the `RowidPrefixEq` panic for the empty-flush case. This addresses only the uninitialized object-level-ZM scan panic reproduced by the empty tombstone lifecycle. It does **not** fix the independent historical `GetValue` row-offset panic or establish the state of the object in the original CI failure; #32185 remains open and is referenced, not closed. No latency or allocation improvement is claimed: initialized summaries add one O(1) initialized-bit check, while unknown non-empty candidates use the necessary existing collector path. ### Validation on rebased HEAD `1ec57307c7` (based on main `c8accd138b`) - `pkg/vm/engine/tae/tables`: full package normal and `-race` both passed (127 terminal test events each). - `pkg/vm/engine/tae/db/test`: full package `-race` passed (318 test PASS events, one pre-existing `TestUpdate` SKIP; 73.981s). - The exact uninitialized-ZM regression and adjacent scan/lifecycle controls passed adaptive race repetitions (100/100 each; empty-flush replay 50/50); the exact regression also passed normal mode. - `TestCollectAppendWaitsForTxnPrepareTS`: adaptive race repetition passed 100/100. - Incremental pinned `static-check-selected` for both changed TAE packages passed with 0 issues. - Independent `gpt-6.1-terra` max review: **APPROVE** for this commit and bounded fix scope. The review explicitly notes that this is not a complete fix for #32185. The pre-rebase PR validation above remains labeled where applicable; the branch patch IDs were preserved across rebase, but the evidence here is the exact current-HEAD TAE rerun. ### CI failure follow-up on `071380402a` The race UT on `1ec57307c7` passed both SQL topologies in `TestIssue28234ExceptAllCluster` but failed during cluster shutdown when CN final metadata withdrawal timed out. The cluster's LogService had already logged repeated Raft tick and heartbeat timeouts, while the runner showed elevated I/O wait before and during the embedded-cluster stage. The available evidence does not establish the source of the runner's I/O stall or prove that this change prevents it. The concurrently running restore test used a fresh private cluster, yet performed six unchecked catalog-cleanup statements at setup and teardown. Remove that redundant work and the resulting unused helper; the two account drops required to exercise restore, all restore/permission assertions, and cluster close remain. In a small local ABBA comparison under 8 CPU/16 GiB caps, the restore root took 38.53/42.25 seconds on the baseline and 36.54/31.55 seconds with the cleanup removed. These are two samples per arm on local NVMe, not a guarantee for a loaded CI runner. The restore, shutdown, and EXCEPT ALL test roots passed in both normal and race modes before the final deletion of the now-unused helper. Incremental `static-check-selected` for `pkg/tests/issues/isolated` passed on the final source. The restore and EXCEPT ALL roots also passed a separate final-HEAD race rerun (34.60s and 9.83s, respectively). The failure artifact itself dropped the 1.2 MiB failed-test output because the raw report consumed the budget. Its reusable workflow lives in another repository; the independently tested fix is matrixorigin/CI#476.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The UT diagnostic artifact for matrixorigin/matrixone#32574 exhausted its 250 MiB budget on the raw report and omitted the actual failing test output. This makes an already expensive CI failure harder to diagnose.
Preserve the failure summary, stderr, checkpoint and failing test output before the raw report, and share the available budget among these critical files so one oversized diagnostic cannot consume every other failure. Keep head/tail truncation and the manifest for oversized files; also honor budgets smaller than the manifest reservation.
Validation:
python3 scripts/test_ut_diagnostic_artifact.py -v: four checks covering oversized reports, multiple oversized failures, oversized stderr, and a budget smaller than the manifest reservation. The original workflow fails the first two regression cases.git diff --check.The MatrixOne CI test's I/O stall and heartbeat timeout are investigated separately; preserving diagnostics does not claim to fix the stall.