From 28bf6020afef3667a659bcf2de7f22d3ffb719c1 Mon Sep 17 00:00:00 2001 From: wecharyu Date: Sat, 25 Mar 2023 01:37:13 +0800 Subject: [PATCH 1/3] HIVE-27172: Add the HMS client connection timeout config --- .../hive/metastore/HiveMetaStoreClient.java | 6 +- .../hive/metastore/conf/MetastoreConf.java | 2 + .../hive/metastore/utils/SecurityUtils.java | 7 ++- .../HiveMetaStoreClientPreCatalog.java | 7 ++- .../metastore/TestHiveMetaStoreTimeout.java | 55 +++++++++++++------ .../hive/metastore/tools/HMSClient.java | 6 +- 6 files changed, 57 insertions(+), 26 deletions(-) diff --git a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java index e229ab1a0b47..fae80536b6e1 100644 --- a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java +++ b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java @@ -738,6 +738,8 @@ private TTransport createBinaryClient(URI store, boolean useSSL) throws TTranspo try { int clientSocketTimeout = (int) MetastoreConf.getTimeVar(conf, ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS); + int connectionTimeout = (int) MetastoreConf.getTimeVar(conf, + ConfVars.CLIENT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); if (useSSL) { String trustStorePath = MetastoreConf.getVar(conf, ConfVars.SSL_TRUSTSTORE_PATH).trim(); if (trustStorePath.isEmpty()) { @@ -751,10 +753,10 @@ private TTransport createBinaryClient(URI store, boolean useSSL) throws TTranspo String trustStoreAlgorithm = MetastoreConf.getVar(conf, ConfVars.SSL_TRUSTMANAGERFACTORY_ALGORITHM).trim(); binaryTransport = SecurityUtils.getSSLSocket(store.getHost(), store.getPort(), clientSocketTimeout, - trustStorePath, trustStorePassword, trustStoreType, trustStoreAlgorithm); + connectionTimeout, trustStorePath, trustStorePassword, trustStoreType, trustStoreAlgorithm); } else { binaryTransport = new TSocket(new TConfiguration(), store.getHost(), store.getPort(), - clientSocketTimeout); + clientSocketTimeout, connectionTimeout); } binaryTransport = createAuthBinaryTransport(store, binaryTransport); } catch (Exception e) { 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 f3f8be618ece..1b2379fa2473 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 @@ -402,6 +402,8 @@ public enum ConfVars { "has an infinite lifetime."), CLIENT_SOCKET_TIMEOUT("metastore.client.socket.timeout", "hive.metastore.client.socket.timeout", 600, TimeUnit.SECONDS, "MetaStore Client socket timeout in seconds"), + CLIENT_CONNECTION_TIMEOUT("metastore.client.connection.timeout", "hive.metastore.client.connection.timeout", 10, + TimeUnit.SECONDS, "MetaStore Client connection timeout in seconds"), COMPACTOR_HISTORY_RETENTION_DID_NOT_INITIATE("metastore.compactor.history.retention.did.not.initiate", "hive.compactor.history.retention.did.not.initiate", 2, new RangeValidator(0, 100), "Determines how many compaction records in state " + diff --git a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SecurityUtils.java b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SecurityUtils.java index cb5b170808aa..ec593ae7a366 100644 --- a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SecurityUtils.java +++ b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SecurityUtils.java @@ -270,7 +270,7 @@ public static TServerSocket getServerSSLSocket(String hiveHost, int portNum, Str return thriftServerSocket; } - public static TTransport getSSLSocket(String host, int port, int loginTimeout, + public static TTransport getSSLSocket(String host, int port, int socketTimeout, int connectionTimeout, String trustStorePath, String trustStorePassWord, String trustStoreType, String trustStoreAlgorithm) throws TTransportException { TSSLTransportFactory.TSSLTransportParameters params = @@ -282,8 +282,9 @@ public static TTransport getSSLSocket(String host, int port, int loginTimeout, tStoreAlgorithm, tStoreType); params.requireClientAuth(true); // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT and - // SSLContext created with the given params - TSocket tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout, params); + // connection timeout and SSLContext created with the given params + TSocket tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, socketTimeout, params); + tSSLSocket.setConnectTimeout(connectionTimeout); return getSSLSocketWithHttps(tSSLSocket); } diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java index cda432fd6685..96695ee1eaaf 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java @@ -443,6 +443,8 @@ private void open() throws MetaException { boolean useCompactProtocol = MetastoreConf.getBoolVar(conf, ConfVars.USE_THRIFT_COMPACT_PROTOCOL); int clientSocketTimeout = (int) MetastoreConf.getTimeVar(conf, ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS); + int connectionTimeout = (int) MetastoreConf.getTimeVar(conf, + ConfVars.CLIENT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); for (int attempt = 0; !isConnected && attempt < retries; ++attempt) { for (URI store : metastoreUris) { @@ -466,7 +468,7 @@ private void open() throws MetaException { // Create an SSL socket and connect transport = SecurityUtils.getSSLSocket(store.getHost(), store.getPort(), clientSocketTimeout, - trustStorePath, trustStorePassword, trustStoreType, trustStoreAlgorithm ); + connectionTimeout, trustStorePath, trustStorePassword, trustStoreType, trustStoreAlgorithm); LOG.info("Opened an SSL connection to metastore, current connections: " + connCount.incrementAndGet()); } catch(IOException e) { throw new IllegalArgumentException(e); @@ -476,7 +478,8 @@ private void open() throws MetaException { } } else { try { - transport = new TSocket(new TConfiguration(),store.getHost(), store.getPort(), clientSocketTimeout); + transport = new TSocket(new TConfiguration(), + store.getHost(), store.getPort(), clientSocketTimeout, connectionTimeout); } catch (TTransportException e) { tte = e; throw new MetaException(e.toString()); diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java index b4c857272555..ba63730dec8d 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java @@ -18,6 +18,8 @@ package org.apache.hadoop.hive.metastore; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import org.apache.hadoop.conf.Configuration; @@ -28,9 +30,10 @@ import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; import org.apache.hadoop.util.StringUtils; -import org.junit.AfterClass; +import org.apache.thrift.transport.TTransportException; +import org.junit.After; import org.junit.Assert; -import org.junit.BeforeClass; +import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -43,9 +46,10 @@ public class TestHiveMetaStoreTimeout { protected static HiveMetaStoreClient client; protected static Configuration conf; protected static Warehouse warehouse; + protected static int port; - @BeforeClass - public static void setUp() throws Exception { + @Before + public void setUp() throws Exception { HMSHandler.testTimeoutEnabled = true; conf = MetastoreConf.newMetastoreConf(); MetastoreConf.setClass(conf, ConfVars.EXPRESSION_PROXY_CLASS, @@ -54,11 +58,14 @@ public static void setUp() throws Exception { TimeUnit.MILLISECONDS); MetaStoreTestUtils.setConfForStandloneMode(conf); warehouse = new Warehouse(conf); + port = MetaStoreTestUtils.startMetaStoreWithRetry(conf); + MetastoreConf.setVar(conf, ConfVars.THRIFT_URIS, "thrift://localhost:" + port); + MetastoreConf.setBoolVar(conf, ConfVars.EXECUTE_SET_UGI, false); client = new HiveMetaStoreClient(conf); } - @AfterClass - public static void tearDown() throws Exception { + @After + public void tearDown() throws Exception { HMSHandler.testTimeoutEnabled = false; try { client.close(); @@ -96,9 +103,8 @@ public void testTimeout() throws Exception { try { client.createDatabase(db); Assert.fail("should throw timeout exception."); - } catch (MetaException e) { - Assert.assertTrue("unexpected MetaException", e.getMessage().contains("Timeout when " + - "executing method: create_database")); + } catch (TTransportException e) { + Assert.assertTrue("unexpected Exception", e.getMessage().contains("Read timed out")); } // restore @@ -117,7 +123,7 @@ public void testResetTimeout() throws Exception { .build(conf); try { client.createDatabase(db); - } catch (MetaException e) { + } catch (Exception e) { Assert.fail("should not throw timeout exception: " + e.getMessage()); } client.dropDatabase(dbName, true, true); @@ -130,13 +136,28 @@ public void testResetTimeout() throws Exception { try { client.createDatabase(db); Assert.fail("should throw timeout exception."); - } catch (MetaException e) { - Assert.assertTrue("unexpected MetaException", e.getMessage().contains("Timeout when " + - "executing method: create_database")); + } catch (TTransportException e) { + Assert.assertTrue("unexpected Exception", e.getMessage().contains("Read timed out")); } + } - // restore - client.dropDatabase(dbName, true, true); - client.setMetaConf(ConfVars.CLIENT_SOCKET_TIMEOUT.getVarname(), "10s"); + @Test + public void testConnectionTimeout() throws Exception { + Configuration newConf = new Configuration(conf); + MetastoreConf.setTimeVar(newConf, ConfVars.CLIENT_CONNECTION_TIMEOUT, 1000, + TimeUnit.MILLISECONDS); + // fake host to mock connection time out + MetastoreConf.setVar(newConf, ConfVars.THRIFT_URIS, "thrift://1.1.1.1:" + port); + MetastoreConf.setLongVar(newConf, ConfVars.THRIFT_CONNECTION_RETRIES, 1); + + Future future = Executors.newSingleThreadExecutor().submit(() -> { + try(HiveMetaStoreClient c = new HiveMetaStoreClient(newConf)) { + Assert.fail("should throw connection timeout exception."); + } catch (MetaException e) { + Assert.assertTrue("unexpected Exception", e.getMessage().contains("connect timed out")); + } + return null; + }); + future.get(5, TimeUnit.SECONDS); } -} \ No newline at end of file +} diff --git a/standalone-metastore/metastore-tools/tools-common/src/main/java/org/apache/hadoop/hive/metastore/tools/HMSClient.java b/standalone-metastore/metastore-tools/tools-common/src/main/java/org/apache/hadoop/hive/metastore/tools/HMSClient.java index cd4032be4256..463e83e2f89b 100644 --- a/standalone-metastore/metastore-tools/tools-common/src/main/java/org/apache/hadoop/hive/metastore/tools/HMSClient.java +++ b/standalone-metastore/metastore-tools/tools-common/src/main/java/org/apache/hadoop/hive/metastore/tools/HMSClient.java @@ -398,6 +398,8 @@ private TTransport open(Configuration conf, @NotNull URI uri) throws boolean useCompactProtocol = MetastoreConf.getBoolVar(conf, MetastoreConf.ConfVars.USE_THRIFT_COMPACT_PROTOCOL); int clientSocketTimeout = (int) MetastoreConf.getTimeVar(conf, MetastoreConf.ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS); + int connectionTimeout = (int) MetastoreConf.getTimeVar(conf, + MetastoreConf.ConfVars.CLIENT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); LOG.debug("Connecting to {}, framedTransport = {}", uri, useFramedTransport); @@ -406,7 +408,7 @@ private TTransport open(Configuration conf, @NotNull URI uri) throws // Sasl/SSL code is copied from HiveMetastoreCLient if (!useSSL) { - transport = new TSocket(new TConfiguration(),host, port, clientSocketTimeout); + transport = new TSocket(new TConfiguration(),host, port, clientSocketTimeout, connectionTimeout); } else { String trustStorePath = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.SSL_TRUSTSTORE_PATH).trim(); if (trustStorePath.isEmpty()) { @@ -421,7 +423,7 @@ private TTransport open(Configuration conf, @NotNull URI uri) throws MetastoreConf.getVar(conf, MetastoreConf.ConfVars.SSL_TRUSTMANAGERFACTORY_ALGORITHM).trim(); // Create an SSL socket and connect - transport = SecurityUtils.getSSLSocket(host, port, clientSocketTimeout, + transport = SecurityUtils.getSSLSocket(host, port, clientSocketTimeout, connectionTimeout, trustStorePath, trustStorePassword, trustStoreType, trustStoreAlgorithm); LOG.info("Opened an SSL connection to metastore, current connections"); } From f554c283b9190e6c92df3ed81884111a9354b291 Mon Sep 17 00:00:00 2001 From: wecharyu Date: Wed, 26 Apr 2023 00:58:57 +0800 Subject: [PATCH 2/3] change default connection timeout value to match socket timeout --- .../org/apache/hadoop/hive/metastore/conf/MetastoreConf.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1b2379fa2473..a2d74672d1dc 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 @@ -402,7 +402,7 @@ public enum ConfVars { "has an infinite lifetime."), CLIENT_SOCKET_TIMEOUT("metastore.client.socket.timeout", "hive.metastore.client.socket.timeout", 600, TimeUnit.SECONDS, "MetaStore Client socket timeout in seconds"), - CLIENT_CONNECTION_TIMEOUT("metastore.client.connection.timeout", "hive.metastore.client.connection.timeout", 10, + CLIENT_CONNECTION_TIMEOUT("metastore.client.connection.timeout", "hive.metastore.client.connection.timeout", 600, TimeUnit.SECONDS, "MetaStore Client connection timeout in seconds"), COMPACTOR_HISTORY_RETENTION_DID_NOT_INITIATE("metastore.compactor.history.retention.did.not.initiate", "hive.compactor.history.retention.did.not.initiate", 2, From 7ef0e052574cebdf3ebd81d580e531b8af24baf0 Mon Sep 17 00:00:00 2001 From: wecharyu Date: Sat, 29 Apr 2023 19:29:27 +0800 Subject: [PATCH 3/3] make the start of MetaStore Server execute before test class. --- .../metastore/TestHiveMetaStoreTimeout.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java index ba63730dec8d..a07225733442 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java @@ -29,11 +29,12 @@ import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; -import org.apache.hadoop.util.StringUtils; import org.apache.thrift.transport.TTransportException; import org.junit.After; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -48,8 +49,8 @@ public class TestHiveMetaStoreTimeout { protected static Warehouse warehouse; protected static int port; - @Before - public void setUp() throws Exception { + @BeforeClass + public static void startMetaStoreServer() throws Exception { HMSHandler.testTimeoutEnabled = true; conf = MetastoreConf.newMetastoreConf(); MetastoreConf.setClass(conf, ConfVars.EXPRESSION_PROXY_CLASS, @@ -61,19 +62,22 @@ public void setUp() throws Exception { port = MetaStoreTestUtils.startMetaStoreWithRetry(conf); MetastoreConf.setVar(conf, ConfVars.THRIFT_URIS, "thrift://localhost:" + port); MetastoreConf.setBoolVar(conf, ConfVars.EXECUTE_SET_UGI, false); + } + + @AfterClass + public static void tearDown() { + HMSHandler.testTimeoutEnabled = false; + } + + @Before + public void setup() throws MetaException { client = new HiveMetaStoreClient(conf); } @After - public void tearDown() throws Exception { - HMSHandler.testTimeoutEnabled = false; - try { - client.close(); - } catch (Throwable e) { - System.err.println("Unable to close metastore"); - System.err.println(StringUtils.stringifyException(e)); - throw e; - } + public void cleanup() { + client.close(); + client = null; } @Test