fix(frontend): clean account-owned metadata on DROP ACCOUNT

[gouhongshen]

What type of PR is this?

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

Which issue(s) this PR fixes:

Related to #27660

What this PR does / why we need it:

Root cause

mo_catalog.mo_branch_metadata and mo_catalog.mo_feature_limit are physically global, predefined catalog tables. Generic DROP ACCOUNT cleanup only deletes cluster-table rows through their physical account_id ownership column. The affected rows are owned semantically by mo_branch_metadata.creator and mo_feature_limit.account_id, so the original cleanup missed them and left permanent orphan metadata.

For Branch metadata, creator identifies the destination account that created an edge; it is not exclusive ownership of the edge's lineage. An unconditional delete by creator can remove an ancestor edge and its protect snapshot while a live descendant account still depends on that edge.

Changes

• Add one reusable DAG-aware account-drop planner for Branch metadata. It uses creator only to select the dropped account's roots, walks their ancestors, and reclaims a normal edge only when its complete descendant subtree is deleted.
• Load Branch metadata with the semantic creator field under FOR UPDATE, retain deleted ancestor edges needed by live cross-account descendants, and reclaim their protect snapshots and metadata together when the final descendant account is dropped.
• Guard the metadata delete with table_deleted and normal-lineage predicates. DROP ACCOUNT invokes the existing ownership-aware historical-lineage compactor after removing the dropped account user snapshots and PITR rows, so ALTER generations are reclaimed only when no external historical source or live descendant needs them.
• Execute Branch and feature-limit cleanup in doDropAccount under the system-account context and the existing account-drop transaction, propagating errors through the existing rollback/finish path.
• Preallocate the computed snapshot-drop list to satisfy the repository's static-analysis requirement without changing reclaim semantics.
• Add unit coverage for both transaction modes, system-account execution, cleanup failure short-circuit/rollback, DAG retention/reclamation, ALTER exclusion, and guarded SQL generation.
• Add the original issue BVT plus a deterministic cross-account B -> A -> C BVT. The chain uses ordinary publication/subscription setup followed by real Data Branch operations, drops A while C is live, verifies A's deleted edge and both protect snapshots remain, then drops C and verifies the complete chain and quotas are gone.

Issue-to-test proof

• drop_account_branch_metadata.sql/.result proves the reported lifecycle: a real Branch operation creates one lineage row and one BRANCH quota row, both semantic rows are gone after DROP ACCOUNT, same-name recreation receives a new account ID, old IDs remain isolated, and the recreated account is cleaned too.
• drop_account_branch_metadata_chain.sql/.result proves the review boundary: it creates and asserts a B -> A -> C lineage, checks that dropping A retains the deleted A edge and both snapshots while C is live, then checks that dropping C removes both chain metadata rows, both snapshots, all rows for both creators, and C's quota.
• TestComputeAccountBranchReclaimPlan proves that live descendants retain an ancestor edge, the final child drop reclaims the retained ancestor, and ALTER rows are excluded.
• Test_doDropAccount_InTransaction and the restore-path coverage prove both transaction modes execute the semantic scan/cleanup under the system account.
• Test_doDropAccount_AccountOwnedMetadataCleanupError proves a Branch metadata scan error is returned, the newly opened transaction is rolled back, and feature-limit cleanup is not attempted.

Tests

• .agents/skills/mo-dev/scripts/mo-cgo-test -count=1 ./pkg/frontend
• .agents/skills/mo-dev/scripts/mo-cgo-test -count=1 ./pkg/frontend/databranchutils
• .agents/skills/mo-dev/scripts/mo-cgo-test -count=1 -run 'TestBranchProtectSnapshot_(CrossAccount|CrossAccount_DropSourceFirst)' ./pkg/vm/engine/test
• go vet ./pkg/frontend ./pkg/frontend/databranchutils ./pkg/vm/engine/test with the repository CGo include/library paths
• golangci-lint run -c .golangci.yml ./pkg/frontend/databranchutils with the repository CGo include/library paths (0 issues)
• make build (after make thirdparties and make cgo)
• mo-tester method=run for test/distributed/cases/tenant/drop_account_branch_metadata.sql: 28/28 statements passed, 100% success rate
• mo-tester method=run for test/distributed/cases/tenant/drop_account_branch_metadata_chain.sql: 43/43 statements passed, 100% success rate
• mo-tester method=run for test/distributed/cases/tenant/drop_account_branch_metadata_alter.sql: 45/45 statements passed, 100% success rate

Residual risks

