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 2013/04/28 22:45:22 UTC

svn commit: r1476833 - in /hive/branches/branch-0.11/service/src: java/org/apache/hive/service/cli/session/HiveSessionImpl.java test/org/apache/hive/service/cli/session/ test/org/apache/hive/service/cli/session/TestHiveSession.java

Author: hashutosh
Date: Sun Apr 28 20:45:04 2013
New Revision: 1476833

URL: http://svn.apache.org/r1476833
Log:
HIVE-4398 : HS2 Resource leak: operation handles not cleaned when originating session is closed (Ashish Vaidya via Ashutosh Chauhan)

Added:
    hive/branches/branch-0.11/service/src/test/org/apache/hive/service/cli/session/
    hive/branches/branch-0.11/service/src/test/org/apache/hive/service/cli/session/TestHiveSession.java
Modified:
    hive/branches/branch-0.11/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java

Modified: hive/branches/branch-0.11/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
URL: http://svn.apache.org/viewvc/hive/branches/branch-0.11/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java?rev=1476833&r1=1476832&r2=1476833&view=diff
==============================================================================
--- hive/branches/branch-0.11/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java (original)
+++ hive/branches/branch-0.11/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java Sun Apr 28 20:45:04 2013
@@ -19,8 +19,10 @@
 package org.apache.hive.service.cli.session;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
@@ -64,6 +66,7 @@ public class HiveSessionImpl implements 
   private SessionManager sessionManager;
   private OperationManager operationManager;
   private IMetaStoreClient metastoreClient = null;
+  private final Set<OperationHandle> opHandleSet = new HashSet<OperationHandle>();
 
   public HiveSessionImpl(String username, String password, Map<String, String> sessionConf) {
     this.username = username;
@@ -164,7 +167,9 @@ public class HiveSessionImpl implements 
       ExecuteStatementOperation operation = getOperationManager()
           .newExecuteStatementOperation(getSession(), statement, confOverlay);
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -176,7 +181,9 @@ public class HiveSessionImpl implements 
     try {
       GetTypeInfoOperation operation = getOperationManager().newGetTypeInfoOperation(getSession());
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -188,7 +195,9 @@ public class HiveSessionImpl implements 
     try {
       GetCatalogsOperation operation = getOperationManager().newGetCatalogsOperation(getSession());
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -201,7 +210,9 @@ public class HiveSessionImpl implements 
       GetSchemasOperation operation =
           getOperationManager().newGetSchemasOperation(getSession(), catalogName, schemaName);
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -215,7 +226,9 @@ public class HiveSessionImpl implements 
       MetadataOperation operation =
           getOperationManager().newGetTablesOperation(getSession(), catalogName, schemaName, tableName, tableTypes);
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -227,7 +240,9 @@ public class HiveSessionImpl implements 
     try {
       GetTableTypesOperation operation = getOperationManager().newGetTableTypesOperation(getSession());
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -240,7 +255,9 @@ public class HiveSessionImpl implements 
     GetColumnsOperation operation = getOperationManager().newGetColumnsOperation(getSession(),
         catalogName, schemaName, tableName, columnName);
     operation.run();
-    return operation.getHandle();
+    OperationHandle opHandle = operation.getHandle();
+    opHandleSet.add(opHandle);
+    return opHandle;
     } finally {
       release();
     }
@@ -253,7 +270,9 @@ public class HiveSessionImpl implements 
       GetFunctionsOperation operation = getOperationManager()
           .newGetFunctionsOperation(getSession(), catalogName, schemaName, functionName);
       operation.run();
-      return operation.getHandle();
+      OperationHandle opHandle = operation.getHandle();
+      opHandleSet.add(opHandle);
+      return opHandle;
     } finally {
       release();
     }
@@ -270,6 +289,11 @@ public class HiveSessionImpl implements 
       if (metastoreClient != null) {
         metastoreClient.close();
       }
+      // Iterate through the opHandles and close their operations
+      for (OperationHandle opHandle : opHandleSet) {
+        operationManager.closeOperation(opHandle);
+      }
+      opHandleSet.clear();
     } finally {
       release();
     }
@@ -300,7 +324,8 @@ public class HiveSessionImpl implements 
   public void closeOperation(OperationHandle opHandle) throws HiveSQLException {
     acquire();
     try {
-      sessionManager.getOperationManager().closeOperation(opHandle);
+      operationManager.closeOperation(opHandle);
+      opHandleSet.remove(opHandle);
     } finally {
       release();
     }
@@ -341,4 +366,4 @@ public class HiveSessionImpl implements 
   protected HiveSession getSession() {
     return this;
   }
-}
+}
\ No newline at end of file

Added: hive/branches/branch-0.11/service/src/test/org/apache/hive/service/cli/session/TestHiveSession.java
URL: http://svn.apache.org/viewvc/hive/branches/branch-0.11/service/src/test/org/apache/hive/service/cli/session/TestHiveSession.java?rev=1476833&view=auto
==============================================================================
--- hive/branches/branch-0.11/service/src/test/org/apache/hive/service/cli/session/TestHiveSession.java (added)
+++ hive/branches/branch-0.11/service/src/test/org/apache/hive/service/cli/session/TestHiveSession.java Sun Apr 28 20:45:04 2013
@@ -0,0 +1,69 @@
+/**
+ * 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.hive.service.cli.session;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+
+import org.apache.hive.service.cli.OperationHandle;
+import org.apache.hive.service.cli.OperationState;
+import org.apache.hive.service.cli.HiveSQLException;
+import org.apache.hive.service.cli.operation.OperationManager;
+
+
+public class TestHiveSession {
+
+  @Test
+  public void checkOperationClosing() throws Exception {
+    OperationManager opMgr = new OperationManager();
+    HiveSession session = new HiveSessionImpl("user", "passw", null);
+
+    session.setOperationManager(opMgr);
+
+    OperationHandle opHandle1 = session.executeStatement("use default", null);
+    assertNotNull(opHandle1);
+    assertEquals(OperationState.FINISHED, opMgr.getOperationState(opHandle1));
+
+    OperationHandle opHandle2 = session.executeStatement("show databases", null);
+    assertNotNull(opHandle2);
+    assertEquals(OperationState.FINISHED, opMgr.getOperationState(opHandle2));
+
+    session.closeOperation(opHandle1); // Don't close directly from opMgr!
+
+    try {
+      opMgr.getOperationState(opHandle1);
+      fail("Shouldn't work!");
+    } catch (Exception e) {
+      assertTrue(e instanceof HiveSQLException);
+    }
+
+    session.close(); // Close all remaining associated operations
+
+    try {
+      opMgr.getOperationState(opHandle2);
+      fail("Shouldn't work this way now that it's fixed (HIVE-4398)!");
+    } catch (Exception e) {
+      assertTrue(e instanceof HiveSQLException);
+    }
+  }
+}