You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by td...@apache.org on 2019/07/31 18:13:20 UTC

[phoenix] branch master updated: PHOENIX-5403 Optimize metadata cache lookup of global tables using a tenant specific connection

This is an automated email from the ASF dual-hosted git repository.

tdsilva pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new 12c538c  PHOENIX-5403 Optimize metadata cache lookup of global tables using a tenant specific connection
12c538c is described below

commit 12c538ce2db4779b91bc294ebe8e4880673ff8c2
Author: Thomas D'Silva <td...@apache.org>
AuthorDate: Mon Jul 22 22:45:04 2019 -0700

    PHOENIX-5403 Optimize metadata cache lookup of global tables using a tenant specific connection
---
 .../java/org/apache/phoenix/end2end/Array2IT.java  | 17 ++---
 .../org/apache/phoenix/compile/FromCompiler.java   |  6 +-
 .../phoenix/query/ConnectionQueryServicesImpl.java |  7 +-
 .../org/apache/phoenix/schema/MetaDataClient.java  | 74 ++++++++--------------
 4 files changed, 39 insertions(+), 65 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/Array2IT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/Array2IT.java
index 52bfb86..0cb60c2 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/Array2IT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/Array2IT.java
@@ -37,6 +37,7 @@ import org.apache.phoenix.schema.types.PhoenixArray;
 import org.apache.phoenix.util.PropertiesUtil;
 import org.apache.phoenix.util.SchemaUtil;
 import org.apache.phoenix.util.StringUtil;
