You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by lv...@apache.org on 2019/06/10 16:18:36 UTC

[impala] 02/05: IMPALA-8635. Use local metastore URIs when checking for Kudu/HMS integration

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

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

commit 1f407fd8d347618fdc18a85b5350556681998ce3
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Thu Jun 6 23:41:17 2019 -0700

    IMPALA-8635. Use local metastore URIs when checking for Kudu/HMS integration
    
    The patch for IMPALA-8504 (part 2) (6bb404dc359) checks to see if Impala
    and Kudu are configured against the same metastore to determine if the
    HMS integration is enabled. However, instead of using its own metastore
    URI config, it uses the configuration stored on the remote HMS. This is
    error prone because it's not common for the HMS configuration to store
    its own URI. Instead, we should use our own config.
    
    This patch changes to using the local configuration for this purpose.
    More robust would be to use the HMS "UUID" support, since it's possible
    that Kudu and Impala are talking to different HMS instances sharing a
    backing DB, but that work is deferred to a later commit since it depends
    on Kudu-side changes.
    
    This commit doesn't add any tests, but fixes the existing tests when
    running against Hive 3, where the HMS server side uses a different
    configuration key for the metastore URIs.
    
    Change-Id: Id7a4c2cc0580f7c4dc5cfceed30b91e87c547612
    Reviewed-on: http://gerrit.cloudera.org:8080/13555
    Reviewed-by: Hao Hao <ha...@cloudera.com>
    Reviewed-by: Thomas Marshall <tm...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/catalog/KuduTable.java     |  6 +-----
 .../java/org/apache/impala/service/CatalogOpExecutor.java     |  9 +--------
 fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java    | 11 +++++++----
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
index e059013..0f0540f 100644
--- a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
@@ -212,11 +212,7 @@ public class KuduTable extends Table implements FeKuduTable {
   public static boolean isHMSIntegrationEnabledAndValidate(String kuduMasters,
       String hmsUris) throws ImpalaRuntimeException {
     Preconditions.checkNotNull(hmsUris);
-    // Skip validation if the HMS URIs in impala is empty for some reason.
-    // TODO: Is this a valid case?
-    if (hmsUris.isEmpty()) {
-      return true;
-    }
+    Preconditions.checkArgument(!hmsUris.isEmpty());
     HiveMetastoreConfig hmsConfig = getHiveMetastoreConfig(kuduMasters);
     if (hmsConfig == null) {
       return false;
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index c16bc38..63d4166 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1515,14 +1515,7 @@ public class CatalogOpExecutor {
     // the configuration.
     Preconditions.checkState(KuduTable.isKuduTable(msTbl));
     String masterHosts = msTbl.getParameters().get(KuduTable.KEY_MASTER_HOSTS);
-    String hmsUris;
-    try {
-      hmsUris = MetaStoreUtil.getHiveMetastoreUrisKeyValue(
-          catalog_.getMetaStoreClient().getHiveClient());
-    } catch (Exception e) {
-      throw new RuntimeException(String.format("Failed to get the Hive Metastore " +
-          "configuration for table '%s' ", msTbl.getTableName()), e);
-    }
+    String hmsUris = MetaStoreUtil.getHiveMetastoreUris();
     return KuduTable.isHMSIntegrationEnabledAndValidate(masterHosts, hmsUris);
   }
 
diff --git a/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java b/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
index 789892d..ee026d8 100644
--- a/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
+++ b/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
@@ -88,6 +88,8 @@ public class MetaStoreUtil {
   // The default value for the above configuration key.
   public static final String DEFAULT_HIVE_METASTORE_URIS = "";
 
+  public static final String hiveMetastoreUris_;
+
   static {
     // Get the value from the Hive configuration, if present.
     HiveConf hiveConf = new HiveConf(HdfsTable.class);
@@ -105,6 +107,9 @@ public class MetaStoreUtil {
           "default: %d", maxPartitionsPerRpc_, DEFAULT_MAX_PARTITIONS_PER_RPC));
       maxPartitionsPerRpc_ = DEFAULT_MAX_PARTITIONS_PER_RPC;
     }
+
+    hiveMetastoreUris_ = hiveConf.get(HIVE_METASTORE_URIS_KEY,
+        DEFAULT_HIVE_METASTORE_URIS);
   }
 
   /**
@@ -119,10 +124,8 @@ public class MetaStoreUtil {
   /**
    * Return the value of thrift URI for the remote Hive Metastore.
    */
-  public static String getHiveMetastoreUrisKeyValue(IMetaStoreClient client)
-      throws ConfigValSecurityException, TException {
-    return client.getConfigValue(
-        HIVE_METASTORE_URIS_KEY, DEFAULT_HIVE_METASTORE_URIS);
+  public static String getHiveMetastoreUris() {
+    return hiveMetastoreUris_;
   }
 
   /**