[hive] branch master updated: HIVE-22275: OperationManager.queryIdOperation does not properly clean up multiple queryIds (Jason Dere, reviewed by Prasanth Jayachandran)

[jd...@apache.org]

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

jdere 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 33fa702  HIVE-22275: OperationManager.queryIdOperation does not properly clean up multiple queryIds (Jason Dere, reviewed by Prasanth Jayachandran)
33fa702 is described below

commit 33fa702db68035cb882ffd948ef82acf28924ad9
Author: Jason Dere <jd...@cloudera.com>
AuthorDate: Thu Oct 3 23:29:13 2019 -0700

    HIVE-22275: OperationManager.queryIdOperation does not properly clean up multiple queryIds (Jason Dere, reviewed by Prasanth Jayachandran)
---
 .../service/cli/operation/OperationManager.java    |  2 +-
 .../service/cli/session/TestSessionCleanup.java    | 29 ++++++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/service/src/java/org/apache/hive/service/cli/operation/OperationManager.java b/service/src/java/org/apache/hive/service/cli/operation/OperationManager.java
index 40515da..506ffe6 100644
--- a/service/src/java/org/apache/hive/service/cli/operation/OperationManager.java
+++ b/service/src/java/org/apache/hive/service/cli/operation/OperationManager.java
@@ -194,7 +194,7 @@ public class OperationManager extends AbstractService {
   }
 
   private String getQueryId(Operation operation) {
-    return operation.getParentSession().getHiveConf().getVar(ConfVars.HIVEQUERYID);
+    return operation.getQueryId();
   }
 
   private void addOperation(Operation operation) {
diff --git a/service/src/test/org/apache/hive/service/cli/session/TestSessionCleanup.java b/service/src/test/org/apache/hive/service/cli/session/TestSessionCleanup.java
index 83748c3..51ce2c2 100644
--- a/service/src/test/org/apache/hive/service/cli/session/TestSessionCleanup.java
+++ b/service/src/test/org/apache/hive/service/cli/session/TestSessionCleanup.java
@@ -29,6 +29,8 @@ import java.util.Set;
 
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.apache.hive.service.cli.CLIService;
+import org.apache.hive.service.cli.OperationHandle;
 import org.apache.hive.service.cli.SessionHandle;
 import org.apache.hive.service.cli.thrift.EmbeddedThriftBinaryCLIService;
 import org.apache.hive.service.cli.thrift.ThriftCLIServiceClient;
@@ -39,11 +41,22 @@ import org.junit.Test;
  * TestSessionCleanup.
  */
 public class TestSessionCleanup {
+  // Create subclass of EmbeddedThriftBinaryCLIService, just so we can get an accessor to the CLIService.
+  // Needed for access to the OperationManager.
+  private class MyEmbeddedThriftBinaryCLIService extends EmbeddedThriftBinaryCLIService {
+    public MyEmbeddedThriftBinaryCLIService() {
+      super();
+    }
+
+    public CLIService getCliService() {
+      return cliService;
+    }
+  }
 
   @Test
   // This is to test session temporary files are cleaned up after HIVE-11768
   public void testTempSessionFileCleanup() throws Exception {
-    EmbeddedThriftBinaryCLIService service = new EmbeddedThriftBinaryCLIService();
+    MyEmbeddedThriftBinaryCLIService service = new MyEmbeddedThriftBinaryCLIService();
     HiveConf hiveConf = new HiveConf();
     hiveConf
         .setVar(HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER,
@@ -54,7 +67,12 @@ public class TestSessionCleanup {
     Set<String> existingPipeoutFiles = new HashSet<String>(Arrays.asList(getPipeoutFiles()));
     SessionHandle sessionHandle = client.openSession("user1", "foobar",
           Collections.<String, String>emptyMap());
-    client.executeStatement(sessionHandle, "set a=b", null);
+    OperationHandle opHandle1 = client.executeStatement(sessionHandle, "set a=b", null);
+    String queryId1 = service.getCliService().getQueryId(opHandle1.toTOperationHandle());
+    Assert.assertNotNull(queryId1);
+    OperationHandle opHandle2 = client.executeStatement(sessionHandle, "set b=c", null);
+    String queryId2 = service.getCliService().getQueryId(opHandle2.toTOperationHandle());
+    Assert.assertNotNull(queryId2);
     File operationLogRootDir = new File(
         new HiveConf().getVar(ConfVars.HIVE_SERVER2_LOGGING_OPERATION_LOG_LOCATION));
     Assert.assertNotEquals(operationLogRootDir.list().length, 0);
@@ -67,6 +85,13 @@ public class TestSessionCleanup {
     Set<String> finalPipeoutFiles = new HashSet<String>(Arrays.asList(getPipeoutFiles()));
     finalPipeoutFiles.removeAll(existingPipeoutFiles);
     Assert.assertTrue(finalPipeoutFiles.isEmpty());
+
+    // Verify both operationHandles are no longer held by the OperationManager
+    Assert.assertEquals(0, service.getCliService().getSessionManager().getOperations().size());
+
+    // Verify both queryIds are no longer held by the OperationManager
+    Assert.assertNull(service.getCliService().getSessionManager().getOperationManager().getOperationByQueryId(queryId2));
+    Assert.assertNull(service.getCliService().getSessionManager().getOperationManager().getOperationByQueryId(queryId1));
   }
 
   private String[] getPipeoutFiles() {