You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/01/23 16:36:49 UTC
[impala] 02/04: IMPALA-8073: Fix FE tests that leak HMS connections
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 9105c5c5427dd0479caf280b5754b31845df5fc2
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Fri Jan 18 09:38:23 2019 -0800
IMPALA-8073: Fix FE tests that leak HMS connections
This patch fixes FE tests that leak the HMS connections. I was to
reproduce the issue by in SentryTestProxy by looping the test multiple
times.
Testing:
- Ran SentryProxyTest in a loop without any issue.
- Ran all FE tests
Change-Id: I56930bc5f7a7bda4db11d4447461f907b1017b91
Reviewed-on: http://gerrit.cloudera.org:8080/12241
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../java/org/apache/impala/catalog/Catalog.java | 4 +-
.../org/apache/impala/analysis/AuditingTest.java | 19 +--
.../apache/impala/analysis/AuthorizationTest.java | 149 +++++++++++----------
.../impala/analysis/StmtMetadataLoaderTest.java | 26 ++--
.../catalog/CatalogObjectToFromThriftTest.java | 1 -
.../org/apache/impala/catalog/CatalogTest.java | 4 +
.../catalog/CatalogdTableInvalidatorTest.java | 6 +-
.../impala/catalog/PartialCatalogInfoTest.java | 4 +
.../org/apache/impala/common/FrontendTestBase.java | 1 +
.../apache/impala/testutil/BlockIdGenerator.java | 53 ++++----
.../apache/impala/testutil/ImpaladTestCatalog.java | 6 +
.../org/apache/impala/util/SentryProxyTest.java | 131 +++++++++---------
12 files changed, 215 insertions(+), 189 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/Catalog.java b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
index 1f46882..7a3aa93 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Catalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
@@ -57,7 +57,7 @@ import com.google.common.base.Preconditions;
* database that the user cannot modify.
* Builtins are populated on startup in initBuiltins().
*/
-public abstract class Catalog {
+public abstract class Catalog implements AutoCloseable {
// Initial catalog version and ID.
public final static long INITIAL_CATALOG_VERSION = 0L;
public static final TUniqueId INITIAL_CATALOG_SERVICE_ID = new TUniqueId(0L, 0L);
@@ -322,9 +322,9 @@ public abstract class Catalog {
* Release the Hive Meta Store Client resources. Can be called multiple times
* (additional calls will be no-ops).
*/
+ @Override
public void close() { metaStoreClientPool_.close(); }
-
/**
* Returns a managed meta store client from the client connection pool.
*/
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
index 836020b..484ae8e 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
@@ -370,15 +370,16 @@ public class AuditingTest extends FrontendTestBase {
// an AuthorizationError
AuthorizationConfig config = AuthorizationConfig.createHadoopGroupAuthConfig(
"server1", "/does/not/exist", "");
- ImpaladCatalog catalog = new ImpaladTestCatalog(config);
- Frontend fe = new Frontend(config, catalog);
- AnalysisContext analysisCtx = createAnalysisCtx(config);
- // We should get an audit event even when an authorization failure occurs.
- try {
- parseAndAnalyze("create table foo_does_not_exist(i int)", analysisCtx, fe);
- Assert.fail("Expected AuthorizationException");
- } catch (AuthorizationException e) {
- Assert.assertEquals(1, analysisCtx.getAnalyzer().getAccessEvents().size());
+ try (ImpaladCatalog catalog = new ImpaladTestCatalog(config)) {
+ Frontend fe = new Frontend(config, catalog);
+ AnalysisContext analysisCtx = createAnalysisCtx(config);
+ // We should get an audit event even when an authorization failure occurs.
+ try {
+ parseAndAnalyze("create table foo_does_not_exist(i int)", analysisCtx, fe);
+ Assert.fail("Expected AuthorizationException");
+ } catch (AuthorizationException e) {
+ Assert.assertEquals(1, analysisCtx.getAnalyzer().getAccessEvents().size());
+ }
}
}
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
index 7e1214e..9de90d6 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -842,58 +842,59 @@ public class AuthorizationTest extends FrontendTestBase {
AuthorizationConfig authzConfig = new AuthorizationConfig("server1",
AUTHZ_POLICY_FILE, "",
LocalGroupResourceAuthorizationProvider.class.getName());
- ImpaladCatalog catalog = new ImpaladTestCatalog(authzConfig);
-
- // This test relies on the auth_to_local rule -
- // "RULE:[2:$1@$0](authtest@REALM.COM)s/(.*)@REALM.COM/auth_to_local_user/"
- // which converts any principal of type authtest/<hostname>@REALM.COM to user
- // auth_to_local_user. We have a sentry privilege in place that grants 'SELECT'
- // privilege on tpcds.* to user auth_to_local_user. To test the integration, we try
- // running the query with authtest/hostname@REALM.COM user which is converted to user
- // 'auth_to_local_user' and the authz should pass.
- User.setRulesForTesting(
- new Configuration().get(HADOOP_SECURITY_AUTH_TO_LOCAL, "DEFAULT"));
- User user = new User("authtest/hostname@REALM.COM");
- AnalysisContext ctx = createAnalysisCtx(authzConfig, user.getName());
- Frontend fe = new Frontend(authzConfig, catalog);
-
- // Can select from table that user has privileges on.
- AuthzOk(fe, ctx, "select * from tpcds.customer");
- // Does not have privileges to execute a query
- AuthzError(fe, ctx, "select * from functional.alltypes",
- "User '%s' does not have privileges to execute 'SELECT' on: functional.alltypes");
-
- // Unit tests for User#getShortName()
- // Different auth_to_local rules to apply on the username.
- List<String> rules = Lists.newArrayList(
- // Expects user@REALM and returns 'usera' (appends a in the end)
- "RULE:[1:$1@$0](.*@REALM.COM)s/(.*)@REALM.COM/$1a/g",
- // Same as above but expects user/host@REALM
- "RULE:[2:$1@$0](.*@REALM.COM)s/(.*)@REALM.COM/$1a/g");
-
- // Map from the user to the expected getShortName() output after applying
- // the corresponding rule from 'rules' list.
- List<Map.Entry<User, String>> users = Lists.newArrayList(
- Maps.immutableEntry(new User(USER.getName() + "@REALM.COM"), USER.getName()
- + "a"),
- Maps.immutableEntry(new User(USER.getName() + "/abc.host.com@REALM.COM"),
- USER.getName() + "a"));
-
- for (int idx = 0; idx < users.size(); ++idx) {
- Map.Entry<User, String> userObj = users.get(idx);
- assertEquals(userObj.getKey().getShortNameForTesting(rules.get(idx)),
- userObj.getValue());
- }
+ try (ImpaladCatalog catalog = new ImpaladTestCatalog(authzConfig)) {
+ // This test relies on the auth_to_local rule -
+ // "RULE:[2:$1@$0](authtest@REALM.COM)s/(.*)@REALM.COM/auth_to_local_user/"
+ // which converts any principal of type authtest/<hostname>@REALM.COM to user
+ // auth_to_local_user. We have a sentry privilege in place that grants 'SELECT'
+ // privilege on tpcds.* to user auth_to_local_user. To test the integration, we try
+ // running the query with authtest/hostname@REALM.COM user which is converted to
+ // user 'auth_to_local_user' and the authz should pass.
+ User.setRulesForTesting(
+ new Configuration().get(HADOOP_SECURITY_AUTH_TO_LOCAL, "DEFAULT"));
+ User user = new User("authtest/hostname@REALM.COM");
+ AnalysisContext ctx = createAnalysisCtx(authzConfig, user.getName());
+ Frontend fe = new Frontend(authzConfig, catalog);
- // Test malformed rules. RuleParser throws an IllegalArgumentException.
- String malformedRule = "{((()";
- try {
- user.getShortNameForTesting(malformedRule);
- } catch (IllegalArgumentException e) {
- Assert.assertTrue(e.getMessage().contains("Invalid rule"));
- return;
+ // Can select from table that user has privileges on.
+ AuthzOk(fe, ctx, "select * from tpcds.customer");
+ // Does not have privileges to execute a query
+ AuthzError(fe, ctx, "select * from functional.alltypes",
+ "User '%s' does not have privileges to execute 'SELECT' on: " +
+ "functional.alltypes");
+
+ // Unit tests for User#getShortName()
+ // Different auth_to_local rules to apply on the username.
+ List<String> rules = Lists.newArrayList(
+ // Expects user@REALM and returns 'usera' (appends a in the end)
+ "RULE:[1:$1@$0](.*@REALM.COM)s/(.*)@REALM.COM/$1a/g",
+ // Same as above but expects user/host@REALM
+ "RULE:[2:$1@$0](.*@REALM.COM)s/(.*)@REALM.COM/$1a/g");
+
+ // Map from the user to the expected getShortName() output after applying
+ // the corresponding rule from 'rules' list.
+ List<Map.Entry<User, String>> users = Lists.newArrayList(
+ Maps.immutableEntry(new User(USER.getName() + "@REALM.COM"), USER.getName()
+ + "a"),
+ Maps.immutableEntry(new User(USER.getName() + "/abc.host.com@REALM.COM"),
+ USER.getName() + "a"));
+
+ for (int idx = 0; idx < users.size(); ++idx) {
+ Map.Entry<User, String> userObj = users.get(idx);
+ assertEquals(userObj.getKey().getShortNameForTesting(rules.get(idx)),
+ userObj.getValue());
+ }
+
+ // Test malformed rules. RuleParser throws an IllegalArgumentException.
+ String malformedRule = "{((()";
+ try {
+ user.getShortNameForTesting(malformedRule);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Invalid rule"));
+ return;
+ }
+ Assert.fail("No exception caught while using getShortName() on a malformed rule");
}
- Assert.fail("No exception caught while using getShortName() on a malformed rule");
}
@Test
@@ -1027,30 +1028,32 @@ public class AuthorizationTest extends FrontendTestBase {
AuthorizationConfig authzConfig = new AuthorizationConfig("server1",
AUTHZ_POLICY_FILE, "",
LocalGroupResourceAuthorizationProvider.class.getName());
- ImpaladCatalog catalog = new ImpaladTestCatalog(authzConfig);
+ try (ImpaladCatalog catalog = new ImpaladTestCatalog(authzConfig)) {
+ // Create an analysis context + FE with the test user
+ // (as defined in the policy file)
+ User user = new User("test_user");
+ AnalysisContext ctx = createAnalysisCtx(authzConfig, user.getName());
+ Frontend fe = new Frontend(authzConfig, catalog);
- // Create an analysis context + FE with the test user (as defined in the policy file)
- User user = new User("test_user");
- AnalysisContext ctx = createAnalysisCtx(authzConfig, user.getName());
- Frontend fe = new Frontend(authzConfig, catalog);
-
- // Can select from table that user has privileges on.
- AuthzOk(fe, ctx, "select * from functional.alltypesagg");
- // Does not have privileges to execute a query
- AuthzError(fe, ctx, "select * from functional.alltypes",
- "User '%s' does not have privileges to execute 'SELECT' on: functional.alltypes");
-
- // Verify with the admin user
- user = new User("admin_user");
- ctx = createAnalysisCtx(authzConfig, user.getName());
- fe = new Frontend(authzConfig, catalog);
-
- // Admin user should have privileges to do anything
- AuthzOk(fe, ctx, "select * from functional.alltypesagg");
- AuthzOk(fe, ctx, "select * from functional.alltypes");
- AuthzOk(fe, ctx, "invalidate metadata");
- AuthzOk(fe, ctx, "create external table tpch.kudu_tbl stored as kudu " +
- "TBLPROPERTIES ('kudu.master_addresses'='127.0.0.1', 'kudu.table_name'='tbl')");
+ // Can select from table that user has privileges on.
+ AuthzOk(fe, ctx, "select * from functional.alltypesagg");
+ // Does not have privileges to execute a query
+ AuthzError(fe, ctx, "select * from functional.alltypes",
+ "User '%s' does not have privileges to execute 'SELECT' on: " +
+ "functional.alltypes");
+
+ // Verify with the admin user
+ user = new User("admin_user");
+ ctx = createAnalysisCtx(authzConfig, user.getName());
+ fe = new Frontend(authzConfig, catalog);
+
+ // Admin user should have privileges to do anything
+ AuthzOk(fe, ctx, "select * from functional.alltypesagg");
+ AuthzOk(fe, ctx, "select * from functional.alltypes");
+ AuthzOk(fe, ctx, "invalidate metadata");
+ AuthzOk(fe, ctx, "create external table tpch.kudu_tbl stored as kudu " +
+ "TBLPROPERTIES ('kudu.master_addresses'='127.0.0.1', 'kudu.table_name'='tbl')");
+ }
}
private void TestWithIncorrectConfig(AuthorizationConfig authzConfig, User user)
diff --git a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
index 9ef923a..5f33683 100644
--- a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
@@ -38,21 +38,23 @@ public class StmtMetadataLoaderTest {
int expectedNumLoadRequests, int expectedNumCatalogUpdates,
String[] expectedDbs, String[] expectedTables)
throws ImpalaException {
- ImpaladTestCatalog catalog = new ImpaladTestCatalog();
- Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
- StatementBase stmt = Parser.parse(stmtStr);
- // Catalog is fresh and no tables are cached.
- validateUncached(stmt, fe, expectedNumLoadRequests, expectedNumCatalogUpdates,
- expectedDbs, expectedTables);
- // All relevant tables should be cached now.
- validateCached(stmt, fe, expectedDbs, expectedTables);
+ try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) {
+ Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
+ StatementBase stmt = Parser.parse(stmtStr);
+ // Catalog is fresh and no tables are cached.
+ validateUncached(stmt, fe, expectedNumLoadRequests, expectedNumCatalogUpdates,
+ expectedDbs, expectedTables);
+ // All relevant tables should be cached now.
+ validateCached(stmt, fe, expectedDbs, expectedTables);
+ }
}
private void testNoLoad(String stmtStr) throws ImpalaException {
- ImpaladTestCatalog catalog = new ImpaladTestCatalog();
- Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
- StatementBase stmt = Parser.parse(stmtStr);
- validateCached(stmt, fe, new String[]{}, new String[]{});
+ try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) {
+ Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
+ StatementBase stmt = Parser.parse(stmtStr);
+ validateCached(stmt, fe, new String[]{}, new String[]{});
+ }
}
private void validateDbs(StmtTableCache stmtTableCache, String[] expectedDbs) {
diff --git a/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java b/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
index 0747464..9762a3e 100644
--- a/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
@@ -24,7 +24,6 @@ import java.util.Collection;
import java.util.Map;
import org.apache.impala.analysis.LiteralExpr;
-import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.SqlCastException;
import org.apache.impala.testutil.CatalogServiceTestCatalog;
diff --git a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
index 0946b38..bac5864 100644
--- a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
@@ -64,6 +64,7 @@ import org.apache.impala.thrift.TPrivilege;
import org.apache.impala.thrift.TPrivilegeLevel;
import org.apache.impala.thrift.TPrivilegeScope;
import org.apache.impala.thrift.TTableName;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -80,6 +81,9 @@ public class CatalogTest {
catalog_ = CatalogServiceTestCatalog.create();
}
+ @After
+ public void cleanUp() { catalog_.close(); }
+
public static void checkTableCols(FeDb db, String tblName, int numClusteringCols,
String[] colNames, Type[] colTypes) throws TableLoadingException {
FeTable tbl = db.getTable(tblName);
diff --git a/fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java b/fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java
index 69e99e3..4410ea5 100644
--- a/fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java
@@ -22,6 +22,7 @@ import org.apache.impala.common.Reference;
import org.apache.impala.testutil.CatalogServiceTestCatalog;
import org.apache.impala.thrift.TTableName;
import org.junit.After;
+import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Test;
@@ -31,7 +32,10 @@ import static java.lang.Thread.sleep;
public class CatalogdTableInvalidatorTest {
- private CatalogServiceCatalog catalog_ = CatalogServiceTestCatalog.create();
+ private static CatalogServiceCatalog catalog_ = CatalogServiceTestCatalog.create();
+
+ @AfterClass
+ public static void tearDown() { catalog_.close(); }
private long waitForTrigger(long previousTriggerCount) throws InterruptedException {
long triggerCount;
diff --git a/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java b/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
index 1b5e288..657509a 100644
--- a/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
@@ -49,6 +49,7 @@ import org.apache.impala.thrift.TTableInfoSelector;
import org.apache.thrift.TDeserializer;
import org.apache.thrift.TException;
import org.apache.thrift.TSerializer;
+import org.junit.AfterClass;
import org.junit.Test;
import com.google.common.base.Preconditions;
@@ -58,6 +59,9 @@ public class PartialCatalogInfoTest {
private static CatalogServiceCatalog catalog_ =
CatalogServiceTestCatalog.create();
+ @AfterClass
+ public static void cleanUp() { catalog_.close(); }
+
/**
* A Callable wrapper around getPartialCatalogObject() call.
*/
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
index 0ea33a7..85ecc4c 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
@@ -97,6 +97,7 @@ public class FrontendTestBase {
@AfterClass
public static void cleanUp() throws Exception {
RuntimeEnv.INSTANCE.reset();
+ catalog_.close();
}
// Adds a Udf: default.name(args) to the catalog.
diff --git a/fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java b/fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java
index f04ce4a..ef03d67 100644
--- a/fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java
+++ b/fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java
@@ -61,38 +61,39 @@ public class BlockIdGenerator {
writer = new FileWriter(output);
// Load all tables in the catalog
- Catalog catalog = CatalogServiceTestCatalog.create();
- for (FeDb database: catalog.getDbs(PatternMatcher.MATCHER_MATCH_ALL)) {
- for (String tableName: database.getAllTableNames()) {
- FeTable table = database.getTable(tableName);
- // Only do this for hdfs tables
- if (table == null || !(table instanceof HdfsTable)) {
- continue;
- }
- HdfsTable hdfsTable = (HdfsTable)table;
+ try (Catalog catalog = CatalogServiceTestCatalog.create()) {
+ for (FeDb database : catalog.getDbs(PatternMatcher.MATCHER_MATCH_ALL)) {
+ for (String tableName : database.getAllTableNames()) {
+ FeTable table = database.getTable(tableName);
+ // Only do this for hdfs tables
+ if (table == null || !(table instanceof HdfsTable)) {
+ continue;
+ }
+ HdfsTable hdfsTable = (HdfsTable) table;
- // Write the output as <tablename>: <blockid1> <blockid2> <etc>
- writer.write(tableName + ":");
- Collection<? extends FeFsPartition> parts =
- FeCatalogUtils.loadAllPartitions(hdfsTable);
- for (FeFsPartition partition: parts) {
- List<FileDescriptor> fileDescriptors = partition.getFileDescriptors();
- for (FileDescriptor fd : fileDescriptors) {
- Path p = new Path(partition.getLocation(), fd.getFileName());
+ // Write the output as <tablename>: <blockid1> <blockid2> <etc>
+ writer.write(tableName + ":");
+ Collection<? extends FeFsPartition> parts =
+ FeCatalogUtils.loadAllPartitions(hdfsTable);
+ for (FeFsPartition partition : parts) {
+ List<FileDescriptor> fileDescriptors = partition.getFileDescriptors();
+ for (FileDescriptor fd : fileDescriptors) {
+ Path p = new Path(partition.getLocation(), fd.getFileName());
- // Use a deprecated API to get block ids
- DistributedFileSystem dfs =
- (DistributedFileSystem)p.getFileSystem(hdfsConfig);
- LocatedBlocks locations = dfs.getClient().getNamenode().getBlockLocations(
- p.toUri().getPath(), 0, fd.getFileLength());
+ // Use a deprecated API to get block ids
+ DistributedFileSystem dfs =
+ (DistributedFileSystem) p.getFileSystem(hdfsConfig);
+ LocatedBlocks locations = dfs.getClient().getNamenode().getBlockLocations(
+ p.toUri().getPath(), 0, fd.getFileLength());
- for (LocatedBlock lb : locations.getLocatedBlocks()) {
- long id = lb.getBlock().getBlockId();
- writer.write(" " + id);
+ for (LocatedBlock lb : locations.getLocatedBlocks()) {
+ long id = lb.getBlock().getBlockId();
+ writer.write(" " + id);
+ }
}
}
+ writer.write("\n");
}
- writer.write("\n");
}
}
} finally {
diff --git a/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java b/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
index 60bbaeb..3395c54 100644
--- a/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
+++ b/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
@@ -147,4 +147,10 @@ public class ImpaladTestCatalog extends ImpaladCatalog {
}
public void removeUser(String userName) { srcCatalog_.removeUser(userName); }
+
+ @Override
+ public void close() {
+ super.close();
+ srcCatalog_.close();
+ }
}
diff --git a/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java b/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
index d3246ab..7ab8e56 100644
--- a/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
+++ b/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
@@ -274,41 +274,41 @@ public class SentryProxyTest {
String mixedCaseRoleName = lowerCaseRoleName.substring(0, 1).toUpperCase() +
lowerCaseRoleName.substring(1);
- CatalogServiceCatalog catalog = CatalogServiceTestCatalog.createWithAuth(
- authzConfig_.getSentryConfig());
-
- addSentryRolePrivileges(sentryService_, lowerCaseRoleName, "functional");
- CatalogState noReset = refreshSentryAuthorization(catalog, sentryService_, false);
-
- // Impala stores the role name in case insensitive way.
- for (String roleName: new String[]{
- lowerCaseRoleName, upperCaseRoleName, mixedCaseRoleName}) {
- Role role = catalog.getAuthPolicy().getRole(roleName);
- assertEquals(lowerCaseRoleName, role.getName());
- assertEquals(1, role.getPrivileges().size());
- assertNotNull(role.getPrivilege(
- "server=server1->db=functional->grantoption=false"));
- }
+ try (CatalogServiceCatalog catalog = CatalogServiceTestCatalog.createWithAuth(
+ authzConfig_.getSentryConfig())) {
+ addSentryRolePrivileges(sentryService_, lowerCaseRoleName, "functional");
+ CatalogState noReset = refreshSentryAuthorization(catalog, sentryService_, false);
+
+ // Impala stores the role name in case insensitive way.
+ for (String roleName : new String[]{
+ lowerCaseRoleName, upperCaseRoleName, mixedCaseRoleName}) {
+ Role role = catalog.getAuthPolicy().getRole(roleName);
+ assertEquals(lowerCaseRoleName, role.getName());
+ assertEquals(1, role.getPrivileges().size());
+ assertNotNull(role.getPrivilege(
+ "server=server1->db=functional->grantoption=false"));
+ }
- try {
- sentryService_.createRole(USER, upperCaseRoleName, false);
- fail("Exception should be thrown when creating a duplicate role name.");
- } catch (Exception e) {
- assertTrue(e.getMessage().startsWith("Error creating role"));
- }
+ try {
+ sentryService_.createRole(USER, upperCaseRoleName, false);
+ fail("Exception should be thrown when creating a duplicate role name.");
+ } catch (Exception e) {
+ assertTrue(e.getMessage().startsWith("Error creating role"));
+ }
- // No new role should be added.
- for (String roleName: new String[]{
- lowerCaseRoleName, upperCaseRoleName, mixedCaseRoleName}) {
- Role role = catalog.getAuthPolicy().getRole(roleName);
- assertEquals(lowerCaseRoleName, role.getName());
- assertEquals(1, role.getPrivileges().size());
- assertNotNull(role.getPrivilege(
- "server=server1->db=functional->grantoption=false"));
- }
+ // No new role should be added.
+ for (String roleName : new String[]{
+ lowerCaseRoleName, upperCaseRoleName, mixedCaseRoleName}) {
+ Role role = catalog.getAuthPolicy().getRole(roleName);
+ assertEquals(lowerCaseRoleName, role.getName());
+ assertEquals(1, role.getPrivileges().size());
+ assertNotNull(role.getPrivilege(
+ "server=server1->db=functional->grantoption=false"));
+ }
- CatalogState reset = refreshSentryAuthorization(catalog, sentryService_, true);
- checkCatalogState(noReset, reset);
+ CatalogState reset = refreshSentryAuthorization(catalog, sentryService_, true);
+ checkCatalogState(noReset, reset);
+ }
}
@Test
@@ -319,34 +319,34 @@ public class SentryProxyTest {
String mixedCaseUserName = lowerCaseUserName.substring(0, 1).toUpperCase() +
lowerCaseUserName.substring(1);
- CatalogServiceCatalog catalog = CatalogServiceTestCatalog.createWithAuth(
- authzConfig_.getSentryConfig());
-
- SentryPolicyServiceStub sentryService = createSentryPolicyServiceStub(
- authzConfig_.getSentryConfig());
- // We grant different privileges to different users to ensure each user is
- // granted with a distinct privilege.
- addSentryUserPrivileges(sentryService, lowerCaseUserName, "functional");
- addSentryUserPrivileges(sentryService, upperCaseUserName, "functional_kudu");
- addSentryUserPrivileges(sentryService, mixedCaseUserName, "functional_parquet");
-
- CatalogState noReset = refreshSentryAuthorization(catalog, sentryService, false);
-
- // Impala stores the user name in case sensitive way.
- for (Pair<String, String> userPrivilege: new Pair[]{
- new Pair(lowerCaseUserName, "server=server1->db=functional->grantoption=false"),
- new Pair(upperCaseUserName, "server=server1->db=functional_kudu" +
- "->grantoption=false"),
- new Pair(mixedCaseUserName, "server=server1->db=functional_parquet" +
- "->grantoption=false")}) {
- User user = catalog.getAuthPolicy().getUser(userPrivilege.first);
- assertEquals(userPrivilege.first, user.getName());
- assertEquals(1, user.getPrivileges().size());
- assertNotNull(user.getPrivilege(userPrivilege.second));
- }
+ try (CatalogServiceCatalog catalog = CatalogServiceTestCatalog.createWithAuth(
+ authzConfig_.getSentryConfig())) {
+ SentryPolicyServiceStub sentryService = createSentryPolicyServiceStub(
+ authzConfig_.getSentryConfig());
+ // We grant different privileges to different users to ensure each user is
+ // granted with a distinct privilege.
+ addSentryUserPrivileges(sentryService, lowerCaseUserName, "functional");
+ addSentryUserPrivileges(sentryService, upperCaseUserName, "functional_kudu");
+ addSentryUserPrivileges(sentryService, mixedCaseUserName, "functional_parquet");
+
+ CatalogState noReset = refreshSentryAuthorization(catalog, sentryService, false);
+
+ // Impala stores the user name in case sensitive way.
+ for (Pair<String, String> userPrivilege : new Pair[]{
+ new Pair(lowerCaseUserName, "server=server1->db=functional->grantoption=false"),
+ new Pair(upperCaseUserName, "server=server1->db=functional_kudu" +
+ "->grantoption=false"),
+ new Pair(mixedCaseUserName, "server=server1->db=functional_parquet" +
+ "->grantoption=false")}) {
+ User user = catalog.getAuthPolicy().getUser(userPrivilege.first);
+ assertEquals(userPrivilege.first, user.getName());
+ assertEquals(1, user.getPrivileges().size());
+ assertNotNull(user.getPrivilege(userPrivilege.second));
+ }
- CatalogState reset = refreshSentryAuthorization(catalog, sentryService, true);
- checkCatalogState(noReset, reset);
+ CatalogState reset = refreshSentryAuthorization(catalog, sentryService, true);
+ checkCatalogState(noReset, reset);
+ }
}
private static void addCatalogPrincipalPrivileges(TPrincipalType type,
@@ -601,13 +601,14 @@ public class SentryProxyTest {
private void withAllPrincipalTypes(Consumer<SentryProxyTestContext> consumer) {
for (TPrincipalType type: TPrincipalType.values()) {
- CatalogServiceCatalog catalog = CatalogServiceTestCatalog.createWithAuth(
- authzConfig_.getSentryConfig());
- SentryPolicyService sentryService = sentryService_;
- if (type == TPrincipalType.USER) {
- sentryService = createSentryPolicyServiceStub(authzConfig_.getSentryConfig());
+ try (CatalogServiceCatalog catalog = CatalogServiceTestCatalog.createWithAuth(
+ authzConfig_.getSentryConfig())) {
+ SentryPolicyService sentryService = sentryService_;
+ if (type == TPrincipalType.USER) {
+ sentryService = createSentryPolicyServiceStub(authzConfig_.getSentryConfig());
+ }
+ consumer.accept(new SentryProxyTestContext(type, catalog, sentryService));
}
- consumer.accept(new SentryProxyTestContext(type, catalog, sentryService));
}
}
}
\ No newline at end of file