Account drop now performs one full locked read of the global Branch metadata table and up to four semantic cleanup deletes in the existing synchronous transaction. The read is required to evaluate cross-account DAG dependencies atomically; it may add latency for catalogs with very large Branch metadata history. No asynchronous cleanup or fallback path is introduced. Local BVT execution used a direct CN endpoint because the standard multi-CN Docker image was unavailable in this environment; the committed cases remain in the distributed BVT suite and use separate tenant sessions.

Latest implementation update

DROP ACCOUNT now invokes the shared ownership-aware historical ALTER compactor after the dropped account user snapshots and PITR rows are removed. Deleted account-owned ALTER generations and their branch-protect snapshots are reclaimed in the same transaction when no external historical source or live descendant remains, while a live external historical source retains them.

Added drop_account_branch_metadata_alter.sql/.result, covering a real cross-account copy-and-swap ALTER, no-external-owner cleanup, and an external account-snapshot control that retains the lineage across account drop and releases it when the owner snapshot is dropped.

After rebasing onto origin/main at 1d3483ac97 with current PR head ea9635da89, verification passed: rebase-focused DROP ACCOUNT frontend tests and cross-account Branch engine tests passed; prior full pkg/frontend/..., CGo-configured go vet ./pkg/frontend/..., make build, and mo-tester 116/116 statements remain green.

Freshness rebase update

After origin/main advanced to 22d56a3, the PR branch was rebased to 24a8465. Rebase-focused DROP ACCOUNT frontend tests and cross-account Branch engine tests passed again.

Freshness rebase update

After origin/main advanced to c1fe7b4, the PR branch was rebased to 6f550e5. Rebase-focused DROP ACCOUNT frontend tests and cross-account Branch engine tests passed again.

Freshness rebase update

After origin/main advanced to 43d361f, the PR branch was rebased to 225ae83. Rebase-focused DROP ACCOUNT frontend tests and cross-account Branch engine tests passed again.

[qodo-code-review]

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 →

[aptend]

Reviewed exact head 317458b and the full account-drop transaction, including system/tenant context switches, both transaction modes, semantic ownership keys, rollback/short-circuit behavior, cross-account branch ownership, restore interaction, and the distributed same-name recreation case. Build, vet, focused tests, full pkg/frontend tests, and 100 targeted race repetitions pass locally. No blocking issue found.

[XuPeng-SH]

Blocking correctness issue: the unconditional delete from mo_catalog.mo_branch_metadata where creator = <accountID> is unsafe for chained cross-account branches such as B -> A -> C. creator is the destination account, not exclusive lineage ownership. When A is dropped, A's branch metadata is marked deleted, but C is still live, so the existing reclaim algorithm intentionally keeps the A-edge protect snapshot. This new DELETE then removes A's metadata while its snapshot remains, breaking C's ancestor/LCA chain and leaving an orphan snapshot that cannot be reclaimed when C is later dropped. The existing cross-account source-drop test confirms that a protect snapshot must remain while a child is alive:

matrixone/pkg/vm/engine/test/branch_protect_snapshot_test.go

Lines 550 to 575 in 317458b

	// ---------------------------------------------------------------------------
	// ET-G7 — CrossAccount drop of the *source* leaves the branch snapshot
	// alive because the child (in account b) is still referenced.
	// ---------------------------------------------------------------------------
	func TestBranchProtectSnapshot_CrossAccount_DropSourceFirst(t *testing.T) {
	env := setupBranchProtectSnapshotEnv(t)
	defer env.close(t)
	const (
	parentTID = uint64(7001)
	childTID = uint64(7002)
	)
	env.simulateBranchCreate(t, childTID, parentTID, 700_000, "acc_a", "db", "t1", parentTID)
	// The "parent dropped" flow does not update mo_branch_metadata for
	// the parent (the parent is not a branch). The reclaim hook only
	// runs against the child tid. So if the operator only drops the
	// parent, the child row stays.
	remaining := env.runReclaim(t, []uint64{parentTID})
	require.Equal(t,
	[]string{databranchutils.BranchSnapshotName(childTID)},
	remaining,
	"dropping only the parent must NOT reclaim the child's branch snapshot",
	)
	}

. Please make account cleanup DAG-aware: retain deleted rows still needed by live descendants, reclaim only after the complete descendant subtree is gone, and add a BVT covering B -> A -> C, DROP ACCOUNT A, then DROP C, checking lineage and snapshot cleanup.

[gouhongshen]

Addressed in dfb0ecebf3.

