PHOENIX-3757 System mutex table not being created in SYSTEM namespace…

… when namespace mapping is enabled

phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemTablePermissionsIT.java

@@ -57,10 +57,9 @@ public class SystemTablePermissionsIT {
     private static final Set<String> PHOENIX_SYSTEM_TABLES = new HashSet<>(Arrays.asList(
             "SYSTEM.CATALOG", "SYSTEM.SEQUENCE", "SYSTEM.STATS", "SYSTEM.FUNCTION",
                 "SYSTEM.MUTEX"));
-    // PHOENIX-XXXX SYSTEM.MUTEX isn't being created in the SYSTEM namespace as it should be.
     private static final Set<String> PHOENIX_NAMESPACE_MAPPED_SYSTEM_TABLES = new HashSet<>(
             Arrays.asList("SYSTEM:CATALOG", "SYSTEM:SEQUENCE", "SYSTEM:STATS", "SYSTEM:FUNCTION",
-                "SYSTEM.MUTEX"));
+                "SYSTEM:MUTEX"));
 
     private static final String TABLE_NAME =
         SystemTablePermissionsIT.class.getSimpleName().toUpperCase();

phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataProtocol.java

@@ -68,6 +68,8 @@ public abstract class MetaDataProtocol extends MetaDataService {
             VersionUtil.encodeVersion(PHOENIX_MAJOR_VERSION, PHOENIX_MINOR_VERSION, PHOENIX_PATCH_NUMBER);
 
     public static final long MIN_TABLE_TIMESTAMP = 0;
+    public static final long MIN_SYSTEM_TABLE_MIGRATION_TIMESTAMP = 0;
+    public static final String MIGRATION_IN_PROGRESS = "MigrationInProgress";
 
     public static final int DEFAULT_MAX_META_DATA_VERSIONS = 1000;
     public static final boolean DEFAULT_META_DATA_KEEP_DELETED_CELLS = true;
@@ -95,6 +97,7 @@ public abstract class MetaDataProtocol extends MetaDataService {
     // Key is the SYSTEM.CATALOG timestamp for the version and value is the version string.
     private static final NavigableMap<Long, String> TIMESTAMP_VERSION_MAP = new TreeMap<>();
     static {
+        TIMESTAMP_VERSION_MAP.put(MIN_SYSTEM_TABLE_MIGRATION_TIMESTAMP, MIGRATION_IN_PROGRESS);
         TIMESTAMP_VERSION_MAP.put(MIN_SYSTEM_TABLE_TIMESTAMP_4_1_0, "4.1.x");
         TIMESTAMP_VERSION_MAP.put(MIN_SYSTEM_TABLE_TIMESTAMP_4_2_0, "4.2.0");
         TIMESTAMP_VERSION_MAP.put(MIN_SYSTEM_TABLE_TIMESTAMP_4_2_1, "4.2.1");

phoenix-core/src/main/java/org/apache/phoenix/exception/UpgradeInProgressException.java

@@ -18,10 +18,14 @@
 package org.apache.phoenix.exception;
 
 
+import org.apache.phoenix.coprocessor.MetaDataProtocol;
+
 public class UpgradeInProgressException extends RetriableUpgradeException {
     public UpgradeInProgressException(String upgradeFrom, String upgradeTo) {
-        super("Cluster is being concurrently upgraded from " + upgradeFrom + " to " + upgradeTo
+        super((upgradeFrom.equals(MetaDataProtocol.MIGRATION_IN_PROGRESS) ?
+                "System Tables are concurrently being migrated to system namespace" :
+                "Cluster is being concurrently upgraded from " + upgradeFrom + " to " + upgradeTo)
                 + ". Please retry establishing connection.", SQLExceptionCode.CONCURRENT_UPGRADE_IN_PROGRESS
                 .getSQLState(), SQLExceptionCode.CONCURRENT_UPGRADE_IN_PROGRESS.getErrorCode());
     }
-}
+}

phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java

@@ -1735,26 +1735,7 @@ private static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metata
                         ? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE
                                 + " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED
                         : QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); }
-        boolean srcTableExists=admin.tableExists(srcTableName);
-        // we need to move physical table in actual namespace for TABLE and Index
-        if (srcTableExists && (PTableType.TABLE.equals(pTableType)
-                || PTableType.INDEX.equals(pTableType) || PTableType.SYSTEM.equals(pTableType))) {
-            boolean destTableExists=admin.tableExists(destTableName);
-            if (!destTableExists) {
-                String snapshotName = QueryConstants.UPGRADE_TABLE_SNAPSHOT_PREFIX + srcTableName;
-                logger.info("Disabling table " + srcTableName + " ..");
-                admin.disableTable(srcTableName);
-                logger.info(String.format("Taking snapshot %s of table %s..", snapshotName, srcTableName));
-                admin.snapshot(snapshotName, srcTableName);
-                logger.info(
-                        String.format("Restoring snapshot %s in destination table %s..", snapshotName, destTableName));
-                admin.cloneSnapshot(Bytes.toBytes(snapshotName), Bytes.toBytes(destTableName));
-                logger.info(String.format("deleting old table %s..", srcTableName));
-                admin.deleteTable(srcTableName);
-                logger.info(String.format("deleting snapshot %s..", snapshotName));
-                admin.deleteSnapshot(snapshotName);
-            }
-        }
+        mapTableToNamespace(admin, srcTableName, destTableName, pTableType);
 
         byte[] tableKey = SchemaUtil.getTableKey(tenantId != null ? tenantId.getString() : null,
                 SchemaUtil.getSchemaNameFromFullName(phoenixTableName),
@@ -1778,6 +1759,29 @@ private static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metata
         }
     }
 
