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.


---

Reply via email to