sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lsk...@apache.org
Subject incubator-sentry git commit: SENTRY-835: Drop table leaves a connection open when using metastorelistener (Hao Hao via Lenni Kuff)
Date Thu, 03 Dec 2015 21:04:54 GMT
Repository: incubator-sentry
Updated Branches:
  refs/heads/master d2a512ea3 -> c2747d9e8


SENTRY-835: Drop table leaves a connection open when using metastorelistener (Hao Hao via
Lenni Kuff)

Change-Id: If7a018d5f4d129dae7944cf87cd0d4d5fd103b7e


Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/c2747d9e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/c2747d9e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/c2747d9e

Branch: refs/heads/master
Commit: c2747d9e82d03724100f01d0f24b316de400f3fe
Parents: d2a512e
Author: Lenni Kuff <lskuff@cloudera.com>
Authored: Thu Dec 3 13:04:22 2015 -0800
Committer: Lenni Kuff <lskuff@cloudera.com>
Committed: Thu Dec 3 13:04:22 2015 -0800

----------------------------------------------------------------------
 .../SentryMetastorePostEventListener.java       | 11 +++-
 .../tests/e2e/dbprovider/TestDbConnections.java | 58 ++++++++++----------
 2 files changed, 39 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c2747d9e/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
index ecdfe1f..3c8ad1f 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
@@ -300,6 +300,9 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener
{
         .getShortUserName();
     SentryPolicyServiceClient sentryClient = getSentryServiceClient();
     sentryClient.dropPrivileges(requestorUserName, authorizableTable);
+
+    // Close the connection after dropping privileges is done.
+    sentryClient.close();
   }
 
   private void renameSentryTablePrivilege(String oldDbName, String oldTabName,
@@ -317,10 +320,12 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener
{
 
     if (!oldTabName.equalsIgnoreCase(newTabName)
         && syncWithPolicyStore(AuthzConfVars.AUTHZ_SYNC_ALTER_WITH_POLICY_STORE))
{
+
+      SentryPolicyServiceClient sentryClient = getSentryServiceClient();
+
       try {
         String requestorUserName = UserGroupInformation.getCurrentUser()
             .getShortUserName();
-        SentryPolicyServiceClient sentryClient = getSentryServiceClient();
         sentryClient.renamePrivileges(requestorUserName, oldAuthorizableTable, newAuthorizableTable);
       } catch (SentryUserException e) {
         throw new MetaException(
@@ -329,6 +334,10 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener
{
             + " Error: " + e.getMessage());
       } catch (IOException e) {
         throw new MetaException("Failed to find local user " + e.getMessage());
+      } finally {
+
+        // Close the connection after renaming privileges is done.
+        sentryClient.close();
       }
     }
     // The HDFS plugin needs to know if it's a path change (set location)

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c2747d9e/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java
index 3c9908c..d89b50e 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java
@@ -50,7 +50,7 @@ public class TestDbConnections extends AbstractTestWithStaticConfiguration
{
   /**
    * Currently the hive binding opens a new server connection for each
    * statement. This test verifies that the client connection is closed properly
-   * at the end. Test Queries, DDLs, Auth DDLs and metdata filtering (eg show
+   * at the end. Test Queries, DDLs, Auth DDLs and metadata filtering (eg show
    * tables/databases)
    * @throws Exception
    */
@@ -58,6 +58,7 @@ public class TestDbConnections extends AbstractTestWithStaticConfiguration
{
   public void testClientConnections() throws Exception {
     String roleName = "connectionTest";
     long preConnectionClientId;
+    // Connect through user admin1.
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
 
@@ -68,84 +69,83 @@ public class TestDbConnections extends AbstractTestWithStaticConfiguration
{
     statement.execute("CREATE DATABASE DB_1");
     statement.execute("USE DB_1");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // If turn on setMetastoreListener ( = true), getNumActiveClients != 0,
-    // Also when run tests on a real cluster,
-    // occasionally getNumActiveClients != 0,
-    // need to clean up this issue. SENTRY-835
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
-
-    // client connection is closed after DDLs
+    // Verify that client connection is closed after DDLs.
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.execute("CREATE TABLE t1 (c1 string)");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // client connection is closed after queries
+    // Verify that client connection is closed after queries.
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.execute("SELECT * FROM t1");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // client invocation via metastore filter
+    // Verify client invocation via metastore filter.
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.executeQuery("show tables");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
+    // Verify that client connection is closed after drop table.
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.execute("DROP TABLE t1");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // client connection is closed after auth DDL
+    // Verify that client connection is closed after auth DDL.
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.execute("CREATE ROLE " + roleName);
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
+
     context.assertSentryException(statement, "CREATE ROLE " + roleName,
         SentryAlreadyExistsException.class.getSimpleName());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
     statement.execute("DROP ROLE " + roleName);
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // client invocation via metastore filter
+    // Verify client invocation via metastore filter
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.executeQuery("show tables");
+    // There are no tables, so auth check does not happen
     // sentry will create connection to get privileges for cache
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
     statement.close();
     connection.close();
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
+    // Connect through user user1_1.
     connection = context.createConnection(USER1_1);
     statement = context.createStatement(connection);
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // verify client connection is closed after statement auth error
+    // Verify that client connection is closed after statement auth error.
     preConnectionClientId = getSentrySrv().getTotalClients();
     context.assertAuthzException(statement, "USE DB_1");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // verify client connection is closed after auth DDL error
+    // Verify that client connection is closed after auth DDL error.
     preConnectionClientId = getSentrySrv().getTotalClients();
     context.assertSentryException(statement, "CREATE ROLE " + roleName,
         SentryAccessDeniedException.class.getSimpleName());
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
-    // client invocation via metastore filter
+    // Verify that client invocation via metastore filter.
     preConnectionClientId = getSentrySrv().getTotalClients();
     statement.executeQuery("show databases");
     assertTrue(preConnectionClientId < getSentrySrv().getTotalClients());
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
 
     statement.close();
     connection.close();
-
-    // assertEquals(0, getSentrySrv().getNumActiveClients());
+    assertEquals(0, getSentrySrv().getNumActiveClients());
   }
 
 }


Mime
View raw message