Skip to content

feat(sirius): introduce execution backend contract - #28973

Merged
XuPeng-SH merged 5 commits into
matrixorigin:mainfrom
aunjgr:feature/28966-sirius-backend-contract
Sep 16, 2026
Merged

XuPeng-SH merged 5 commits into
matrixorigin:mainfrom
aunjgr:feature/28966-sirius-backend-contract

Conversation

@aunjgr

@aunjgr aunjgr commented Sep 16, 2026 •

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:

Refs #28966 (migration PR 2 of 10). Numeric compatibility remains separately tracked by #28968.

What this PR does / why we need it:

Introduce the service-owned Sirius backend/execution contract so the compiler
does not own concrete Flight types. The existing Flight adapter preserves its
prepare, result delivery, fallback classification, reconciliation and cleanup
behavior. No Flight protocol or native execution implementation changes here.

  • Record approved design version 1 and all ten PR boundaries in
    docs/design/sirius-embedded.md; no separate design-only PR.
  • Add sirius.backend, defaulting to flight during coexistence. embedded
    fails explicitly as unavailable in this build; unknown values fail closed.
    Both config validation and service startup reject unsupported selection
    before leases, certificates or sockets are initialized.
  • Make query cleanup depend on the narrow execution interface, retaining
    bounded cleanup after request cancellation and joined run/cleanup errors.
  • Preserve nil-backend validation and avoid a typed-nil execution on failed
    preparation. Keep malformed plans terminal rather than falling back.
  • Add fake-execution coverage for success, consumer failure, cancellation,
    cleanup failure and consumer panic; adapt existing real Flight/CN tests.
  • Correct two validation time budgets exposed by the first CI run: tenant
    fixture cleanup now has the scenario's bounded five-minute allowance, and
    the all-stage UT runner has a two-hour outer deadline. Per-package timeouts,
    assertions, stage selection and cancellation diagnostics are unchanged.

This is the MO foundation only. It does not yet implement CGo, enable embedded
queries, port #27599's MO-reader stream, change GPU scheduling, or remove Flight.
PRs 3-8 provide those components; PRs 9-10 remain blocked on all-22 parity and
numeric compatibility. The old #27599 branch is unchanged.

Validation

  • Focused compiler/CN Sirius tests passed.
  • Full pkg/sql/compile, pkg/sql/compile/sidecarflight, pkg/cnservice tests passed normally and under -race through mo-cgo-test.
  • Finalized new tests (including consumer-panic cleanup) passed normally and under -race.
  • make config passed; no module metadata changed.
  • Focused golangci-lint with the repository's Go 1.26.4 toolchain: 0 issues.
  • The original-head GitHub SCA, build, coverage and BVT jobs passed. Fresh
    exact-head CI is tracked separately below; no lint suppression was added.
  • GPU/SF10 tests are not claimed for this Go-only adapter foundation. The
    native synchronization is a separate Sirius PR.
  • Exact-head GitHub CI is pending.
  • Fresh post-main-merge validation with Go 1.26.4 and the existing GCC 15 CPU
    toolchain: tenant regression normal 11.708s, race 19.549s; all three
    compiler/Flight/CN owning packages passed normally and under race.
  • The UT-runner optools package passed (6.175s); incremental lint for
    compiler, CN and issue-test packages reported 0 issues. Test selection,
    SQL assertions and cleanup error checks are unchanged.

Review scope

R0: design and delivery map. R2: backend/adapter/config consumer boundary.
Lifecycle review follows admitted lease -> Prepare/execution -> Run ->
Cleanup/retained reconciliation -> backend Close. The adapter adds no workers,
queues or retry state. Existing Flight cancellation/reconciliation tests remain
in the owning-package normal/race evidence. No new SQL behavior or wire
representation is enabled, so no unrelated BVT/SF10 run is substituted for
the focused real CN/Flight consumer tests.

Lifecycle check Result for this PR
Q1: ownership Prepare passes the same release callback to Flight; execution/reconciliation retain their existing cleanup owner. Failed preparation returns a genuinely nil interface.
Q2: waits The adapter adds no locks or waits. Cleanup still uses an independently bounded, uncanceled context and Flight's cancel/join acknowledgement.
Q3: growth One adapter per CN; no new queue, worker, retry collection or batch retention.

CI failure diagnosis and closure