The account-drop path now loads mo_branch_metadata under the system account with FOR UPDATE and uses creator only to select roots. The reclaim planner walks each root's ancestors and deletes a normal metadata edge and its protect snapshot only when the complete descendant subtree is deleted. Therefore, in B -> A -> C, dropping A retains A's deleted edge and both protect snapshots while C is live; dropping C then reclaims the C edge and the retained A edge together. ALTER generations remain owned by the historical-lineage compactor, and the metadata DELETE repeats the normal/deleted predicates at execution time.

Added drop_account_branch_metadata_chain.sql/.result, which builds the chain through ordinary publication/subscription setup and real Data Branch operations, asserts the chain and quotas, checks retention after dropping A, and checks metadata, snapshots, creator rows, and quota cleanup after dropping C. The original lifecycle/same-name recreation BVT remains covered as well.

Verification after rebasing onto origin/main at beeda8aef8: full pkg/frontend, full pkg/frontend/databranchutils, cross-account Branch engine tests, and CGo-configured go vet passed; make build passed; both BVT cases passed (71/71 statements overall).

[XuPeng-SH]

Re-reviewed exact head 3b0b740 after my earlier DAG-lifecycle request changes.

The B -> A -> C blocker is fixed: creator is now only the root selector, ancestor reclaim is subtree-aware, live descendants retain the deleted ancestor edge, and the final child drop reclaims both normal edges and their protect snapshots.

One blocking lifecycle gap remains for ALTER generations.

branchReclaimableTableIDs deliberately skips every level='alter' / alter:* row, and BuildBranchMetadataDeleteSQL repeats that exclusion. However, doDropAccount calls only reclaimAccountOwnedBranchMetadataWithBH; it never invokes compactHistoricalAlterLineageWithBH. The compactor is currently triggered only by Data Branch DELETE and snapshot/PITR operations. Once the account is gone, none of those provides a bounded cleanup guarantee.

Deterministic counterexample:

1. account A creates a normal branch row T2;
2. copy-and-swap ALTER creates an ALTER lineage row T3 owned by A;
3. DROP ACCOUNT marks both generations deleted;
4. this new planner can delete T2 and __mo_branch_2 but explicitly retains T3 and __mo_branch_3;
5. no later operation is guaranteed to run the historical-lineage compactor, so the row keyed by the deleted account and its protection snapshot can remain indefinitely.

The added TestComputeAccountBranchReclaimPlan confirms the skip but does not close the lifecycle. This violates issue #27660's requirement that account-owned branch tombstones not accumulate indefinitely (or have a bounded documented cleanup process).

Please run the historical ALTER compaction safely in the account-drop transaction after account snapshot/PITR ownership has been removed, or otherwise include eligible ALTER generations in a bounded ownership-aware cleanup. Add an end-to-end case that creates a branch, produces an ALTER generation, drops the account, and proves both creator metadata and branch snapshots are gone when no external historical owner/live descendant remains. Also retain a control where an external snapshot/PITR or live descendant still requires the generation.

I also rechecked the normal-edge delete guards, system-account context, rollback propagation, feature-limit cleanup, full-DAG lock ordering, cycle bounds, and the exact-head green CI. No other blocker found.

[gouhongshen]

Addressed in 1065662c00.

The lifecycle gap was valid. DROP ACCOUNT now invokes the shared ownership-aware historical ALTER compactor after removing the dropped account user snapshots and PITR rows. Deleted account-owned ALTER generations and their branch-protect snapshots are reclaimed in the same transaction when no external historical source or live descendant remains; a live external account snapshot retains them.

Added drop_account_branch_metadata_alter.sql/.result, covering a real cross-account copy-and-swap ALTER, no-external-owner cleanup, and an external account-snapshot control. The new case observes 1 ALTER row and 2 branch snapshots before DROP ACCOUNT, 0/0 after drop without an external source, 1/1 retained with an external owner, and 0/0 after that owner snapshot is dropped. Verification after rebasing onto origin/main at 6b8987741d: full pkg/frontend/..., cross-account Branch engine tests, CGo go vet, make build, and 116/116 mo-tester statements passed.

[XuPeng-SH]

Re-reviewed exact head ca25d0d5b1be811e0724091b7963d0cbe747f1a1 against merge-base a73f76d109bd528ae2e53a4b8ab294422fb13e21, with special focus on the delta after my review of 3b0b74027bb6c3b4f8acff235c4715b82c95ccc3.

Both prior blockers are now closed.

