Skip to content

fix(view): activate durable metadata refresh - #27734

Open
ck89119 wants to merge 42 commits into
matrixorigin:mainfrom
ck89119:issue-26227-activation-main-clean
Open

fix(view): activate durable metadata refresh#27734
ck89119 wants to merge 42 commits into
matrixorigin:mainfrom
ck89119:issue-26227-activation-main-clean

Conversation

@ck89119

@ck89119 ck89119 commented Aug 27, 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 #26227

What this PR does / why we need it:

This is the final activation layer for the View metadata lifecycle built by #27267 and #27370 on top of the durable admission contract from #27430.

It:

  • advertises refresh support only after a CN observes the exact final catalog version and offset in READY state;
  • lets HAKeeper activate refresh only after every HAKeeper replica and known CN supports the lifecycle;
  • fences public metadata readers before a new membership epoch can require catalog revalidation;
  • runs one bounded, stopper-owned recovery worker per CN and acknowledges an epoch only after the durable catalog pass reaches a terminal state;
  • keeps refresh sealed when an epoch advance is canceled, rejects metadata-sensitive work from an older transaction epoch, and fences lifecycle-unaware CN or HAKeeper rollback;
  • enables fail-closed behavior for DESC, SHOW COLUMNS, information_schema.columns, and current-catalog CTAS while ordinary View queries continue to rebind the stored SQL;
  • preserves snapshot, subscription, restore, identity, and typed catalog-readiness behavior;
  • seeds revalidation through an idempotent version-offset upgrade.

The activation authority is the replicated HAKeeper admission epoch. No local MOCluster snapshot can independently authorize refresh.

Validation:

Design artifact

  • Pending re-approval design: View Metadata Lifecycle Admission, Activation, and Recovery
  • Semantic checkpoint: d0b5e7ac2d75c67cdac777ced8921ac1072f9401; documentation/evidence head: 5d59cccd0d4e3b5e189786ffb122116ffc79c2c5.
  • Status: Pending re-approval. There is currently no auditable reviewer identity or GitHub approval record; this PR must not be treated as design-approved until one is recorded.

Review closure evidence

  • Metadata-sensitive statement classification is AST-based at the frontend admission boundary. It walks table references for information_schema.columns; comments/formatting cannot bypass the fence, while CTE aliases and string literals do not create false positives.
  • Exact PR-head binary: 5d59cccd0d4e3b5e189786ffb122116ffc79c2c5; exact old/base binary: 6b8987741df912f38cece5bd026349d79672faf9.
  • A real 1 LogService/HAKeeper + 1 TN + 2 independently replaced CN sequence passed: all-old stale metadata (VARCHAR(12)), one-new/one-old capability-disabled legacy metadata (VARCHAR(12)), all-new activation (VARCHAR(48)), same-UUID CN restart, old-binary rollback denied before ingress, new-binary recovery, snapshot restore revalidation, and immediate whole-cluster restart.
  • The first restart run exposed a 30s startup deadline vs. old-generation expiry race. The final head preserves the old-owner safety fence and gives admission a second discovery window; CN/HAKeeper normal and race owning-package tests pass.
  • Historical CI checkpoint 6f15067b02 fixed Rows lifecycle SCA findings and added bounded lifecycle waits to four pre-existing View BVTs; the lifecycle BVT followed by all four affected tests passed 220/220. That historical BVT/build/lint evidence remains supplementary rather than being relabeled as current-head evidence.
  • Current semantic checkpoint d0b5e7ac2d75c67cdac777ced8921ac1072f9401 separates provisional epoch admission from metadata authority: ordinary base-table and binary prepared/cached planning remain available after authority expiry, while SHOW COLUMNS and information_schema.columns fail closed and recover after renewal. The frontend boundary test passes normally and with -race -count=100; full frontend, compile, and CN owning-package UTs pass.
  • The previous unrelated TestIssue26114CrossAccountBranchQuotaAndOwnership failure is outside this View lifecycle diff; Coverage failed only because a required producer failed.
  • Upstream upgrade Target/Release jobs are hard-disabled with if: false; those SKIPPED jobs are not counted as evidence.

Review round 2 closure

  • Epoch-0 metadata-sensitive readers now acquire a generation lease; TestViewMetadataInitialRefreshLeaseBlocksFirstAdvance proves the first Advance(1) cannot publish until that lease drains.
  • Same-UUID replacement snapshots now carry the authoritative remaining owner-expiry ticks and tick rate persisted by the HAKeeper RSM. CN startup derives its deadline from that owner expiry plus one discovery/reconciliation window; custom discovery=5s, owner remaining=30.1s yields a 36s budget rather than the old fixed 10s.
  • The AST detector is CTE-scope-aware. An unqualified columns shadowed by a CTE under default information_schema is ordinary work, while an explicitly qualified relation and a real information_schema.columns reference inside a CTE still acquire the lease.
  • Historical lifecycle-protocol revision (49364cd460, superseded and not current-head evidence) mixed-version run against base (6b8987741d) passed all-old, partial fail-closed, all-new activation, snapshot restore, same-UUID restart, old-binary rollback denial, recovery, and immediate full-cluster restart.