The first run's complete UT diagnostics identified two separate cutoffs:

  • TestTenantCatalogRegressions passed its scenario assertions, then its
    DROP ACCOUNT fixture cleanup exceeded 30 seconds while catalog DDL was still
    progressing. Cleanup now uses the same bounded five-minute allowance as the
    scenario; SQL assertions and cleanup error checks remain intact.
  • The overall runner received SIGTERM at its 70-minute outer deadline after
    light, exclusive-issues and embedded stages consumed most of that allowance.
    Heavy/engine tests were progressing and plan testing had not completed. The
    outer budget is now 120 minutes, with the existing per-package timeout and
    TERM/KILL diagnostic handling unchanged. No parallelism increase or test skip.

CI evidence: original failed UT job.

Current-main integration includes 125d943786ecdf59be842ce381b9cc22d6a5c8bc.
Delivery head: 399e0ee601.
This foundation is independently reviewable; it does not wait for migration
stages 3–8 to implement the native backend.

@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 →

@XuPeng-SH XuPeng-SH 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 review @ 399e0ee

结论:approve。

Design gate:PASS。docs/design/sirius-embedded.md version 1 明确记录已批准的十 PR 计划,并定义本 PR 为 2/10 的 backend contract/Flight adapter 边界;native ABI、协议和后续 cutover 不属于本 PR。

实现与生命周期检查通过:

  • Flight adapter 保留 Prepare 参数、失败清理和 fallback 分类;
  • nil runtime 和 Prepare 失败不会产生 typed-nil execution;
  • unsupported backend 在 lease、证书和 socket 初始化前 fail closed;
  • Prepare/Run、cancel/join、partial failure、cleanup/reconciliation、consumer panic 路径未发现新增所有权或 liveness 问题;
  • teardown 30 秒到 5 分钟以及 UT 外层 70 到 120 分钟的调整仍保留错误检查、TERM/KILL 和既有 per-package timeout,未发现 skip、retry、sleep 或断言弱化。

exact-head UT、BVT、SCA、build、coverage 和 CI Required 均通过。未发现 P1/P2 或 mandatory design blocker。

@mergify mergify Bot added the queued label Sep 16, 2026
@mergify

mergify Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-09-16 13:38 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🟠 Checks running · in-place
  • 🚫 Left the queue — 2026-09-16 13:39 UTC · at c61f3dd4ec1f5fc1d585ab7bf546f23c011c5ce1

This pull request spent 1 minute 15 seconds in the queue, with no time running CI.

Waiting for any of
  • check-neutral = CI Required
  • check-skipped = CI Required
  • check-success = CI Required
All conditions

Reason

Pull request #28973 has been dequeued

Pull request from fork cannot be queued. This pull request comes from a fork, and Mergify needs the author's permission to update its branch.

The author needs to enable "Allow edits from maintainers" on this pull request.

Hint

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

  • Requeue this pull request

@XuPeng-SH
XuPeng-SH merged commit c7554a5 into matrixorigin:main Sep 16, 2026
27 of 29 checks passed
@mergify mergify Bot added dequeued and removed queued labels Sep 16, 2026
VioletQwQ-0 pushed a commit to VioletQwQ-0/matrixone that referenced this pull request Sep 17, 2026
## What type of PR is this?

- [x] API-change
- [ ] BUG
- [ ] Improvement
- [ ] Documentation
- [x] Feature
- [x] Test and CI
- [x] Code Refactoring

## Which issue(s) this PR fixes:

Refs matrixorigin#28966 (migration PR 2 of 10). Numeric compatibility remains
separately tracked by matrixorigin#28968.

## What this PR does / why we need it:

Introduce the service-owned Sirius backend/execution contract so the
compiler
does not own concrete Flight types. The existing Flight adapter
preserves its
prepare, result delivery, fallback classification, reconciliation and
cleanup
behavior. No Flight protocol or native execution implementation changes
here.

- Record approved design version 1 and all ten PR boundaries in
  `docs/design/sirius-embedded.md`; no separate design-only PR.
- Add `sirius.backend`, defaulting to `flight` during coexistence.
`embedded`
fails explicitly as unavailable in this build; unknown values fail
closed.
Both config validation and service startup reject unsupported selection
  before leases, certificates or sockets are initialized.
- Make query cleanup depend on the narrow execution interface, retaining
bounded cleanup after request cancellation and joined run/cleanup
errors.
- Preserve nil-backend validation and avoid a typed-nil execution on
failed
  preparation. Keep malformed plans terminal rather than falling back.
- Add fake-execution coverage for success, consumer failure,
cancellation,
cleanup failure and consumer panic; adapt existing real Flight/CN tests.
- Correct two validation time budgets exposed by the first CI run:
tenant
fixture cleanup now has the scenario's bounded five-minute allowance,
and
the all-stage UT runner has a two-hour outer deadline. Per-package
timeouts,
assertions, stage selection and cancellation diagnostics are unchanged.

