fix(proxy): preserve temporary tables during CN migration

[iamlinjunhong]

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #27602

What this PR does / why we need it:

fix(proxy): preserve temporary tables during CN migration

[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]

There is one blocking temporary-table lifecycle case that still makes migration fail permanently. See the inline comment.

[aptend]

DROP DATABASE does not remove entries from ses.tempTables, and snapshotTempTables exports those entries. Reproducer: CREATE DATABASE d; CREATE TEMPORARY TABLE d.t(i INT); USE mysql; DROP DATABASE d;, then mark the source CN as draining. This code generates CREATE TEMPORARY TABLE d.t CLONE d.__mo_migrate_source_0; Scope.createTable must open database d, so the target returns Unknown database d. Migrate already treats a deleted current req.DB as non-fatal, but this per-table path turns the same state into a target migration failure, and Proxy retries it every 5 seconds. The otherwise usable client connection therefore remains pinned to the draining CN, preserving the endless-retry failure mode from #27602. A temporary counterexample added to TestMigrateTempTablesClonesIntoTargetOwnership with the clone executor returning moerr.NewBadDB failed exactly with Unknown database db.with.dot. Please clean or exclude definitively stale mappings before export, or handle this target case without converting it into a permanently retried migration.

[iamlinjunhong]

Fixed in 5f01b70. DropDatabase now removes all session temporary-table mappings for the deleted database after its cleanup path completes. The removal is forwarded through background sessions and uses the existing statement/transaction journal, so failed statements and rolled-back transactions restore the original identities. Focused coverage now verifies the deleted database is absent from the migration snapshot, statement rollback, recreate using only the new physical name, and transaction rollback.

[XuPeng-SH]

Blocking on exact head 173a615.

The migration snapshot can contain definitively stale temporary-table mappings after the database has been dropped. DROP DATABASE removes the database but does not remove entries from ses.tempTables, while snapshotTempTables exports every non-internal entry. For example: CREATE DATABASE d; CREATE TEMPORARY TABLE d.t(i INT); USE mysql; DROP DATABASE d; then drain the source CN. The target runs CREATE TEMPORARY TABLE d.t CLONE d.__mo_migrate_source_0 and fails with Unknown database d.

This is not a one-off migration error: Proxy retries the same state every five seconds, so an otherwise usable connection remains pinned to the draining CN and the endless-retry half of #27602 is still reproducible. Recreating d before migration can similarly leave an alias pointing at a deleted source physical relation.

Please remove or exclude stale per-database mappings with transaction and statement rollback semantics, or make the migration path explicitly recognize and discard only entries proven stale. Add a counterexample covering DROP DATABASE followed by migration, including rollback/recreate behavior. The focused frontend and proxy tests added by this PR pass, but none exercise this lifecycle transition.

[aptend]

Deep re-reviewed exact head c8026dc9d6290aad4a8d774548dec83710a1bceb, including the complete diff, all reviews/inline comments/replies, the unresolved thread, and the delta after my prior reviewed head 173a6152054515f00e8f676373aaadd2ec626371. The stale DROP DATABASE mapping is now handled for both same-session and cross-session cases, snapshot bounds are present, and the v32 rebase is coherent. Build, vet, full frontend/proxy/compile tests, and 20 focused race repetitions pass locally. Two acceptance/design blockers remain: the RFC still records that its independent approval gate is unmet, and the new topology test does not actually prove two of the acceptance claims it is cited for. The Java test could not be compiled on this host because its installed JDK does not support the project's Java 8 release target; the Go-side validation is complete.

[aptend]

[blocking design gate] This exact revision still declares the RFC draft, and line 140 explicitly says independent design approval is still required before it can advance. This change adds a versioned distributed protocol, source/proxy/target negotiation, cross-CN physical ownership transfer, partial-clone/commit/retry cleanup, and rolling-upgrade behavior, so merging the implementation while its own design gate remains unmet makes those invariants de facto API behavior before the owning design is approved. Please obtain a traceable independent approval of this exact RFC revision and advance the status according to the RFC's stated process; a generic or earlier code approval does not establish which ownership/compatibility revision was approved.

[aptend]

[blocking acceptance gap] This loop does not exercise the five-second failure/retry condition it claims to guard: all three probes can finish in milliseconds. It also never proves that idx_grp survived—the post-handoff WHERE grp = 1 query can return the same rows by a full table scan even if the hidden index was not cloned. A counterexample implementation that drops the hidden index and schedules another migration/failure at t=5s passes every assertion here. That leaves two explicit RFC/issue acceptance requirements untested and lets the distributed path be declared validated while index state or retry termination is broken. Assert the index metadata/access path after handoff, observe the connection beyond at least one retry interval (or assert a migration-attempt counter/log invariant), and attach the exact-head two-CN run artifact rather than only stating that an environment-gated test was run.