ck89119 and others added 19 commits August 28, 2026 11:48
…in-clean-fix

# Conflicts:
#	pkg/bootstrap/versions/v4_0_6/upgrade_test.go
…in-clean-fix

# Conflicts:
#	pkg/sql/compile/view_metadata_recovery.go
#	pkg/sql/compile/view_metadata_recovery_test.go
…in-clean-fix

# Conflicts:
#	pkg/frontend/mysql_cmd_executor.go
#	pkg/frontend/plan_cache.go
#	pkg/frontend/session.go
@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.

The mandatory design gate is still open, and the remaining gaps affect the architecture rather than implementation polish.

  1. Both the PR body and docs/design/20260827-view-metadata-lifecycle.md explicitly say Pending re-approval and record no auditable approval for the semantic checkpoint. This is an R3 distributed lifecycle/protocol change across HAKeeper, LogService, CN, catalog, frontend, planner, mixed-version restart and rollback. The document's own acceptance gate requires an exact reviewed revision and traceable approval before implementation approval.

  2. The failure-containment contract is not closed. After a valid admission response, renewViewMetadataAuthorityLease installs a timer; heartbeat loss beyond that lease reaches expireViewMetadataAuthority -> revokeViewMetadataGeneration, which seals all ingress, stops the frontend, and asynchronously closes the entire CN. Therefore a sufficiently long transient HAKeeper/CN partition can remove every CN and interrupt ordinary SQL, not only lifecycle-sensitive metadata reads. The design needs to state and justify the availability/RTO budget for this blast radius, explain why a narrower fail-closed boundary cannot preserve the invariant, and provide deterministic outage/partition acceptance evidence.

  3. The design puts a mutex-backed provisional lease plus an O(AST-nodes) detector on relation-reading statement paths, including prepared/cached paths, but provides no numeric latency/allocation/contention budget or base-vs-head evidence. Add an explicit acceptance threshold and representative before/after measurements for simple TP reads, cached/prepared execution, and metadata-sensitive statements.

Please obtain approval on the exact revised design checkpoint first. Per the design-first gate, implementation approval remains blocked until these decisions are resolved and recorded; I intentionally did not treat green CI as proof of the missing availability and hot-path contracts.

@aunjgr aunjgr 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 exact head 706359696ab071fe8cc6c53fe4747c57eb9a61c4 against merge base 1a4f656bb2b5b7f2fb1a6d951c4806ce7d124ba5 and newest main. This is an R3 distributed lifecycle/protocol activation across HAKeeper RSM, LogService wire state, CN generation/admission, catalog durability, frontend ingress, planner classification, restart and rollback. The mandatory design gate remains explicitly open: both the PR body and docs/design/20260827-view-metadata-lifecycle.md say Pending re-approval, record no auditable reviewer/approval for the semantic checkpoint, and state implementation approval is blocked until that record exists.

Two architectural decisions still require approval and evidence before the code can be accepted:

  1. Authority-lease expiry has whole-CN blast radius. A long HAKeeper-only partition reaches expireViewMetadataAuthority -> revokeViewMetadataGeneration, synchronously seals ingress/stops the frontend and requests asynchronous full CN closure. The deterministic test confirms exactly that behavior while an existing metadata reader is blocked. A control-plane partition affecting all CNs can therefore remove the entire SQL serving fleet and interrupt ordinary, metadata-insensitive work. Record and justify the availability/RTO budget, why a narrower metadata-sensitive fail-closed boundary cannot preserve the invariant, and provide deterministic multi-CN partition/recovery acceptance evidence for the chosen blast radius.

  2. Relation-reading statement admission adds a mutex-backed provisional lease plus O(AST-nodes) classification to ordinary, prepared and cached paths. The design lists the mechanism but no numeric latency/allocation/contention threshold or base-vs-head evidence for simple TP reads, cached/prepared reuse, and lifecycle-sensitive statements. Establish that hot-path budget and demonstrate it on the exact semantic checkpoint.

The epoch ownership, durable required/completion transitions, stale-generation fencing, worker bounds, restart/rollback tests, cursor epoch handling, and catalog publication logic are otherwise extensively covered. Obtain traceable approval of the final design revision after closing these decisions, then record implementation conformance against that exact revision; green CI cannot substitute for the explicitly pending design/availability gate.

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 size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants