Skip to content

fix(frontend): replay publications after snapshot database restore - #30675

Merged
XuPeng-SH merged 22 commits into
matrixorigin:mainfrom
daviszhen:fix-30610-snapshot-publication
Oct 24, 2026
Merged

XuPeng-SH merged 22 commits into
matrixorigin:mainfrom
daviszhen:fix-30610-snapshot-publication

Conversation

@daviszhen

@daviszhen daviszhen commented Oct 20, 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:

issue #30610

What this PR does / why we need it:

RESTORE DATABASE <db> {snapshot="..."} drops and recreates the database, removing its publication rows without restoring them. The publisher's data returns but existing subscribers fail with there is no publication.

Replay snapshot-time publications at the top-level database restore coordinator, after ordinary tables, deferred foreign-key tables and views have finished. Reuse the PITR publication recreation path so the current database identity is resolved and the stored publication principals are retained. Keep the same-account guard; table and cross-account restores do not replay publications.

Do not replay publications inside shared database workers. Account/cluster restores also call those workers; doing so changed account behavior and collided with the cluster's existing final publication phase. Cluster restore continues to recreate publications once, followed by subscriptions and identity-bearing catalogs. This PR does not change the existing account-restore publication contract or suppress duplicate-key errors.

Update only the full-text database-restore golden that expected the missing-publication defect. Drop the restored publication before fixture database cleanup, preventing leaked databases from breaking later SHOW DATABASES tests. Add a public SQL regression with a real subscriber, foreign-key tables, a view, table-only restore and repeated database restore; add UT guards for restore-level ownership and shared workers.

Verification

  • Full pkg/frontend suite: passed (-short -tags=matrixone_test -count=1, 15.103s).
  • Focused publication-owner/shared-worker UTs and go vet ./pkg/frontend: passed.
  • Focused publication-owner/shared-worker race UTs: passed (-race -count=1, 3.246s).
  • Configured incremental SCA for pkg/frontend: passed, zero issues (molint and configured golangci linters).
  • Isolated local service, MySQL protocol and real JDBC/mo-tester execution: the exact 17 previously failing CI scripts plus the new regression all pass. Final total: 3,576 assertions, 3,569 passed, 7 existing issue#23700 cases skipped with the normal CI ignore setting, zero failures. Coverage includes original duplicate-publication cluster cases, account controls, table/repeated DB restore, FK/view ordering, full-text subscriber rows and later SHOW DATABASES controls. No account/cluster golden changes were needed.
  • Post-run counts for leaked pub_fulltext_table, pub_full01 and the new fixture publication are zero.

Validation was on macOS arm64 / Go 1.27.1 with matching CGo dependencies, not the Linux multi-CN Compose lane. Full remote CI remains pending; local results do not claim full Compose CI passed. Repair revision: dc5953a8a8bf480beac8c45920ad793d12b964b8.