[iamlinjunhong]

Addressed by eed54db:

• The JDBC connection now sets useServerPrepStmts=true explicitly, so the PreparedStatement path uses the server binary protocol.
• After handoff, the test verifies idx_grp with SHOW INDEX, then keeps issuing a binary prepared query while asserting the same target server for six seconds—longer than Proxy's five-second retry interval.
• The commit and rollback cases similarly remain on the source for that full interval before the transaction boundary.

The environment-gated class compiles on this revision with ./mvnw -q -Dtest=ProxyTempTableMigrationE2ETest test.

[XuPeng-SH]

Deep re-review of exact head c8026dc9d6290aad4a8d774548dec83710a1bceb, including the full prior lifecycle review, range-diff from the previously reviewed a01d167524347c84d04b036af31450e679125537, current RFC, topology test, issue acceptance matrix, and exact-head CI.

The production patch is equivalent across the rebase and the earlier code blockers remain closed: same- and cross-session stale DROP DATABASE mappings are handled, snapshot count/size is bounded, source/target ownership is separated, borrowed aliases are deferred-cleaned, and non-stale clone failures remain fail-closed. Two mandatory gates still block approval:

1. [Design gate] The current RFC revision is still explicitly unapproved. docs/rfcs/20260826_proxy_temp_table_migration.md:1 remains Status: draft, and lines 138-140 say independent design approval is still required before advancing. The only independent approval predates the new topology/RFC commit and does not identify this revised design. This feature changes a versioned distributed protocol, cross-CN physical ownership, retry/idempotency, partial-failure cleanup, and rolling-upgrade behavior; obtain traceable approval of this exact RFC revision and advance it to in-progress before implementation approval.

2. [Acceptance gate] The checked-in topology test does not prove several contracts it and the RFC claim, and no exact-head terminal artifact is attached.

   • Creating idx_grp and querying WHERE grp = 1 proves only row visibility; the same result is returned by a full scan if the hidden index was not cloned. Assert post-handoff index metadata/physical index existence.
   • The three immediate probes at lines 106-110 can finish in milliseconds, before Proxy's defaultScalingInterval = 5s; they cannot detect a repeated migration attempt/failure at the next interval. Likewise, the transaction tests make one immediate source assertion after observing Draining, so they do not prove an attempted handoff was rejected while the transaction was active. Cross at least one real attempt boundary or assert a migration-attempt/log/metric invariant deterministically.
   • DriverManager.getConnection(url) does not force Connector/J's useServerPrepStmts=true; without the external URL option, PreparedStatement can be client-emulated and the test never exercises binary COM_STMT_PREPARE/EXECUTE. Set the property in test code (and preferably assert it), rather than relying on undocumented runner discipline.
   • Ordinary CI only compiles/runs this class with the environment assumption skipped; the PR contains no exact-head two-CN + Proxy terminal report showing these acceptance cells passed.

Please close those cells and attach the exact-head run artifact requested by issue #27602 and the RFC. git diff --check and ordinary exact-head CI are green, but they do not execute this environment-gated distributed path.

[XuPeng-SH]

Deep re-reviewed exact head eed54db29b02a70a777fefd086871d3cda09d548.

The latest acceptance-test delta closes the concrete assertion gaps from my previous review: it forces useServerPrepStmts=true, verifies idx_grp, observes the target beyond Proxy's five-second retry interval while executing binary prepared queries, and holds active commit/rollback transactions on the source across that interval.

The production Q1-Q3 lifecycle audit also remains closed: source/target physical ownership is separate; borrowed aliases are defer-cleaned; failure cleanup owns only target clones; only proven-stale BadDB/NoSuchTable entries are discarded; other errors fail closed; waits are bounded; and the snapshot is capped at 1,024 tables / 16 MiB.

One mandatory gate remains unproven. The topology class is guarded by MO_PROXY_TEMP_TABLE_E2E_URL; ordinary CI compiles it but skips all three tests. There is no exact-head terminal artifact showing this revision passed on the declared two-CN + Proxy topology. Please attach that run, covering the indexed table, SQL PREPARE, binary COM_STMT, transaction admission, and >5s stable handoff.

The RFC also still says Status: draft and explicitly claims independent approval remains outstanding. If an existing approval covers the unchanged exact RFC, update its status/text so the checked-in design no longer declares its own gate incomplete.

[aunjgr]

Requesting changes on exact head .\n\nThe latest Java topology test now closes the earlier test-design gaps: it enables real server prepares, observes both retry windows for six seconds, holds the active-transaction case long enough, and verifies the hidden index. The remaining blocker is execution/approval closure for this distributed state-transfer change. The environment-gated test is not evidence that the two-CN+Proxy topology actually ran on this exact head, and the governing RFC is still draft without the required independent design approval.\n\nPlease attach a traceable exact-head terminal run covering the required migration/retry/transaction matrix and move the RFC through its approval gate. Standard green CI does not substitute for the skipped topology validation this change depends on.

[aunjgr]

Requesting changes on exact head eed54db29b02a70a777fefd086871d3cda09d548.

The latest Java topology test now closes the earlier test-design gaps: it enables real server prepares, observes both retry windows for six seconds, holds the active-transaction case long enough, and verifies the hidden index. The remaining blocker is execution/approval closure for this distributed state-transfer change. The environment-gated test is not evidence that the two-CN+Proxy topology actually ran on this exact head, and the governing RFC is still draft without the required independent design approval.

Please attach a traceable exact-head terminal run covering the required migration/retry/transaction matrix and move the RFC through its approval gate. Standard green CI does not substitute for the skipped topology validation this change depends on.

[XuPeng-SH]

Deep re-reviewed exact head cbb1ebe3611917194b0da07e5ffd7a959e4f0e53 without waiting for the new generic CI.

The new checked-in execution record closes the topology-execution blocker. It binds the production-code head eed54db29b02a70a777fefd086871d3cda09d548, identifies the repository's two-CN + Proxy launch, forces Connector/J server prepares, records the three-test suite as 3/3 passing, and covers the indexed-table/prepared-statement/retry-window/transaction matrix exercised by the strengthened test. The current head changes only this documentation, so that evidence applies to the reviewed implementation.

One declared design gate still blocks approval. The RFC remains Status: draft, and its current Open questions section explicitly says that an independent design approval is still required before the status changes. This PR introduces a versioned distributed protocol and cross-CN physical-ownership lifecycle, so its own design gate must not be waived implicitly. Please obtain a traceable independent approval of the current RFC revision and update the RFC status/text accordingly. No additional production-code correctness, performance, hang, or unhappy-path blocker was found.

[XuPeng-SH]

Deep re-reviewed exact head ce5bf5ae82d01b5444a0bea13e6e9a46a50afdd9 against merge base 6b8987741df912f38cece5bd026349d79672faf9, including a range-diff from my previously reviewed head cbb1ebe3611917194b0da07e5ffd7a959e4f0e53.

The rebase is coherent. Commits 2-8 are patch-equivalent; commit 1 only moves the temporary-table migration protocol gate from v32 to v33 because main now owns v32, and the final commit updates the RFC/test wording. The v33 source and target gates agree, and focused frontend/proxy migration tests pass.

The prior production Q1-Q3 audit remains closed:

• Q1 ownership: source physical tables remain source-owned; borrowed aliases are defer-cleaned; failed targets clean only target clones.
• Q2 termination: migration/clone work remains under the transfer context; stale catalog entries terminate by bounded discard, while other errors fail closed.
• Q3 bounds: the snapshot remains capped at 1,024 visible tables and 16 MiB.

One mandatory design gate is still explicitly open. docs/rfcs/20260826_proxy_temp_table_migration.md:1 remains Status: draft, and its Open questions section still says “an independent design approval is still required before status changes from draft.” The only new commit renumbers the protocol and does not provide that approval or advance the RFC. This change introduces a versioned distributed protocol plus cross-CN physical-ownership and retry semantics, so the PR cannot declare its own governing design unapproved and still be implementation-approved.

Please obtain/record traceable independent approval of the current RFC revision and update the status/open-question text. No additional correctness, performance, hang, cleanup, or acceptance blocker was found on this head.

[XuPeng-SH]

Additional protocol blocker found while reviewing the next queued PR: #27553 and this PR currently reserve the same MORPCVersion33 for two different wire capabilities (MongoScan explicit-query fields versus temporary-table migration fields).

These cannot share one deployment gate. If one feature lands in a v33 binary and the other is later added without a new version, oldest-live v33 no longer proves that every receiver understands both payloads, so the later feature can cross a rolling-upgrade boundary to a v33 receiver that silently ignores/does not implement it. The overlapping pkg/defines/const.go edits may cause a merge conflict, but conflict resolution must not keep both at v33.

Please coordinate merge order and allocate the later feature a fresh version (currently v34), updating all source/send/receive/Prepare gates, tests, and RFC wording. Given commit times, #27553 reserved v33 first and this PR's later ce5bf5ae82 reservation should presently move to v34, unless the merge order is explicitly reversed.