ngsg commented on code in PR #5771:
URL: https://github.com/apache/hive/pull/5771#discussion_r2174271963


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -187,5249 +66,146 @@ public HiveMetaStoreClient(Configuration conf, 
HiveMetaHookLoader hookLoader) th
 
   public HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader 
hookLoader, Boolean allowEmbedded)
     throws MetaException {
-
-    this.hookLoader = hookLoader;
-    if (conf == null) {
-      conf = MetastoreConf.newMetastoreConf();
-      this.conf = conf;
-    } else {
-      this.conf = new Configuration(conf);
-    }
-    version = MetastoreConf.getBoolVar(conf, ConfVars.HIVE_IN_TEST) ? 
TEST_VERSION : VERSION;
-    filterHook = loadFilterHooks();
-    isClientFilterEnabled = getIfClientFilterEnabled();
-    uriResolverHook = loadUriResolverHook();
-    fileMetadataBatchSize = MetastoreConf.getIntVar(
-        conf, ConfVars.BATCH_RETRIEVE_OBJECTS_MAX);
-
-    if ((MetastoreConf.get(conf, "hive.metastore.client.capabilities")) != 
null) {
-      String[] capabilities = MetastoreConf.get(conf, 
"hive.metastore.client.capabilities").split(",");
-      setProcessorCapabilities(capabilities);
-      String hostName = "unknown";
-      try {
-        hostName = InetAddress.getLocalHost().getCanonicalHostName();
-      } catch (UnknownHostException ue) {
-      }
-      setProcessorIdentifier("HMSClient-" + "@" + hostName);
-    }
-
-    String msUri = MetastoreConf.getVar(conf, ConfVars.THRIFT_URIS);
-    localMetaStore = MetastoreConf.isEmbeddedMetaStore(msUri);
-    if (localMetaStore) {
-      if (!allowEmbedded) {
-        throw new MetaException("Embedded metastore is not allowed here. 
Please configure "
-            + ConfVars.THRIFT_URIS.toString() + "; it is currently set to [" + 
msUri + "]");
-      }
-
-      client = callEmbeddedMetastore(this.conf);
-
-      // instantiate the metastore server handler directly instead of 
connecting
-      // through the network
-      isConnected = true;
-      snapshotActiveConf();
-      return;
-    }
-
-    // get the number retries
-    retries = MetastoreConf.getIntVar(conf, 
ConfVars.THRIFT_CONNECTION_RETRIES);
-    retryDelaySeconds = MetastoreConf.getTimeVar(conf,
-        ConfVars.CLIENT_CONNECT_RETRY_DELAY, TimeUnit.SECONDS);
-
-    // user wants file store based configuration
-    if (MetastoreConf.getVar(conf, ConfVars.THRIFT_URIS) != null) {
-      resolveUris();
-    } else {
-      LOG.error("NOT getting uris from conf");
-      throw new MetaException("MetaStoreURIs not found in conf file");
-    }
-
-    generateProxyUserDelegationToken();
-
-    // finally open the store
-    open();
-  }
-
-  /**
-   * Instantiate the metastore server handler directly instead of connecting
-   * through the network
-   *
-   * @param conf Configuration object passed to embedded metastore
-   * @return embedded client instance
-   * @throws MetaException
-   */
-  static ThriftHiveMetastore.Iface callEmbeddedMetastore(Configuration conf) 
throws MetaException {
-    // Instantiate the metastore server handler directly instead of connecting
-    // through the network
-    //
-    // The code below simulates the following code
-    //
-    // client = HiveMetaStore.newHMSHandler(this.conf);
-    //
-    // using reflection API. This is done to avoid dependency of 
MetastoreClient on Hive Metastore.
-    // Note that newHMSHandler is static method, so we pass null as the object 
reference.
-    //
-    try {
-      Class<?> clazz = Class.forName(HIVE_METASTORE_CLASS);
-      //noinspection JavaReflectionMemberAccess
-      Method method = 
clazz.getDeclaredMethod(HIVE_METASTORE_CREATE_HANDLER_METHOD, 
Configuration.class);
-      method.setAccessible(true);
-      return (ThriftHiveMetastore.Iface) method.invoke(null, conf);
-    } catch (InvocationTargetException e) {
-      if (e.getCause() != null) {
-        MetaStoreUtils.throwMetaException((Exception) e.getCause());
-      }
-      MetaStoreUtils.throwMetaException(e);
-    } catch (ClassNotFoundException
-        | NoSuchMethodException
-        | IllegalAccessException e) {
-      MetaStoreUtils.throwMetaException(e);
-    }
-    return null;
+    super(createUnderlyingClient(conf, hookLoader, allowEmbedded), conf);
+    HookMetaStoreClientProxy hookProxy = (HookMetaStoreClientProxy) 
getDelegate();
+    this.thriftClient = (ThriftHiveMetaStoreClient) hookProxy.getDelegate();
   }
 