daviszhen and others added 4 commits October 20, 2026 12:43
…arser (matrixorigin#30607)

RESTORE DATABASE ... FROM PITR reads the restored database's publications
through getPubInfoWithPitrFormat, which projected mo_pubs as
(pub_name, database_name, ...). createPubByPitr then parsed the rows with
extractPubInfosFromExecResult, which expects the 12-column layout starting
with (account_id, account_name). The pub name landed in the first column
and restore aborted with
"strconv.ParseInt: parsing \"<pub_name>\": invalid syntax", so a
published database could never be restored via PITR (while the same
restore succeeded for unpublished databases).

Make the PITR query project the same 12-column layout as the extractor,
matching getAllPubInfosBySnapshotName's already-correct snapshot query.
Regression tests pin the projection against getPubInfoSql and verify a
realistic row parses into the right PubInfo fields; both fail on main.
…atrixorigin#30610)

RESTORE DATABASE <db> {snapshot=...} drops the database before recreating
it, and that drop erases the mo_pubs rows targeting the database: every
subscriber then breaks permanently on 'there is no publication' while
'show subscriptions' still lists the dangling databases.  The PITR path
rebuilds the publications after its database restore (createPubByPitr),
so the two restore stacks disagreed and only the snapshot one lost data.

Replay the snapshot-time publications at the end of the database restore
in both snapshot stacks — the clone-based restoreToDatabaseOrTable (the
RESTORE DATABASE {snapshot} mainline) and the from-TS
restoreDatabaseFromTS — through a shared wrapper that:

- reuses the PITR 'read mo_pubs at {MO_TS} / re-CREATE PUBLICATION'
  semantics, replaying with the stored owner and creator;
- stays silent on cross-account restores: the source account kept its
  database and publications there, so replaying would collide;
- never runs for table-level restores, which do not drop the database
  and its publications in the first place.

Verified live on a fresh single-node instance with the issue flow
(account acc1, publication pubS -> acc1, subscriber spb_sub, snapshot,
post-snapshot insert, restore): the publication row survives, the data
rolls back, and the subscriber keeps serving.  Two complete independent
cycles stay green.  Existing snapshot-restore UT fixtures learn the new
mo_pubs projection query; a new wrapper UT pins the same-account replay,
the cross-account no-op and error propagation.
@daviszhen
daviszhen requested a review from XuPeng-SH as a code owner October 20, 2026 13:01
@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 Oct 20, 2026
@matrix-meow matrix-meow added the size/M Denotes a PR that changes [100,499] lines label Oct 20, 2026
@daviszhen

Copy link
Copy Markdown
Contributor Author

Evidence at head 6b0cd3454c (live single-node: 1 LOG / 1 TN / 1 CN)

Exact issue flow, fresh instance of this branch:

sys:  create account acc1 ...
      create database spb; create table spb.t (id int primary key);
      insert into spb.t values (1),(2);
      create publication pubS database spb account acc1;
acc1: create database spb_sub from sys publication pubS;   -- works
      use spb_sub; select * from t;                          -- 1, 2
sys:  create snapshot snapS for database spb;
      insert into spb.t values (3);
      restore database spb {snapshot="snapS"};
      show publications;                                    -- pubS present (was: EMPTY)
acc1: use spb_sub; select * from t;                          -- 1, 2 (was: ERROR 20101)
sys:  select count(*) from spb.t;                            -- 2, mutation rolled back

Two additional fully independent cycles (unique account/db/pub/snapshot names each round):

== CYCLE r1: pub_rows=1 master_count=2 sub_result='2'
== CYCLE r2: pub_rows=1 master_count=2 sub_result='2'

Path notes for reviewers, verified by debug log:

  • RESTORE DATABASE {snapshot} flows through the clone-based
    restoreToDatabaseOrTable (not restoreDatabaseFromTS); hooks are
    added at the tail of both stacks, guarded by restoreAccount == toAccountId and !restoreToTbl.
  • The replay logs start to recreate publications of db: spb ->
    get pub info sql: select ... mo_pubs {MO_TS=<ts>} ... ->
    create pub sql: CREATE PUBLICATION pubs DATABASE spb ACCOUNT acc1.
  • Control experiment on an upstream-main binary confirms the plain
    DROP DATABASE guard (can not drop database 'spb' which is publishing) — the restore's internal drop is what bypasses it,
    which is exactly the row-erasure this PR repairs.

UTs: Test_recreatePublicationsAfterDbRestoreTS (same-account replay
with owner/creator principals preserved in the mo_pubs INSERT,
cross-account executes nothing, mo_pubs read failure aborts the
restore); existing snapshot/restore fixtures extended with the empty
mo_pubs projection. Full pkg/frontend suite passes on this head.

@matrix-meow matrix-meow removed the size/L Denotes a PR that changes [500,999] lines label Oct 21, 2026
@XuPeng-SH

XuPeng-SH commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: a8dd8082ae563fb8f779ad41ba5210e2426a23d6

Review completed. Read the assessment.

@XuPeng-SH

XuPeng-SH commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-xupeng · superseded

Commit: a8dd8082ae563fb8f779ad41ba5210e2426a23d6

"interrupted"

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

mo-bot review · mo-bot-local · review

Commit: a8dd8082ae563fb8f779ad41ba5210e2426a23d6

Assessment: request_changes

Reviewed the complete delta and supplied comments. Replay ownership and backend reuse are reasonable; prior quoting, skipped-database and empty-list concerns are addressed. One reproduced regression remains: explicitly publishing an external table prevents database restore.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

  • P2 pkg/frontend/snapshot_restore_with_ts.go:566: Create a database containing an ordinary table and external table ext_t, publish TABLE ext_t ACCOUNT ALL, take a database snapshot, then restore it.
    Impact: Bulk restore deliberately skips external tables, but publication replay validates the unchanged historical selection. The missing ext_t aborts and rolls back the entire restore, preventing recovery of ordinary tables. Reconcile replay with the external-table exclusion policy while preserving explicit empty selections, and add regression coverage.
    Evidence: Real SQL probe TestReviewSnapshotExternalPublication/external_selection fails on the candidate with "table 'ext_t' not exists", leaving two ordinary rows after rollback. The identical merge-base probe succeeds, restoring one row. Candidate controls with no publication, ALL tables, and an ordinary-table selection all pass. Retained logs: external-candidate.log and external-base.log.

Validation:

  • Reviewed all 15 changed files, restore/PITR/cluster callers, transaction cleanup, authorization, serialization, and test contracts.

  • Isolation preflight passed. Verified 192 native inputs against candidate and base, plus seven reused build outputs against the build owner's receipt and log.

  • Candidate probe: three controls passed, external selection failed; checks/1792596324791694213-20.json records exit 1. Base probe: all four passed; checks/1792596383849604362-3.json records exit 0.

  • Reconciled commands, clean source fingerprints, logs, identical probe bytes and base overlays. Evidence retained under /home/xupeng/.local/state/mo-bot/jobs/148480debcda396ee9412082/attempt-1, including review-evidence-audit.json.

  • Frozen exact-head CI reports successful Ubuntu UT, Linux/arm64 SCA, shared build and proxy/pessimistic BVT.

  • Performance tracing found added catalog/subscriber work confined to publication replay; ordinary SELECT/DML paths gain no work.

Limitations:

  • The focused reproduction used single-CN real SQL; no additional race, multi-CN or performance benchmark runs were performed.

  • local_validation contained no retained author gates; raw CI logs and the linked issue body were absent. CI conclusions do not establish coverage of the reproduced boundary.

@daviszhen

Copy link
Copy Markdown
Contributor Author

Addressed the external-table replay P2 in 3be8d01.

The replay helper now reads source-tenant table metadata at the restore timestamp and applies the existing bulk-restore exclusion policy to explicit publication selections. Only intentionally omitted external-table names are removed. Missing ordinary tables still fail normal creation validation, and historical-read errors abort before catalog writes. An external-only selection remains all_table=false, table_list='', never ALL; mixed selections retain ordinary tables. Snapshot metadata, grants, comments, current database-ID resolution/locking, and subscriber rebinding are preserved. DATABASE/PITR and cluster replay pass their timestamp through the shared helper; account restore policy is unchanged.

Reproduced the reported failure with real tenant publisher/subscriber SQL on the reviewed production source: external-only and mixed selections abort RESTORE DATABASE with table 'ext-t' not exists; no-publication, ALL, and ordinary selections are controls. New real SQL tests cover those five cases with two restores each, exact publication/subscriber metadata, ordinary-row recovery, external-table absence, no empty-to-ALL widening, and denied access after an excluded name is reused. Unit tests cover source/history selection, immutable metadata, exact insertion policy, missing-ordinary rejection and historical-read error propagation. The previous 12 SQL/subscriber scenarios remain covered.

Validation on the final source (Go 1.27.1, darwin/arm64, repository-native provenance verified):

  • Full frontend/pubsub: PASS (19.128s / 1.444s).
  • Focused native frontend + real SQL: PASS.
  • Replay/gating tests and all 17 SQL scenarios with -race -count=3 -shuffle=on: PASS (frontend 4.319s; embed 123.255s).
  • Configured frontend/embed static target: exit 0; golangci-lint 0 actionable issues. git diff --check clean.

Local SQL uses the single-CN fixture; no full-repository, distributed-BVT, cluster-restore SQL or throughput benchmark is claimed. Both this PR's reviewed head and #30719's reviewed head had successful applicable CI when inspected, so there was no failed old run to retry. The new push has triggered fresh CI; its results are pending, not claimed green. Please re-review.

@daviszhen

Copy link
Copy Markdown
Contributor Author

Merged current main (a949c88af0) and resolved the publication/restore conflicts without dropping either behavior:

  • The shared snapshot/PITR replay helper still preserves historical principals, current database identity, external-table omission, empty table/account selections and subscriber rebinding.
  • Main's publication view-dependency validation remains active. Replay resolves lower_case_table_names from its background session after filtering and building the replay statement, then passes it into the shared creation backend. ALL and explicitly empty selections do not request an unused mode; retained explicit selections fail closed if the mode is unavailable.
  • No ACCOUNT ALL or TABLE ALL fallback, bypassed validation, silent missing ordinary table, grammar change or unrelated manual main edits.

Updated the upstream PITR mode-resolution fixture to include the historical table read that now precedes mode resolution, retaining the exact rejection and adding an exact no-write query assertion. Unit tests still cover external-only/mixed/unknown selections and source metadata immutability.

Extended the existing real MySQL restore fixture, without another cluster, with two view-scope controls. A complete view/table dependency set restores twice and remains queryable by the subscriber. A publication whose view was subsequently redefined to require an unselected private table is rejected twice; the two live rows and publication selection remain unchanged after rollback. The previous 17 restore/subscriber scenarios remain covered, including external-only and empty selections.

Validation on Go 1.27.1 / darwin-arm64, using the repository wrapper and freshly rebuilt, provenance-verified native artifacts:

  • Full frontend normal: PASS (15.649s); full pubsub normal: PASS (2.571s).
  • Full frontend/pubsub with -race -vet=all: PASS (25.597s / 3.156s).
  • All 19 real-client scenarios: normal PASS (16.990s), race PASS (44.870s). Each scenario retains two restore attempts. After the final test-only per-iteration result-cleanup improvement, the affected boundary root passes separately in normal mode (8.955s) and race mode (21.750s). Its measured race duration selected one final run under the 30-second adaptive budget.
  • Configured frontend/embed static check: exit 0, golangci-lint 0 actionable issues. The final test-only cleanup change additionally passes the embed static target (3.865s). Formatting and git diff --check pass.

Local SQL validation uses the canonical single-CN fixture; no new distributed BVT, cluster-restore SQL, full-repository UT or performance benchmark is claimed. The latest external-table reviewer finding was already repaired in 3be8d01636; no newer reviewer finding was present at inspection. Reviewer re-approval remains pending. The old head had no failed CI check: this update resolves its main conflicts and triggers fresh CI. Pending new-head checks are not claimed green.

@XuPeng-SH

XuPeng-SH commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: 45730bd08bd20183f70fe2785dbbcf8844ad917b

Review completed. Read the assessment.

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

mo-bot review · mo-bot-local · review

Commit: 45730bd08bd20183f70fe2785dbbcf8844ad917b

Assessment: request_changes

Reviewed the complete 16-file delta and restore/publication callers. Coordinator-owned replay, recreation gating and reuse of the creation backend are sound; earlier reported defects are addressed. One confirmed authorization regression remains: replay can publish a different, previously private table by lowercasing its saved name.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

  • P1 pkg/frontend/publication_replay.go:26: With lower_case_table_names=0, create distinct tables PublishedT and publishedt, publish only PublishedT to another account, snapshot the database, then RESTORE DATABASE.
    Impact: The hard-coded parser mode 1 changes PublishedT to publishedt. Restore succeeds but grants the subscriber access to the previously private lowercase table. Without that colliding table, restore instead fails and rolls back. Preserve catalog identifier bytes during replay and add case-sensitive collision and noncollision regressions.
    Evidence: TestReviewPublicationCaseReplayV3/case_collision verified private-table access was denied before restore, then observed table_list="publishedt" and a successful subscriber read of private value 901 afterward. The noncollision case failed with "table 'publishedt' not exists" and retained two live rows. Both controls passed against the merge-base production overlay. Candidate receipt: checks/1792648580367420366-3.json (exit 1); base receipt: checks/1792648610039050219-3.json (exit 0). Logs: candidate-case-v3.jsonl and base-case-v3.jsonl.

Validation:

  • Artifacts are retained under /home/xupeng/.local/state/mo-bot/jobs/bac1210cab099de45d8da8be/attempt-1. evidence-audit.json reconciles commands, source fingerprints, exit receipts, logs and overlay hashes; source remained unchanged.

  • Isolation preflight passed. After the repository wrapper rejected incompatible host artifacts, make cgo built frozen-source prerequisites. Independently verified 194 native inputs and 13 output hashes.

  • Focused frontend/pubsub tests passed: focused-tests.jsonl, receipt checks/1792648532928084812-3.json. All 19 added real-client SQL scenarios passed in 12.916s: existing-sql-tests.jsonl, receipt checks/1792648600425470836-3.json.

  • The final four-case public-SQL probe reproduced two consequences of the identifier defect; unpublished and ordinary-name controls passed. All four cases passed against verified merge-base production sources.

  • git diff --check passed. Performance tracing found additional historical metadata scans and publication/subscriber work confined to restore; ordinary SELECT/DML paths gain no work.

Limitations:

  • Local SQL execution used one embedded CN. Frozen CI metadata reports successful UT, coverage, static analysis and multi-CN BVT checks; raw CI logs and retained author-validation receipts were not supplied.

  • No local race, full-repository suite or performance benchmark was run. Initial probe setup failures were retained separately and excluded from the finding.

  • The linked issue body was not supplied; its contract was assessed through the frozen PR description, comments and regression tests.

@daviszhen

Copy link
Copy Markdown
Contributor Author

@XuPeng-SH Addressed the case-sensitive publication replay P1 from review 5609853967 in commit e653b52.

Replay now constructs the internal CREATE PUBLICATION AST directly from canonical catalog fields for every selection shape. It no longer formats SQL and reparses it with hard-coded mode 1. This preserves identifier bytes without changing the ordinary backend's table/view scope validation, historical principals, subscriber handling, or ALL-versus-empty semantics.

The new real-client SQL regressions cover both PublishedT without a collision and distinct PublishedT / publishedt tables. They set and verify case-sensitive sessions in both tenants, check the publisher's private value 901 exists while subscriber access is denied, restore twice, and verify restored selected data, exact publication metadata, successful selected-table subscription reads, and continued denial of the private table. The unchanged production code reproduced both the missing-table restore failure and the authorization leak. Unit coverage also preserves mixed-case/quoted identifiers and empty/ALL/explicit account selections.

Local validation:

  • Full frontend and pubsub packages passed (13.883s / 1.794s).
  • All 21 named publication-replay SQL scenarios passed, reusing the existing single-CN fixture.
  • The complete 13-case boundary root passed under race (37.30s root); focused replay/backend frontend race tests passed (2.944s package).
  • Incremental frontend/embed static checks: 0 issues.

Transparency: the first full frontend run hit the unchanged TestConnMeasuresOnlyPhysicalOutputWrite assertion that elapsed time must be positive. An exact clean-HEAD source overlay reproduced six zero-nanosecond failures in 100 repetitions; this is separate from replay. No unrelated assertion was weakened. The subsequent full frontend run passed.

Both PRs' required CI was green before these review fixes. The pushed commit starts fresh checks; the local results above are not a claim that those new checks have completed.

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

Deep re-review of exact head 1eadf5008d3e984f50a07c9548d9e32aabad594b against base/merge-base 6cbe375b49696021d72778baa292a66287f080bd, including the increment from my last audited head 4c453e7f5ae5e08497b3b2db76ffe05e4e151d41. Read all16 changed files,9 historical reviews,22 discussion comments,2 inline comments/author reply and the unresolved thread. No remaining blocking finding.

My previous empty-authorization counterexample was rerun byte-for-byte: restore now succeeds and restores1 row (1.46s). Also verified all21 real-client scenarios: empty grants deny old/new subscribers until reauthorized; empty table selections stay empty; external-only/mixed filtering never widens to ALL; case-sensitive collision/noncollision preserves selected names and denies private-table access; incomplete view scope fails atomically. Comment/quoted-identifier and external-FK skip repairs remain covered. The old inline thread is left unresolved in GitHub, but its failure is demonstrably fixed.

Traced coordinator recreation ownership, FK/view ordering, source timestamp/tenant and historical principals, ordinary creation validation, subscriber rebinding, rollback/locks and AST cleanup. Each helper frees its AST on success/error; restore failure reaches the existing bounded transaction cleanup. Public grammar/authorization checks are not bypassed.

Independent exact-head evidence:35 focused frontend/pubsub roots pass; canonical LOAD + unchanged prior counterexample + all21 SQL scenarios pass in one process (23.948s); probe vet, formatting and diff checks pass. Reused verified CI39749370567: SCA/UT and complete non-race coverage producer exit0 with no failure event, plus both compose BVT groups. Raw PROXY/mo-tester results explicitly show the new regression102/102, restore_pub_sub354/354, and cluster/privilege controls all passing. No author-only execution is relabeled as mine, and no new physical archive/upgrade/performance claim is made.

@XuPeng-SH

XuPeng-SH commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: 1eadf5008d3e984f50a07c9548d9e32aabad594b

Review completed. Read the assessment.

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

mo-bot review · mo-bot-local · review

Commit: 1eadf5008d3e984f50a07c9548d9e32aabad594b

Assessment: request_changes

Reviewed the complete 16-file delta and historical concerns. Coordinator ownership, shared creation validation, and authorization preservation are sound, but replay misses an existing intentional view omission and introduces a confirmed database-restore regression.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

  • P2 pkg/frontend/publication_replay.go:86: Create a valid FULLTEXT view v_ft over docs, publish TABLE docs,v_ft, drop its FULLTEXT index, take a database snapshot, modify docs, then RESTORE DATABASE from that snapshot.
    Impact: Restore deliberately skips the now-unusable view, but publication replay removes only external tables from its selection. Validation then rejects the missing v_ft and rolls back the entire database restore. Propagate the restore owner's known omitted-view set into replay while retaining rejection of unexpectedly missing objects.
    Evidence: The identical real-client probe passes on merge-base 6cbe375 and restores one row. The candidate returns "table 'v_ft' not exists" and retains two rows after rollback; no-publication and ALL-tables controls pass. Retained under /home/xupeng/.local/state/mo-bot/jobs/b0b646bd7887a4db8cdbfb09/attempt-1: review_unservable_publication_test.go, candidate-unservable-probe.jsonl, base-unservable-probe.jsonl, and the audited exact-base overlay. Receipts: 1792667728926398164-9.json (candidate exit 1), 1792667832381642605-21.json (base exit 0).

Validation:

  • Command/source/exit receipts, retained logs, probe hashes, and base overlays reconciled in attempt-1/review-evidence-audit.json; audit receipt 1792667910986605084-3.json passed. Source remained clean.

  • Isolation preflight passed. Verified current native prerequisites against 192 input files and 46 retained output hashes.

  • All 21 added SQL scenarios plus the existing LOAD regression passed, covering case-sensitive access, empty grants/selections, external tables, quoted names/comments, repeated restore, and scope-validation rollback.

  • Independent cluster and PITR client probes passed, including recreated publisher account IDs, current database IDs, and subscriber access restrictions.

  • Frontend: 2,582 root tests passed, one skipped, and one unrelated export test failed because the checkout is read-only. That test passed with an audited test-only output-path overlay. All 11 pubsub roots and 11 focused frontend race roots passed.

  • Configured incremental golangci-lint reported zero issues; changed Go formatting and git diff checks passed.

Limitations:

  • No local multi-CN run or performance benchmark was performed. Historical table enumeration adds restore-only work per explicit publication; ordinary SELECT/DML paths are unchanged.

  • Frozen CI statuses and author summaries were contextual evidence, not substitutes for audited execution. The race selector exercised frontend tests only.

@daviszhen

Copy link
Copy Markdown
Contributor Author

Fixed the omitted-view replay regression in 1180cd9361.

Publication replay now receives the restore coordinator's database-qualified set of views deliberately omitted as unservable. It filters a historical selection only when the historical object is a view in that set (or an external table already intentionally skipped by bulk restore). Unexpectedly missing objects still reach ordinary publication validation and fail. ALL, empty selections, authorization, source tenant/timestamp and ordinary CREATE validation are unchanged. Runtime-discovered unservable views are recorded by the view owner too.

The real-client regression creates a valid FULLTEXT view and publication/subscription, drops its FULLTEXT index before the snapshot, and restores twice. It verifies restored rows, omitted view metadata, the retained publication selection, subscriber access, and denial after the omitted view's name is reused. No-publication and ALL controls are included.

Validation on this change:

  • Red: the corrected real-client regression against the exact old production sources at 1eadf5008d fails the explicit-selection case with table 'v_ft' not exists; no-publication and ALL controls pass.
  • Green: all 24 publication replay SQL scenarios pass (16.102 s).
  • Full frontend and pubsub packages pass (19.005 s / 2.153 s).
  • Focused frontend plus all 24 SQL scenarios pass under -race (2.838 s / 50.977 s).
  • Configured incremental golangci-lint: zero issues; formatting and diff checks pass.

The new head is mergeable and fresh CI has started. These are local embedded real-client regressions, not a claim of multi-CN or physical cross-version archive testing. Please re-review.

@XuPeng-SH

XuPeng-SH commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

mo-bot review · mo-bot-local · done

Commit: 1180cd9361046a1b7cd52560d4e0a7c0c7cba28a

Review completed. Read the assessment.

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

mo-bot review · mo-bot-local · review

Commit: 1180cd9361046a1b7cd52560d4e0a7c0c7cba28a

Assessment: approve

Reviewed the complete 16-file delta, callers and supplied review concerns. Coordinator-owned replay, explicit recreation gating and reuse of publication validation are coherent. The omitted-view repair preserves authorization, empty selections and rejection of unexpectedly missing objects. No actionable findings remain. Added catalog work is confined to restore paths.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

Validation:

  • Verified frozen head, clean source, complete patch, commands, source fingerprints, terminal receipts and retained logs. Audit: attempt-1/review-audit.json; receipt checks/1792676699570174695-4.json, exit 0.

  • Isolation preflight passed. Independently verified 192 native inputs and 46 retained build outputs before reuse.

  • 37 focused frontend/pubsub test roots passed: focused-tests.jsonl; receipt 1792676266108955696-3.json.

  • All 24 real-client publication replay scenarios plus the existing LOAD regression passed on the shared single-CN fixture, including repeated restore, subscriber restrictions and omitted-name reuse: sql-tests.jsonl; receipt 1792676358468921535-3.json.

  • Owning-package execution passed 2,594 test roots, with one existing skip and one export-test failure caused by writing into the read-only checkout. That unchanged test passed with a path-only overlay directing output to private /tmp; receipts 1792676382469669881-3.json and 1792676472696084313-5.json.

  • Reconciled retained cluster/PITR probe receipts, logs and diagnostic hashes. Their tested external-selection and identity-rebinding paths remain semantically unchanged by this revision.

  • Formatting and diff checks passed. Frozen current-head CI reports successful UT, coverage, SCA and both multi-CN BVT lanes.

Limitations:

  • Raw CI logs, retained author gates and the separate linked issue body were not supplied; author summaries were not treated as independently audited executions.

  • No new local race, multi-CN, BVT-harness or performance run was performed. Local public SQL validation used one CN; no throughput or restore-latency improvement is claimed.

@daviszhen

Copy link
Copy Markdown
Contributor Author

@mergify refresh

@mergify

mergify Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

refresh

✅ Pull request refreshed

@daviszhen

Copy link
Copy Markdown
Contributor Author

Investigated run 39797131945 / UT job 119887273624 at exact head 93a72247caacc871efb3dce27cdbcf05798a5761. Downloaded the UT diagnostics artifact, the coverage incident report, and the complete job log. The diagnostic JSON was budget-truncated, so the complete failure output in the job log was also checked.

There are three independent failing tests, not a coverage-threshold failure:

  • TestInactiveAfterCannotConnect: the initial successful Unix-socket connection had only a 100 ms budget. It timed out on this runner, and the test helper continued past a non-fatal assertion into a nil-backend type assertion, causing the panic. Fixed in e7e40871c1: allow 10 seconds for setup, fail immediately on creation/send/receive errors, guarantee listener cleanup, and replace the unbounded inactivity loop with a bounded eventual assertion. The pre-stop active-state assertion and post-stop inactive-state assertion are both retained. Production timeout behavior is unchanged; the independent virtual-clock connect-budget regressions remain intact.
  • TestIssue29095RewriteMetadataVisibility/mode2: failed in deferred cleanup (drop database if exists DbCase29095) when the shared 30-second cleanup context expired, after the functional scenarios. The log has Dragonboat tick accumulation and approximately 8.2-second WAL append/commit delays. This is evidence of service/runner latency, not a metadata wrong-result assertion.
  • TestIssue25526And27935PreparedUpdateJoinBinaryProtocol: failed before entering its SQL callback, during shared-cluster startup. CN0's initial cluster refresh timed out; CN1's connection-refused errors followed teardown. No prepared UPDATE assertion ran in this failing invocation.

The checkpoint telemetry records no OOM kill. Coverage UT (exit 0), coverage aggregation, SCA, and both required BVT jobs passed at the failing head. No publication-replay functional failure is reported by this job.

Local verification (macOS arm64, not the Linux/x86 runner):

  • Final morpc tree, -race -tags matrixone_test -short: complete package passed (12.263 s).
  • Inactive-connect test and both connect-budget regressions: 100 race repetitions each passed (2.718 s).
  • Both unchanged SQL regressions: 2 normal repetitions each passed (12.084 s), then 3 repetitions each with CI's -race -tags matrixone_test -short mode passed (65.347 s).
  • Incremental lint: 0 issues; git diff --check clean.

Only the morpc test fixture was changed. No SQL expectation was relaxed, no test was skipped, and no global/production cluster timeout was increased. A fresh full CI result is still required; these local results do not establish that the new head is CI-green.

@mergify

mergify Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-10-24 02:27 UTC · Rule: main · triggered by rule Automatic queue on approval for main
  • 🟠 Checks running · in-place
  • 🚫 Left the queue — 2026-10-24 03:33 UTC · at c57ecbcddd6b49c48b0fe374eb258b416926df53

This pull request spent 1 hour 6 minutes 3 seconds in the queue, with no time running CI.

Reason

Pull request #30675 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

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

dequeued kind/bug Something isn't working kind/test-ci size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants