lokiore opened a new pull request, #2534:
URL: https://github.com/apache/phoenix/pull/2534

   ## Summary
   
   Introduces a coarse advisory lock primitive (`acquireTransformLock` / 
`releaseTransformLock`) on `ConnectionQueryServices`, backed by `SYSTEM.MUTEX` 
with the existing 15-min TTL auto-expiry. Wires both transform-triggering DDL 
paths in `MetaDataClient` (`addColumn`, `alterIndex`) with a **narrow** scope: 
acquire only inside the `if (isTransformNeeded)` branch after pre-validation, 
with an under-lock re-check via `getTableNoCache` to close the TOCTOU window 
between the first `checkIsTransformNeeded` call and the lock acquire.
   
   Non-transform-triggering ALTERs (`SET TTL`, plain `ADD COLUMN`, 
`IMMUTABLE_ROWS` toggle, etc.) do **not** contend on this lock. The lock is 
held through the addColumn / alterIndex CAS so that a concurrent caller can't 
sneak in between `addTransform` and the metadata CAS. When held by another 
caller, throws `CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS` (SQLState 
`42Z25`) — message instructs the caller to retry once the in-progress 
modification completes (or after the SYSTEM.MUTEX 15-min TTL auto-expires).
   
   This is a coarse advisory lock with no fencing token; the TTL bounds 
worst-case hold time.
   
   ## Scope-narrowing matrix
   
   | ALTER variant | Lock acquired? |
   |---|---|
   | `ALTER TABLE T ADD COLUMN c INTEGER` | no |
   | `ALTER TABLE T SET TTL = 100` | no |
   | `ALTER TABLE T SET IMMUTABLE_ROWS = true` | no |
   | `ALTER TABLE T SET IMMUTABLE_STORAGE_SCHEME = 
SINGLE_CELL_ARRAY_WITH_OFFSETS` | yes (transform-triggering) |
   | `ALTER INDEX I ON T REBUILD` | no |
   | `ALTER INDEX I ON T (transform-triggering)` | yes |
   
   ## Files changed
   
   - 
`phoenix-core-client/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java`
 — adds `CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS` (1225, `42Z25`)
   - 
`phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java`
 — adds `acquireTransformLock` / `releaseTransformLock` interface methods with 
Javadoc
   - 
`phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java`
 — adds `TRANSFORM_LOCK_MARKER = "TRANSFORM_LOCK"` + impl on `writeMutexCell` / 
`deleteMutexCell`
   - 
`phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java`
 — connectionless stubs
   - 
`phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java`
 — forward-delegate
   - 
`phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java`
 — wires lock into `addColumn` and `alterIndex` with narrow scope, under-lock 
re-check, lock-held-through-CAS shape
   - 
`phoenix-core/src/it/java/org/apache/phoenix/end2end/transform/TransformLockIT.java`
 — new IT class, 7 tests including `testNonTransformAlterDoesNotAcquireLock` 
(proves narrow scope) and `testConcurrentAlterTableSerializesViaTransformLock` 
(proves serialization at the DDL callsite)
   
   ## Verification
   
   - `mvn spotless:apply -DskipTests -q` — clean, no drift
   - `mvn install -pl phoenix-core-client,phoenix-core,phoenix-core-server -am 
-DskipTests` — BUILD SUCCESS (14.76s)
   - `mvn -pl phoenix-core failsafe:integration-test failsafe:verify 
-Dit.test=TransformLockIT` — **7/7 PASS** (61.15s)
   - `TransformIT` regression sweep — same 4 failures + 2 errors as the 
upstream/PHOENIX-7904-feature baseline; **pre-existing**, NOT introduced by 
this PR. These inherit from PHOENIX-7794 (Eventually Consistent Global 
Secondary Indexes, dual-write code path) and are tracked for fix as part of 
PHOENIX-7907 / Gap B which touches the same code path. Verified pre-existing 
via baseline-Edit-revert against the worktree before this change.
   
   ## Cross-link
   
   Part of the PHOENIX-7904 Online Schema Change umbrella. PR targets 
`PHOENIX-7904-feature` (NOT master).
   
   ## AI Disclosure
   
   Per the ASF generative-tooling policy, this PR was authored with AI 
assistance.
   
   - **Tool:** Claude Code (Anthropic), model `claude-opus-4-7[1m]`
   - **Author oversight:** Each diff was reviewed line-by-line by the human 
author. Architectural shape (lock-held-through-CAS vs. narrow scope vs. TOCTOU 
re-check) was decided by the author after architectural review; the AI 
implemented the agreed shape.
   - **Tests:** All 7 new IT methods in `TransformLockIT` were authored under 
the same AI-assisted process and reviewed by the human author. Pass/fail counts 
above are from local runs on the author's machine.
   - **License:** All contributed code is original to this PR and licensed 
under the Apache License 2.0 in line with project conventions.
   
   The `Generated-by:` commit trailer follows the precedent set by PHOENIX-7844 
/ PR #2462.


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

Reply via email to