+    public static void mapTableToNamespace(HBaseAdmin admin, String srcTableName, String destTableName, PTableType pTableType) throws IOException {
+        boolean srcTableExists=admin.tableExists(srcTableName);
+        // we need to move physical table in actual namespace for TABLE and Index
+        if (srcTableExists && (PTableType.TABLE.equals(pTableType)
+                || PTableType.INDEX.equals(pTableType) || PTableType.SYSTEM.equals(pTableType))) {
+            boolean destTableExists=admin.tableExists(destTableName);
+            if (!destTableExists) {
+                String snapshotName = QueryConstants.UPGRADE_TABLE_SNAPSHOT_PREFIX + srcTableName;
+                logger.info("Disabling table " + srcTableName + " ..");
+                admin.disableTable(srcTableName);
+                logger.info(String.format("Taking snapshot %s of table %s..", snapshotName, srcTableName));
+                admin.snapshot(snapshotName, srcTableName);
+                logger.info(
+                        String.format("Restoring snapshot %s in destination table %s..", snapshotName, destTableName));
+                admin.cloneSnapshot(Bytes.toBytes(snapshotName), Bytes.toBytes(destTableName));
+                logger.info(String.format("deleting old table %s..", srcTableName));
+                admin.deleteTable(srcTableName);
+                logger.info(String.format("deleting snapshot %s..", snapshotName));
+                admin.deleteSnapshot(snapshotName);
+            }
+        }
+    }
+
     /*
      * Method to map existing phoenix table to a namespace. Should not be use if tables has views and indexes ,instead
      * use map table utility in psql.py

phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java

@@ -20,6 +20,7 @@
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -46,18 +47,20 @@ public void testExceptionHandlingOnSystemNamespaceCreation() throws Exception {
         ConnectionQueryServicesImpl cqs = mock(ConnectionQueryServicesImpl.class);
         // Invoke the real methods for these two calls
         when(cqs.createSchema(any(List.class), anyString())).thenCallRealMethod();
-        doCallRealMethod().when(cqs).ensureSystemTablesUpgraded(any(ReadOnlyProps.class));
+        doCallRealMethod().when(cqs).ensureSystemTablesMigratedToSystemNamespace(any(ReadOnlyProps.class));
+        // Do nothing for this method, just check that it was invoked later
+        doNothing().when(cqs).createSysMutexTable(any(HBaseAdmin.class), any(ReadOnlyProps.class));
 
         // Spoof out this call so that ensureSystemTablesUpgrade() will return-fast.
         when(cqs.getSystemTableNames(any(HBaseAdmin.class))).thenReturn(Collections.<TableName> emptyList());
 
         // Throw a special exception to check on later
         doThrow(PHOENIX_IO_EXCEPTION).when(cqs).ensureNamespaceCreated(anyString());
 
-        // Make sure that ensureSystemTablesUpgraded will try to migrate the system tables.
+        // Make sure that ensureSystemTablesMigratedToSystemNamespace will try to migrate the system tables.
         Map<String,String> props = new HashMap<>();
         props.put(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, "true");
-        cqs.ensureSystemTablesUpgraded(new ReadOnlyProps(props));
+        cqs.ensureSystemTablesMigratedToSystemNamespace(new ReadOnlyProps(props));
 
         // Should be called after upgradeSystemTables()
         // Proves that execution proceeded