-  private boolean getIfClientFilterEnabled() {
-    boolean isEnabled = MetastoreConf.getBoolVar(conf, 
ConfVars.METASTORE_CLIENT_FILTER_ENABLED);
-    LOG.info("HMS client filtering is " + (isEnabled ? "enabled." : 
"disabled."));
-
-    return isEnabled;
+  private static IMetaStoreClient createUnderlyingClient(Configuration conf, 
HiveMetaHookLoader hookLoader,
+      Boolean allowEmbedded) throws MetaException {
+    IMetaStoreClient thriftClient = new ThriftHiveMetaStoreClient(conf, 
allowEmbedded);
+    IMetaStoreClient clientWithHook = new HookMetaStoreClientProxy(conf, 
hookLoader, thriftClient);
+    return clientWithHook;
   }
 
-  private void resolveUris() throws MetaException {
-    String thriftUris = MetastoreConf.getVar(conf, ConfVars.THRIFT_URIS);
-    String serviceDiscoveryMode = MetastoreConf.getVar(conf, 
ConfVars.THRIFT_SERVICE_DISCOVERY_MODE);
-    List<String> metastoreUrisString = null;
-
-    // The metastore URIs can come from THRIFT_URIS directly or need to be 
fetched from the
-    // Zookeeper
-    try {
-      if (serviceDiscoveryMode == null || 
serviceDiscoveryMode.trim().isEmpty()) {
-        metastoreUrisString = Arrays.asList(thriftUris.split(","));
-      } else if (serviceDiscoveryMode.equalsIgnoreCase("zookeeper")) {
-        metastoreUrisString = new ArrayList<String>();
-        // Add scheme to the bare URI we get.
-        for (String s : MetastoreConf.getZKConfig(conf).getServerUris()) {
-          metastoreUrisString.add("thrift://" + s);
-        }
-      } else {
-        throw new IllegalArgumentException("Invalid metastore dynamic service 
discovery mode " +
-                serviceDiscoveryMode);
-      }
-    } catch (Exception e) {
-      MetaStoreUtils.throwMetaException(e);
-    }
-
-    if (metastoreUrisString.isEmpty() && 
"zookeeper".equalsIgnoreCase(serviceDiscoveryMode)) {
-      throw new MetaException("No metastore service discovered in ZooKeeper. "
-          + "Please ensure that at least one metastore server is online");
-    }
-
-    LOG.info("Resolved metastore uris: {}", metastoreUrisString);
-
-    List<URI> metastoreURIArray = new ArrayList<URI>();
-    try {
-      for (String s : metastoreUrisString) {
-        URI tmpUri = new URI(s);
-        if (tmpUri.getScheme() == null) {
-          throw new IllegalArgumentException("URI: " + s
-                  + " does not have a scheme");
-        }
-        if (uriResolverHook != null) {
-          metastoreURIArray.addAll(uriResolverHook.resolveURI(tmpUri));
-        } else {
-          metastoreURIArray.add(tmpUri);
-        }
-      }
-      metastoreUris = new URI[metastoreURIArray.size()];
-      for (int j = 0; j < metastoreURIArray.size(); j++) {
-        metastoreUris[j] = metastoreURIArray.get(j);
-      }
-
-      if (MetastoreConf.getVar(conf, 
ConfVars.THRIFT_URI_SELECTION).equalsIgnoreCase("RANDOM")) {
-        List<URI> uriList = Arrays.asList(metastoreUris);
-        Collections.shuffle(uriList);
-        metastoreUris = uriList.toArray(new URI[uriList.size()]);
-      }
-    } catch (IllegalArgumentException e) {
-      throw (e);
-    } catch (Exception e) {
-      MetaStoreUtils.throwMetaException(e);
-    }
+  public boolean createType(Type type) throws TException {
+    return thriftClient.createType(type);
   }
 
-  private void generateProxyUserDelegationToken() throws MetaException {
-    //If HADOOP_PROXY_USER is set in env or property,
-    //then need to create metastore client that proxies as that user.
-    String HADOOP_PROXY_USER = "HADOOP_PROXY_USER";
-    String proxyUser = System.getenv(HADOOP_PROXY_USER);
-    if (proxyUser == null) {
-      proxyUser = System.getProperty(HADOOP_PROXY_USER);
-    }
-    //if HADOOP_PROXY_USER is set, create DelegationToken using real user
-    if (proxyUser != null) {
-      LOG.info(HADOOP_PROXY_USER + " is set. Using delegation "
-          + "token for HiveMetaStore connection.");
-      try {
-        
UserGroupInformation.getRealUserOrSelf(UserGroupInformation.getLoginUser()).doAs(
-            new PrivilegedExceptionAction<Void>() {
-              @Override
-              public Void run() throws Exception {
-                open();
-                return null;
-              }
-            });
-        String delegationTokenPropString = 
"DelegationTokenForHiveMetaStoreServer";
-        String delegationTokenStr = getDelegationToken(proxyUser, proxyUser);
-        SecurityUtils.setTokenStr(UserGroupInformation.getCurrentUser(), 
delegationTokenStr,
-            delegationTokenPropString);
-        MetastoreConf.setVar(this.conf, ConfVars.TOKEN_SIGNATURE, 
delegationTokenPropString);
-      } catch (Exception e) {
-        LOG.error("Error while setting delegation token for " + proxyUser, e);
-        if (e instanceof MetaException) {
-          throw (MetaException) e;
-        } else {
-          throw new MetaException(e.getMessage());
-        }
-      } finally {
-        close();
-      }
-    }
+  public boolean dropType(String type) throws TException {

Review Comment:
   `dropType` and some methods in `HiveMetaStoreClient` are not defined in 
`IMetaStoreClient`. Therefore, there are no `@Override` annotations on those 
methods.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to