[phoenix] branch 4.x-HBase-1.5 updated: PHOENIX-5274: ConnectionQueryServiceImpl#ensureNamespaceCreated and ensureTableCreated should use HBase APIs that do not require ADMIN permissions for existence checks (Use hbaseAdmin listNamespaces API rather than getNamespaceDescriptor)

[gj...@apache.org]

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

gjacoby pushed a commit to branch 4.x-HBase-1.5
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/4.x-HBase-1.5 by this push:
     new e859f09  PHOENIX-5274: ConnectionQueryServiceImpl#ensureNamespaceCreated and ensureTableCreated should use HBase APIs that do not require ADMIN permissions for existence checks (Use hbaseAdmin listNamespaces API rather than getNamespaceDescriptor)
e859f09 is described below

commit e859f091aed1a3125a9fae338f28b0566e92844f
Author: Ankit Jain <ja...@salesforce.com>
AuthorDate: Thu Sep 5 21:31:18 2019 -0700

    PHOENIX-5274: ConnectionQueryServiceImpl#ensureNamespaceCreated and ensureTableCreated should use HBase APIs that do not require ADMIN permissions for existence checks (Use hbaseAdmin listNamespaces API rather than getNamespaceDescriptor)
---
 .../org/apache/phoenix/end2end/CreateSchemaIT.java | 11 ++++---
 .../org/apache/phoenix/end2end/DropSchemaIT.java   | 14 +++------
 .../SystemCatalogCreationOnConnectionIT.java       |  9 ++----
 .../phoenix/query/ConnectionQueryServicesImpl.java | 23 ++++----------
 .../java/org/apache/phoenix/util/ServerUtil.java   | 24 +++++++++++++++
 .../org/apache/phoenix/util/ServerUtilTest.java    | 36 ++++++++++++++++++++++
 6 files changed, 79 insertions(+), 38 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java
index 8002dc1..6e61c57 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java
@@ -18,7 +18,7 @@
 package org.apache.phoenix.end2end;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.sql.Connection;
@@ -33,6 +33,7 @@ import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.schema.SchemaAlreadyExistsException;
 import org.apache.phoenix.util.PropertiesUtil;
 import org.apache.phoenix.util.SchemaUtil;
+import org.apache.phoenix.util.ServerUtil;
 import org.apache.phoenix.util.TestUtil;
 import org.junit.Test;
 
@@ -54,9 +55,9 @@ public class CreateSchemaIT extends ParallelStatsDisabledIT {
         try (Connection conn = DriverManager.getConnection(getUrl(), props);
                 HBaseAdmin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin();) {
             conn.createStatement().execute(ddl1);
-            assertNotNull(admin.getNamespaceDescriptor(schemaName1));
+            assertTrue(ServerUtil.isHbaseNamespaceAvailable(admin, schemaName1));
             conn.createStatement().execute(ddl2);
-            assertNotNull(admin.getNamespaceDescriptor(schemaName2.toUpperCase()));
+            assertTrue(ServerUtil.isHbaseNamespaceAvailable(admin, schemaName2.toUpperCase()));
         }
         // Try creating it again and verify that it throws SchemaAlreadyExistsException
         try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
@@ -95,8 +96,8 @@ public class CreateSchemaIT extends ParallelStatsDisabledIT {
             conn.createStatement().execute("CREATE SCHEMA \""
                     + SchemaUtil.HBASE_NAMESPACE.toUpperCase() + "\"");
 
-            assertNotNull(admin.getNamespaceDescriptor(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE.toUpperCase()));
-            assertNotNull(admin.getNamespaceDescriptor(SchemaUtil.HBASE_NAMESPACE.toUpperCase()));
+            assertTrue(ServerUtil.isHbaseNamespaceAvailable(admin, SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE.toUpperCase()));
+            assertTrue(ServerUtil.isHbaseNamespaceAvailable(admin, SchemaUtil.HBASE_NAMESPACE.toUpperCase()));
 
         }
     }
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/DropSchemaIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/DropSchemaIT.java
index 5c5420c..9dfc829 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/DropSchemaIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/DropSchemaIT.java
@@ -18,8 +18,8 @@
 package org.apache.phoenix.end2end;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
