From efd04b858d2c545746045034ef5da3884b542f5a Mon Sep 17 00:00:00 2001 From: Noritaka Sekiyama Date: Thu, 13 Aug 2020 22:04:44 +0900 Subject: [PATCH 1/4] HIVE-12679: Cherry-pick 1558b1ad12c6f32d1eab21b6e0865fec5752a2c3 --- .../org/apache/hadoop/hive/conf/HiveConf.java | 3 + .../apache/hadoop/hive/ql/metadata/Hive.java | 25 +++++++-- .../metadata/HiveMetaStoreClientFactory.java | 55 +++++++++++++++++++ .../SessionHiveMetaStoreClientFactory.java | 52 ++++++++++++++++++ .../hadoop/hive/ql/metadata/TestHive.java | 45 +++++++++++++++ 5 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java create mode 100644 ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClientFactory.java diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index f4d1c376d3fc..886e0b455e3b 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -889,6 +889,9 @@ public static enum ConfVars { HADOOPNUMREDUCERS("mapreduce.job.reduces", -1, "", true), // Metastore stuff. Be sure to update HiveConf.metaVars when you add something here! + METASTORE_CLIENT_FACTORY_CLASS("hive.metastore.client.factory.class", + "org.apache.hadoop.hive.ql.metadata.SessionHiveMetaStoreClientFactory", + "The name of the factory class that produces objects implementing the IMetaStoreClient interface."), METASTOREDBTYPE("hive.metastore.db.type", "DERBY", new StringSet("DERBY", "ORACLE", "MYSQL", "MSSQL", "POSTGRES"), "Type of database used by the metastore. Information schema & JDBCStorageHandler depend on it."), /** diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java index 53e95ef0e929..3395e312f72e 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java @@ -236,6 +236,7 @@ import org.apache.hadoop.mapred.InputFormat; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.StringUtils; +import org.apache.hadoop.util.ReflectionUtils; import org.apache.hive.common.util.HiveVersionInfo; import org.apache.thrift.TException; import org.apache.thrift.TApplicationException; @@ -5700,11 +5701,25 @@ public HiveMetaHook getHook( } }; - if (conf.getBoolVar(ConfVars.METASTORE_FASTPATH)) { - return new SessionHiveMetaStoreClient(conf, hookLoader, allowEmbedded); - } else { - return RetryingMetaStoreClient.getProxy(conf, hookLoader, metaCallTimeMap, - SessionHiveMetaStoreClient.class.getName(), allowEmbedded); + return createMetaStoreClientFactory(conf) + .createMetaStoreClient(conf, hookLoader, allowEmbedded, metaCallTimeMap); + } + + private static HiveMetaStoreClientFactory createMetaStoreClientFactory(HiveConf conf) throws + MetaException { + String metaStoreClientFactoryClassName = conf.getVar(HiveConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS); + + try { + Class factoryClass = + conf.getClassByName(metaStoreClientFactoryClassName) + .asSubclass(HiveMetaStoreClientFactory.class); + return ReflectionUtils.newInstance(factoryClass, conf); + } catch (Exception e) { + String errorMessage = String.format( + "Unable to instantiate a metastore client factory %s due to: %s", + metaStoreClientFactoryClassName, e); + LOG.error(errorMessage, e); + throw new MetaException(errorMessage); } } diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java new file mode 100644 index 000000000000..13bd9ec97a99 --- /dev/null +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hive.ql.metadata; + +import java.util.concurrent.ConcurrentHashMap; +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.HiveMetaHookLoader; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.MetaException; + +/** + * Abstract factory that defines an interface for other factories that produce concrete + * MetaStoreClient objects. + * + */ +public interface HiveMetaStoreClientFactory { + + /** + * A method for producing IMetaStoreClient objects. + * + * The implementation returned by this method must throw a MetaException if allowEmbedded = true + * and it does not support embedded mode. + * + * @param conf + * Hive Configuration. + * @param hookLoader + * Hook for handling events related to tables. + * @param allowEmbedded + * Flag indicating the implementation must run in-process, e.g. for unit testing or + * "fast path". + * @param metaCallTimeMap + * A container for storing entry and exit timestamps of IMetaStoreClient method + * invocations. + * @return IMetaStoreClient An implementation of IMetaStoreClient. + * @throws MetaException + */ + IMetaStoreClient createMetaStoreClient(HiveConf conf, HiveMetaHookLoader hookLoader, + boolean allowEmbedded, ConcurrentHashMap metaCallTimeMap) throws MetaException; +} diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClientFactory.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClientFactory.java new file mode 100644 index 000000000000..b7bac5b16b5d --- /dev/null +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClientFactory.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hive.ql.metadata; + +import static com.google.common.base.Preconditions.checkNotNull; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.conf.HiveConf.ConfVars; +import org.apache.hadoop.hive.metastore.HiveMetaHookLoader; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.MetaException; + +/** + * Default MetaStoreClientFactory for Hive which produces SessionHiveMetaStoreClient objects. + * + */ +public final class SessionHiveMetaStoreClientFactory implements HiveMetaStoreClientFactory { + + @Override + public IMetaStoreClient createMetaStoreClient(HiveConf conf, HiveMetaHookLoader hookLoader, + boolean allowEmbedded, + ConcurrentHashMap metaCallTimeMap) throws MetaException { + + checkNotNull(conf, "conf cannot be null!"); + checkNotNull(hookLoader, "hookLoader cannot be null!"); + checkNotNull(metaCallTimeMap, "metaCallTimeMap cannot be null!"); + + if (conf.getBoolVar(ConfVars.METASTORE_FASTPATH)) { + return new SessionHiveMetaStoreClient(conf, hookLoader, allowEmbedded); + } else { + return RetryingMetaStoreClient.getProxy(conf, hookLoader, metaCallTimeMap, + SessionHiveMetaStoreClient.class.getName(), allowEmbedded); + } + } +} diff --git a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java index 897e2e20026d..d06e4457f6ae 100755 --- a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hive.ql.metadata; import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_DATABASE_NAME; +import static org.junit.Assert.assertThat; +import static org.hamcrest.CoreMatchers.instanceOf; import java.io.OutputStream; import java.util.ArrayList; @@ -38,7 +40,11 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; +<<<<<<< HEAD import org.apache.hadoop.hive.metastore.MetaStoreEventListener; +======= +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +>>>>>>> 1558b1ad12 (HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf) import org.apache.hadoop.hive.metastore.PartitionDropOptions; import org.apache.hadoop.hive.metastore.Warehouse; import org.apache.hadoop.hive.metastore.api.Database; @@ -890,6 +896,7 @@ public void testHiveRefreshOnConfChange() throws Throwable{ assertTrue(prevHiveObj != newHiveObj); } +<<<<<<< HEAD public void testFireInsertEvent() throws Throwable { Hive hiveDb = Hive.getWithFastCheck(hiveConf, false); String tableName = "test_fire_insert_event"; @@ -953,6 +960,44 @@ private String getFileCheckSum(FileSystem fileSystem, Path p) throws Exception { return ""; } +======= + @Test + public void testLoadingHiveMetaStoreClientFactory() throws Throwable { + String factoryClassName = SessionHiveMetaStoreClientFactory.class.getName(); + HiveConf conf = new HiveConf(); + conf.setVar(ConfVars.METASTORE_CLIENT_FACTORY_CLASS, factoryClassName); + // Make sure we instantiate the embedded version + // so the implementation chosen is SessionHiveMetaStoreClient, not a retryable version of it. + conf.setBoolVar(ConfVars.METASTORE_FASTPATH, true); + // The current object was constructed in setUp() before we got here + // so clean that up so we can inject our own dummy implementation of IMetaStoreClient + Hive.closeCurrent(); + Hive hive = Hive.get(conf); + IMetaStoreClient tmp = hive.getMSC(); + assertNotNull("getMSC() failed.", tmp); + assertThat("Invalid default client implementation created.", tmp, + instanceOf(SessionHiveMetaStoreClient.class)); + } + + @Test + public void testLoadingInvalidHiveMetaStoreClientFactory() throws Throwable { + // Intentionally invalid class + String factoryClassName = String.class.getName(); + HiveConf conf = new HiveConf(); + conf.setVar(HiveConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS, factoryClassName); + // The current object was constructed in setUp() before we got here + // so clean that up so we can inject our own dummy implementation of IMetaStoreClient + Hive.closeCurrent(); + Hive hive = Hive.get(conf); + try { + hive.getMSC(); + fail("getMSC() was expected to throw MetaException."); + } catch (Exception e) { + assertTrue("getMSC() failed, which IS expected.", true); + } + } + +>>>>>>> 1558b1ad12 (HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf) // shamelessly copied from Path in hadoop-2 private static final String SEPARATOR = "/"; private static final char SEPARATOR_CHAR = '/'; From 160c37731925aef44e55649aace6c8a5568529cc Mon Sep 17 00:00:00 2001 From: okumin Date: Tue, 20 Jun 2023 19:57:51 +0900 Subject: [PATCH 2/4] HIVE-12679: Resolve conflict --- .../test/org/apache/hadoop/hive/ql/metadata/TestHive.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java index d06e4457f6ae..6de8d2e331e9 100755 --- a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java @@ -40,11 +40,8 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; -<<<<<<< HEAD -import org.apache.hadoop.hive.metastore.MetaStoreEventListener; -======= import org.apache.hadoop.hive.metastore.IMetaStoreClient; ->>>>>>> 1558b1ad12 (HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf) +import org.apache.hadoop.hive.metastore.MetaStoreEventListener; import org.apache.hadoop.hive.metastore.PartitionDropOptions; import org.apache.hadoop.hive.metastore.Warehouse; import org.apache.hadoop.hive.metastore.api.Database; @@ -896,7 +893,6 @@ public void testHiveRefreshOnConfChange() throws Throwable{ assertTrue(prevHiveObj != newHiveObj); } -<<<<<<< HEAD public void testFireInsertEvent() throws Throwable { Hive hiveDb = Hive.getWithFastCheck(hiveConf, false); String tableName = "test_fire_insert_event"; @@ -960,7 +956,6 @@ private String getFileCheckSum(FileSystem fileSystem, Path p) throws Exception { return ""; } -======= @Test public void testLoadingHiveMetaStoreClientFactory() throws Throwable { String factoryClassName = SessionHiveMetaStoreClientFactory.class.getName(); @@ -997,7 +992,6 @@ public void testLoadingInvalidHiveMetaStoreClientFactory() throws Throwable { } } ->>>>>>> 1558b1ad12 (HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf) // shamelessly copied from Path in hadoop-2 private static final String SEPARATOR = "/"; private static final char SEPARATOR_CHAR = '/'; From ff4b527aaed0e9add7fd02e38d64a1ebad4f5c2d Mon Sep 17 00:00:00 2001 From: okumin Date: Tue, 20 Jun 2023 21:31:31 +0900 Subject: [PATCH 3/4] HIVE-12679: Follow warning of IDE --- .../hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java index 13bd9ec97a99..0a19e4538791 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMetaStoreClientFactory.java @@ -48,7 +48,7 @@ public interface HiveMetaStoreClientFactory { * A container for storing entry and exit timestamps of IMetaStoreClient method * invocations. * @return IMetaStoreClient An implementation of IMetaStoreClient. - * @throws MetaException + * @throws MetaException if this method fails to create IMetaStoreClient */ IMetaStoreClient createMetaStoreClient(HiveConf conf, HiveMetaHookLoader hookLoader, boolean allowEmbedded, ConcurrentHashMap metaCallTimeMap) throws MetaException; From 071d78aec8950f0eda8759728f6df356fd954600 Mon Sep 17 00:00:00 2001 From: okumin Date: Wed, 21 Jun 2023 22:03:21 +0900 Subject: [PATCH 4/4] HIVE-12679: Move the parameter to MetastoreConf --- common/src/java/org/apache/hadoop/hive/conf/HiveConf.java | 3 --- ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java | 5 +++-- ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java | 4 ++-- .../org/apache/hadoop/hive/metastore/conf/MetastoreConf.java | 5 +++++ 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index 886e0b455e3b..f4d1c376d3fc 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -889,9 +889,6 @@ public static enum ConfVars { HADOOPNUMREDUCERS("mapreduce.job.reduces", -1, "", true), // Metastore stuff. Be sure to update HiveConf.metaVars when you add something here! - METASTORE_CLIENT_FACTORY_CLASS("hive.metastore.client.factory.class", - "org.apache.hadoop.hive.ql.metadata.SessionHiveMetaStoreClientFactory", - "The name of the factory class that produces objects implementing the IMetaStoreClient interface."), METASTOREDBTYPE("hive.metastore.db.type", "DERBY", new StringSet("DERBY", "ORACLE", "MYSQL", "MSSQL", "POSTGRES"), "Type of database used by the metastore. Information schema & JDBCStorageHandler depend on it."), /** diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java index 3395e312f72e..b05e250d351c 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java @@ -114,6 +114,7 @@ import org.apache.hadoop.hive.metastore.api.SourceTable; import org.apache.hadoop.hive.metastore.api.UpdateTransactionalStatsRequest; import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.ql.ddl.table.AlterTableType; import org.apache.hadoop.hive.ql.io.HdfsUtils; import org.apache.hadoop.hive.metastore.HiveMetaException; @@ -123,7 +124,6 @@ import org.apache.hadoop.hive.metastore.HiveMetaStoreUtils; import org.apache.hadoop.hive.metastore.IMetaStoreClient; import org.apache.hadoop.hive.metastore.PartitionDropOptions; -import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient; import org.apache.hadoop.hive.metastore.SynchronizedMetaStoreClient; import org.apache.hadoop.hive.metastore.TableType; import org.apache.hadoop.hive.metastore.Warehouse; @@ -5707,7 +5707,8 @@ public HiveMetaHook getHook( private static HiveMetaStoreClientFactory createMetaStoreClientFactory(HiveConf conf) throws MetaException { - String metaStoreClientFactoryClassName = conf.getVar(HiveConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS); + String metaStoreClientFactoryClassName = MetastoreConf.getVar(conf, + MetastoreConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS); try { Class factoryClass = diff --git a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java index 6de8d2e331e9..dfbb01ce45b3 100755 --- a/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java @@ -960,7 +960,7 @@ private String getFileCheckSum(FileSystem fileSystem, Path p) throws Exception { public void testLoadingHiveMetaStoreClientFactory() throws Throwable { String factoryClassName = SessionHiveMetaStoreClientFactory.class.getName(); HiveConf conf = new HiveConf(); - conf.setVar(ConfVars.METASTORE_CLIENT_FACTORY_CLASS, factoryClassName); + MetastoreConf.setVar(conf, MetastoreConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS, factoryClassName); // Make sure we instantiate the embedded version // so the implementation chosen is SessionHiveMetaStoreClient, not a retryable version of it. conf.setBoolVar(ConfVars.METASTORE_FASTPATH, true); @@ -979,7 +979,7 @@ public void testLoadingInvalidHiveMetaStoreClientFactory() throws Throwable { // Intentionally invalid class String factoryClassName = String.class.getName(); HiveConf conf = new HiveConf(); - conf.setVar(HiveConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS, factoryClassName); + MetastoreConf.setVar(conf, MetastoreConf.ConfVars.METASTORE_CLIENT_FACTORY_CLASS, factoryClassName); // The current object was constructed in setUp() before we got here // so clean that up so we can inject our own dummy implementation of IMetaStoreClient Hive.closeCurrent(); diff --git a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java index 62ea1a63eaec..2de836ac5baf 100644 --- a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java +++ b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java @@ -185,6 +185,7 @@ public String toString() { ConfVars.KERBEROS_PRINCIPAL, ConfVars.USE_THRIFT_SASL, ConfVars.METASTORE_CLIENT_AUTH_MODE, + ConfVars.METASTORE_CLIENT_FACTORY_CLASS, ConfVars.METASTORE_CLIENT_PLAIN_USERNAME, ConfVars.CACHE_PINOBJTYPES, ConfVars.CONNECTION_POOLING_TYPE, @@ -1609,6 +1610,10 @@ public enum ConfVars { " and password. Any other value is ignored right now but may be used later." + "If JWT- Supported only in HTTP transport mode. If set, HMS Client will pick the value of JWT from " + "environment variable HMS_JWT and set it in Authorization header in http request"), + METASTORE_CLIENT_FACTORY_CLASS("metastore.client.factory.class", + "hive.metastore.client.factory.class", + "org.apache.hadoop.hive.ql.metadata.SessionHiveMetaStoreClientFactory", + "The name of the factory class that produces objects implementing the IMetaStoreClient interface."), METASTORE_CLIENT_ADDITIONAL_HEADERS("metastore.client.http.additional.headers", "hive.metastore.client.http.additional.headers", "", "Comma separated list of headers which are passed to the metastore service in the http headers"),