You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by pv...@apache.org on 2020/06/08 07:35:53 UTC

[hive] branch master updated: HIVE-23242: Fix flaky tests testHouseKeepingThreadExistence in TestMetastoreHousekeepingLeaderEmptyConfig and TestMetastoreHousekeepingLeader (Peter Varga via Peter Vary)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 388d37c  HIVE-23242: Fix flaky tests testHouseKeepingThreadExistence in TestMetastoreHousekeepingLeaderEmptyConfig and TestMetastoreHousekeepingLeader (Peter Varga via Peter Vary)
388d37c is described below

commit 388d37cca082e9069197f7db4b6e50de9d8595e0
Author: Peter Varga <pv...@cloudera.com>
AuthorDate: Mon Jun 8 09:35:39 2020 +0200

    HIVE-23242: Fix flaky tests testHouseKeepingThreadExistence in TestMetastoreHousekeepingLeaderEmptyConfig and TestMetastoreHousekeepingLeader (Peter Varga via Peter Vary)
---
 .../MetastoreHousekeepingLeaderTestBase.java       |  20 ++---
 .../MetastoreTaskThreadAlwaysTestImpl.java         |   2 +-
 .../RemoteMetastoreTaskThreadTestImpl1.java        |   2 +-
 .../RemoteMetastoreTaskThreadTestImpl2.java        |   2 +-
 .../metastore/TestMetastoreHousekeepingLeader.java |   5 +-
 ...TestMetastoreHousekeepingLeaderEmptyConfig.java |   5 +-
 .../TestMetastoreHousekeepingNonLeader.java        |   2 +-
 .../hadoop/hive/metastore/HiveMetaStore.java       |  44 ++++-----
 .../hadoop/hive/metastore/MetaStoreTestUtils.java  | 100 +++++++++------------
 9 files changed, 77 insertions(+), 105 deletions(-)

diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreHousekeepingLeaderTestBase.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreHousekeepingLeaderTestBase.java
index a39a9c8..d165bb2 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreHousekeepingLeaderTestBase.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreHousekeepingLeaderTestBase.java
@@ -47,15 +47,12 @@ class MetastoreHousekeepingLeaderTestBase {
   private static boolean isServerStarted = false;
   private static int port;
   private static MiniDFSCluster miniDFS;
-  // How long should we wait for the housekeeping threads to start in ms.
-  private static final long SLEEP_INTERVAL_FOR_THREADS_TO_START = 10000;
-  // Threads using ThreadPool will start after the configured interval. So, start them some time
-  // before we check the existence of threads.
-  private static final long REMOTE_TASKS_INTERVAL = SLEEP_INTERVAL_FOR_THREADS_TO_START - 3000;
+  // Threads using ThreadPool will start after the configured interval. Start them right away
+  private static final long REMOTE_TASKS_INTERVAL = 1;
   static final String METASTORE_THREAD_TASK_FREQ_CONF = "metastore.leader.test.task.freq";
 
   static Map<String, Boolean> threadNames = new HashMap<>();
-  static Map<Class, Boolean> threadClasses = new HashMap<>();
+  static Map<Class<? extends Thread>, Boolean> threadClasses = new HashMap<>();
 
   void internalSetup(final String leaderHostName) throws Exception {
     MetaStoreTestUtils.setConfForStandloneMode(conf);
@@ -70,9 +67,9 @@ class MetastoreHousekeepingLeaderTestBase {
       Assert.assertNotNull("Unable to connect to the MetaStore server", client);
       return;
     }
-
+    // Start the metastore and wait for the background threads
     port = MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(),
-            conf, true);
+            conf, false, false,  true, true);
     System.out.println("Starting MetaStore Server on port " + port);
     isServerStarted = true;
 
@@ -136,7 +133,7 @@ class MetastoreHousekeepingLeaderTestBase {
     return 2;
   }
 