1. Normal cross-account lineage is DAG-aware: creator selects roots only; ancestor candidates are retained while any descendant is live, ALTER rows are excluded from this delete path, and the SQL repeats table_deleted plus level guards. The B -> A -> C test proves dropping A retains the ancestor edge/snapshots and dropping C later reclaims the complete normal chain.
2. The remaining ALTER lifecycle is now bounded. doDropAccount invokes the shared ownership-aware historical compactor in the same transaction after deleting the dropped account's user snapshots and PITR rows. No-source deleted ALTER generations and their deterministic protect snapshots are reclaimed; an external account snapshot retains the generation, and dropping that owner later releases it. This is the exact control requested in the previous review.

I rechecked transaction/context behavior: both caller transaction modes execute the scans/deletes as the system account; errors return through the existing rollback/short-circuit path; normal metadata and its snapshot are deleted as one transaction; the compactor retains live logical components, identity-mismatched edges, live rows missing identity snapshots, and components covered by user snapshot/PITR timestamps. DAG walks are cycle-bounded and memoized, so corrupt cycles do not hang and shared descendants do not cause quadratic subtree recomputation.

Performance cost is confined to the low-frequency synchronous DROP ACCOUNT path: currently it performs two full reads of global branch metadata under the same transaction lock plus the historical-source scans. That can increase account-drop latency for very large catalogs, but it avoids per-row queries, keeps memory O(lineage rows), and is required for atomic cross-account ownership decisions; it does not affect normal query/DML hot paths. No new goroutine, channel, file, or asynchronous cleanup ownership is introduced.

Exact-head CI is green, including Ubuntu UT, SCA, coverage, multi-CN proxy BVT, and multi-CN pessimistic BVT. The targeted three BVTs cover same-name account recreation, chained normal branches, ALTER cleanup, and external historical retention. I found no remaining blocking correctness, performance, lifecycle, or hang issue.

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-08-27 19:30 UTC · Rule: main · triggered by rule Automatic queue on approval for main
• 🚫 Left the queue — 2026-08-27 19:34 UTC · at ca25d0d5b1be811e0724091b7963d0cbe747f1a1

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

Reason

The pull request #27690 has been manually updated

Requeued — the merge queue status continues in this comment ↓.

[XuPeng-SH]

Incremental deep re-review completed at exact head 24a846528dfa3dce2fee3fe8323eee22289a1eb2 on current main 22d56a30b102b58050a31ff04e4f5187541c0f7b.

The rebased PR aggregate patch-id is identical to the fully reviewed head ca25d0d5b1be811e0724091b7963d0cbe747f1a1 (b6340f717ac73b616c90ac75b88eaa1d4926b4c1). There are no files changed by both the PR and the intervening main range, and git diff --check is clean. The prior DAG ownership, ALTER-generation compaction, external historical owner, correctness, catalog-scan performance, and cleanup unhappy-path conclusions therefore remain valid on this exact head. Per reviewer direction, I am not waiting for rerun CI.

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-08-27 21:03 UTC · Rule: main · triggered by rule Automatic queue on approval for main
• ✅ Checks passed · in-place
• ✅ Merged — 2026-08-27 22:23 UTC · at fcdebf46c7deb9d946324b3144d3d0551bf95bae · squash

This pull request spent 1 hour 19 minutes 47 seconds in the queue, including 47 minutes 45 seconds running CI.

Required conditions to merge

• #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • fix(frontend): clean account-owned metadata on DROP ACCOUNT #27690
• github-review-approved [🛡 GitHub branch protection]
  • fix(frontend): clean account-owned metadata on DROP ACCOUNT #27690
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone CI / UT Test on Ubuntu/x86
  • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
  • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone CI / SCA Test on Linux/arm64
  • check-neutral = Matrixone CI / SCA Test on Linux/arm64
  • check-skipped = Matrixone CI / SCA Test on Linux/arm64
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PROXY)
  • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PROXY)
  • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PROXY)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Utils CI / Coverage
  • check-neutral = Matrixone Utils CI / Coverage
  • check-skipped = Matrixone Utils CI / Coverage
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone UT Coverage / UT Coverage on Ubuntu/x86
  • check-neutral = Matrixone UT Coverage / UT Coverage on Ubuntu/x86
  • check-skipped = Matrixone UT Coverage / UT Coverage on Ubuntu/x86
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Standlone CI / multi CN e2e BVT Test on Linux/x64(COMPOSE, PESSIMISTIC)
  • check-neutral = Matrixone Standlone CI / multi CN e2e BVT Test on Linux/x64(COMPOSE, PESSIMISTIC)
  • check-skipped = Matrixone Standlone CI / multi CN e2e BVT Test on Linux/x64(COMPOSE, PESSIMISTIC)