+import static org.junit.Assert.assertTrue;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
@@ -30,7 +30,6 @@ import java.util.Map;
 import java.util.Properties;
 
 import org.apache.hadoop.hbase.NamespaceDescriptor;
-import org.apache.hadoop.hbase.NamespaceNotFoundException;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.jdbc.PhoenixConnection;
@@ -38,6 +37,7 @@ import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.schema.SchemaNotFoundException;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.apache.phoenix.util.SchemaUtil;
+import org.apache.phoenix.util.ServerUtil;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -92,22 +92,18 @@ public class DropSchemaIT extends BaseUniqueNamesOwnClusterIT {
             } catch (SQLException e) {
                 assertEquals(e.getErrorCode(), SQLExceptionCode.CANNOT_MUTATE_SCHEMA.getErrorCode());
             }
-            assertNotNull(admin.getNamespaceDescriptor(normalizeSchemaIdentifier));
+            assertTrue(ServerUtil.isHbaseNamespaceAvailable(admin, normalizeSchemaIdentifier));
 
             conn.createStatement().execute("DROP TABLE " + schema + "." + tableName);
             conn.createStatement().execute(ddl);
-            try {
-                admin.getNamespaceDescriptor(normalizeSchemaIdentifier);
+            if(ServerUtil.isHbaseNamespaceAvailable(admin, normalizeSchemaIdentifier))
                 fail();
-            } catch (NamespaceNotFoundException ne) {
-                // expected
-            }
             
             conn.createStatement().execute("DROP SCHEMA IF EXISTS " + schema);
             
             admin.createNamespace(NamespaceDescriptor.create(normalizeSchemaIdentifier).build());
             conn.createStatement().execute("DROP SCHEMA IF EXISTS " + schema);
