You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by sp...@apache.org on 2017/05/02 15:29:21 UTC
hive git commit: HIVE-16213: ObjectStore can leak Queries when
rollbackTransaction throws an exception (Vihang Karajgaonkar,
reviewed by Sergio Pena)
Repository: hive
Updated Branches:
refs/heads/master 53b70bdc3 -> 812fa3946
HIVE-16213: ObjectStore can leak Queries when rollbackTransaction throws an exception (Vihang Karajgaonkar, reviewed by Sergio Pena)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/812fa394
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/812fa394
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/812fa394
Branch: refs/heads/master
Commit: 812fa39463a49a6391b5f82c6bfbac589573c500
Parents: 53b70bd
Author: Vihang Karajgaonkar <vi...@cloudera.com>
Authored: Tue May 2 10:27:51 2017 -0500
Committer: Sergio Pena <se...@cloudera.com>
Committed: Tue May 2 10:29:04 2017 -0500
----------------------------------------------------------------------
.../hadoop/hive/metastore/ObjectStore.java | 550 ++++---------------
.../hadoop/hive/metastore/TestObjectStore.java | 14 +
2 files changed, 132 insertions(+), 432 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/812fa394/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
----------------------------------------------------------------------
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 58b7730..29aa642 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -234,26 +234,22 @@ public class ObjectStore implements RawStore, Configurable {
private Pattern partitionValidationPattern;
/**
- * A class to pass the Query object to the caller to let the caller release
- * resources by calling QueryWrapper.query.closeAll() after consuming all the query results.
+ * A Autocloseable wrapper around Query class to pass the Query object to the caller and let the caller release
+ * the resources when the QueryWrapper goes out of scope
*/
- public static class QueryWrapper {
+ public static class QueryWrapper implements AutoCloseable {
public Query query;
/**
* Explicitly closes the query object to release the resources
*/
+ @Override
public void close() {
if (query != null) {
query.closeAll();
query = null;
}
}
-
- @Override
- protected void finalize() {
- this.close();
- }
}
public ObjectStore() {
@@ -700,12 +696,7 @@ public class ObjectStore implements RawStore, Configurable {
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 +815,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return success;
}
@@ -858,12 +846,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return databases;
}
@@ -883,12 +866,7 @@ public class ObjectStore implements RawStore, Configurable {
databases = new ArrayList<String>((Collection<String>) query.execute());
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
Collections.sort(databases);
return databases;
@@ -956,12 +934,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return type;
}
@@ -985,12 +958,7 @@ public class ObjectStore implements RawStore, Configurable {
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 +1202,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return tbls;
}
@@ -1271,12 +1234,7 @@ public class ObjectStore implements RawStore, Configurable {
result = (Long) query.execute();
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return result.intValue();
}
@@ -1314,12 +1272,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return metas;
}
@@ -1405,12 +1358,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
nmtbl.mtbl = mtbl;
return nmtbl;
@@ -1453,15 +1401,10 @@ public class ObjectStore implements RawStore, Configurable {
}
committed = commitTransaction();
} finally {
- if (!committed) {
- rollbackTransaction();
- }
+ rollbackAndCleanup(committed, query);
if (dbExistsQuery != null) {
dbExistsQuery.closeAll();
}
- if (query != null) {
- query.closeAll();
- }
}
return tables;
}
@@ -1979,12 +1922,7 @@ public class ObjectStore implements RawStore, Configurable {
}
}
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return ret;
}
@@ -2216,10 +2154,7 @@ public class ObjectStore implements RawStore, Configurable {
success = commitTransaction();
return parts;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
}
@@ -2321,6 +2256,7 @@ public class ObjectStore implements RawStore, Configurable {
for (Iterator i = names.iterator(); i.hasNext();) {
pns.add((String) i.next());
}
+
if (query != null) {
query.closeAll();
}
@@ -2415,10 +2351,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return partitions;
}
@@ -2440,10 +2373,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return partitionNames;
}
@@ -3209,12 +3139,7 @@ public class ObjectStore implements RawStore, Configurable {
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 +3185,7 @@ public class ObjectStore implements RawStore, Configurable {
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 +3404,7 @@ public class ObjectStore implements RawStore, Configurable {
success = commitTransaction();
LOG.debug("successfully deleted a CD in removeUnusedColumnDescriptor");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
}
@@ -3571,12 +3488,7 @@ public class ObjectStore implements RawStore, Configurable {
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 +3736,7 @@ public class ObjectStore implements RawStore, Configurable {
pm.retrieve(midx);
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return midx;
}
@@ -3892,12 +3799,7 @@ public class ObjectStore implements RawStore, Configurable {
return indexes;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -3924,12 +3826,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return pns;
}
@@ -4052,12 +3949,7 @@ public class ObjectStore implements RawStore, Configurable {
pm.retrieve(mRoleMember);
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return mRoleMember;
}
@@ -4126,11 +4018,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
-
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return success;
}
@@ -4200,12 +4088,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listRoles");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
if (principalType == PrincipalType.USER) {
@@ -4271,7 +4154,6 @@ public class ObjectStore implements RawStore, Configurable {
mRoleMemebership = (List<MRoleMap>) query.execute(roleName, principalType.toString());
pm.retrieveAll(mRoleMemebership);
success = commitTransaction();
-
LOG.debug("Done retrieving all objects for listMSecurityPrincipalMembershipRole");
} finally {
if (!success) {
@@ -4305,12 +4187,7 @@ public class ObjectStore implements RawStore, Configurable {
pm.retrieve(mrole);
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return mrole;
}
@@ -4332,12 +4209,7 @@ public class ObjectStore implements RawStore, Configurable {
success = commitTransaction();
return roleNames;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -5163,12 +5035,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listRoleMembers");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mRoleMemeberList;
}
@@ -5219,12 +5086,7 @@ public class ObjectStore implements RawStore, Configurable {
userNameDbPriv.addAll(mPrivs);
}
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return userNameDbPriv;
}
@@ -5264,12 +5126,7 @@ public class ObjectStore implements RawStore, Configurable {
commited = commitTransaction();
return convertGlobal(userNameDbPriv);
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
}
@@ -5312,12 +5169,7 @@ public class ObjectStore implements RawStore, Configurable {
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 +5292,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listAllTableGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityTabList;
}
@@ -5472,12 +5319,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listTableAllPartitionGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityTabPartList;
}
@@ -5505,12 +5347,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listTableAllColumnGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mTblColPrivilegeList;
}
@@ -5539,12 +5376,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listTableAllPartitionColumnGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityColList;
}
@@ -5587,7 +5419,6 @@ public class ObjectStore implements RawStore, Configurable {
private List<MDBPrivilege> listDatabaseGrants(String dbName, QueryWrapper queryWrapper) {
dbName = HiveStringUtils.normalizeIdentifier(dbName);
boolean success = false;
-
try {
LOG.debug("Executing listDatabaseGrants");
@@ -5695,12 +5526,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listAllTableGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityTabPartList;
}
@@ -5760,12 +5586,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalPartitionGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityTabPartList;
}
@@ -5829,12 +5650,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalTableColumnGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityColList;
}
@@ -5896,12 +5712,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalPartitionColumnGrants");
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return mSecurityColList;
}
@@ -5963,12 +5774,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalPartitionColumnGrantsAll");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -5996,12 +5802,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPartitionColumnGrantsAll");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6076,12 +5877,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalAllTableGrants");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6104,12 +5900,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalAllTableGrants");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6149,7 +5940,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalAllPartitionGrants");
} finally {
if (!success) {
- rollbackTransaction();
+ rollbackTransaction();
}
}
return mSecurityTabPartList;
@@ -6181,12 +5972,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalPartitionGrantsAll");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6212,12 +5998,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalPartitionGrantsAll");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6295,12 +6076,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalTableColumnGrantsAll");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6325,12 +6101,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalTableColumnGrantsAll");
return result;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6407,12 +6178,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done executing isPartitionMarkedForEvent");
return (partEvents != null && !partEvents.isEmpty()) ? true : false;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -6466,7 +6232,6 @@ public class ObjectStore implements RawStore, Configurable {
public Collection<?> executeJDOQLSelect(String queryStr, QueryWrapper queryWrapper) {
boolean committed = false;
Collection<?> result = null;
-
try {
openTransaction();
Query query = queryWrapper.query = pm.newQuery(queryStr);
@@ -6507,12 +6272,7 @@ public class ObjectStore implements RawStore, Configurable {
return -1;
}
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6542,12 +6302,7 @@ public class ObjectStore implements RawStore, Configurable {
return null;
}
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6658,12 +6413,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6751,12 +6501,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6790,12 +6535,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6886,12 +6626,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6968,12 +6703,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -7184,7 +6914,6 @@ public class ObjectStore implements RawStore, Configurable {
}
boolean committed = false;
-
try {
openTransaction();
@@ -7546,12 +7275,7 @@ public class ObjectStore implements RawStore, Configurable {
rollbackTransaction();
throw e;
} finally {
- if (!ret) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(ret, query);
}
return ret;
}
@@ -7621,12 +7345,7 @@ public class ObjectStore implements RawStore, Configurable {
rollbackTransaction();
throw e;
} finally {
- if (!ret) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(ret, query);
}
return ret;
}
@@ -7648,12 +7367,7 @@ public class ObjectStore implements RawStore, Configurable {
delCnt = query.deletePersistentAll(curTime, expiryTime);
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
LOG.debug("Done executing cleanupEvents");
}
return delCnt;
@@ -7757,12 +7471,7 @@ public class ObjectStore implements RawStore, Configurable {
return tokenIdents;
} finally {
LOG.debug("Done executing getAllTokenIdentifers with status : " + committed);
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -7805,12 +7514,7 @@ public class ObjectStore implements RawStore, Configurable {
}
committed = commitTransaction();
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
LOG.debug("Done executing updateMasterKey with status : " + committed);
if (null == masterKey) {
@@ -7838,12 +7542,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
LOG.debug("Done executing removeMasterKey with status : " + success);
return (null != masterKey) && success;
@@ -7869,12 +7568,7 @@ public class ObjectStore implements RawStore, Configurable {
return masterKeys;
} finally {
LOG.debug("Done executing getMasterKeys with status : " + committed);
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -7986,12 +7680,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return mVerTables.get(0);
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -8217,12 +7906,7 @@ public class ObjectStore implements RawStore, Configurable {
pm.retrieve(mfunc);
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return mfunc;
}
@@ -8286,12 +7970,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return funcs;
}
@@ -8322,11 +8001,8 @@ public class ObjectStore implements RawStore, Configurable {
}
return result;
} finally {
- if (query != null) {
- query.closeAll();
- }
if (!commited) {
- rollbackTransaction();
+ rollbackAndCleanup(commited, query);
return null;
}
}
@@ -8356,12 +8032,7 @@ public class ObjectStore implements RawStore, Configurable {
pm.makePersistent(translateThriftToDb(entry));
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
}
@@ -8381,12 +8052,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
}
@@ -8405,12 +8071,7 @@ public class ObjectStore implements RawStore, Configurable {
commited = commitTransaction();
return new CurrentNotificationEventId(id);
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
}
@@ -8629,12 +8290,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return primaryKeys;
}
@@ -8659,12 +8315,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return ret;
}
@@ -8787,12 +8438,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return foreignKeys;
}
@@ -8819,4 +8465,44 @@ public class ObjectStore implements RawStore, Configurable {
}
}
}
+
+ /**
+ * 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();
+ }
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/hive/blob/812fa394/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java
----------------------------------------------------------------------
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 6524ee7..69e8826 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.Assert;
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;
@@ -410,4 +413,15 @@ public class TestObjectStore {
} 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(3))
+ .rollbackAndCleanup(Mockito.anyBoolean(), Mockito.<Query>anyObject());
+ }
}