Skip to content

fix(compile): preserve remote pipeline receiver ownership - #26603

Merged
XuPeng-SH merged 3 commits into
mainfrom
fix/26595-remote-pipeline-ownership
Aug 3, 2026
Merged

fix(compile): preserve remote pipeline receiver ownership#26603
XuPeng-SH merged 3 commits into
mainfrom
fix/26595-remote-pipeline-ownership

Conversation

@XuPeng-SH

Copy link
Copy Markdown
Contributor

What type of PR is this?

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

Which issue(s) this PR fixes:

Fixes #26595

What this PR does / why we need it:

Root cause

A distributed shuffle FULL OUTER JOIN creates multiple DOP receiver buckets per CN. A dispatch attached to one bucket can target local receivers owned by sibling buckets. When a following global operator such as WINDOW calls the common merge path, each bucket becomes a separate RemoteRun tree, so the dispatch-bearing tree is not standalone executable and is rejected before remote start.

Fix

  • Extract the existing standalone receiver-ownership check into a pure compile-time detector while retaining the RemoteRun fail-fast guard.
  • At the common newMergeScope boundary, group an invalid remote input stage by CN so each serialized tree owns every in-process receiver it targets.
  • Preserve the direct path for local dependencies and independent remote inputs.
  • Add focused Dispatch and Connector counterexamples plus a distributed FULL OUTER JOIN to WINDOW topology regression.
  • Add an end-to-end multi-CN BVT using the existing optimizer shuffle data.

Validation

  • Pre-fix focused counterexamples reproduced the defect: Dispatch and Connector inputs remained split, and FULL OUTER JOIN to WINDOW produced four non-standalone RemoteRun inputs instead of two per-CN trees.
  • Focused compile tests passed.
  • Full pkg/sql/compile passed under normal and race modes.
  • New topology tests each passed under race with count 100.
  • go list, go build, and go vet passed for pkg/sql/compile.
  • A latest-main binary at d29b475 passed the exact BVT SQL on a real two-CN cluster: MULTICN plan, FULL OUTER range shuffle, Window, and result both=2 / left_only=2 in 0.161s.

Lifecycle audit

  • Q1 ownership: regrouped scopes, receivers, and connectors remain in the existing scope tree and retain single cleanup ownership.
  • Q2 unhappy paths: remote-start and merge failures keep the existing sibling cancellation and fail-before-start cleanup paths.
  • Q3 boundedness: detection is bounded by the input scope tree and added merge scopes are bounded by the CN count.

Scope and risk

No API, wire, catalog, or storage-format changes. Regrouping is compile-time only and triggers only when a remote input would otherwise be non-standalone. Independent stages retain their existing topology.

@XuPeng-SH
XuPeng-SH requested review from aunjgr and heni02 as code owners August 3, 2026 03:59
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@mergify mergify Bot added the kind/bug Something isn't working label Aug 3, 2026
@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label Aug 3, 2026
@mergify mergify Bot added kind/test-ci kind/refactor Code refactor labels Aug 3, 2026
Distributed shuffle joins can leave a dispatch in one DOP bucket targeting local receivers owned by sibling buckets on the same CN. A following global merge then treats each bucket as an independent RemoteRun tree and rejects the dispatch-bearing tree.

Detect this ownership violation at the common merge boundary and group the stage by CN before creating RemoteRun units. Keep independent pipelines unchanged and retain the runtime standalone guard.

Add focused topology coverage and a multi-CN FULL OUTER JOIN plus WINDOW regression.

Fixes #26595

@LeftHandCold LeftHandCold left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep-reviewed head 8d9b02d: no blocking issues found.

I traced the complete path from newMergeScope regrouping through per-CN scope construction, standalone validation, RemoteRun root stripping/serialization, local and remote dispatch registration, receiver notification, cancellation, and cleanup.

The regrouping is gated to non-local trees with an external in-process receiver dependency. Independent remote inputs and local dependencies retain the existing topology. Each original scope is attached exactly once, receiver/end counts remain consistent, wrapper growth is bounded by CN/input count, and error paths retain sibling cancellation plus bounded cleanup.

Focused race tests for the new Dispatch/Connector counterexamples, independent-input fast path, FULL OUTER JOIN -> WINDOW topology, and the existing standalone detector passed 3 consecutive runs with -p=1. The PR is based on current main, diff check is clean, and current GitHub checks have no failures or pending jobs.

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex automated review

No findings. The per-CN regrouping restores remote receiver ownership while preserving local and independent-input paths, cancellation, cleanup, boundedness, and compatibility. Tests cover Dispatch, Connector, fast-path, and FULL OUTER JOIN→WINDOW topologies. Diff and formatting checks are clean; focused local execution was unavailable because this worktree lacks MatrixOne’s native CGo prerequisites.

@aptend aptend left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the receiver-ownership regrouping and RemoteRun lifecycle paths. The focused topology tests, package tests, race tests, build, and vet pass locally.

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

Labels

kind/bug Something isn't working kind/refactor Code refactor kind/test-ci size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Full Outer Join + CTE: remote pipeline is not standalone executable (20101)

6 participants