Repository navigation
test: repair CI TERM handshakes, port candidates, and TopN coverage - #29978
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
mo-bot work · Commit: new same-head PR feedback/CI evidence requires a fresh task |
|
mo-bot work · Commit: Coverage fails because TRUNCATE lifecycle tests do not expect Snapshot(). Independence from this PR lacks the required baseline/control evidence. |
|
mo-bot work · Commit: Coverage aggregation fails because its upstream UT coverage producer failed. The upstream failure still lacks baseline/control evidence establishing independence from this PR. |
|
mo-bot work · Commit: Final review accepts the unchanged candidate and independent baseline-test diagnosis. Affected-package race and coverage tests and repository SCA pass. Overall CI still fails on the inherited TRUNCATE mock mismatch; reuse #29971 without an immediate rerun. Cause issue: #30006 |
|
mo-bot work · Commit: unresolved or indeterminate merge conflict requires a rebase plan; an independent CI diagnosis does not resolve it |
|
mo-bot work · Commit: Implementation is blocked by a coordinator handoff mismatch: the checkpoint reports rebased=true, but the checkout remains clean at the original head and lacks the diagnosed target commit. A coherent resolution patch is prepared in diagnostics; source remains unchanged. |
|
mo-bot work · Commit: new same-head PR feedback/CI evidence requires a fresh task |
842d806 to
9c45883
Compare
|
mo-bot review · Commit: Assessment: approve Reviewed the complete four-file delta, affected callers, cleanup paths, and frozen comments. TERM sequencing, ephemeral-port exclusion, and TopN fixture ownership are sound. No actionable defects remain. Model: Session: Validation:
Limitations:
|
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
mo-bot review · mo-bot-local · review
Commit: 9c45883ae41ba1d8192463e59cbb3c908baf1126
Assessment: approve
Reviewed the complete four-file delta, affected callers, cleanup paths, and frozen comments. TERM sequencing, ephemeral-port exclusion, and TopN fixture ownership are sound. No actionable defects remain.
Model: gpt-6-astra / xhigh
Session: 01a12d8f-8e48-7622-ad17-b605f08450ce · slot 1 · generation 0
Validation:
-
Verified HEAD, merge-base, patch identity, whitespace, and unchanged source checkout.
-
Traced TERM success, sender failure, timeout, and cleanup; port allocation/probe errors and gossip consumers; TopN partial construction, ownership transfer, reset, and cleanup.
-
A standard-library-only probe using extracted source passed 348,275 configured ranges, single-port exhaustion, and equal-interval controls. This was not a MatrixOne integration test.
-
Reused author-reported exact-candidate normal/race package tests, focused coverage tests, TERM stress and mutation controls, gossip restart repetitions, and static checks.
-
Recorded complete coverage and comment dispositions in attempt-1/review-evidence.json.
Limitations:
-
Fresh CI remained pending in the frozen snapshot. No pending check was counted as passing, and native package tests were not rerun.
-
Raw author logs and original linked-issue bodies were unavailable; validation reuse relies on the frozen reports identifying the verified commit.
-
Port probes still permit races with explicit binders, and customized non-Linux ephemeral ranges are not inspected. These limits are documented.
-
No performance benchmark or full coverage-threshold verdict was produced.
This branch had an error being deployed
3 participants
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #29960
Fixes #29968
Related CI failure: #29921.
What this PR does / why we need it:
The TERM/PID fixture could acknowledge a signal before its trap returned. The sender then issued the next TERM too early and stalled waiting for an acknowledgment. A timeout's extra TERM could release the stall and misleadingly print
TERMS 200. Acknowledgments now come from ordinary main-loop execution after the trap returns, with sequence/readiness assertions. A sender failure publishes its completion marker so the main process can join it and report the protocol error promptly.Logservice test listener candidates previously overlapped automatic client-port allocation. Probes closed their sockets before service startup; outbound traffic could occupy a candidate in that gap. On Linux, the existing allocator now excludes the configured inclusive
ip_local_port_range, choosing the larger remaining interval while preserving its probes, process-local uniqueness and bounded attempts. The existing gossip topology and restart/convergence assertions remain exercised.TopN's unsupported-type regression used
T_any, whose domainless NULL values now have a comparator. It now rejectsT_tupleand positively exercises flat/constant all-NULL ANY keys across reset generations with source-row and mpool cleanup assertions. This adopts Lundomn's original #29975 commitfbc1abc9f7bfdda6d907904d03ef34cb4793eac8, preserving authorship, and closes partial batch-construction cleanup in a follow-up. The CI corrections are consolidated here as requested.Validation
Linux/amd64, Go 1.27.1, CGo enabled,
matrixone_test, bounded build/test concurrency. Matching native source/platform/profile provenance was verified before using the repository CGo test wrapper.optools,pkg/logservice, andpkg/sql/colexec/partitionpackages pass in normal and race modes.gpt-6.1-sol/xhighproduced the design and reviewed the complete final change. No unresolved material findings.The fix adds no sleeps, retry loops, reduced topology, or skipped coverage. Port probing still has an interval before the actual bind, so independent explicit binders can race it. Non-Linux uses a conservative candidate ceiling suitable for default macOS ranges; customized non-Linux ephemeral ranges are not inspected. BVT is not applicable to these CI/test-fixture contracts. Remote CI is pending after publication.