commit b776449f5df6c3ece16247e9cf1642b0755e9d97 Author: Vihang Karajgaonkar Date: Mon Apr 3 15:57:31 2017 -0700 HIVE-16213 : ObjectStore can leak Queries when rollbackTransaction throws an exception diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java index a63519a744537fb381de84e02eefed44a820b9ef..7f521d1388f35910a60f5b163f4941aa222cae97 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java @@ -700,12 +700,7 @@ private MDatabase getMDatabase(String name) throws NoSuchObjectException { pm.retrieve(mdb); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } if (mdb == null) { throw new NoSuchObjectException("There is no database named " + name); @@ -824,10 +819,7 @@ public boolean dropDatabase(String dbname) throws NoSuchObjectException, MetaExc } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackIfNotSuccessful(success, queryWrapper); } return success; } @@ -858,12 +850,7 @@ public boolean dropDatabase(String dbname) throws NoSuchObjectException, MetaExc } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return databases; } @@ -883,12 +870,7 @@ public boolean dropDatabase(String dbname) throws NoSuchObjectException, MetaExc databases = new ArrayList((Collection) query.execute()); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } Collections.sort(databases); return databases; @@ -956,12 +938,7 @@ public Type getType(String typeName) { } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return type; } @@ -985,12 +962,7 @@ public boolean dropType(String typeName) { success = commitTransaction(); LOG.debug("type not found " + typeName, e); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return success; } @@ -1231,12 +1203,7 @@ public Table getTable(String dbName, String tableName) throws MetaException { } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return tbls; } @@ -1268,12 +1235,7 @@ private int getObjectCount(String fieldName, String objName) { result = (Long) query.execute(); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return result.intValue(); } @@ -1311,12 +1273,7 @@ private int getObjectCount(String fieldName, String objName) { } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return metas; } @@ -1402,12 +1359,7 @@ private AttachedMTableInfo getMTable(String db, String table, boolean retrieveCD } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } nmtbl.mtbl = mtbl; return nmtbl; @@ -1450,15 +1402,10 @@ private MTable getMTable(String db, String table) { } committed = commitTransaction(); } finally { - if (!committed) { - rollbackTransaction(); - } + rollbackIfNotSuccessful(committed, query); if (dbExistsQuery != null) { dbExistsQuery.closeAll(); } - if (query != null) { - query.closeAll(); - } } return tables; } @@ -1976,12 +1923,7 @@ private MPartition getMPartition(String dbName, String tableName, List p } } } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return ret; } @@ -2213,10 +2155,7 @@ private boolean dropPartitionCommon(MPartition part) throws NoSuchObjectExceptio success = commitTransaction(); return parts; } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackIfNotSuccessful(success, queryWrapper); } } @@ -2412,10 +2351,7 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackIfNotSuccessful(success, queryWrapper); } return partitions; } @@ -2437,10 +2373,7 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackIfNotSuccessful(success, queryWrapper); } return partitionNames; } @@ -3206,12 +3139,7 @@ private String makeParameterDeclarationStringObj(Map params) { success = commitTransaction(); LOG.debug("Done retrieving all objects for listTableNamesByFilter"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return tableNames; } @@ -3257,12 +3185,7 @@ private String makeParameterDeclarationStringObj(Map params) { success = commitTransaction(); LOG.debug("Done retrieving all objects for listMPartitionNamesByFilter"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return partNames; } @@ -3481,10 +3404,7 @@ private void removeUnusedColumnDescriptor(MColumnDescriptor oldCD) { success = commitTransaction(); LOG.debug("successfully deleted a CD in removeUnusedColumnDescriptor"); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackIfNotSuccessful(success, queryWrapper); } } @@ -3568,12 +3488,7 @@ private boolean constraintNameAlreadyExists(String name) { constraintNameIfExists = (String) constraintExistsQuery.execute(name); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (constraintExistsQuery != null) { - constraintExistsQuery.closeAll(); - } + rollbackIfNotSuccessful(commited, constraintExistsQuery); } return constraintNameIfExists != null && !constraintNameIfExists.isEmpty(); } @@ -3821,12 +3736,7 @@ private MIndex getMIndex(String dbName, String originalTblName, String indexName pm.retrieve(midx); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return midx; } @@ -3889,12 +3799,7 @@ private Index convertToIndex(MIndex mIndex) throws MetaException { return indexes; } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } } @@ -3921,12 +3826,7 @@ private Index convertToIndex(MIndex mIndex) throws MetaException { } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return pns; } @@ -4049,12 +3949,7 @@ private MRoleMap getMSecurityUserRoleMap(String userName, PrincipalType principa pm.retrieve(mRoleMember); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return mRoleMember; } @@ -4123,11 +4018,7 @@ public boolean removeRole(String roleName) throws MetaException, } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - - queryWrapper.close(); + rollbackIfNotSuccessful(success, queryWrapper); } return success; } @@ -4197,12 +4088,7 @@ private void getAllRoleAncestors(Set processedRoleNames, List LOG.debug("Done retrieving all objects for listRoles"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } if (principalType == PrincipalType.USER) { @@ -4302,12 +4188,7 @@ private MRole getMRole(String roleName) { pm.retrieve(mrole); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return mrole; } @@ -4329,12 +4210,7 @@ private MRole getMRole(String roleName) { success = commitTransaction(); return roleNames; } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } } @@ -5160,12 +5036,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) LOG.debug("Done retrieving all objects for listRoleMembers"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return mRoleMemeberList; } @@ -5216,12 +5087,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) userNameDbPriv.addAll(mPrivs); } } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } return userNameDbPriv; } @@ -5261,12 +5127,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) commited = commitTransaction(); return convertGlobal(userNameDbPriv); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(commited, query); } } @@ -5309,12 +5170,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) mSecurityDBList.addAll(mPrivs); LOG.debug("Done retrieving all objects for listPrincipalDBGrants"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return mSecurityDBList; } @@ -5437,12 +5293,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) LOG.debug("Done retrieving all objects for listAllTableGrants"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return mSecurityTabList; } @@ -5469,12 +5320,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) LOG.debug("Done retrieving all objects for listTableAllPartitionGrants"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return mSecurityTabPartList; } @@ -5502,12 +5348,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) LOG.debug("Done retrieving all objects for listTableAllColumnGrants"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return mTblColPrivilegeList; } @@ -5536,12 +5377,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) LOG.debug("Done retrieving all objects for listTableAllPartitionColumnGrants"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackIfNotSuccessful(success, query); } return mSecurityColList; } @@ -5692,12 +5528,7 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List