You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ha...@apache.org on 2020/07/21 15:59:04 UTC

[hive] branch master updated: HIVE-23855: TestQueryShutdownHooks is flaky (Mustafa Iman via Panos G, Ashutosh Chauhan)

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

hashutosh 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 2864d8d  HIVE-23855: TestQueryShutdownHooks is flaky (Mustafa Iman via Panos G, Ashutosh Chauhan)
2864d8d is described below

commit 2864d8dbeafebfa3059e07fb34206906466f99f9
Author: Mustafa Iman <mu...@gmail.com>
AuthorDate: Fri Jul 17 15:07:36 2020 -0700

    HIVE-23855: TestQueryShutdownHooks is flaky (Mustafa Iman via Panos G, Ashutosh Chauhan)
    
    Increased timeout for async query. Test were not isolated very well. Test async query did not clean up properly. State leaked to test sync causing it to fail. Cleanup is moved to @After so cleanup is always run.
    
    Signed-off-by: Ashutosh Chauhan <ha...@apache.org>
---
 .../hadoop/util/ShutdownHookManagerInspector.java  | 15 ++++-
 .../cli/operation/TestQueryShutdownHooks.java      | 64 +++++++++-------------
 2 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/service/src/test/org/apache/hadoop/util/ShutdownHookManagerInspector.java b/service/src/test/org/apache/hadoop/util/ShutdownHookManagerInspector.java
index d360475..2221f20 100644
--- a/service/src/test/org/apache/hadoop/util/ShutdownHookManagerInspector.java
+++ b/service/src/test/org/apache/hadoop/util/ShutdownHookManagerInspector.java
@@ -20,9 +20,20 @@ package org.apache.hadoop.util;
 
 import java.util.List;
 
