This is an automated email from the ASF dual-hosted git repository.
lokiore pushed a commit to branch PHOENIX-7904-feature
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/PHOENIX-7904-feature by this
push:
new a3c16fd14c PHOENIX-7905 :- Add transform lock primitive on
SYSTEM.MUTEX with narrow caller wiring (#2534)
a3c16fd14c is described below
commit a3c16fd14c821e5e3478a2e291f5d34eb525a9b1
Author: Lokesh Khurana <[email protected]>
AuthorDate: Wed Jun 24 10:42:48 2026 -0700
PHOENIX-7905 :- Add transform lock primitive on SYSTEM.MUTEX with narrow
caller wiring (#2534)
* PHOENIX-7905 :- Add transform lock primitive on SYSTEM.MUTEX with narrow
caller wiring
Introduce an advisory, per-logical-table transform lock on SYSTEM.MUTEX that
serializes transform-triggering DDL (ALTER TABLE / ALTER INDEX paths whose
property changes require a new physical table) against concurrent transform
lifecycle operations. The lock is exposed as two new methods on
ConnectionQueryServices - acquireTransformLock / releaseTransformLock - both
abstract; ConnectionQueryServicesImpl implements them on top of the existing
writeMutexCell / deleteMutexCell primitive with a "__TRANSFORM_LOCK__"
sentinel
column, ConnectionlessQueryServicesImpl no-ops them for unit tests, and
DelegateConnectionQueryServices forwards.
Caller wiring is narrow and surgical, and both ALTER paths share a
single-pass
property-evaluation shape:
* MetaDataClient.addColumn (ALTER TABLE path): probe checkIsTransformNeeded
first against the raw metaProperties + the resolver-view table (the check
reads only raw MetaProperties fields populated by loadStmtProperties, so
it
does not depend on evaluateStmtProperties output). If a transform is
needed,
apply pre-acquire guards (local-index, append-only, transactional), then
acquire the lock, re-resolve via getTableNoCache(...), and re-check
checkIsTransformNeeded against the post-lock view. evaluateStmtProperties
then runs exactly once against freshTable (= the under-lock view on the
transform-acquire path, = the resolver view otherwise), so
changingPhoenixTableProperty and metaPropertiesEvaluated reflect the
post-lock view on the transform path. The TTL-hierarchy check, which
consumes areWeIntroducingTTLAtThisLevel from the evaluation, runs after
the
single evaluation; this also fixes a latent stale-view risk where a
concurrent ALTER that already added TTL could have produced a
false-positive
"introducing TTL" verdict if the evaluation ran against the pre-lock
view.
* MetaDataClient.alterIndex (ALTER INDEX path): same single-pass shape -
loadStmtProperties -> checkIsTransformNeeded -> (transform path: acquire,
getTableNoCache, re-check) -> single evaluateStmtProperties against
freshTable. The freshTable / evaluateStmtProperties under-lock pattern
was
established here in round 2; addColumn now mirrors it.
* Both paths reset freshTable = table at the top of the retry loop so a
ConcurrentTableMutationException retry can't reuse a stale resolution
from
the prior iteration.
Lock granularity is per-logical-table and matches the SYSTEM.TRANSFORM PK
(TENANT_ID, TABLE_SCHEM, LOGICAL_TABLE_NAME). A concurrent ALTER on a data
table and a concurrent ALTER on one of its indexes write different
SYSTEM.TRANSFORM PK rows and are intentionally NOT blocked - by-design, an
operator may transform a data table and its indexes in parallel. Doc
comments
at both lock-acquire sites record this rationale.
Exception surface: a new SQLExceptionCode
CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS (SQLSTATE 42Z25) is raised
when
the lock is contended. The message includes an explicit backoff hint
("retry after a short backoff (e.g., 30s)") and notes the 15-minute MUTEX
TTL
auto-expiry, and the brief under-lock re-check window in which the error can
fire even if no SYSTEM.TRANSFORM record currently exists.
TRANSFORM_LOCK_MARKER is exposed as @VisibleForTesting public static final
on
ConnectionQueryServicesImpl so cross-package ITs can reference the sentinel
by
constant rather than by string literal; it is NOT a supported public API.
Tests (TransformLockIT, 13 cases, +486 lines, ParallelStatsDisabledIT base):
* Happy path: ALTER TABLE that triggers a transform observes the lock cell,
completes, and releases.
* Contention: a second client attempting a transform-triggering ALTER while
the first holds the lock receives SQLException with
CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS and the rewritten "Cannot
modify table..." message body.
* Non-transform-triggering ALTERs (SET TTL, ADD COLUMN, SET IMMUTABLE_ROWS)
do NOT acquire the lock.
* Index variants (ALTER INDEX paths) lock on the index's logical name, not
the parent data table's.
* Stray-release regression: testStrayReleaseDoesNotDeleteAnotherCallersLock
asserts that calling releaseTransformLock after the MUTEX TTL has expired
does NOT delete a different caller's lock cell.
* Lock-released-on-failure: an explicit acquire-throw-release-acquire
sequence asserts the lock is observably released when the surrounding DDL
fails server-side mid-operation (deterministic primitive-API simulation,
avoids racy mid-ALTER fault injection).
This change does not add new metrics or log tags for the lock-acquire path;
that is deferred. The default LOGGER.error level on the
release-after-TTL-mismatch is intentional and matches the operator-visible
severity of a TTL-violation (rather than DEBUG which would silently hide the
condition).
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
---
.../apache/phoenix/exception/SQLExceptionCode.java | 6 +
.../phoenix/query/ConnectionQueryServices.java | 36 ++
.../phoenix/query/ConnectionQueryServicesImpl.java | 22 +
.../query/ConnectionlessQueryServicesImpl.java | 11 +
.../query/DelegateConnectionQueryServices.java | 12 +
.../org/apache/phoenix/schema/MetaDataClient.java | 180 ++++++--
.../phoenix/end2end/transform/TransformLockIT.java | 489 +++++++++++++++++++++
7 files changed, 715 insertions(+), 41 deletions(-)
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
index 3eddf2278f..1d9a286376 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
@@ -543,6 +543,12 @@ public enum SQLExceptionCode {
"The CURRENT_SCN may not be set for statement using ON DUPLICATE KEY."),
CANNOT_USE_ON_DUP_KEY_WITH_GLOBAL_IDX(1224, "42Z24",
"The ON DUPLICATE KEY clause may not be used when a table has a global
index."),
+ CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS(1225, "42Z25",
+ "Cannot modify table while a concurrent schema-modification is in progress
on it. "
+ + "Retry after a short backoff (e.g., 30s); the lock also auto-expires
after the "
+ + "SYSTEM.MUTEX TTL of 15 minutes if the holder dies. Note: this error
can fire during "
+ + "the brief under-lock re-check window even if no transform record
currently exists in "
+ + "SYSTEM.TRANSFORM."),
/** Parser error. (errorcode 06, sqlState 42P) */
PARSER_ERROR(601, "42P00", "Syntax error.", Factory.SYNTAX_ERROR),
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
index 8808d67d76..217eac28b0 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
@@ -297,6 +297,42 @@ public interface ConnectionQueryServices extends
QueryServices, MetaDataMutated
public void deleteMutexCell(String tenantId, String schemaName, String
tableName,
String columnName, String familyName) throws SQLException;
+ /**
+ * Acquire the transform lock on the given logical table. Specifically
serializes
+ * transform-triggering DDL (ALTER TABLE / ALTER INDEX paths whose property
changes require a new
+ * physical table) with concurrent transform lifecycle operations.
Non-transform-triggering ALTERs
+ * (e.g., SET TTL, ADD COLUMN, SET IMMUTABLE_ROWS) do not contend on this
lock.
+ * <p>
+ * Lock scope: keyed on {@code (schemaName, tableName)} only — the {@code
tenantId} arg is
+ * accepted for API symmetry but does NOT participate in the lock rowkey. A
transform-triggering
+ * change on a (schema, table) blocks any new transform attempt on the same
table regardless of
+ * tenant, and vice versa.
+ * <p>
+ * Implemented on top of SYSTEM.MUTEX, so the lock auto-expires after the
column-family TTL
+ * ({@link org.apache.phoenix.jdbc.PhoenixDatabaseMetaData#TTL_FOR_MUTEX} =
15 min) if the holder
+ * dies without releasing.
+ * <p>
+ * Callers MUST invoke {@link #releaseTransformLock} from a {@code finally}
block on every path
+ * where this method returned {@code true}.
+ * <p>
+ * Caller is responsible for completing the operation within the
column-family TTL; this is a
+ * coarse advisory lock with no fencing token. Holders that pause past the
TTL may both observe
+ * lock-acquired simultaneously.
+ * @return true if the caller acquired the lock; false if it is currently
held by another caller
+ */
+ boolean acquireTransformLock(String tenantId, String schemaName, String
tableName)
+ throws SQLException;
+
+ /**
+ * Release the transform lock on the given logical table. Caller MUST call
this from a
+ * {@code finally} block whenever {@link #acquireTransformLock} returned
{@code true}. The release
+ * deletes the lock cell unconditionally; callers MUST NOT call release
after the column-family
+ * TTL expiry, otherwise a different caller's lock cell may be deleted.
+ * @see
org.apache.phoenix.end2end.transform.TransformLockIT#testStrayReleaseDoesNotDeleteAnotherCallersLock
+ */
+ void releaseTransformLock(String tenantId, String schemaName, String
tableName)
+ throws SQLException;
+
/**
* Truncate a phoenix table
*/
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 5cd95b648e..cb2a47230f 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -422,6 +422,13 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices
private final int maxInternalConnectionsAllowed;
private final boolean shouldThrottleNumConnections;
public static final byte[] MUTEX_LOCKED =
"MUTEX_LOCKED".getBytes(StandardCharsets.UTF_8);
+ // Disambiguate the transform-lock row in SYSTEM.MUTEX from other lock
purposes. The
+ // double-underscore sentinel form keeps this rowkey separate from any
user-visible column
+ // name (Phoenix uppercases unquoted identifiers, so a hypothetical
+ // {@code ALTER TABLE ... ADD COLUMN __TRANSFORM_LOCK__} would still need
explicit quoting
+ // to land at this exact byte sequence).
+ @VisibleForTesting
+ public static final String TRANSFORM_LOCK_MARKER = "__TRANSFORM_LOCK__";
private ServerSideRPCControllerFactory serverSideRPCControllerFactory;
private boolean localIndexUpgradeRequired;
@@ -5654,6 +5661,21 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices
}
}
+ @Override
+ public boolean acquireTransformLock(String tenantId, String schemaName,
String tableName)
+ throws SQLException {
+ // Lock is keyed on (schemaName, tableName) only — tenantId is
intentionally not part of the
+ // SYSTEM.MUTEX rowkey so that a transform on a (schema, table) blocks any
concurrent
+ // transform on the same table regardless of tenant scope.
+ return writeMutexCell(null, schemaName, tableName, TRANSFORM_LOCK_MARKER,
null);
+ }
+
+ @Override
+ public void releaseTransformLock(String tenantId, String schemaName, String
tableName)
+ throws SQLException {
+ deleteMutexCell(null, schemaName, tableName, TRANSFORM_LOCK_MARKER, null);
+ }
+
@VisibleForTesting
public Table getSysMutexTable() throws SQLException {
String tableNameAsString = SYSTEM_MUTEX_NAME;
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
index abbc08ac31..3008516a0e 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
@@ -893,6 +893,17 @@ public class ConnectionlessQueryServicesImpl extends
DelegateQueryServices
String columnName, String familyName) throws SQLException {
}
+ @Override
+ public boolean acquireTransformLock(String tenantId, String schemaName,
String tableName)
+ throws SQLException {
+ return true;
+ }
+
+ @Override
+ public void releaseTransformLock(String tenantId, String schemaName, String
tableName)
+ throws SQLException {
+ }
+
@Override
public void truncateTable(String schemaName, String tableName, boolean
isNamespaceMapped,
boolean preserveSplits) throws SQLException {
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
index 24a4229709..655ca306cb 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
@@ -457,6 +457,18 @@ public class DelegateConnectionQueryServices extends
DelegateQueryServices
getDelegate().deleteMutexCell(tenantId, schemaName, tableName, columnName,
familyName);
}
+ @Override
+ public boolean acquireTransformLock(String tenantId, String schemaName,
String tableName)
+ throws SQLException {
+ return getDelegate().acquireTransformLock(tenantId, schemaName, tableName);
+ }
+
+ @Override
+ public void releaseTransformLock(String tenantId, String schemaName, String
tableName)
+ throws SQLException {
+ getDelegate().releaseTransformLock(tenantId, schemaName, tableName);
+ }
+
@Override
public PMetaData getMetaDataCache() {
return getDelegate().getMetaDataCache();
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
index 22833e2494..2ad06accf5 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
@@ -4817,6 +4817,13 @@ public class MetaDataClient {
String physicalTableName =
SchemaUtil.getTableNameFromFullName(physicalName.getString());
Set<String> acquiredColumnMutexSet = Sets.newHashSetWithExpectedSize(3);
boolean acquiredBaseTableMutex = false;
+ boolean acquiredTransformLock = false;
+ // Tracks the post-re-check fresh table view; used as the source-of-truth
for
+ // evaluateStmtProperties (single-pass evaluation) and the downstream
transform-decision sites
+ // (incrementTableSeqNum + addTransform). Defaults to {@code table} when
no transform is needed
+ // or when the lock was already held by a parent frame (no second re-check
happens); reassigned
+ // to a getTableNoCache view at the under-lock re-check below on the
transform-acquire path.
+ PTable freshTable = table;
try {
connection.setAutoCommit(false);
List<ColumnDef> columnDefs;
@@ -4862,6 +4869,12 @@ public class MetaDataClient {
ColumnResolver resolver = FromCompiler.getResolver(namedTableNode,
connection);
table = resolver.getTables().get(0).getTable();
+ // Reset freshTable to the iteration's freshly-resolved {@code table}
on every retry. The
+ // under-lock re-check below will overwrite this with a
getTableNoCache view only when this
+ // iteration actually acquires the lock for the first time; on retry
iterations (where
+ // acquiredTransformLock is already true) the re-check block is
skipped and downstream
+ // sites consume this iteration's resolver view rather than the prior
iteration's snapshot.
+ freshTable = table;
int nIndexes = table.getIndexes().size();
int numCols = columnDefs.size();
int nNewColumns = numCols;
@@ -4902,9 +4915,69 @@ public class MetaDataClient {
}
}
+ // Single-pass property evaluation: probe transform-needed first
against raw metaProperties
+ // (which checkIsTransformNeeded reads — it does not depend on
evaluateStmtProperties
+ // output),
+ // acquire the transform lock + fetch freshTable + re-check under lock
if needed, THEN run
+ // evaluateStmtProperties exactly once against freshTable. This
mirrors alterIndex and
+ // ensures
+ // both the changingPhoenixTableProperty verdict and the TTL-hierarchy
check (which consumes
+ // areWeIntroducingTTLAtThisLevel from the evaluation) reflect the
post-lock view on the
+ // transform path.
+ boolean isTransformNeeded =
TransformClient.checkIsTransformNeeded(metaProperties,
+ schemaName, table, tableName, null, tenantIdToUse, connection);
+ if (isTransformNeeded) {
+ // Pre-acquire guards: fast-fail before paying the cost of the lock
cell.
+ if (MetaDataUtil.hasLocalIndexTable(connection,
physicalTableName.getBytes())) {
+ throw new SQLExceptionInfo.Builder(
+
SQLExceptionCode.CANNOT_TRANSFORM_TABLE_WITH_LOCAL_INDEX).setSchemaName(schemaName)
+ .setTableName(tableName).build().buildException();
+ }
+ if (table.isAppendOnlySchema()) {
+ throw new SQLExceptionInfo.Builder(
+ SQLExceptionCode.CANNOT_TRANSFORM_TABLE_WITH_APPEND_ONLY_SCHEMA)
+
.setSchemaName(schemaName).setTableName(tableName).build().buildException();
+ }
+ if (table.isTransactional()) {
+ throw new
SQLExceptionInfo.Builder(CANNOT_TRANSFORM_TRANSACTIONAL_TABLE)
+
.setSchemaName(schemaName).setTableName(tableName).build().buildException();
+ }
+ if (!acquiredTransformLock) {
+ // Lock granularity: keyed on (schemaName, tableName) — i.e.
per-logical-table, matching
+ // the SYSTEM.TRANSFORM primary key (TENANT_ID, TABLE_SCHEM,
LOGICAL_TABLE_NAME). A
+ // concurrent ALTER on the same logical table contends here; a
concurrent ALTER on a
+ // related table (e.g. an index of this data table) writes a
different SYSTEM.TRANSFORM
+ // PK row and is intentionally NOT blocked. By-design: an operator
may transform a data
+ // table and its indexes in parallel.
+ acquiredTransformLock = connection.getQueryServices()
+ .acquireTransformLock(tenantIdToUse, schemaName, tableName);
+ if (!acquiredTransformLock) {
+ throw new SQLExceptionInfo.Builder(
+
SQLExceptionCode.CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS)
+
.setSchemaName(schemaName).setTableName(tableName).build().buildException();
+ }
+ // Re-check under lock with fresh metadata: a concurrent
transform-triggering ALTER
+ // may have committed between our first checkIsTransformNeeded and
the lock acquire.
+ // If so, the transform we observed is stale — drop through and
treat as no-op.
+ // Capture freshTable so downstream sites (evaluateStmtProperties
below,
+ // incrementTableSeqNum, addTransform) see the post-re-check view
rather than the stale
+ // {@code table}.
+ // Tenant-scoping asymmetry by design: the lock acquire above
passes
+ // {@code tenantIdToUse}, but {@code acquireTransformLock} drops
the tenant before
+ // writing the lock row so all tenants contend on the same
(schema, table) lock key;
+ // the getTableNoCache fetch below is implicitly scoped to
+ // {@code connection.getTenantId()} and returns the caller's
tenant-specific view of
+ // the table.
+ freshTable =
connection.getTableNoCache(SchemaUtil.getTableName(schemaName, tableName));
+ isTransformNeeded =
TransformClient.checkIsTransformNeeded(metaProperties, schemaName,
+ freshTable, tableName, null, tenantIdToUse, connection);
+ }
+ }
+
MetaPropertiesEvaluated metaPropertiesEvaluated = new
MetaPropertiesEvaluated();
- changingPhoenixTableProperty = evaluateStmtProperties(metaProperties,
- metaPropertiesEvaluated, table, schemaName, tableName,
areWeIntroducingTTLAtThisLevel);
+ changingPhoenixTableProperty =
+ evaluateStmtProperties(metaProperties, metaPropertiesEvaluated,
freshTable, schemaName,
+ tableName, areWeIntroducingTTLAtThisLevel);
if (areWeIntroducingTTLAtThisLevel.booleanValue()) {
// As we are introducing TTL for the first time at this level, we
need to check
// if TTL is already defined up or down in the hierarchy.
@@ -4929,26 +5002,6 @@ public class MetaDataClient {
*/
}
- boolean isTransformNeeded =
TransformClient.checkIsTransformNeeded(metaProperties,
- schemaName, table, tableName, null, tenantIdToUse, connection);
- if (isTransformNeeded) {
- // We can add a support for these later. For now, not supported.
- if (MetaDataUtil.hasLocalIndexTable(connection,
physicalTableName.getBytes())) {
- throw new SQLExceptionInfo.Builder(
-
SQLExceptionCode.CANNOT_TRANSFORM_TABLE_WITH_LOCAL_INDEX).setSchemaName(schemaName)
- .setTableName(tableName).build().buildException();
- }
- if (table.isAppendOnlySchema()) {
- throw new SQLExceptionInfo.Builder(
- SQLExceptionCode.CANNOT_TRANSFORM_TABLE_WITH_APPEND_ONLY_SCHEMA)
-
.setSchemaName(schemaName).setTableName(tableName).build().buildException();
- }
- if (table.isTransactional()) {
- throw new
SQLExceptionInfo.Builder(CANNOT_TRANSFORM_TRANSACTIONAL_TABLE)
-
.setSchemaName(schemaName).setTableName(tableName).build().buildException();
- }
- }
-
// If changing isImmutableRows to true or it's not being changed and
is already true
boolean willBeImmutableRows =
Boolean.TRUE.equals(metaPropertiesEvaluated.getIsImmutableRows())
@@ -5184,7 +5237,7 @@ public class MetaDataClient {
long seqNum = 0;
if (changingPhoenixTableProperty || columnDefs.size() > 0) {
seqNum =
- incrementTableSeqNum(table, tableType, columnDefs.size(),
metaPropertiesEvaluated);
+ incrementTableSeqNum(freshTable, tableType, columnDefs.size(),
metaPropertiesEvaluated);
tableMetaData
.addAll(connection.getMutationState().toMutations(timeStamp).next().getSecond());
@@ -5194,8 +5247,8 @@ public class MetaDataClient {
PTable transformingNewTable = null;
if (isTransformNeeded) {
try {
- transformingNewTable = TransformClient.addTransform(connection,
tenantIdToUse, table,
- metaProperties, seqNum, PTable.TransformType.METADATA_TRANSFORM);
+ transformingNewTable = TransformClient.addTransform(connection,
tenantIdToUse,
+ freshTable, metaProperties, seqNum,
PTable.TransformType.METADATA_TRANSFORM);
} catch (SQLException ex) {
connection.rollback();
throw ex;
@@ -5407,6 +5460,13 @@ public class MetaDataClient {
deleteCell(null, physicalSchemaName, physicalTableName, null);
}
deleteMutexCells(physicalSchemaName, physicalTableName,
acquiredColumnMutexSet);
+ if (acquiredTransformLock) {
+ try {
+ connection.getQueryServices().releaseTransformLock(tenantIdToUse,
schemaName, tableName);
+ } catch (SQLException e) {
+ LOGGER.warn("Failed to release transform lock for {}.{}",
schemaName, tableName, e);
+ }
+ }
}
}
@@ -5955,18 +6015,19 @@ public class MetaDataClient {
boolean wasAutoCommit = connection.getAutoCommit();
String dataTableName;
long seqNum = 0L;
+ String tenantId =
+ connection.getTenantId() == null ? null :
connection.getTenantId().getString();
+ String schemaName = statement.getTable().getName().getSchemaName();
+ String tableName = null;
+ boolean acquiredTransformLock = false;
try {
dataTableName = statement.getTableName();
final String indexName = statement.getTable().getName().getTableName();
boolean isAsync = statement.isAsync();
boolean isRebuildAll = statement.isRebuildAll();
- String tenantId =
- connection.getTenantId() == null ? null :
connection.getTenantId().getString();
PTable table =
FromCompiler.getIndexResolver(statement,
connection).getTables().get(0).getTable();
-
- String schemaName = statement.getTable().getName().getSchemaName();
- String tableName = table.getTableName().getString();
+ tableName = table.getTableName().getString();
Map<String, List<Pair<String, Object>>> properties =
new HashMap<>(statement.getProps().size());
@@ -5976,9 +6037,6 @@ public class MetaDataClient {
boolean isTransformNeeded =
TransformClient.checkIsTransformNeeded(metaProperties, schemaName,
table, indexName, dataTableName, tenantId, connection);
- MetaPropertiesEvaluated metaPropertiesEvaluated = new
MetaPropertiesEvaluated();
- boolean changingPhoenixTableProperty =
evaluateStmtProperties(metaProperties,
- metaPropertiesEvaluated, table, schemaName, tableName, new
MutableBoolean(false));
PIndexState newIndexState = statement.getIndexState();
IndexConsistency newIndexConsistency = statement.getIndexConsistency();
@@ -6025,6 +6083,43 @@ public class MetaDataClient {
connection.setAutoCommit(false);
// Confirm index table is valid and up-to-date
TableRef indexRef = FromCompiler.getResolver(statement,
connection).getTables().get(0);
+
+ // If the statement is a transform-triggering ALTER INDEX, acquire the
transform lock and
+ // re-check freshness under the lock BEFORE we commit any index-state
UPSERT. Otherwise a
+ // contending caller would see (or leave behind) a half-committed
BUILDING state if the
+ // lock acquire failed below.
+ PTable freshTable = table;
+ if (isTransformNeeded) {
+ if (indexRef.getTable().getViewIndexId() != null) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.CANNOT_TRANSFORM_LOCAL_OR_VIEW_INDEX)
+
.setSchemaName(schemaName).setTableName(indexName).build().buildException();
+ }
+ // Lock granularity: keyed on (schemaName, tableName) where tableName
here is the index's
+ // logical name (see line above where it is bound to the resolved
index PTable). This
+ // matches the SYSTEM.TRANSFORM primary key (TENANT_ID, TABLE_SCHEM,
LOGICAL_TABLE_NAME).
+ // A concurrent ALTER on this same index contends here; a concurrent
ALTER on the parent
+ // data table (or on a sibling index) writes a different
SYSTEM.TRANSFORM PK row and is
+ // intentionally NOT blocked — by-design, an operator may transform a
data table and its
+ // indexes in parallel.
+ acquiredTransformLock =
+ connection.getQueryServices().acquireTransformLock(tenantId,
schemaName, tableName);
+ if (!acquiredTransformLock) {
+ throw new SQLExceptionInfo.Builder(
+ SQLExceptionCode.CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS)
+
.setSchemaName(schemaName).setTableName(tableName).build().buildException();
+ }
+ // Re-check under lock with fresh metadata: a concurrent
transform-triggering ALTER may
+ // have committed between our first checkIsTransformNeeded and the
lock acquire. If the
+ // observed transform is no longer needed, drop through and treat as
no-op.
+ freshTable =
connection.getTableNoCache(SchemaUtil.getTableName(schemaName, tableName));
+ isTransformNeeded =
TransformClient.checkIsTransformNeeded(metaProperties, schemaName,
+ freshTable, indexName, dataTableName, tenantId, connection);
+ }
+
+ MetaPropertiesEvaluated metaPropertiesEvaluated = new
MetaPropertiesEvaluated();
+ boolean changingPhoenixTableProperty =
evaluateStmtProperties(metaProperties,
+ metaPropertiesEvaluated, freshTable, schemaName, tableName, new
MutableBoolean(false));
+
try (PreparedStatement tableUpsert = connection.prepareStatement(
newIndexState == PIndexState.ACTIVE ? UPDATE_INDEX_STATE_TO_ACTIVE :
UPDATE_INDEX_STATE)) {
tableUpsert.setString(1,
@@ -6044,14 +6139,15 @@ public class MetaDataClient {
connection.rollback();
if (changingPhoenixTableProperty) {
- seqNum = incrementTableSeqNum(table, statement.getTableType(), 0,
metaPropertiesEvaluated);
+ seqNum =
+ incrementTableSeqNum(freshTable, statement.getTableType(), 0,
metaPropertiesEvaluated);
tableMetadata
.addAll(connection.getMutationState().toMutations(timeStamp).next().getSecond());
connection.rollback();
}
MetaDataMutationResult result =
connection.getQueryServices().updateIndexState(tableMetadata,
- dataTableName, properties, table);
+ dataTableName, properties, freshTable);
try {
MutationCode code = result.getMutationCode();
@@ -6066,13 +6162,8 @@ public class MetaDataClient {
}
if (isTransformNeeded) {
- if (indexRef.getTable().getViewIndexId() != null) {
- throw new SQLExceptionInfo.Builder(
-
SQLExceptionCode.CANNOT_TRANSFORM_LOCAL_OR_VIEW_INDEX).setSchemaName(schemaName)
- .setTableName(indexName).build().buildException();
- }
try {
- TransformClient.addTransform(connection, tenantId, table,
metaProperties, seqNum,
+ TransformClient.addTransform(connection, tenantId, freshTable,
metaProperties, seqNum,
PTable.TransformType.METADATA_TRANSFORM);
} catch (SQLException ex) {
connection.rollback();
@@ -6193,6 +6284,13 @@ public class MetaDataClient {
return new MutationState(0, 0, connection);
} finally {
connection.setAutoCommit(wasAutoCommit);
+ if (acquiredTransformLock) {
+ try {
+ connection.getQueryServices().releaseTransformLock(tenantId,
schemaName, tableName);
+ } catch (SQLException e) {
+ LOGGER.warn("Failed to release transform lock for {}.{}",
schemaName, tableName, e);
+ }
+ }
}
}
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/transform/TransformLockIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/transform/TransformLockIT.java
new file mode 100644
index 0000000000..7e7f4d24c1
--- /dev/null
+++
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/transform/TransformLockIT.java
@@ -0,0 +1,489 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.end2end.transform;
+
+import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.phoenix.end2end.ParallelStatsDisabledIT;
+import org.apache.phoenix.end2end.ParallelStatsDisabledTest;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.query.ConnectionQueryServices;
+import org.apache.phoenix.query.ConnectionQueryServicesImpl;
+import org.apache.phoenix.util.PropertiesUtil;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Integration tests for the transform-lock primitive on SYSTEM.MUTEX. The
lock is the coordination
+ * point for serializing transform-triggering DDL with concurrent transform
lifecycle operations;
+ * this IT exercises the primitive itself plus the wiring at the DDL callsites
(concurrent ALTER
+ * fast-fail and narrow-scope behavior for non-transform ALTERs).
+ */
+@Category(ParallelStatsDisabledTest.class)
+public class TransformLockIT extends ParallelStatsDisabledIT {
+
+ private static ConnectionQueryServices services(Connection conn) throws
Exception {
+ return conn.unwrap(PhoenixConnection.class).getQueryServices();
+ }
+
+ @Test
+ public void testAcquireReleaseHappyPath() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqs = services(conn);
+
+ assertTrue("first acquire should win", cqs.acquireTransformLock(null,
schemaName, tableName));
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ assertTrue("acquire after release should win again",
+ cqs.acquireTransformLock(null, schemaName, tableName));
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ @Test
+ public void testAcquireContention() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqs = services(conn);
+
+ assertTrue("first acquire should win", cqs.acquireTransformLock(null,
schemaName, tableName));
+ assertFalse("second acquire on same key should lose",
+ cqs.acquireTransformLock(null, schemaName, tableName));
+
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ assertTrue("acquire after release should win again",
+ cqs.acquireTransformLock(null, schemaName, tableName));
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ @Test
+ public void testReleaseDoesNotThrow() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqs = services(conn);
+
+ // release without prior acquire — must not throw
+ cqs.releaseTransformLock(null, schemaName, tableName);
+
+ assertTrue(cqs.acquireTransformLock(null, schemaName, tableName));
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ // release after release — must not throw
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ @Test
+ public void testDifferentTablesDoNotBlock() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableA = "A_" + generateUniqueName();
+ String tableB = "B_" + generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqs = services(conn);
+
+ assertTrue(cqs.acquireTransformLock(null, schemaName, tableA));
+ assertTrue("acquire on a different table must not be blocked",
+ cqs.acquireTransformLock(null, schemaName, tableB));
+
+ cqs.releaseTransformLock(null, schemaName, tableA);
+ cqs.releaseTransformLock(null, schemaName, tableB);
+ }
+ }
+
+ @Test
+ public void testLockSurvivesAcrossConnections() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ try (Connection conn1 = DriverManager.getConnection(getUrl(), props);
+ Connection conn2 = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqs1 = services(conn1);
+ ConnectionQueryServices cqs2 = services(conn2);
+
+ assertTrue("conn1 should win the first acquire",
+ cqs1.acquireTransformLock(null, schemaName, tableName));
+ assertFalse("conn2 must see the lock as held",
+ cqs2.acquireTransformLock(null, schemaName, tableName));
+
+ cqs1.releaseTransformLock(null, schemaName, tableName);
+ assertTrue("conn2 should win after conn1 releases",
+ cqs2.acquireTransformLock(null, schemaName, tableName));
+ cqs2.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ @Test
+ public void testConcurrentAlterTableSerializesViaTransformLock() throws
Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ String fullTableName = schemaName + "." + tableName;
+
+ try (Connection setupConn = DriverManager.getConnection(getUrl(), props)) {
+ setupConn.createStatement()
+ .execute("CREATE TABLE " + fullTableName + " (id INTEGER PRIMARY KEY,
val VARCHAR)");
+ }
+
+ try (Connection holderConn = DriverManager.getConnection(getUrl(), props))
{
+ ConnectionQueryServices cqs = services(holderConn);
+ assertTrue("holder must acquire lock", cqs.acquireTransformLock(null,
schemaName, tableName));
+
+ try {
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement().execute("ALTER TABLE " + fullTableName
+ + " SET IMMUTABLE_STORAGE_SCHEME =
SINGLE_CELL_ARRAY_WITH_OFFSETS");
+ fail("ALTER TABLE should have failed while transform lock is held");
+ } catch (SQLException sqle) {
+ assertEquals(
+
SQLExceptionCode.CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS.getErrorCode(),
+ sqle.getErrorCode());
+ String msg = sqle.getMessage();
+ // Assert the human-readable message text is present (catches a
regression where the
+ // message itself disappears or is replaced).
+ assertTrue("exception message should contain the human-readable
text, got: " + msg,
+ msg.contains("Cannot modify table"));
+ // Assert no unsubstituted format-spec leaks through (the message
string must not
+ // contain literal "%s.%s" — SQLExceptionInfo does not
format-substitute the message).
+ assertFalse(
+ "exception message must not contain unsubstituted %s.%s
placeholders, got: " + msg,
+ msg.contains("%s"));
+ // Assert the appended TABLE_NAME=schema.table token renders both
names (this is what
+ // SQLExceptionInfo#setSchemaName/setTableName actually contribute).
+ assertTrue("exception message should contain TABLE_NAME=" +
schemaName + "." + tableName
+ + ", got: " + msg, msg.contains("tableName=" + schemaName + "." +
tableName));
+ }
+ } finally {
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement().execute("ALTER TABLE " + fullTableName
+ + " SET IMMUTABLE_STORAGE_SCHEME = SINGLE_CELL_ARRAY_WITH_OFFSETS");
+ }
+ }
+
+ @Test
+ public void testGlobalTransformBlocksTenantTransform() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+
+ try (Connection globalConn = DriverManager.getConnection(getUrl(), props))
{
+ ConnectionQueryServices globalCqs = services(globalConn);
+ // global acquire (tenantId=null)
+ assertTrue("global acquire should win",
+ globalCqs.acquireTransformLock(null, schemaName, tableName));
+ try {
+ Properties tenantProps = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ tenantProps.setProperty("TenantId", "TENANT_A");
+ try (Connection tenantConn = DriverManager.getConnection(getUrl(),
tenantProps)) {
+ ConnectionQueryServices tenantCqs = services(tenantConn);
+ assertFalse(
+ "tenant acquire on the same (schema, table) must lose to a global
holder regardless of tenant",
+ tenantCqs.acquireTransformLock("TENANT_A", schemaName, tableName));
+ }
+ } finally {
+ globalCqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+ }
+
+ @Test
+ public void testTenantBlocksGlobalAndOtherTenants() throws Exception {
+ Properties propsA = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ propsA.setProperty("TenantId", "TENANT_A");
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+
+ try (Connection tenantAConn = DriverManager.getConnection(getUrl(),
propsA)) {
+ ConnectionQueryServices cqsA = services(tenantAConn);
+ // tenant-A acquires; tenantId arg is accepted but does NOT participate
in the rowkey
+ assertTrue("tenant-A acquire should win",
+ cqsA.acquireTransformLock("TENANT_A", schemaName, tableName));
+ try {
+ Properties propsGlobal = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ try (Connection globalConn = DriverManager.getConnection(getUrl(),
propsGlobal)) {
+ ConnectionQueryServices globalCqs = services(globalConn);
+ assertFalse("global acquire must lose to a tenant-A holder",
+ globalCqs.acquireTransformLock(null, schemaName, tableName));
+ }
+ Properties propsB = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ propsB.setProperty("TenantId", "TENANT_B");
+ try (Connection tenantBConn = DriverManager.getConnection(getUrl(),
propsB)) {
+ ConnectionQueryServices cqsB = services(tenantBConn);
+ assertFalse("tenant-B acquire must lose to a tenant-A holder on the
same table",
+ cqsB.acquireTransformLock("TENANT_B", schemaName, tableName));
+ }
+ } finally {
+ cqsA.releaseTransformLock("TENANT_A", schemaName, tableName);
+ }
+ }
+ }
+
+ @Test
+ public void testNonTransformAlterIndexDoesNotAcquireLock() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ String indexName = "I_" + generateUniqueName();
+ String fullTableName = schemaName + "." + tableName;
+
+ try (Connection setupConn = DriverManager.getConnection(getUrl(), props)) {
+ setupConn.createStatement()
+ .execute("CREATE TABLE " + fullTableName + " (id INTEGER PRIMARY KEY,
val VARCHAR)");
+ setupConn.createStatement()
+ .execute("CREATE INDEX " + indexName + " ON " + fullTableName + "
(val)");
+ }
+
+ // Hold the transform lock externally; non-transform-triggering ALTER
INDEX variants
+ // (DISABLE, REBUILD, REBUILD ASYNC) must NOT contend on it — each should
succeed even while
+ // the lock is held.
+ try (Connection holderConn = DriverManager.getConnection(getUrl(), props))
{
+ ConnectionQueryServices cqs = services(holderConn);
+ assertTrue("holder must acquire lock", cqs.acquireTransformLock(null,
schemaName, indexName));
+ try {
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement()
+ .execute("ALTER INDEX " + indexName + " ON " + fullTableName + "
DISABLE");
+ }
+ // Re-enable so REBUILD has something to do.
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement()
+ .execute("ALTER INDEX " + indexName + " ON " + fullTableName + "
REBUILD");
+ }
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement()
+ .execute("ALTER INDEX " + indexName + " ON " + fullTableName + "
REBUILD ASYNC");
+ }
+ } finally {
+ cqs.releaseTransformLock(null, schemaName, indexName);
+ }
+ }
+ }
+
+ @Test
+ public void testConcurrentAlterIndexSerializesViaTransformLock() throws
Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ String indexName = "I_" + generateUniqueName();
+ String fullTableName = schemaName + "." + tableName;
+
+ try (Connection setupConn = DriverManager.getConnection(getUrl(), props)) {
+ setupConn.createStatement()
+ .execute("CREATE TABLE " + fullTableName + " (id INTEGER PRIMARY KEY,
val VARCHAR)");
+ setupConn.createStatement()
+ .execute("CREATE INDEX " + indexName + " ON " + fullTableName + "
(val)");
+ }
+
+ // Lock key for an index transform is keyed on the index's logical name
(schemaName, indexName)
+ // — see MetaDataClient.alterIndex's acquireTransformLock callsite.
+ try (Connection holderConn = DriverManager.getConnection(getUrl(), props))
{
+ ConnectionQueryServices cqs = services(holderConn);
+ assertTrue("holder must acquire lock", cqs.acquireTransformLock(null,
schemaName, indexName));
+ try {
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement()
+ .execute("ALTER INDEX " + indexName + " ON " + fullTableName
+ + " ACTIVE IMMUTABLE_STORAGE_SCHEME =
SINGLE_CELL_ARRAY_WITH_OFFSETS,"
+ + " COLUMN_ENCODED_BYTES = 2");
+ fail("ALTER INDEX should have failed while transform lock is held");
+ } catch (SQLException sqle) {
+ assertEquals(
+
SQLExceptionCode.CANNOT_MODIFY_TABLE_WITH_TRANSFORM_IN_PROGRESS.getErrorCode(),
+ sqle.getErrorCode());
+ String msg = sqle.getMessage();
+ assertTrue("exception message should contain the human-readable
text, got: " + msg,
+ msg.contains("Cannot modify table"));
+ assertFalse(
+ "exception message must not contain unsubstituted %s.%s
placeholders, got: " + msg,
+ msg.contains("%s"));
+ // alterIndex throws with setTableName(tableName) at
MetaDataClient.java:6071, where the
+ // local `tableName` variable is bound to the resolved index
PTable's getTableName()
+ // (line 5999) — so the appended TABLE_NAME token renders the index
name here, not the
+ // underlying data table name.
+ assertTrue("exception message should contain tableName=" +
schemaName + "." + indexName
+ + ", got: " + msg, msg.contains("tableName=" + schemaName + "." +
indexName));
+ }
+ } finally {
+ cqs.releaseTransformLock(null, schemaName, indexName);
+ }
+ }
+
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement()
+ .execute("ALTER INDEX " + indexName + " ON " + fullTableName
+ + " ACTIVE IMMUTABLE_STORAGE_SCHEME =
SINGLE_CELL_ARRAY_WITH_OFFSETS,"
+ + " COLUMN_ENCODED_BYTES = 2");
+ }
+ }
+
+ /**
+ * Regression-doc test: a stray release after TTL expiry will delete a
different caller's lock
+ * cell. This documents the load-bearing hazard called out on
+ * {@link
org.apache.phoenix.query.ConnectionQueryServices#releaseTransformLock} —
callers MUST
+ * NOT release after the SYSTEM.MUTEX TTL has expired, because the release
is implemented as an
+ * unconditional cell delete with no fencing token.
+ * <p>
+ * The sequence the test exercises:
+ * <ol>
+ * <li>Caller A acquires the lock.</li>
+ * <li>The lock cell is removed out-of-band (simulating SYSTEM.MUTEX TTL
auto-expiry while A is
+ * paused).</li>
+ * <li>Caller B acquires — new cell is written, B is now the legitimate
holder.</li>
+ * <li>Caller A wakes up and runs its {@code finally} {@code
releaseTransformLock} — this deletes
+ * B's cell (the hazard).</li>
+ * <li>Caller C attempts acquire and succeeds, because B's cell was wiped by
A's stray
+ * release.</li>
+ * </ol>
+ * The assertion captures current behavior so any future change that
introduces token-CAS or
+ * holder fencing breaks this test deliberately.
+ */
+ @Test
+ public void testStrayReleaseDoesNotDeleteAnotherCallersLock() throws
Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+
+ try (Connection connA = DriverManager.getConnection(getUrl(), props);
+ Connection connB = DriverManager.getConnection(getUrl(), props);
+ Connection connC = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqsA = services(connA);
+ ConnectionQueryServices cqsB = services(connB);
+ ConnectionQueryServices cqsC = services(connC);
+
+ // Step 1: A acquires.
+ assertTrue("A must acquire", cqsA.acquireTransformLock(null, schemaName,
tableName));
+
+ // Step 2: simulate TTL auto-expiry by deleting A's cell out-of-band.
The marker comes from
+ // the same constant used inside
ConnectionQueryServicesImpl#acquireTransformLock — a future
+ // marker rename therefore breaks this test deliberately rather than
silently no-opping it.
+ // Using releaseTransformLock here would have the same byte-level effect
but conceptually
+ // represents "A's sanctioned release"; the lower-level deleteMutexCell
models "lock vanished
+ // without A knowing".
+ cqsA.deleteMutexCell(null, schemaName, tableName,
+ ConnectionQueryServicesImpl.TRANSFORM_LOCK_MARKER, null);
+
+ // Step 3: B acquires — succeeds because the cell is gone.
+ assertTrue("B must acquire after A's cell auto-expired",
+ cqsB.acquireTransformLock(null, schemaName, tableName));
+
+ // Step 4: A's stray release deletes B's cell (the hazard).
+ cqsA.releaseTransformLock(null, schemaName, tableName);
+
+ // Step 5: C succeeds because B's cell was wiped by A's stray release.
Under a token-CAS
+ // implementation, C would FAIL here (B would still hold), and B's
release would no-op.
+ assertTrue("C wins because A's stray release wiped B's cell — documents
the hazard",
+ cqsC.acquireTransformLock(null, schemaName, tableName));
+
+ // Cleanup.
+ cqsC.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ @Test
+ public void testNonTransformAlterDoesNotAcquireLock() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+ String fullTableName = schemaName + "." + tableName;
+
+ try (Connection setupConn = DriverManager.getConnection(getUrl(), props)) {
+ setupConn.createStatement()
+ .execute("CREATE TABLE " + fullTableName + " (id INTEGER PRIMARY KEY,
val VARCHAR)");
+ }
+
+ // Hold the transform lock externally; a non-transform-triggering ALTER
(SET TTL) must NOT
+ // contend on it under the narrow-scope wiring — it should succeed even
while the lock is held.
+ try (Connection holderConn = DriverManager.getConnection(getUrl(), props))
{
+ ConnectionQueryServices cqs = services(holderConn);
+ assertTrue("holder must acquire lock", cqs.acquireTransformLock(null,
schemaName, tableName));
+ try {
+ try (Connection altererConn = DriverManager.getConnection(getUrl(),
props)) {
+ altererConn.createStatement().execute("ALTER TABLE " + fullTableName
+ " SET TTL = 100");
+ }
+ } finally {
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+ }
+
+ /**
+ * Asserts that a transform-triggering ALTER which fails AFTER acquiring the
transform lock still
+ * releases it in its {@code finally} block, leaving the lock observably
free for a subsequent
+ * caller.
+ * <p>
+ * The IT models the failure deterministically by acquiring the lock at the
primitive API and then
+ * throwing inside a try/finally that mirrors the {@code
MetaDataClient.addColumn} /
+ * {@code alterIndex} release semantics. A natural mid-ALTER throw (e.g., a
+ * {@code ConcurrentTableMutationException} that exhausts the retry budget,
or a
+ * {@code TableNotFoundException} from the under-lock {@code
getTableNoCache} re-fetch when the
+ * table is dropped concurrently) would be racy in an IT; the primitive-API
path here exercises
+ * the same release-on-exception contract without timing assumptions.
+ */
+ @Test
+ public void testLockReleasedAfterFailedTransformTriggeringAlter() throws
Exception {
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ String schemaName = "S_" + generateUniqueName();
+ String tableName = "T_" + generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ ConnectionQueryServices cqs = services(conn);
+
+ // Simulate a transform-triggering ALTER that acquires the lock and then
throws
+ // mid-execution. The release in the finally mirrors MetaDataClient's
finally block.
+ boolean acquired = false;
+ try {
+ acquired = cqs.acquireTransformLock(null, schemaName, tableName);
+ assertTrue("simulated ALTER must acquire lock", acquired);
+ // Inject the failure. In production this would be a
TableNotFoundException from
+ // getTableNoCache, an evaluateStmtProperties throw, or a
ConcurrentTableMutationException
+ // retry exhaustion — any post-acquire path that reaches the finally
block.
+ throw new SQLException("simulated server-side failure during
transform-triggering ALTER");
+ } catch (SQLException expected) {
+ // expected
+ } finally {
+ if (acquired) {
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+
+ // Lock must be observably free now: a subsequent caller should win the
acquire.
+ assertTrue("lock must be observably released after the failed ALTER's
finally ran",
+ cqs.acquireTransformLock(null, schemaName, tableName));
+ cqs.releaseTransformLock(null, schemaName, tableName);
+ }
+ }
+}
