Github user karanmehta93 commented on a diff in the pull request: https://github.com/apache/phoenix/pull/277#discussion_r146698425 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java --- @@ -3101,49 +3124,68 @@ void ensureSystemTablesUpgraded(ReadOnlyProps props) // Regardless of the case 1 or 2, if the NS does not exist, we will error expectedly // below. If the NS does exist and is mapped, the below check will exit gracefully. } - + List<TableName> tableNames = getSystemTableNames(admin); // No tables exist matching "SYSTEM\..*", they are all already in "SYSTEM:.*" if (tableNames.size() == 0) { return; } // Try to move any remaining tables matching "SYSTEM\..*" into "SYSTEM:" if (tableNames.size() > 5) { logger.warn("Expected 5 system tables but found " + tableNames.size() + ":" + tableNames); } + + // Try acquiring a lock in SYSMUTEX table before migrating the tables since it involves disabling the table + // If we cannot acquire lock, it means some old client is either migrating SYSCAT or trying to upgrade the + // schema of SYSCAT table and hence it should not be interrupted + acquiredMutexLock = acquireUpgradeMutex(0, mutexRowKey); + if(acquiredMutexLock) { + logger.debug("Acquired lock in SYSMUTEX table for migrating SYSTEM tables to SYSTEM namespace"); + } + // We will not reach here if we fail to acquire the lock, since it throws UpgradeInProgressException + + // Handle the upgrade of SYSMUTEX table separately since it doesn't have any entries in SYSCAT + String sysMutexSrcTableName = PhoenixDatabaseMetaData.SYSTEM_MUTEX_NAME; + String sysMutexDestTableName = SchemaUtil.getPhysicalName(sysMutexSrcTableName.getBytes(), props).getNameAsString(); + UpgradeUtil.mapTableToNamespace(admin, sysMutexSrcTableName, sysMutexDestTableName, PTableType.SYSTEM); + byte[] mappedSystemTable = SchemaUtil .getPhysicalName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, props).getName(); metatable = getTable(mappedSystemTable); if (tableNames.contains(PhoenixDatabaseMetaData.SYSTEM_CATALOG_HBASE_TABLE_NAME)) { if (!admin.tableExists(mappedSystemTable)) { + // Actual migration of SYSCAT table UpgradeUtil.mapTableToNamespace(admin, metatable, PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME, props, null, PTableType.SYSTEM, null); + // Invalidate the client-side metadataCache ConnectionQueryServicesImpl.this.removeTable(null, PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME, null, MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_1_0); } tableNames.remove(PhoenixDatabaseMetaData.SYSTEM_CATALOG_HBASE_TABLE_NAME); } - tableNames.remove(PhoenixDatabaseMetaData.SYSTEM_MUTEX_HBASE_TABLE_NAME); for (TableName table : tableNames) { UpgradeUtil.mapTableToNamespace(admin, metatable, table.getNameAsString(), props, null, PTableType.SYSTEM, null); ConnectionQueryServicesImpl.this.removeTable(null, table.getNameAsString(), null, MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_1_0); } - if (!tableNames.isEmpty()) { - clearCache(); - } + + // Clear the server-side metadataCache when all tables are migrated so that the new PTable can be loaded with NS mapping + clearCache(); } finally { if (metatable != null) { metatable.close(); } + if(acquiredMutexLock) { --- End diff -- @joshelser @ankitsinghal Can probably comment on this one.
---