+import static org.junit.Assert.assertEquals;
+
 public class ShutdownHookManagerInspector {
 
-  public static List<ShutdownHookManager.HookEntry> getShutdownHooksInOrder() {
-    return ShutdownHookManager.get().getShutdownHooksInOrder();
+  public static int getShutdownHookCount() {
+    return ShutdownHookManager.get().getShutdownHooksInOrder().size();
+  }
+
+  public static void assertShutdownHookCount(int expected) {
+    List<ShutdownHookManager.HookEntry> entries = ShutdownHookManager.get().getShutdownHooksInOrder();
+    StringBuilder errorBuilder = new StringBuilder("Shutdown hooks:\n");
+    for (ShutdownHookManager.HookEntry entry: entries) {
+      errorBuilder.append(entry.getHook()).append(" Priority:").append(entry.getPriority()).append("\n");
+    }
+    assertEquals(errorBuilder.toString(), expected, entries.size());
   }
 }
diff --git a/service/src/test/org/apache/hive/service/cli/operation/TestQueryShutdownHooks.java b/service/src/test/org/apache/hive/service/cli/operation/TestQueryShutdownHooks.java
index 0233e8b..0170c71 100644
--- a/service/src/test/org/apache/hive/service/cli/operation/TestQueryShutdownHooks.java
+++ b/service/src/test/org/apache/hive/service/cli/operation/TestQueryShutdownHooks.java
@@ -28,6 +28,7 @@ import org.apache.hive.service.cli.OperationStatus;
 import org.apache.hive.service.cli.SessionHandle;
 import org.apache.hive.service.cli.thrift.EmbeddedThriftBinaryCLIService;
 import org.apache.hive.service.cli.thrift.ThriftCLIServiceClient;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -40,11 +41,13 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 
-@org.junit.Ignore("HIVE-23855 TestQueryShutdownHooks is flaky")
 public class TestQueryShutdownHooks {
 
+  private static final long ASYNC_QUERY_TIMEOUT_MS = 600000;
   private EmbeddedThriftBinaryCLIService service;
   private ThriftCLIServiceClient client;
+  private SessionHandle sessionHandle;
+  private final Map<String, String> confOverlay = new HashMap<>();
 
   @Before
   public void setUp() throws Exception {
@@ -57,21 +60,23 @@ public class TestQueryShutdownHooks {
     hiveConf.setVar(ConfVars.HIVE_LOCK_MANAGER, "org.apache.hadoop.hive.ql.lockmgr.EmbeddedLockManager");
     service.init(hiveConf);
     client = new ThriftCLIServiceClient(service);
-    SessionHandle tempSession = client.openSession("anonymous", "anonymous", new HashMap<>());
+    sessionHandle = client.openSession("anonymous", "anonymous", new HashMap<>());
     // any job causes creation of HadoopJobExecHelper's shutdown hook. It is once per JVM
     // We want it to be created before we count the hooks so it does not cause off by one error in our count
-    client.executeStatement(tempSession, "select reflect(\"java.lang.System\", \"currentTimeMillis\")", new HashMap<>());
-    client.closeSession(tempSession);
+    client.executeStatement(sessionHandle, "select reflect(\"java.lang.System\", \"currentTimeMillis\")", new HashMap<>());
+  }
+
+  @After
+  public void cleanup() throws HiveSQLException {
+    if (sessionHandle != null) {
+      client.closeSession(sessionHandle);
+    }
+    service.stop();
   }
 
   @Test
   public void testSync() throws Exception {
-    Map<String, String> opConf = new HashMap<String, String>();
-
-    SessionHandle sessHandle = client.openSession("anonymous",
-            "anonymous", opConf);
-
-    int shutdownHooksBeforeQueries = ShutdownHookManagerInspector.getShutdownHooksInOrder().size();
+    int shutdownHooksBeforeQueries = ShutdownHookManagerInspector.getShutdownHookCount();
 
     String[] someQueries = {
             "CREATE TABLE sample_shutdown_hook (sample_id int, sample_value string)",
@@ -86,7 +91,7 @@ public class TestQueryShutdownHooks {
             "DROP TABLE sample_shutdown_hook",
     };
     for (String queryStr : someQueries) {
-      OperationHandle opHandle = client.executeStatement(sessHandle, queryStr, opConf);
+      OperationHandle opHandle = client.executeStatement(sessionHandle, queryStr, confOverlay);
       assertNotNull(opHandle);
       OperationStatus opStatus = client.getOperationStatus(opHandle, false);
       assertNotNull(opStatus);
@@ -94,19 +99,12 @@ public class TestQueryShutdownHooks {
       assertEquals("Query should be finished", OperationState.FINISHED, state);
     }
 
-    int shutdownHooksAfterFinished = ShutdownHookManagerInspector.getShutdownHooksInOrder().size();
-
-    assertEquals(shutdownHooksBeforeQueries, shutdownHooksAfterFinished);
-
-    client.closeSession(sessHandle);
+    ShutdownHookManagerInspector.assertShutdownHookCount(shutdownHooksBeforeQueries);
   }
 
   @Test
   public void testAsync() throws Exception {
-    Map<String, String> opConf = new HashMap<String, String>();
-
-    SessionHandle sessHandle = client.openSession("anonymous", "anonymous", opConf);
-    int shutdownHooksBeforeQueries = ShutdownHookManagerInspector.getShutdownHooksInOrder().size();
+    int shutdownHooksBeforeQueries = ShutdownHookManagerInspector.getShutdownHookCount();
 
     String[] someQueries = {
             "select reflect(\"java.lang.Thread\", \"sleep\", bigint(1000))",
@@ -117,14 +115,14 @@ public class TestQueryShutdownHooks {
 
     List<OperationHandle> operationHandles = new ArrayList<>();
     for (String queryStr : someQueries) {
-      OperationHandle opHandle = client.executeStatementAsync(sessHandle, queryStr, opConf);
+      OperationHandle opHandle = client.executeStatementAsync(sessionHandle, queryStr, confOverlay);
       assertNotNull(opHandle);
       operationHandles.add(opHandle);
     }
 
     boolean allComplete = false;
     final long step = 200;
-    final long timeout = System.currentTimeMillis() + 60000;
+    final long timeout = System.currentTimeMillis() + ASYNC_QUERY_TIMEOUT_MS;
 
     while (!allComplete) {
       allComplete = true;
@@ -141,27 +139,21 @@ public class TestQueryShutdownHooks {
       }
     }
 
-    int shutdownHooksAfterFinished = ShutdownHookManagerInspector.getShutdownHooksInOrder().size();
-
-    assertEquals(shutdownHooksBeforeQueries, shutdownHooksAfterFinished);
-    client.closeSession(sessHandle);
+    ShutdownHookManagerInspector.assertShutdownHookCount(shutdownHooksBeforeQueries);
   }
 
   @Test
   public void testShutdownHookManagerIsRegistered() throws HiveSQLException, InterruptedException {
-    Map<String, String> opConf = new HashMap<String, String>();
-
-    SessionHandle sessHandle = client.openSession("anonymous", "anonymous", opConf);
-    int shutdownHooksBeforeQuery = ShutdownHookManagerInspector.getShutdownHooksInOrder().size();
+    int shutdownHooksBeforeQuery = ShutdownHookManagerInspector.getShutdownHookCount();
 
     String queryStr = "select reflect(\"java.lang.Thread\", \"sleep\", bigint(5000))";
-    OperationHandle opHandle = client.executeStatementAsync(sessHandle, queryStr, opConf);
+    OperationHandle opHandle = client.executeStatementAsync(sessionHandle, queryStr, confOverlay);
     assertNotNull(opHandle);
 
-    assertEquals(shutdownHooksBeforeQuery + 1, ShutdownHookManagerInspector.getShutdownHooksInOrder().size());
+    ShutdownHookManagerInspector.assertShutdownHookCount(shutdownHooksBeforeQuery + 1);
 
     final long step = 200;
-    final long timeout = System.currentTimeMillis() + 60000;
+    final long timeout = System.currentTimeMillis() + ASYNC_QUERY_TIMEOUT_MS;
 
     while (true) {
       OperationStatus operationStatus = client.getOperationStatus(opHandle, false);
@@ -173,10 +165,6 @@ public class TestQueryShutdownHooks {
       }
       Thread.sleep(step);
     }
-
-    int shutdownHooksAfterFinished = ShutdownHookManagerInspector.getShutdownHooksInOrder().size();
-
-    assertEquals(shutdownHooksBeforeQuery, shutdownHooksAfterFinished);
-    client.closeSession(sessHandle);
+    ShutdownHookManagerInspector.assertShutdownHookCount(shutdownHooksBeforeQuery);
   }
 }