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 <lsk...@cloudera.com> Authored: Thu Dec 3 13:04:22 2015 -0800 Committer: Lenni Kuff <lsk...@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()); } }