-            assertNotNull(admin.getNamespaceDescriptor(normalizeSchemaIdentifier));
+            assertTrue(ServerUtil.isHbaseNamespaceAvailable(admin, normalizeSchemaIdentifier));
             conn.createStatement().execute("CREATE SCHEMA " + schema);
             conn.createStatement().execute("DROP SCHEMA " + schema);
             try {
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemCatalogCreationOnConnectionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemCatalogCreationOnConnectionIT.java
index d42ea28..ebf08c0 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemCatalogCreationOnConnectionIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemCatalogCreationOnConnectionIT.java
@@ -41,7 +41,6 @@ import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HTableDescriptor;
-import org.apache.hadoop.hbase.NamespaceNotFoundException;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.phoenix.coprocessor.MetaDataProtocol;
 import org.apache.phoenix.exception.SQLExceptionCode;
@@ -56,6 +55,7 @@ import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.query.QueryServicesTestImpl;
 import org.apache.phoenix.util.ReadOnlyProps;
+import org.apache.phoenix.util.ServerUtil;
 import org.apache.phoenix.util.UpgradeUtil;
 import org.junit.After;
 import org.junit.Before;
@@ -456,12 +456,7 @@ public class SystemCatalogCreationOnConnectionIT {
 
     // Check if the SYSTEM namespace has been created
     private boolean isSystemNamespaceCreated() throws IOException {
-        try {
-            testUtil.getHBaseAdmin().getNamespaceDescriptor(SYSTEM_CATALOG_SCHEMA);
-        } catch (NamespaceNotFoundException ex) {
-            return false;
-        }
-        return true;
+        return ServerUtil.isHbaseNamespaceAvailable(testUtil.getConnection().getAdmin(), SYSTEM_CATALOG_SCHEMA);
     }
 
     /**
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 fd9092f..cd7923c 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
@@ -118,6 +118,7 @@ import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.NamespaceNotFoundException;
 import org.apache.hadoop.hbase.TableExistsException;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.Append;
 import org.apache.hadoop.hbase.client.Delete;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
@@ -1139,15 +1140,9 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
     boolean ensureNamespaceCreated(String schemaName) throws SQLException {
         SQLException sqlE = null;
         boolean createdNamespace = false;
-        try (HBaseAdmin admin = getAdmin()) {
-            NamespaceDescriptor namespaceDescriptor = null;
-            try {
-                namespaceDescriptor = admin.getNamespaceDescriptor(schemaName);
-            } catch (NamespaceNotFoundException ignored) {
-
-            }
-            if (namespaceDescriptor == null) {
-                namespaceDescriptor = NamespaceDescriptor.create(schemaName).build();
+        try (Admin admin = connection.getAdmin()){
+        if (!ServerUtil.isHbaseNamespaceAvailable(admin, schemaName)) {
+                NamespaceDescriptor namespaceDescriptor = NamespaceDescriptor.create(schemaName).build();
                 admin.createNamespace(namespaceDescriptor);
                 createdNamespace = true;
             }
@@ -5236,17 +5231,11 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement
 
     private void ensureNamespaceDropped(String schemaName) throws SQLException {
         SQLException sqlE = null;
-        try (HBaseAdmin admin = getAdmin()) {
+        try (Admin admin = connection.getAdmin()) {
             final String quorum = ZKConfig.getZKQuorumServersString(config);
             final String znode = this.props.get(HConstants.ZOOKEEPER_ZNODE_PARENT);
             LOGGER.debug("Found quorum: " + quorum + ":" + znode);
-            boolean nameSpaceExists = true;
-            try {
-                admin.getNamespaceDescriptor(schemaName);
-            } catch (org.apache.hadoop.hbase.NamespaceNotFoundException e) {
-                nameSpaceExists = false;
-            }
-            if (nameSpaceExists) {
+            if (ServerUtil.isHbaseNamespaceAvailable(admin, schemaName)) {
                 admin.deleteNamespace(schemaName);
             }
         } catch (IOException e) {
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java
index 39986fb..3b8f185 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java
@@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.NotServingRegionException;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.ClusterConnection;
 import org.apache.hadoop.hbase.client.CoprocessorHConnection;
 import org.apache.hadoop.hbase.client.HTableInterface;
@@ -450,4 +451,27 @@ public class ServerUtil {
 
     }
 
+    /**
+     * Returns true if HBase namespace exists, else returns false
+     * @param admin HbaseAdmin Object
+     * @param schemaName Phoenix schema name for which we check existence of the HBase namespace
+     * @return true if the HBase namespace exists, else returns false
+     * @throws SQLException If there is an exception checking the HBase namespace
+     */
+    public static boolean isHbaseNamespaceAvailable(Admin admin, String schemaName) throws IOException{
+        boolean namespaceExists = false;
+        try{
+            String[] hbaseNamespaces = admin.listNamespaces();
+            for(String namespace : hbaseNamespaces){
+                if(namespace.equals(schemaName)){
+                    namespaceExists = true;
+                    break;
+                }
+            }
+        } catch (IOException e) {
+            throw e;
+        }
+        return namespaceExists;
+    }
+
 }
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java b/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java
new file mode 100644
index 0000000..49bf198
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java
@@ -0,0 +1,36 @@
+package org.apache.phoenix.util;
+
+import org.apache.hadoop.hbase.client.Admin;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+public class ServerUtilTest {
+
+    String existingNamespaceOne = "existingNamespaceOne";
+    String existingNamespaceTwo = "existingNamespaceTwo";
+    String nonExistingNamespace = "nonExistingNamespace";
+
+    String[] namespaces = { existingNamespaceOne, existingNamespaceTwo };
+
+    @Test
+    public void testIsHbaseNamespaceAvailableWithExistingNamespace() throws Exception {
+        Admin mockAdmin = getMockedAdmin();
+        assertTrue(ServerUtil.isHbaseNamespaceAvailable(mockAdmin, existingNamespaceOne));
+    }
+
+    @Test
+    public void testIsHbaseNamespaceAvailableWithNonExistingNamespace() throws Exception{
+        Admin mockAdmin = getMockedAdmin();
+        assertFalse(ServerUtil.isHbaseNamespaceAvailable(mockAdmin,nonExistingNamespace));
+    }
+
+    private Admin getMockedAdmin() throws Exception {
+        Admin mockAdmin = Mockito.mock(Admin.class);
+        Mockito.when(mockAdmin.listNamespaces()).thenReturn(namespaces);
+        return mockAdmin;
+    }
+
+}