Repository navigation
docs(sirius): record remaining embedding PR sequence - #29690
Conversation
mergify
Bot
added
the
kind/documentation
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? |
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep review of 05050ba9f262f6c7b0a86c9bae4c0b4eb627c5c6 against freshly fetched main / merge-base bab4b3286a0dd5683a9b291763817722233e586c. Both complete documents and every changed hunk reviewed personally; no subagents.
APPROVE this design-status/delivery-map amendment. No blocking finding in this delta. This is not approval of future native/MO implementation, numerical parity, measured performance, default cutover or issue closure.
Scope and prior decisions
Production increment: zero. Test/oracle increment: zero. Documentation: embedded +43/-0; numeric +20/-11. The scope is a delivery refinement of an already reviewed cross-repository design, not an executable feature hidden in a docs change.
The semantic approval claim is verifiable: the #29449 approval explicitly names blob 42a89f09a1d168d02b9583cb3ea7b4de6dbb5634, which is also the numeric document blob on this PR's base. Sections 1–8 are byte-identical between base and head. The earlier scalar/aggregate error-class and division-type findings remain closed: operation-owned statuses 12/13 and MO-bound result scales are unchanged. The outdated “proposed” status is correctly repaired without silently reopening/changing those semantics.
Verified merged states for Sirius #25, importer #3, importer #4, and MO #29547. The merged Sirius #25 C entry point still advertises only the existing engine/input/result capabilities, not exact-decimal support; its primitive milestone is correctly distinguished from full admission. This PR has no prior human review comments or unresolved review threads.
Architectural judgment and QA challenges
| Boundary challenged | Contract retained by this revision |
|---|---|
| Scalar primitives/import complete, aggregates/keys incomplete | A remains capability-disabled; B must complete the entire family before advertising it. C waits for merged, pinned B. The split is review/delivery granularity, not a second execution or ownership path. |
| A plan validates under embedded semantics but serializes through ordinary Flight | C must preserve one export profile through validation and serialization; Flight emission remains unchanged during coexistence. |
| Flight cannot execute the complete exact-numeric suite | Native-MO/embedded-MO all-22 SF1/SF10 correctness/type/error evidence remains mandatory. Flight comparisons explicitly report equivalent common coverage or N/A, never a fabricated full-suite total. |
| Available matched Flight coverage regresses | The 1.0 common-metric/Q9 ratio remains a gate where comparable coverage exists; the distinct 10% numeric baseline gate also remains. N/A is not a waiver for an available failed comparison. |
| Upgrade finds no lease-manager instance or replay is not ready | Neither proves an empty old execution set. Cutover requires authoritative readiness/reconciliation and quiescence; GC protection cannot be dropped merely because the new path avoids Flight. |
| Legacy transport is removed before a usable replacement is distributed | E requires acceptance; F additionally requires a verified available MO release artifact and test-replacement mapping. Independent tools and historical evidence survive. |
Strict legacy configuration rejection is a coherent selected stop-upgrade contract: actionable errors rather than silently reinterpreting enabled Flight configuration. Disabled Sirius/ordinary CPU behavior remains explicitly unchanged. Reusing the previous release for old reconciliation and post-removal rollback avoids maintaining a second recovery stack, while the existing protection owner must remain authoritative until old consumers are quiescent.
The historical ten-PR map is explicitly labeled historical, with the new six-stage sequence defining the remaining work. Numeric semantics retain one authoritative document/blob rather than being copied into a new implementation contract. The added complexity is documentary dependency/acceptance precision, not extra runtime state.
Verification and limits
- Local exact-range
git diff --check: PASS. - Exact numeric-semantic section comparison: identical.
- Referenced in-repository design/inventory paths exist; cross-repository prerequisite and approval records checked live.
- Exact-head documentation CI job 113152707256 ran the documentation diff check successfully; CI Required is successful.
- Current
optools/sirius_parity.pystill uses schema version 1 and includes the obsolete embedded-TAE route. The new text correctly assigns campaign versioning/removal of that requirement to D; that runner work is pending, not already delivered by this PR. - No Go/CGo/SQL/GPU test, race stress or benchmark was run for a documentation-only delta. Those jobs being skipped is not a runtime pass. There is no changed executable lifecycle, persistent format, test selection or performance path to validate here.
Future behavior-changing PRs must still provide their own focused unhappy-path, resource, public-SQL, GPU and performance evidence. This approval neither closes #28968/#28966 nor permits capability/default activation before those gates.
Merge Queue Status
This pull request spent 1 minute 53 seconds in the queue, including 1 minute 25 seconds running CI. Required conditions to merge
|
) Opt-in embedded MO-reader queries now lower MO exact Decimal64/128/256 values and descriptors to Sirius GPU execution. One immutable capability-scoped profile owns admission, physical types/literals, bound scalar/aggregate overloads, input publication, result reconstruction and public numeric error classes. Terminal evidence records verified descriptors and a schema digest without logging headings or values. This implements approved PR C from #29690 and the versioned [design/validation map](https://github.com/matrixorigin/matrixone/blob/a4e34ef05de6a6d02ceb0fa81219cacd452ae6c8/docs/design/sirius-mo-exact-lowering.md). Physical width is preserved at narrow declared precision; MO-bound division metadata and 32-byte Decimal256 coefficients use the existing credit-before-copy and borrowed-result contracts. Embedded fetch defaults and DATE extraction use supported native wire forms. All prerequisites are merged and inherited: - Sirius [#29](matrixorigin/sirius#29): `908ffc75a58b3be496b2172795e416326416e7ec`, with merged importer #5 at `99c7ca3b6f8f3159239e119ed2982d42f98c4690`. - Kernel [#29775](#29775): `af3f7232a31879a2d6ed590785c8e9a560361c3f`. Prepared SELECT metadata comes from the executed generation; incomplete saved SELECTs clear session state before reuse. - Main [#29794](#29794) supplies the existing MORPC shutdown-test race correction. Main now also includes the histogram-export synchronization fix; C uses main's bounded two-send notification wait unchanged. Final release/native integration at executable C head `ad61b5b5d3`, on merged-kernel main `af3f7232a3`, passes: - Default, Sirius-only and combined Sirius/cuVS release builds. The clean merged SDK, all 73 fingerprints, compiler/header/proto identity and runtime package/provenance guards pass. - All 22 native preparations without reader/GPU-task admission. The complete public MySQL numeric fixture passes exact values, metadata/NULLs, arithmetic/aggregates/joins/sort, operation-owned errors, masked errors, healthy reuse and prepared division increments 0/4/10/30/4. Native MO supplies the independent oracle. - All 31 public terminal events show capability 31, `mo-exact-decimal-v1`, completed GPU tasks, no fallback, healthy cleanup and zero retained input/result credit. - Four C-owning packages plus frontend pass default normal/race checks; native-tag owning checks and the complete combined bridge suite pass. cuVS-before/Sirius/cuVS-after passes in one process. - All nine public prepared-metadata/saved-result cases pass normal mode and three race repetitions, including a real persisted-batch failure followed by successful save/replay on the same physical connection. - Native data/cancellation passes 68 race repetitions in one process (measured 0.44s; 30s budget). The exact MORPC shutdown test passes 100 race repetitions. Metrics focused/owning normal/race and 100 focused race repetitions pass. The delivered head is `b34111b4c9e94cda034d199a2c362072fe98d808`, normally merged with authoritative main `e5dd4724f782067e1c76c38c9d2f55cebceda4a9`. Review fixes are committed in `af4f342066`: typed DATE lowering preserves persisted MO zero dates; exact CASE retains the MO-bound descriptor around IfThen; ordinary embedded integer add/subtract/multiply declines before readers because the pinned consumer does not implement MO checked overflow. Flight encoding/admission, reader bytes, native ABI and the merged Sirius pin are unchanged. The installed and loaded host drivers now both report NVIDIA 615.78.08; final GPU checks run directly with the installed driver. The bounded local native fixture uses 256 MiB GPU and 2 GiB host capacity. An initial 512 MiB host fixture correctly rejected larger progress-window reservations; only the test fixture was corrected. Production windows, limits and assertions remain unchanged. Review-resolution validation at prior head `a4e34ef05d`: - Both incoming counterexamples have real GPU red/green proof. Zero DATE now returns 0/0/0 alongside year-0001/leap-day/year-9999/NULL controls, with SQL mode restored. Nullable division conditions with two required decimal arms execute with native-MO values and nullable DECIMAL(15,2) metadata. - The existing public fixture covers integer +/-/* overflow declines and healthy integral/decimal reuse. No new cluster, production hook, native library dependency, sleep or skip was added. - All 22 native preparations pass. Full public GPU normal mode passes (9.596s); measured race mode followed by three in-process repetitions passes (29.571s). Its 34 terminal events verify exact profile/capability31, completed GPU work, no fallback, healthy cleanup and zero retained input/result credits. - Fresh combined release/native provenance and full bridge/coexistence pass. Exporter/compiler/CN owning normal/race, focused new wire/domain regressions, config and full mandatory pre-push SCA pass with zero lint findings. Self-review has no unresolved implementation blocker. **Ready for review; merge gates remain required CI and reviewer approval.** Both incoming findings were reproduced on actual GPU/public MySQL and fixed with regressions. A separate admitted integer-overflow counterexample returned -2 instead of native MO 1690/22003; it now declines before execution. Fresh CI is pending for the delivered head, and the reviewer has been asked to reassess the fixes. Local passes do not establish required CI success. C remains opt-in. D begins after C merges and stays one complete public campaign PR for all-22 SF1/SF10, bounded comparison, lifecycle/resource/process evidence and performance gates. E/F retain their separate recovery-readiness design. References #28968 and #28966; neither issue is closed by C. Conflict resolution at `b34111b4c9` (2026-10-10): normally merged authoritative main `e5dd4724f7`. The single conflict was two equivalent asynchronous histogram-test fixes; main's stopped-timer notification barrier is retained unchanged. All 28 prior C paths were checked: only this test changed, while the DATE/CASE/integer review fixes and merged native pins remain intact. Validation at this merged head passes: affected owning packages normal/race, 100 focused metrics race repetitions, fresh native generations and combined release/package, all 22 actual preparations (1.116s), full public GPU normal/race (9.762s/11.999s), and full combined bridge/coexistence (1.108s). Both public runs' 34 terminal events verify completed GPU work, exact profile/capability31, healthy cleanup, no fallback and zero retained credits. Mandatory pre-push SCA passes with zero findings; self-review has no unresolved implementation blocker. New-head required CI and human reviewer approval remain the merge gates. --------- Co-authored-by: XuPeng-SH <xupeng3112@163.com>
This branch was successfully deployed
3 participants
What type of PR is this?
Which issue(s) this PR fixes:
Refs #28966 and #28968. Does not close either issue.
What this PR does / why we need it:
Record the remaining embedding implementation sequence after merged Sirius #25 and importer #3/#4. Split native import/type/scalar plumbing from the complete aggregate/key/capability milestone, then retain merged dependencies for MO lowering, full acceptance, cutover and sidecar retirement.
Correct the numeric design's stale approval status while preserving the approved semantic revision. Record the owner-selected strict legacy configuration migration and the 1.0 matched Flight+MO gate only on available equivalent coverage. Preserve the historical delivery map and GC protection/reconciliation requirements.
Validation:
git diff --checkand the MatrixOne PR guard pass. This is a documentation/delivery-map change; no production behavior or test oracle changes.