This is the MO foundation only. It does not yet implement CGo, enable
embedded
queries, port matrixorigin#27599's MO-reader stream, change GPU scheduling, or
remove Flight.
PRs 3-8 provide those components; PRs 9-10 remain blocked on all-22
parity and
numeric compatibility. The old matrixorigin#27599 branch is unchanged.

### Validation

- Focused compiler/CN Sirius tests passed.
- Full `pkg/sql/compile`, `pkg/sql/compile/sidecarflight`,
`pkg/cnservice` tests passed normally and under `-race` through
`mo-cgo-test`.
- Finalized new tests (including consumer-panic cleanup) passed normally
and under `-race`.
- `make config` passed; no module metadata changed.
- Focused golangci-lint with the repository's Go 1.26.4 toolchain: **0
issues**.
- The original-head GitHub SCA, build, coverage and BVT jobs passed.
Fresh
exact-head CI is tracked separately below; no lint suppression was
added.
- GPU/SF10 tests are not claimed for this Go-only adapter foundation.
The
  native synchronization is a separate Sirius PR.
- Exact-head GitHub CI is pending.
- Fresh post-main-merge validation with Go 1.26.4 and the existing GCC
15 CPU
toolchain: tenant regression normal **11.708s**, race **19.549s**; all
three
  compiler/Flight/CN owning packages passed normally and under race.
- The UT-runner `optools` package passed (**6.175s**); incremental lint
for
compiler, CN and issue-test packages reported **0 issues**. Test
selection,
  SQL assertions and cleanup error checks are unchanged.

### Review scope

R0: design and delivery map. R2: backend/adapter/config consumer
boundary.
Lifecycle review follows admitted lease -> Prepare/execution -> Run ->
Cleanup/retained reconciliation -> backend Close. The adapter adds no
workers,
queues or retry state. Existing Flight cancellation/reconciliation tests
remain
in the owning-package normal/race evidence. No new SQL behavior or wire
representation is enabled, so no unrelated BVT/SF10 run is substituted
for
the focused real CN/Flight consumer tests.

| Lifecycle check | Result for this PR |
| --- | --- |
| Q1: ownership | Prepare passes the same release callback to Flight;
execution/reconciliation retain their existing cleanup owner. Failed
preparation returns a genuinely nil interface. |
| Q2: waits | The adapter adds no locks or waits. Cleanup still uses an
independently bounded, uncanceled context and Flight's cancel/join
acknowledgement. |
| Q3: growth | One adapter per CN; no new queue, worker, retry
collection or batch retention. |

### CI failure diagnosis and closure

The first run's complete UT diagnostics identified two separate cutoffs:

- `TestTenantCatalogRegressions` passed its scenario assertions, then
its
`DROP ACCOUNT` fixture cleanup exceeded 30 seconds while catalog DDL was
still
progressing. Cleanup now uses the same bounded five-minute allowance as
the
  scenario; SQL assertions and cleanup error checks remain intact.
- The overall runner received SIGTERM at its 70-minute outer deadline
after
light, exclusive-issues and embedded stages consumed most of that
allowance.
Heavy/engine tests were progressing and plan testing had not completed.
The
outer budget is now 120 minutes, with the existing per-package timeout
and
TERM/KILL diagnostic handling unchanged. No parallelism increase or test
skip.