-  private long addAlwaysTasksConfigs() throws Exception {
+  private long addAlwaysTasksConfigs() {
     String alwaysTaskClassPaths = MetastoreTaskThreadAlwaysTestImpl.class.getCanonicalName();
     MetastoreConf.setVar(conf, ConfVars.TASK_THREADS_ALWAYS, alwaysTaskClassPaths);
     threadNames.put(MetastoreTaskThreadAlwaysTestImpl.TASK_NAME, false);
@@ -156,10 +153,7 @@ class MetastoreHousekeepingLeaderTestBase {
   }
 
   void searchHousekeepingThreads() throws Exception {
-    // Client has been created so the metastore has started serving. Sleep for few seconds for
-    // the housekeeping threads to start.
-    Thread.sleep(SLEEP_INTERVAL_FOR_THREADS_TO_START);
-
+    // Client has been created so the metastore has started serving and started the background threads
     LOG.info(getAllThreadsAsString());
 
     // Check if all the housekeeping threads have been started.
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreTaskThreadAlwaysTestImpl.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreTaskThreadAlwaysTestImpl.java
index 4cd2c58..28d9f48 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreTaskThreadAlwaysTestImpl.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreTaskThreadAlwaysTestImpl.java
@@ -52,7 +52,7 @@ public class MetastoreTaskThreadAlwaysTestImpl implements MetastoreTaskThread {
     LOG.info("Name of thread " + Thread.currentThread().getName() + " changed to " + TASK_NAME);
     Thread.currentThread().setName(TASK_NAME);
     try {
-      Thread.sleep(runFrequency(TimeUnit.MILLISECONDS));
+      Thread.sleep(10000);
     } catch (InterruptedException ie) {
       LOG.error("Task " + TASK_NAME + " interrupted: " + ie.getMessage(), ie);
     }
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl1.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl1.java
index c590b6a..f7844a4 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl1.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl1.java
@@ -52,7 +52,7 @@ public class RemoteMetastoreTaskThreadTestImpl1 implements MetastoreTaskThread {
     LOG.info("Name of thread " + Thread.currentThread().getName() + " changed to " + TASK_NAME);
     Thread.currentThread().setName(TASK_NAME);
     try {
-      Thread.sleep(runFrequency(TimeUnit.MILLISECONDS));
+      Thread.sleep(10000);
     } catch (InterruptedException ie) {
       LOG.error("Task " + TASK_NAME + " interrupted: " + ie.getMessage(), ie);
     }
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl2.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl2.java
index 5b50f66..9612d2f 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl2.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/RemoteMetastoreTaskThreadTestImpl2.java
@@ -52,7 +52,7 @@ public class RemoteMetastoreTaskThreadTestImpl2 implements MetastoreTaskThread {
     LOG.info("Name of thread " + Thread.currentThread().getName() + " changed to " + TASK_NAME);
     Thread.currentThread().setName(TASK_NAME);
     try {
-      Thread.sleep(runFrequency(TimeUnit.MILLISECONDS));
+      Thread.sleep(10000);
     } catch (InterruptedException ie) {
       LOG.error("Task " + TASK_NAME + " interrupted: " + ie.getMessage(), ie);
     }
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeader.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeader.java
index 03a8161..320de8d 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeader.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeader.java
@@ -20,7 +20,6 @@ package org.apache.hadoop.hive.metastore;
 
 import org.junit.Assert;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,8 +37,6 @@ public class TestMetastoreHousekeepingLeader extends MetastoreHousekeepingLeader
     internalSetup("localhost");
   }
 
-  @Ignore("HIVE-23221 Ignore flaky test testHouseKeepingThreadExistence in TestMetastoreHousekeepingLeaderEmptyConfig" +
-      " and TestMetastoreHousekeepingLeader")
   @Test
   public void testHouseKeepingThreadExistence() throws Exception {
     searchHousekeepingThreads();
@@ -52,7 +49,7 @@ public class TestMetastoreHousekeepingLeader extends MetastoreHousekeepingLeader
       Assert.assertTrue("No thread with name " + entry.getKey() + " found.", entry.getValue());
     }
 
-    for (Map.Entry<Class, Boolean> entry : threadClasses.entrySet()) {
+    for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
       if (entry.getValue()) {
         LOG.info("Found thread for " + entry.getKey().getSimpleName());
       }
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeaderEmptyConfig.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeaderEmptyConfig.java
index 75ea637..382d19b 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeaderEmptyConfig.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeaderEmptyConfig.java
@@ -20,7 +20,6 @@ package org.apache.hadoop.hive.metastore;
 
 import org.junit.Assert;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,8 +39,6 @@ public class TestMetastoreHousekeepingLeaderEmptyConfig extends MetastoreHouseke
     internalSetup("");
   }
 
-  @Ignore("HIVE-23221 Ignore flaky test testHouseKeepingThreadExistence in TestMetastoreHousekeepingLeaderEmptyConfig" +
-      " and TestMetastoreHousekeepingLeader")
   @Test
   public void testHouseKeepingThreadExistence() throws Exception {
     searchHousekeepingThreads();
@@ -54,7 +51,7 @@ public class TestMetastoreHousekeepingLeaderEmptyConfig extends MetastoreHouseke
       Assert.assertTrue("No thread with name " + entry.getKey() + " found.", entry.getValue());
     }
 
-    for (Map.Entry<Class, Boolean> entry : threadClasses.entrySet()) {
+    for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
       if (entry.getValue()) {
         LOG.info("Found thread for " + entry.getKey().getSimpleName());
       }
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingNonLeader.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingNonLeader.java
index 0341d3c..025f08b 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingNonLeader.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingNonLeader.java
@@ -52,7 +52,7 @@ public class TestMetastoreHousekeepingNonLeader extends MetastoreHousekeepingLea
       Assert.assertFalse("Thread with name " + entry.getKey() + " found.", entry.getValue());
     }
 
-    for (Map.Entry<Class, Boolean> entry : threadClasses.entrySet()) {
+    for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
       // A non-leader HMS will still run the configured number of Compaction worker threads.
       if (entry.getKey() == Worker.class) {
         if (entry.getValue()) {
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index f4f5458..7038aec 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -23,7 +23,6 @@ import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_DATABASE_COMMEN
 import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_DATABASE_NAME;
 import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_CATALOG_NAME;
 import static org.apache.hadoop.hive.metastore.Warehouse.getCatalogQualifiedTableName;
-import static org.apache.hadoop.hive.metastore.api.FireEventRequestData._Fields.INSERT_DATA;
 import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog;
 import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.parseDbName;
 import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.CAT_NAME;
@@ -89,7 +88,6 @@ import org.apache.hadoop.hive.common.ZooKeeperHiveHelper;
 import org.apache.hadoop.hive.common.ZKDeRegisterWatcher;
 import org.apache.hadoop.hive.common.repl.ReplConst;
 import org.apache.hadoop.hive.metastore.api.*;
-import org.apache.hadoop.hive.metastore.api.FireEventRequestData._Fields;
 import org.apache.hadoop.hive.metastore.events.AddForeignKeyEvent;
 import org.apache.hadoop.hive.metastore.events.AcidWriteEvent;
 import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
@@ -10201,11 +10199,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         }
       }
 
-      Lock startLock = new ReentrantLock();
-      Condition startCondition = startLock.newCondition();
-      AtomicBoolean startedServing = new AtomicBoolean();
-      startMetaStore(cli.getPort(), HadoopThriftAuthBridge.getBridge(), conf, startLock,
-          startCondition, startedServing);
+      startMetaStore(cli.getPort(), HadoopThriftAuthBridge.getBridge(), conf, true, null);
     } catch (Throwable t) {
       // Catch the exception, log it and rethrow it.
       HMSHandler.LOG
@@ -10223,7 +10217,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
    */
   public static void startMetaStore(int port, HadoopThriftAuthBridge bridge)
       throws Throwable {
-    startMetaStore(port, bridge, MetastoreConf.newMetastoreConf(), null, null, null);
+    startMetaStore(port, bridge, MetastoreConf.newMetastoreConf(), false, null);
   }
 
   /**
@@ -10235,21 +10229,23 @@ public class HiveMetaStore extends ThriftHiveMetastore {
    */
   public static void startMetaStore(int port, HadoopThriftAuthBridge bridge,
                                     Configuration conf) throws Throwable {
-    startMetaStore(port, bridge, conf, null, null, null);
+    startMetaStore(port, bridge, conf, false, null);
   }
 
   /**
-   * Start Metastore based on a passed {@link HadoopThriftAuthBridge}
+   * Start Metastore based on a passed {@link HadoopThriftAuthBridge}.
    *
-   * @param port
+   * @param port The port on which the Thrift server will start to serve
    * @param bridge
-   * @param conf
-   *          configuration overrides
+   * @param conf Configuration overrides
+   * @param startMetaStoreThreads Start the background threads (initiator, cleaner, statsupdater, etc.)
+   * @param startedBackgroundThreads If startMetaStoreThreads is true, this AtomicBoolean will be switched to true,
+   *  when all of the background threads are scheduled. Useful for testing purposes to wait
+   *  until the MetaStore is fully initialized.
    * @throws Throwable
    */
   public static void startMetaStore(int port, HadoopThriftAuthBridge bridge,
-      Configuration conf, Lock startLock, Condition startCondition,
-      AtomicBoolean startedServing) throws Throwable {
+      Configuration conf, boolean startMetaStoreThreads, AtomicBoolean startedBackgroundThreads) throws Throwable {
     isMetaStoreRemote = true;
     // Server will create new threads up to max as necessary. After an idle
     // period, it will destroy threads to keep the number of threads in the
@@ -10382,10 +10378,13 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     boolean directSqlEnabled = MetastoreConf.getBoolVar(conf, ConfVars.TRY_DIRECT_SQL);
     HMSHandler.LOG.info("Direct SQL optimization = {}",  directSqlEnabled);
 
-    if (startLock != null) {
-      startMetaStoreThreads(conf, startLock, startCondition, startedServing,
-                isMetastoreHousekeepingLeader(conf, getServerHostName()));
-      signalOtherThreadsToStart(tServer, startLock, startCondition, startedServing);
+    if (startMetaStoreThreads) {
+      Lock metaStoreThreadsLock = new ReentrantLock();
+      Condition startCondition = metaStoreThreadsLock.newCondition();
+      AtomicBoolean startedServing = new AtomicBoolean();
+      startMetaStoreThreads(conf, metaStoreThreadsLock, startCondition, startedServing,
+                isMetastoreHousekeepingLeader(conf, getServerHostName()), startedBackgroundThreads);
+      signalOtherThreadsToStart(tServer, metaStoreThreadsLock, startCondition, startedServing);
     }
 
     // If dynamic service discovery through ZooKeeper is enabled, add this server to the ZooKeeper.
@@ -10528,8 +10527,8 @@ public class HiveMetaStore extends ThriftHiveMetastore {
    *                 started only in a leader HMS.
    */
   private static void startMetaStoreThreads(final Configuration conf, final Lock startLock,
-                                            final Condition startCondition, final
-                                            AtomicBoolean startedServing, boolean isLeader) {
+      final Condition startCondition, final AtomicBoolean startedServing, boolean isLeader,
+      final AtomicBoolean startedBackGroundThreads) {
     // A thread is spun up to start these other threads.  That's because we can't start them
     // until after the TServer has started, but once TServer.serve is called we aren't given back
     // control.
@@ -10583,6 +10582,9 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         if (isLeader) {
           ReplChangeManager.scheduleCMClearer(conf);
         }
+        if (startedBackGroundThreads != null) {
+          startedBackGroundThreads.set(true);
+        }
       }
     };
     t.setDaemon(true);
diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/MetaStoreTestUtils.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/MetaStoreTestUtils.java
index 0e2c35a..915586e 100644
--- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/MetaStoreTestUtils.java
+++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/MetaStoreTestUtils.java
@@ -30,9 +30,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.locks.Condition;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
@@ -66,45 +63,37 @@ public class MetaStoreTestUtils {
    * @param port The port to start on
    * @param bridge The bridge to use
    * @param conf The configuration to use
-   * @throws Exception
+   * @param withHouseKeepingThreads Start the housekeeping background threads in the metastore
+   * @param waitForHouseKeepers Wait until the background threads are scheduled
+   * @throws Exception Timeout after 60 seconds
    */
-  public static void startMetaStore(final int port,
-      final HadoopThriftAuthBridge bridge, Configuration conf, boolean withHouseKeepingThreads)
-      throws Exception{
+  public static void startMetaStore(final int port, final HadoopThriftAuthBridge bridge, Configuration conf,
+      boolean withHouseKeepingThreads, boolean waitForHouseKeepers) throws Exception {
     if (conf == null) {
       conf = MetastoreConf.newMetastoreConf();
     }
     final Configuration finalConf = conf;
-    Thread thread = new Thread(new Runnable() {
-      @Override
-      public void run() {
-        try {
-          Lock startLock = null;
-          Condition startCondition = null;
-          AtomicBoolean startedServing = null;
-          if (withHouseKeepingThreads) {
-            startLock = new ReentrantLock();
-            startCondition = startLock.newCondition();
-            startedServing = new AtomicBoolean();
-          }
-          HiveMetaStore.startMetaStore(port, bridge, finalConf, startLock, startCondition,
-                                       startedServing);
-        } catch (Throwable e) {
-          LOG.error("Metastore Thrift Server threw an exception...", e);
-        }
+    final AtomicBoolean startedBackgroundThreads =
+        (withHouseKeepingThreads && waitForHouseKeepers) ? new AtomicBoolean() : null;
+    Thread thread = new Thread(() -> {
+      try {
+        HiveMetaStore.startMetaStore(port, bridge, finalConf, withHouseKeepingThreads, startedBackgroundThreads);
+      } catch (Throwable e) {
+        LOG.error("Metastore Thrift Server threw an exception...", e);
       }
     }, "MetaStoreThread-" + port);
     thread.setDaemon(true);
     thread.start();
     map.put(port,thread);
     String msHost = MetastoreConf.getVar(conf, ConfVars.THRIFT_BIND_HOST);
-    MetaStoreTestUtils.loopUntilHMSReady(msHost, port);
+    MetaStoreTestUtils.loopUntilHMSReady(msHost, port, startedBackgroundThreads);
     String serviceDiscMode = MetastoreConf.getVar(conf, ConfVars.THRIFT_SERVICE_DISCOVERY_MODE);
     if (serviceDiscMode != null && serviceDiscMode.equalsIgnoreCase("zookeeper")) {
       MetaStoreTestUtils.loopUntilZKReady(conf, msHost, port);
     }
   }
 
+  @SuppressWarnings("deprecation")
   public static void close(final int port){
     Thread thread = map.get(port);
     if(thread != null){
@@ -124,7 +113,7 @@ public class MetaStoreTestUtils {
                                             boolean keepWarehousePath)
       throws Exception {
     return MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(), conf,
-        keepJdbcUri, keepWarehousePath, false);
+        keepJdbcUri, keepWarehousePath, false, false);
   }
 
   public static int startMetaStoreWithRetry() throws Exception {
@@ -134,13 +123,13 @@ public class MetaStoreTestUtils {
 
   public static int startMetaStoreWithRetry(HadoopThriftAuthBridge bridge,
                                             Configuration conf) throws Exception {
-    return MetaStoreTestUtils.startMetaStoreWithRetry(bridge, conf, false, false, false);
+    return MetaStoreTestUtils.startMetaStoreWithRetry(bridge, conf, false, false, false, false);
   }
 
   public static int startMetaStoreWithRetry(HadoopThriftAuthBridge bridge,
                                             Configuration conf, boolean withHouseKeepingThreads)
           throws Exception {
-    return MetaStoreTestUtils.startMetaStoreWithRetry(bridge, conf, false, false, withHouseKeepingThreads);
+    return MetaStoreTestUtils.startMetaStoreWithRetry(bridge, conf, false, false, withHouseKeepingThreads, false);
   }
 
   /**
@@ -152,14 +141,13 @@ public class MetaStoreTestUtils {
    * @param conf The configuration to use
    * @param keepJdbcUri If set to true, then the JDBC url is not changed
    * @param keepWarehousePath If set to true, then the Warehouse directory is not changed
-   * @param withHouseKeepingThreads
+   * @param withHouseKeepingThreads Start the housekeeping background threads in the metastore
+   * @param waitForHouseKeepers Wait until the background threads are scheduled
    * @return The port on which the MetaStore finally started
-   * @throws Exception
+   * @throws Exception Timeout after 60 seconds
    */
-  public static int startMetaStoreWithRetry(HadoopThriftAuthBridge bridge,
-                                            Configuration conf, boolean keepJdbcUri,
-                                            boolean keepWarehousePath,
-                                            boolean withHouseKeepingThreads) throws Exception {
+  public static int startMetaStoreWithRetry(HadoopThriftAuthBridge bridge, Configuration conf, boolean keepJdbcUri,
+      boolean keepWarehousePath, boolean withHouseKeepingThreads, boolean waitForHouseKeepers) throws Exception {
     Exception metaStoreException = null;
     String warehouseDir = MetastoreConf.getVar(conf, ConfVars.WAREHOUSE);
 
@@ -187,7 +175,7 @@ public class MetaStoreTestUtils {
           MetastoreConf.setVar(conf, ConfVars.THRIFT_URIS, "thrift://localhost:" + metaStorePort);
         }
 
-        MetaStoreTestUtils.startMetaStore(metaStorePort, bridge, conf, withHouseKeepingThreads);
+        MetaStoreTestUtils.startMetaStore(metaStorePort, bridge, conf, withHouseKeepingThreads, waitForHouseKeepers);
 
         // Creating warehouse dir, if not exists
         Warehouse wh = new Warehouse(conf);
@@ -211,12 +199,12 @@ public class MetaStoreTestUtils {
 
   /**
    * A simple connect test to make sure that the metastore is up
-   * @throws Exception
+   * @throws Exception Timeout after 60 seconds
    */
-  private static void loopUntilHMSReady(String msHost, int port) throws Exception {
+  private static void loopUntilHMSReady(String msHost, int port, AtomicBoolean houseKeepingStarted) throws Exception {
     int retries = 0;
     Exception exc = null;
-    while (true) {
+    while (retries++ < 60) {
       try {
         Socket socket = new Socket();
         SocketAddress sockAddr;
@@ -227,14 +215,19 @@ public class MetaStoreTestUtils {
         }
         socket.connect(sockAddr, 5000);
         socket.close();
-        return;
-      } catch (Exception e) {
-        if (retries++ > 60) { //give up
-          exc = e;
-          break;
+        if (houseKeepingStarted == null || houseKeepingStarted.get()) {
+          return;
+        } else {
+          LOG.info("HMS started, waiting for housekeeper threads to start.");
         }
-        Thread.sleep(1000);
+      } catch (Exception e) {
+        LOG.info("Waiting the HMS to start.");
+        exc = e;
       }
+      Thread.sleep(1000);
+    }
+    if (exc == null) {
+      exc = new IllegalStateException("HMS started, but housekeeping threads were not started until 60 sec.");
     }
     // something is preventing metastore from starting
     // print the stack from all threads for debugging purposes
@@ -246,7 +239,7 @@ public class MetaStoreTestUtils {
 
   /**
    * A simple connect test to make sure that the metastore URI is available in the ZooKeeper
-   * @throws Exception
+   * @throws Exception Timeout after 60 seconds
    */
   private static void loopUntilZKReady(Configuration conf, String msHost, int port)
           throws Exception {
@@ -301,8 +294,8 @@ public class MetaStoreTestUtils {
   /**
    * Finds a free port on the machine.
    *
-   * @return
-   * @throws IOException
+   * @return The allocated port
+   * @throws IOException -
    */
   public static int findFreePort() throws IOException {
     ServerSocket socket= new ServerSocket(0);
@@ -316,11 +309,7 @@ public class MetaStoreTestUtils {
    * ability to specify a port number to not use, no matter what.
    */
   public static int findFreePortExcepting(int portToExclude) throws IOException {
-    ServerSocket socket1 = null;
-    ServerSocket socket2 = null;
-    try {
-      socket1 = new ServerSocket(0);
-      socket2 = new ServerSocket(0);
+    try (ServerSocket socket1 = new ServerSocket(0); ServerSocket socket2 = new ServerSocket(0)) {
       if (socket1.getLocalPort() != portToExclude) {
         return socket1.getLocalPort();
       }
@@ -328,13 +317,6 @@ public class MetaStoreTestUtils {
       // Since both sockets were open together at a point in time, we're
       // guaranteed that socket2.getLocalPort() is not the same.
       return socket2.getLocalPort();
-    } finally {
-      if (socket1 != null){
-        socket1.close();
-      }
-      if (socket2 != null){
-        socket2.close();
-      }
     }
   }