commit a0497232b2675c1c289e9fb65dda63bb2d7f1637 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 6b217516bc74c612348b8edeca077dfbdbdb1a40..e4c53df1a99a5a882fb1c6a9d18ef3f27afe5f44 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(); - } + rollbackAndCleanup(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(); + rollbackAndCleanup(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(); - } + rollbackAndCleanup(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(); - } + rollbackAndCleanup(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(); - } + rollbackAndCleanup(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(); - } + rollbackAndCleanup(success, query); } return success; } @@ -1234,12 +1206,7 @@ public Table getTable(String dbName, String tableName) throws MetaException { } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return tbls; } @@ -1271,12 +1238,7 @@ private int getObjectCount(String fieldName, String objName) { result = (Long) query.execute(); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return result.intValue(); } @@ -1314,12 +1276,7 @@ private int getObjectCount(String fieldName, String objName) { } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return metas; } @@ -1405,12 +1362,7 @@ private AttachedMTableInfo getMTable(String db, String table, boolean retrieveCD } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } nmtbl.mtbl = mtbl; return nmtbl; @@ -1453,15 +1405,10 @@ private MTable getMTable(String db, String table) { } committed = commitTransaction(); } finally { - if (!committed) { - rollbackTransaction(); - } + rollbackAndCleanup(committed, query); if (dbExistsQuery != null) { dbExistsQuery.closeAll(); } - if (query != null) { - query.closeAll(); - } } return tables; } @@ -1979,12 +1926,7 @@ private MPartition getMPartition(String dbName, String tableName, List p } } } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return ret; } @@ -2216,10 +2158,7 @@ private boolean dropPartitionCommon(MPartition part) throws NoSuchObjectExceptio success = commitTransaction(); return parts; } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackAndCleanup(success, queryWrapper); } } @@ -2321,6 +2260,7 @@ public Partition getPartitionWithAuth(String dbName, String tblName, for (Iterator i = names.iterator(); i.hasNext();) { pns.add((String) i.next()); } + if (query != null) { query.closeAll(); } @@ -2415,10 +2355,7 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackAndCleanup(success, queryWrapper); } return partitions; } @@ -2440,10 +2377,7 @@ private Collection getPartitionPsQueryResults(String dbName, String tableName, } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackAndCleanup(success, queryWrapper); } return partitionNames; } @@ -3209,12 +3143,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(); - } + rollbackAndCleanup(success, query); } return tableNames; } @@ -3260,12 +3189,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(); - } + rollbackAndCleanup(success, query); } return partNames; } @@ -3484,10 +3408,7 @@ private void removeUnusedColumnDescriptor(MColumnDescriptor oldCD) { success = commitTransaction(); LOG.debug("successfully deleted a CD in removeUnusedColumnDescriptor"); } finally { - if (!success) { - rollbackTransaction(); - } - queryWrapper.close(); + rollbackAndCleanup(success, queryWrapper); } } @@ -3522,10 +3443,11 @@ private void preDropStorageDescriptor(MStorageDescriptor msd) { QueryWrapper queryWrapper) { boolean success = false; List sds = null; + Query query = null; try { openTransaction(); LOG.debug("Executing listStorageDescriptorsWithCD"); - Query query = queryWrapper.query = pm.newQuery(MStorageDescriptor.class, "this.cd == inCD"); + query = queryWrapper.query = pm.newQuery(MStorageDescriptor.class, "this.cd == inCD"); query.declareParameters("MColumnDescriptor inCD"); if (maxSDs >= 0) { // User specified a row limit, set it on the Query @@ -3537,9 +3459,7 @@ private void preDropStorageDescriptor(MStorageDescriptor msd) { success = commitTransaction(); LOG.debug("Done retrieving all objects for listStorageDescriptorsWithCD"); } finally { - if (!success) { - rollbackTransaction(); - } + rollbackAndCleanup(success, query); } return sds; } @@ -3571,12 +3491,7 @@ private boolean constraintNameAlreadyExists(String name) { constraintNameIfExists = (String) constraintExistsQuery.execute(name); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (constraintExistsQuery != null) { - constraintExistsQuery.closeAll(); - } + rollbackAndCleanup(commited, constraintExistsQuery); } return constraintNameIfExists != null && !constraintNameIfExists.isEmpty(); } @@ -3824,12 +3739,7 @@ private MIndex getMIndex(String dbName, String originalTblName, String indexName pm.retrieve(midx); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return midx; } @@ -3892,12 +3802,7 @@ private Index convertToIndex(MIndex mIndex) throws MetaException { return indexes; } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(success, query); } } @@ -3924,12 +3829,7 @@ private Index convertToIndex(MIndex mIndex) throws MetaException { } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(success, query); } return pns; } @@ -4052,12 +3952,7 @@ private MRoleMap getMSecurityUserRoleMap(String userName, PrincipalType principa pm.retrieve(mRoleMember); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return mRoleMember; } @@ -4126,11 +4021,7 @@ public boolean removeRole(String roleName) throws MetaException, } success = commitTransaction(); } finally { - if (!success) { - rollbackTransaction(); - } - - queryWrapper.close(); + rollbackAndCleanup(success, queryWrapper); } return success; } @@ -4200,12 +4091,7 @@ private void getAllRoleAncestors(Set processedRoleNames, List LOG.debug("Done retrieving all objects for listRoles"); } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(success, query); } if (principalType == PrincipalType.USER) { @@ -4262,11 +4148,12 @@ private void getAllRoleAncestors(Set processedRoleNames, List QueryWrapper queryWrapper) { boolean success = false; List mRoleMemebership = null; + Query query = null; try { LOG.debug("Executing listMSecurityPrincipalMembershipRole"); openTransaction(); - Query query = queryWrapper.query = pm.newQuery(MRoleMap.class, "principalName == t1 && principalType == t2"); + query = queryWrapper.query = pm.newQuery(MRoleMap.class, "principalName == t1 && principalType == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); mRoleMemebership = (List) query.execute(roleName, principalType.toString()); pm.retrieveAll(mRoleMemebership); @@ -4274,9 +4161,7 @@ private void getAllRoleAncestors(Set processedRoleNames, List LOG.debug("Done retrieving all objects for listMSecurityPrincipalMembershipRole"); } finally { - if (!success) { - rollbackTransaction(); - } + rollbackAndCleanup(success, query); } return mRoleMemebership; } @@ -4305,12 +4190,7 @@ private MRole getMRole(String roleName) { pm.retrieve(mrole); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return mrole; } @@ -4332,12 +4212,7 @@ private MRole getMRole(String roleName) { success = commitTransaction(); return roleNames; } finally { - if (!success) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(success, query); } } @@ -5163,12 +5038,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(); - } + rollbackAndCleanup(success, query); } return mRoleMemeberList; } @@ -5219,12 +5089,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) userNameDbPriv.addAll(mPrivs); } } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return userNameDbPriv; } @@ -5264,12 +5129,7 @@ public boolean revokePrivileges(PrivilegeBag privileges, boolean grantOption) commited = commitTransaction(); return convertGlobal(userNameDbPriv); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } } @@ -5312,12 +5172,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(); - } + rollbackAndCleanup(success, query); } return mSecurityDBList; } @@ -5440,12 +5295,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(); - } + rollbackAndCleanup(success, query); } return mSecurityTabList; } @@ -5472,12 +5322,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(); - } + rollbackAndCleanup(success, query); } return mSecurityTabPartList; } @@ -5505,12 +5350,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(); - } + rollbackAndCleanup(success, query); } return mTblColPrivilegeList; } @@ -5539,12 +5379,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(); - } + rollbackAndCleanup(success, query); } return mSecurityColList; } @@ -5587,12 +5422,12 @@ public void dropPartitionAllColumnGrantsNoTxn( private List listDatabaseGrants(String dbName, QueryWrapper queryWrapper) { dbName = HiveStringUtils.normalizeIdentifier(dbName); boolean success = false; - + Query query = null; try { LOG.debug("Executing listDatabaseGrants"); openTransaction(); - Query query = queryWrapper.query = pm.newQuery(MDBPrivilege.class, "database.name == t1"); + query = queryWrapper.query = pm.newQuery(MDBPrivilege.class, "database.name == t1"); query.declareParameters("java.lang.String t1"); List mSecurityDBList = (List) query.executeWithArray(dbName); pm.retrieveAll(mSecurityDBList); @@ -5600,9 +5435,7 @@ public void dropPartitionAllColumnGrantsNoTxn( LOG.debug("Done retrieving all objects for listDatabaseGrants"); return mSecurityDBList; } finally { - if (!success) { - rollbackTransaction(); - } + rollbackAndCleanup(success, query); } } @@ -5695,12 +5528,7 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List mSecurityTabPartList = null; + Query query = null; try { LOG.debug("Executing listPrincipalAllTableGrants"); openTransaction(); - Query query = queryWrapper.query = pm.newQuery(MTablePrivilege.class, + query = queryWrapper.query = pm.newQuery(MTablePrivilege.class, "principalName == t1 && principalType == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); mSecurityTabPartList = (List) query.execute( @@ -6044,9 +5848,7 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List mSecurityTabPartList = null; + Query query = null; try { openTransaction(); LOG.debug("Executing listPrincipalAllPartitionGrants"); - Query query = queryWrapper.query = pm.newQuery(MPartitionPrivilege.class, "principalName == t1 && principalType == t2"); + query = queryWrapper.query = pm.newQuery(MPartitionPrivilege.class, "principalName == t1 && principalType == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); mSecurityTabPartList = (List) query.execute(principalName, principalType.toString()); @@ -6148,9 +5941,7 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List mSecurityColumnList = null; + Query query = null; try { LOG.debug("Executing listPrincipalAllTableColumnGrants"); openTransaction(); - Query query = queryWrapper.query = + query = queryWrapper.query = pm.newQuery(MTableColumnPrivilege.class, "principalName == t1 && principalType == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); mSecurityColumnList = @@ -6261,9 +6043,7 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List mSecurityColumnList = null; + Query query = null; try { LOG.debug("Executing listPrincipalAllTableColumnGrants"); openTransaction(); - Query query = queryWrapper.query = + query = queryWrapper.query = pm.newQuery(MPartitionColumnPrivilege.class, "principalName == t1 && principalType == t2"); query.declareParameters("java.lang.String t1, java.lang.String t2"); mSecurityColumnList = @@ -6372,9 +6143,7 @@ private void dropPartitionGrantsNoTxn(String dbName, String tableName, List partName) throws In public Collection executeJDOQLSelect(String queryStr, QueryWrapper queryWrapper) { boolean committed = false; Collection result = null; - + Query query = null; try { openTransaction(); - Query query = queryWrapper.query = pm.newQuery(queryStr); + query = queryWrapper.query = pm.newQuery(queryStr); result = ((Collection) query.execute()); committed = commitTransaction(); @@ -6479,9 +6243,7 @@ private String getPartitionStr(Table tbl, Map partName) throws In return null; } } finally { - if (!committed) { - rollbackTransaction(); - } + rollbackAndCleanup(committed, queryWrapper); } } @@ -6507,12 +6269,7 @@ public long executeJDOQLUpdate(String queryStr) { return -1; } } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -6542,12 +6299,7 @@ public long executeJDOQLUpdate(String queryStr) { return null; } } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -6658,12 +6410,7 @@ public UpdateMDatabaseURIRetVal updateMDatabaseURI(URI oldLoc, URI newLoc, boole } return retVal; } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -6751,12 +6498,7 @@ public UpdatePropURIRetVal updateTblPropURI(URI oldLoc, URI newLoc, String tblPr } return retVal; } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -6790,12 +6532,7 @@ public UpdatePropURIRetVal updateMStorageDescriptorTblPropURI(URI oldLoc, URI ne } return retVal; } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -6886,12 +6623,7 @@ public UpdateMStorageDescriptorTblURIRetVal updateMStorageDescriptorTblURI(URI o } return retVal; } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -6968,12 +6700,7 @@ public UpdateSerdeURIRetVal updateSerdeURI(URI oldLoc, URI newLoc, String serdeP } return retVal; } finally { - if (!committed) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(committed, query); } } @@ -7184,13 +6911,13 @@ public boolean updatePartitionColumnStatistics(ColumnStatistics colStats, List result = null; validateTableCols(table, colNames); - Query query = queryWrapper.query = pm.newQuery(MTableColumnStatistics.class); + query = queryWrapper.query = pm.newQuery(MTableColumnStatistics.class); String filter = "tableName == t1 && dbName == t2 && ("; String paramStr = "java.lang.String t1, java.lang.String t2"; Object[] params = new Object[colNames.size() + 2]; @@ -7219,9 +6946,7 @@ public boolean updatePartitionColumnStatistics(ColumnStatistics colStats, List getAllFunctions() throws MetaException { boolean commited = false; + Query query = null; try { openTransaction(); - Query query = pm.newQuery(MFunction.class); + query = pm.newQuery(MFunction.class); List allFunctions = (List) query.execute(); pm.retrieveAll(allFunctions); commited = commitTransaction(); return convertToFunctions(allFunctions); } finally { - if (!commited) { - rollbackTransaction(); - } + rollbackAndCleanup(commited, query); } } @@ -8249,12 +7929,7 @@ public Function getFunction(String dbName, String funcName) throws MetaException } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return funcs; } @@ -8284,11 +7959,8 @@ public NotificationEventResponse getNextNotification(NotificationEventRequest rq } return result; } finally { - if (query != null) { - query.closeAll(); - } if (!commited) { - rollbackTransaction(); + rollbackAndCleanup(commited, query); return null; } } @@ -8318,12 +7990,7 @@ public void addNotificationEvent(NotificationEvent entry) { pm.makePersistent(translateThriftToDb(entry)); commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } } @@ -8343,12 +8010,7 @@ public void cleanNotificationEvents(int olderThan) { } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } } @@ -8367,12 +8029,7 @@ public CurrentNotificationEventId getCurrentNotificationEventId() { commited = commitTransaction(); return new CurrentNotificationEventId(id); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } } @@ -8591,12 +8248,7 @@ private static long clearFieldMap(ClassLoaderResolverImpl clri, String mapFieldN } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return primaryKeys; } @@ -8621,12 +8273,7 @@ private String getPrimaryKeyConstraintName(String db_name, String tbl_name) thro } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return ret; } @@ -8749,12 +8396,7 @@ private String getPrimaryKeyConstraintName(String db_name, String tbl_name) thro } commited = commitTransaction(); } finally { - if (!commited) { - rollbackTransaction(); - } - if (query != null) { - query.closeAll(); - } + rollbackAndCleanup(commited, query); } return foreignKeys; } @@ -8782,4 +8424,43 @@ public void dropConstraint(String dbName, String tableName, } } + /** + * This is a cleanup method which is used to rollback a active transaction + * if the success flag is false and close the associated Query object. This method is used + * internally and visible for testing purposes only + * @param success Rollback the current active transaction if false + * @param query Query object which needs to be closed + */ + @VisibleForTesting + void rollbackAndCleanup(boolean success, Query query) { + try { + if(!success) { + rollbackTransaction(); + } + } finally { + if (query != null) { + query.closeAll(); + } + } + } + + /** + * This is a cleanup method which is used to rollback a active transaction + * if the success flag is false and close the associated QueryWrapper object. This method is used + * internally and visible for testing purposes only + * @param success Rollback the current active transaction if false + * @param queryWrapper QueryWrapper object which needs to be closed + */ + @VisibleForTesting + void rollbackAndCleanup(boolean success, QueryWrapper queryWrapper) { + try { + if(!success) { + rollbackTransaction(); + } + } finally { + if (queryWrapper != null) { + queryWrapper.close(); + } + } + } } diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java b/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java index 9b8eaf2ab72fcf1fe0d58e8913ac1b84ec7e71cf..cd14f1420597a957ae15b15dad3c7a9d40c8d377 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java @@ -59,9 +59,12 @@ import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.jdo.Query; + public class TestObjectStore { private ObjectStore objectStore = null; @@ -405,4 +408,15 @@ public static void dropAllStoreObjects(RawStore store) throws MetaException, Inv } catch (NoSuchObjectException e) { } } + + @Test + public void testQueryCloseOnError() throws Exception { + ObjectStore spy = Mockito.spy(objectStore); + spy.getAllDatabases(); + spy.getAllFunctions(); + spy.getAllTables(DB1); + spy.getPartitionCount(); + Mockito.verify(spy, Mockito.times(4)) + .rollbackAndCleanup(Mockito.anyBoolean(), Mockito.anyObject()); + } }