sanjeet006py commented on code in PR #2594:
URL: https://github.com/apache/phoenix/pull/2594#discussion_r3864290226
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -2459,6 +2460,53 @@ private void disableTable(Admin admin, TableName
tableName) throws IOException {
}
}
+ private void enableTable(Admin admin, TableName tableName) throws
IOException {
+ try {
+ admin.enableTable(tableName);
+ } catch (TableNotDisabledException e) {
+ LOGGER.info("Table already enabled, continuing with next steps", e);
+ }
+ }
+
+ /**
+ * PHOENIX-7788: re-enable a disabled physical HBase table if SYSTEM.CATALOG
has no row for it. If
+ * metadata exists, leave it disabled — an admin may have disabled the
registered table.
+ */
+ private void reenableOrphanedDisabledHBaseTable(byte[] schemaBytes, byte[]
tableBytes,
+ boolean isNamespaceMapped, PTableType tableType) throws SQLException {
+ if (tableType != PTableType.TABLE) {
+ return;
+ }
+ TableName physicalTableName = TableName.valueOf(
+ SchemaUtil.getPhysicalHBaseTableName(schemaBytes, tableBytes,
isNamespaceMapped).getBytes());
+ try (Admin admin = getAdmin()) {
+ if (!AdminUtilWithFallback.tableExists(admin, physicalTableName)) {
+ return;
+ }
+ if (!admin.isTableDisabled(physicalTableName)) {
+ return;
+ }
+ MetaDataMutationResult result = getTable(null, schemaBytes, tableBytes,
+ HConstants.LATEST_TIMESTAMP, HConstants.LATEST_TIMESTAMP);
+ if (result.getMutationCode() != MutationCode.TABLE_NOT_FOUND) {
+ LOGGER.info(
+ "Physical HBase table {} is disabled but SYSTEM.CATALOG has metadata
for it "
Review Comment:
nit: Instead of hardcoding `SYSTEM.CATALOG` how about using
`PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME`? This will keep code DRY.
##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -2557,6 +2605,9 @@ public MetaDataMutationResult createTable(final
List<Mutation> tableMetaData,
(tableType != PTableType.CDC) && ((tableType == PTableType.VIEW &&
physicalTableName != null)
|| (tableType != PTableType.VIEW && (physicalTableName == null ||
localIndexTable)))
) {
+ // PHOENIX-7788: recover from an orphaned disabled physical table before
ensureTableCreated
+ // runs modifyTable on it. See the helper for the metadata-preserving
contract.
+ reenableOrphanedDisabledHBaseTable(schemaBytes, tableBytes,
isNamespaceMapped, tableType);
Review Comment:
How about shifting this to `ensureTableCreated`? That way the responsibility
of ensuring table is created and usable lies with `ensureTableCreated` only.
Further, you will notice that in subsequent lines `ensureViewIndexTableCreated`
is called which eventually calls `ensureTableCreated` for physical HBase table
used by view indexes. And, same situation of physical HBase table for view
index being disabled can happen here also.
--
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]