+import org.junit.Ignore;
 import org.junit.Test;
 
 public class Array2IT extends ArrayIT {
@@ -669,27 +670,19 @@ public class Array2IT extends ArrayIT {
 
     }
 
-    @Test
+    @Test // see PHOENIX-5416
+    @Ignore
     public void testArrayRefToLiteral() throws Exception {
-        Connection conn;
-
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
-        conn = DriverManager.getConnection(getUrl(), props);
-        try {
-            PreparedStatement stmt = conn.prepareStatement("select ?[2] from \"SYSTEM\".\"catalog\" limit 1");
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            PreparedStatement stmt = conn.prepareStatement("select ?[2] from \"SYSTEM\".\"CATALOG\" limit 1");
             Array array = conn.createArrayOf("CHAR", new String[] {"a","b","c"});
             stmt.setArray(1, array);
             ResultSet rs = stmt.executeQuery();
             assertTrue(rs.next());
             assertEquals("b", rs.getString(1));
             assertFalse(rs.next());
-        } catch (SQLException e) {
-        } finally {
-            if (conn != null) {
-                conn.close();
-            }
         }
-
     }
     
     @Test
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
index ce0c3a1..a1ee0bf 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java
@@ -174,11 +174,7 @@ public class FromCompiler {
         NamedTableNode tableNode = NamedTableNode.create(null, baseTable, Collections.<ColumnDef>emptyList());
         // Always use non-tenant-specific connection here
         try {
-            // We need to always get the latest meta data for the parent table of a create view call to ensure that
-            // that we're copying the current table meta data as of when the view is created. Once we no longer
-            // copy the parent meta data, but store only the local diffs (PHOENIX-3534), we will no longer need
-            // to do this.
-            SingleTableColumnResolver visitor = new SingleTableColumnResolver(connection, tableNode, true, true);
+            SingleTableColumnResolver visitor = new SingleTableColumnResolver(connection, tableNode, true);
             return visitor;
         } catch (TableNotFoundException e) {
             // Used for mapped VIEW, since we won't be able to resolve that.
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index fc761dd..4112984 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -228,6 +228,7 @@ import org.apache.phoenix.schema.PSynchronizedMetaData;
 import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.schema.PTableImpl;
 import org.apache.phoenix.schema.PTableKey;
+import org.apache.phoenix.schema.PTableRef;
 import org.apache.phoenix.schema.PTableType;
 import org.apache.phoenix.schema.ReadOnlyTableException;
 import org.apache.phoenix.schema.SaltingUtil;
@@ -271,6 +272,7 @@ import org.apache.phoenix.util.QueryUtil;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.apache.phoenix.util.SchemaUtil;
 import org.apache.phoenix.util.ServerUtil;
+import org.apache.phoenix.util.TimeKeeper;
 import org.apache.phoenix.util.UpgradeUtil;
 import org.apache.twill.zookeeper.ZKClientService;
 import org.slf4j.Logger;
@@ -642,8 +644,11 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
                 throwConnectionClosedIfNullMetaData();
                 // If existing table isn't older than new table, don't replace
                 // If a client opens a connection at an earlier timestamp, this can happen
-                PTable existingTable = latestMetaData.getTableRef(new PTableKey(table.getTenantId(), table.getName().getString())).getTable();
+                PTableRef existingTableRef = latestMetaData.getTableRef(new PTableKey(
+                        table.getTenantId(), table.getName().getString()));
+                PTable existingTable = existingTableRef.getTable();
                 if (existingTable.getTimeStamp() > table.getTimeStamp()) {
+                    existingTableRef.setLastAccessTime(TimeKeeper.SYSTEM.getCurrentTime());
                     return;
                 }
             } catch (TableNotFoundException e) {}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
index 9c4ee2c..4afb0a0 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
@@ -408,8 +408,6 @@ public class MetaDataClient {
                     INDEX_DISABLE_TIMESTAMP +","+
                     ASYNC_REBUILD_TIMESTAMP + " " + PLong.INSTANCE.getSqlTypeName() +
                     ") VALUES (?, ?, ?, ?, ?, ?)";
-    //TODO: merge INSERT_COLUMN_CREATE_TABLE and INSERT_COLUMN_ALTER_TABLE column when
-    // the new major release is out.
     private static final String INSERT_COLUMN_CREATE_TABLE =
             "UPSERT INTO " + SYSTEM_CATALOG_SCHEMA + ".\"" + SYSTEM_CATALOG_TABLE + "\"( " +
                     TENANT_ID + "," +
@@ -434,29 +432,6 @@ public class MetaDataClient {
                     IS_ROW_TIMESTAMP +
                     ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
 
-    private static final String INSERT_COLUMN_ALTER_TABLE =
-            "UPSERT INTO " + SYSTEM_CATALOG_SCHEMA + ".\"" + SYSTEM_CATALOG_TABLE + "\"( " +
-                    TENANT_ID + "," +
-                    TABLE_SCHEM + "," +
-                    TABLE_NAME + "," +
-                    COLUMN_NAME + "," +
-                    COLUMN_FAMILY + "," +
-                    DATA_TYPE + "," +
-                    NULLABLE + "," +
-                    COLUMN_SIZE + "," +
-                    DECIMAL_DIGITS + "," +
-                    ORDINAL_POSITION + "," +
-                    SORT_ORDER + "," +
-                    DATA_TABLE_NAME + "," + // write this both in the column and table rows for access by metadata APIs
-                    ARRAY_SIZE + "," +
-                    VIEW_CONSTANT + "," +
-                    IS_VIEW_REFERENCED + "," +
-                    PK_NAME + "," +  // write this both in the column and table rows for access by metadata APIs
-                    KEY_SEQ + "," +
-                    COLUMN_DEF + "," +
-                    COLUMN_QUALIFIER +
-                    ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
-
     /*
      * Custom sql to add a column to SYSTEM.CATALOG table during upgrade.
      * We can't use the regular INSERT_COLUMN_ALTER_TABLE sql because the COLUMN_QUALIFIER column
@@ -597,12 +572,23 @@ public class MetaDataClient {
         String fullTableName = SchemaUtil.getTableName(schemaName, tableName);
         long tableTimestamp = HConstants.LATEST_TIMESTAMP;
         long tableResolvedTimestamp = HConstants.LATEST_TIMESTAMP;
-        try {
-            tableRef = connection.getTableRef(new PTableKey(tenantId, fullTableName));
-            table = tableRef.getTable();
-            tableTimestamp = table.getTimeStamp();
-            tableResolvedTimestamp = tableRef.getResolvedTimeStamp();
-        } catch (TableNotFoundException e) {
+        int tryCount = 0;
+        // for tenant specific connection, look up the global table without using tenantId
+        int maxTryCount = tenantId == null ? 1 : 2;
+        do {
+            try {
+                tableRef = connection.getTableRef(new PTableKey(tenantId, fullTableName));
+                table = tableRef.getTable();
+                tableTimestamp = table.getTimeStamp();
+                tableResolvedTimestamp = tableRef.getResolvedTimeStamp();
+                break;
+            } catch (TableNotFoundException e) {
+                tenantId = null;
+            }
+        } while (++tryCount < maxTryCount);
+        // reset the tenantId if the global table isn't found in the cache
+        if (table==null) {
+            tenantId = systemTable ? null : origTenantId;
         }
 
         // start a txn if all table are transactional by default or if we found the table in the cache and it is transactional
@@ -622,10 +608,7 @@ public class MetaDataClient {
             return new MetaDataMutationResult(MutationCode.TABLE_ALREADY_EXISTS, QueryConstants.UNSET_TIMESTAMP, table);
         }
 
-        int maxTryCount = tenantId == null ? 1 : 2;
-        int tryCount = 0;
         MetaDataMutationResult result;
-
         // if we are looking up an index on a child view that is inherited from its
         // parent, then we need to resolve the parent of the child view which will also
         // load any of its indexes instead of trying to load the inherited view index
@@ -655,6 +638,7 @@ public class MetaDataClient {
             }
         }
         else {
+            tryCount = 0;
             do {
                 final byte[] schemaBytes = PVarchar.INSTANCE.toBytes(schemaName);
                 final byte[] tableBytes = PVarchar.INSTANCE.toBytes(tableName);
@@ -681,6 +665,9 @@ public class MetaDataClient {
                             && result.getTable() == null) {
                         result.setTable(table);
                     }
+                    if (result.getTable()!=null) {
+                        addTableToCache(result);
+                    }
                     return result;
                 }
                 MutationCode code = result.getMutationCode();
@@ -914,16 +901,12 @@ public class MetaDataClient {
         } else {
             colUpsert.setString(18, column.getExpressionStr());
         }
-        if (colUpsert.getParameterMetaData().getParameterCount() > 18) {
-            if (column.getColumnQualifierBytes() == null) {
-                colUpsert.setNull(19, Types.VARBINARY);
-            } else {
-                colUpsert.setBytes(19, column.getColumnQualifierBytes());
-            }
-        }
-        if (colUpsert.getParameterMetaData().getParameterCount() > 19) {
-            colUpsert.setBoolean(20, column.isRowTimestamp());
+        if (column.getColumnQualifierBytes() == null) {
+            colUpsert.setNull(19, Types.VARBINARY);
+        } else {
+            colUpsert.setBytes(19, column.getColumnQualifierBytes());
         }
+        colUpsert.setBoolean(20, column.isRowTimestamp());
         colUpsert.execute();
     }
 
@@ -3587,10 +3570,7 @@ public class MetaDataClient {
                 Map<String, Integer> changedCqCounters = new HashMap<>(numCols);
                 if (numCols > 0 ) {
                     StatementContext context = new StatementContext(new PhoenixStatement(connection), resolver);
-                    String addColumnSqlToUse = connection.isRunningUpgrade()
-                            && tableName.equals(PhoenixDatabaseMetaData.SYSTEM_CATALOG_TABLE)
-                            && schemaName.equals(PhoenixDatabaseMetaData.SYSTEM_CATALOG_SCHEMA) ? ALTER_SYSCATALOG_TABLE_UPGRADE
-                            : INSERT_COLUMN_ALTER_TABLE;
+                    String addColumnSqlToUse = INSERT_COLUMN_CREATE_TABLE;
                     try (PreparedStatement colUpsert = connection.prepareStatement(addColumnSqlToUse)) {
                         short nextKeySeq = SchemaUtil.getMaxKeySeq(table);
                         for( ColumnDef colDef : columnDefs) {