CI evidence: [original failed UT
job](https://github.com/matrixorigin/matrixone/actions/runs/35058948935/job/104675005101).

Current-main integration includes
`125d943786ecdf59be842ce381b9cc22d6a5c8bc`.
Delivery head: `399e0ee601`.
This foundation is independently reviewable; it does not wait for
migration
stages 3–8 to implement the native backend.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
VioletQwQ-0 pushed a commit to VioletQwQ-0/matrixone that referenced this pull request Sep 17, 2026
## What type of PR is this?

- [x] API-change
- [ ] BUG
- [ ] Improvement
- [ ] Documentation
- [x] Feature
- [x] Test and CI
- [x] Code Refactoring

## Which issue(s) this PR fixes:

Refs matrixorigin#28966 (migration PR 2 of 10). Numeric compatibility remains
separately tracked by matrixorigin#28968.

## What this PR does / why we need it:

Introduce the service-owned Sirius backend/execution contract so the
compiler
does not own concrete Flight types. The existing Flight adapter
preserves its
prepare, result delivery, fallback classification, reconciliation and
cleanup
behavior. No Flight protocol or native execution implementation changes
here.

- Record approved design version 1 and all ten PR boundaries in
  `docs/design/sirius-embedded.md`; no separate design-only PR.
- Add `sirius.backend`, defaulting to `flight` during coexistence.
`embedded`
fails explicitly as unavailable in this build; unknown values fail
closed.
Both config validation and service startup reject unsupported selection
  before leases, certificates or sockets are initialized.
- Make query cleanup depend on the narrow execution interface, retaining
bounded cleanup after request cancellation and joined run/cleanup
errors.
- Preserve nil-backend validation and avoid a typed-nil execution on
failed
  preparation. Keep malformed plans terminal rather than falling back.
- Add fake-execution coverage for success, consumer failure,
cancellation,
cleanup failure and consumer panic; adapt existing real Flight/CN tests.
- Correct two validation time budgets exposed by the first CI run:
tenant
fixture cleanup now has the scenario's bounded five-minute allowance,
and
the all-stage UT runner has a two-hour outer deadline. Per-package
timeouts,
assertions, stage selection and cancellation diagnostics are unchanged.

This is the MO foundation only. It does not yet implement CGo, enable
embedded
queries, port matrixorigin#27599's MO-reader stream, change GPU scheduling, or
remove Flight.
PRs 3-8 provide those components; PRs 9-10 remain blocked on all-22
parity and
numeric compatibility. The old matrixorigin#27599 branch is unchanged.

### Validation

- Focused compiler/CN Sirius tests passed.
- Full `pkg/sql/compile`, `pkg/sql/compile/sidecarflight`,
`pkg/cnservice` tests passed normally and under `-race` through
`mo-cgo-test`.
- Finalized new tests (including consumer-panic cleanup) passed normally
and under `-race`.
- `make config` passed; no module metadata changed.
- Focused golangci-lint with the repository's Go 1.26.4 toolchain: **0
issues**.
- The original-head GitHub SCA, build, coverage and BVT jobs passed.
Fresh
exact-head CI is tracked separately below; no lint suppression was
added.
- GPU/SF10 tests are not claimed for this Go-only adapter foundation.
The
  native synchronization is a separate Sirius PR.
- Exact-head GitHub CI is pending.
- Fresh post-main-merge validation with Go 1.26.4 and the existing GCC
15 CPU
toolchain: tenant regression normal **11.708s**, race **19.549s**; all
three
  compiler/Flight/CN owning packages passed normally and under race.
- The UT-runner `optools` package passed (**6.175s**); incremental lint
for
compiler, CN and issue-test packages reported **0 issues**. Test
selection,
  SQL assertions and cleanup error checks are unchanged.

### Review scope

R0: design and delivery map. R2: backend/adapter/config consumer
boundary.
Lifecycle review follows admitted lease -> Prepare/execution -> Run ->
Cleanup/retained reconciliation -> backend Close. The adapter adds no
workers,
queues or retry state. Existing Flight cancellation/reconciliation tests
remain
in the owning-package normal/race evidence. No new SQL behavior or wire
representation is enabled, so no unrelated BVT/SF10 run is substituted
for
the focused real CN/Flight consumer tests.

| Lifecycle check | Result for this PR |
| --- | --- |
| Q1: ownership | Prepare passes the same release callback to Flight;
execution/reconciliation retain their existing cleanup owner. Failed
preparation returns a genuinely nil interface. |
| Q2: waits | The adapter adds no locks or waits. Cleanup still uses an
independently bounded, uncanceled context and Flight's cancel/join
acknowledgement. |
| Q3: growth | One adapter per CN; no new queue, worker, retry
collection or batch retention. |

### CI failure diagnosis and closure

The first run's complete UT diagnostics identified two separate cutoffs:

- `TestTenantCatalogRegressions` passed its scenario assertions, then
its
`DROP ACCOUNT` fixture cleanup exceeded 30 seconds while catalog DDL was
still
progressing. Cleanup now uses the same bounded five-minute allowance as
the
  scenario; SQL assertions and cleanup error checks remain intact.
- The overall runner received SIGTERM at its 70-minute outer deadline
after
light, exclusive-issues and embedded stages consumed most of that
allowance.
Heavy/engine tests were progressing and plan testing had not completed.
The
outer budget is now 120 minutes, with the existing per-package timeout
and
TERM/KILL diagnostic handling unchanged. No parallelism increase or test
skip.

CI evidence: [original failed UT
job](https://github.com/matrixorigin/matrixone/actions/runs/35058948935/job/104675005101).

Current-main integration includes
`125d943786ecdf59be842ce381b9cc22d6a5c8bc`.
Delivery head: `399e0ee601`.
This foundation is independently reviewable; it does not wait for
migration
stages 3–8 to implement the native backend.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@XuPeng-SH XuPeng-SH mentioned this pull request Sep 23, 2026
3 of 7 tasks

This branch was successfully deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants