lokiore opened a new pull request, #2592: URL: https://github.com/apache/phoenix/pull/2592
### What changes were proposed in this pull request? This replaces the inline partial pass that previously ran during transform cutover with an explicit, restartable **cutover state machine** in `TransformMonitorTask`, and tears down the dual-write links at the cutover commit so a single client cache-invalidation cycle propagates both the physical-table pointer swap and the dual-write shutoff. **Lifecycle** (each state is committed to `SYSTEM.TRANSFORM` and monitored by the self-healing `TransformMonitorTask`): 1. `PENDING_CUTOVER -> PENDING_PARTIAL_PASS` — after `doCutover` swaps the physical-table pointer, persist a wait deadline instead of running the partial pass immediately. The deadline is the logical table's update-cache-frequency scaled by a safety margin (`1.10`), floored at 30 minutes and capped at 24 hours, stored in a new nullable `BIGINT` column `PENDING_PARTIAL_PASS_UNTIL_TS`. The raw frequency is clamped to the 24-hour ceiling *before* scaling: a table configured to never refresh its cache resolves its update-cache-frequency to `Long.MAX_VALUE`, and scaling that then adding it to the current time would saturate into a negative (past) deadline that defeats the wait. This lets clients still holding a cached pointer to the old physical table refresh before the partial pass runs, so late writes routed to the old table are not stranded as unverified rows. 2. `PENDING_PARTIAL_PASS -> PARTIAL_PASS_RUNNING` — once the wait window elapses, commit the transition (clearing the inherited full-pass job id so the monitoring branch cannot mistake the already-successful full pass for the partial pass and complete early), then kick the partial-pass `TransformTool` run. 3. `PARTIAL_PASS_RUNNING -> COMPLETED / FAILED` — monitor the partial-pass job. A pass that cannot be confirmed successful (no job id registered because the initial kick failed before its `STARTED` transition, an unsuccessful job, or a job id that no longer resolves) is routed through a retry budget; once retries are exhausted the record reaches terminal `FAILED` rather than stranding a pointer-swapped table with unverified rows forever. **Dual-write link teardown at cutover commit** (`Transform.doCutover`): - The base-table `TRANSFORMING_NEW_TABLE` link is deleted uncommitted and batched into the same commit as the base-table pointer swap. - Each child view's link is deleted inside the existing `MUTATE_BATCH_SIZE` view loop, paired with that view's pointer swap. A base table can have millions of views, so folding per-view link teardown into the bounded batch loop keeps every commit bounded while still pairing each link removal with its swap in one cache-invalidation cycle. `doGetTable` attaches the transforming-new-table per row from link presence, so a surviving view link would keep dual-write alive for view-routed writes after cutover. **Schema:** `PENDING_PARTIAL_PASS_UNTIL_TS` is the first column ever added to `SYSTEM.TRANSFORM`. On upgrade, `ConnectionQueryServicesImpl` snapshots the table and runs `addColumnsIfNotExists` guarded on `MIN_SYSTEM_TABLE_TIMESTAMP_5_4_0` — the same (unreleased) 5.4.0 system-table timestamp that `SYSTEM.CATALOG`'s header already reaches via its `INDEX_CONSISTENCY` column add — so no new min system-table timestamp is introduced. The client upgrade gate compares against `SYSTEM.CATALOG`'s own reported timestamp, so introducing a new min not matched by a `SYSTEM.CATALOG` column-add at that timestamp would leave the catalog below the gate after an in-place upgrade and loop clients on `UpgradeRequiredException`; riding the existing 5.4.0 timestamp avoids that. A fresh install gets the column directly from the `CREATE TABLE` DDL. `SystemTransformRecord` / `TransformClient` read and write the new column with explicit `BIGINT` null handling. ### Why are the changes needed? Running the partial pass inline at cutover repaired unverified rows immediately, but clients could still hold a cached pointer to the old physical table for up to their update-cache-frequency window. Writes routed to the old table during that window landed after the partial pass had already run, so they were never repaired and remained as unverified rows. Deferring the partial pass until after the cache-refresh window closes, and shutting off dual-write in the same cache cycle as the pointer swap, closes that gap. Making cutover an explicit, committed state machine also lets a partial pass that fails reach a terminal state (retry-budgeted, then `FAILED`) instead of being lost when the monitor process restarts. ### Does this PR introduce _any_ user-facing change? No. This targets the `PHOENIX-7904-feature` branch (Online Schema Change gap-fix initiative), which is pre-launch. Two new `TransformStatus` values (`PENDING_PARTIAL_PASS`, `PARTIAL_PASS_RUNNING`) and one nullable `SYSTEM.TRANSFORM` column are added; both are internal to the transform lifecycle. ### How was this patch tested? New `CutoverLifecycleIT` (`@Category(ParallelStatsDisabledTest.class)`) drives real and seeded cutovers with an injected clock (so no real 30-minute wait) and a job-lookup seam: - Happy path across mutable, immutable, and secondary-index tables, plus a child-view table asserting both base-table and view dual-write links are gone at cutover. - The monitor honors the persisted wait deadline (no-op before it, advances after). - The `PENDING_PARTIAL_PASS -> PARTIAL_PASS_RUNNING` transition clears the inherited full-pass job id. - Strand regressions, each asserting a terminal state: a `PARTIAL_PASS_RUNNING` record with a null job id, with a not-found job, and with retries exhausted all reach terminal `FAILED`. - The new column round-trips a value and a `NULL`. - A never-cached table (`UPDATE_CACHE_FREQUENCY=NEVER`, which resolves to `Long.MAX_VALUE`) yields a bounded, future wait deadline (`> cutover time`, `<= cutover + 24h`) rather than an overflowed past one. Two fast unit tests (no cluster) guard the arithmetic-only invariants: - `TransformMonitorTaskWaitTest` exercises the extracted `boundedPartialPassWaitMs` clamp across the whole input domain (`Long.MAX_VALUE`, zero, negative, mid-range, at/above the 24h ceiling), asserting the wait is always within `[30min, 24h]` and strictly positive so `now + wait` cannot overflow. - `MetaDataUtilTest.testMinSystemTableTimestampIsSystemCatalogReachable` asserts `MIN_SYSTEM_TABLE_TIMESTAMP == MIN_SYSTEM_TABLE_TIMESTAMP_5_4_0`, tripping if a future change bumps the min system-table timestamp without a corresponding `SYSTEM.CATALOG` column-add at that timestamp (which would strand in-place-upgraded clients on `UpgradeRequiredException`). Heavy user-table cutover ITs run on CI; the seeded strand regressions run locally. `mvn spotless:check` and main+test compile are clean on `phoenix-core-client`, `phoenix-core-server`, and `phoenix-core`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
