You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by st...@apache.org on 2017/11/09 01:32:33 UTC
[12/14] hive git commit: HIVE-16213: ObjectStore can leak Queries
when rollbackTransaction throws an exception (Vihang Karajgaonkar,
reviewed by Sergio Pena)
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/c2b5dba7
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/c2b5dba7
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/c2b5dba7
Branch: refs/heads/branch-2.3
Commit: c2b5dba78e2ad1c9884cd4a54f7113532614b605
Parents: 145ed20
Author: Vihang Karajgaonkar <vi...@cloudera.com>
Authored: Tue May 2 10:27:51 2017 -0500
Committer: Sahil Takiar <st...@cloudera.com>
Committed: Tue Nov 7 08:15:48 2017 -0800
----------------------------------------------------------------------
.../hadoop/hive/metastore/ObjectStore.java | 549 ++++---------------
.../hadoop/hive/metastore/TestObjectStore.java | 14 +
2 files changed, 131 insertions(+), 432 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/c2b5dba7/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 358cf17..e6a918b 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;
}
@@ -1231,12 +1199,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return tbls;
}
@@ -1268,12 +1231,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();
}
@@ -1311,12 +1269,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return metas;
}
@@ -1402,12 +1355,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;
@@ -1450,15 +1398,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;
}
@@ -1976,12 +1919,7 @@ public class ObjectStore implements RawStore, Configurable {
}
}
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return ret;
}
@@ -2213,10 +2151,7 @@ public class ObjectStore implements RawStore, Configurable {
success = commitTransaction();
return parts;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
}
@@ -2318,6 +2253,7 @@ public class ObjectStore implements RawStore, Configurable {
for (Iterator i = names.iterator(); i.hasNext();) {
pns.add((String) i.next());
}
+
if (query != null) {
query.closeAll();
}
@@ -2412,10 +2348,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return partitions;
}
@@ -2437,10 +2370,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return partitionNames;
}
@@ -3206,12 +3136,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;
}
@@ -3257,12 +3182,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;
}
@@ -3481,10 +3401,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);
}
}
@@ -3568,12 +3485,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();
}
@@ -3821,12 +3733,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;
}
@@ -3889,12 +3796,7 @@ public class ObjectStore implements RawStore, Configurable {
return indexes;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -3921,12 +3823,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
return pns;
}
@@ -4049,12 +3946,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;
}
@@ -4123,11 +4015,7 @@ public class ObjectStore implements RawStore, Configurable {
}
success = commitTransaction();
} finally {
- if (!success) {
- rollbackTransaction();
- }
-
- queryWrapper.close();
+ rollbackAndCleanup(success, queryWrapper);
}
return success;
}
@@ -4197,12 +4085,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) {
@@ -4268,7 +4151,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) {
@@ -4302,12 +4184,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;
}
@@ -4329,12 +4206,7 @@ public class ObjectStore implements RawStore, Configurable {
success = commitTransaction();
return roleNames;
} finally {
- if (!success) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(success, query);
}
}
@@ -5160,12 +5032,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;
}
@@ -5216,12 +5083,7 @@ public class ObjectStore implements RawStore, Configurable {
userNameDbPriv.addAll(mPrivs);
}
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return userNameDbPriv;
}
@@ -5261,12 +5123,7 @@ public class ObjectStore implements RawStore, Configurable {
commited = commitTransaction();
return convertGlobal(userNameDbPriv);
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
}
@@ -5309,12 +5166,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;
}
@@ -5437,12 +5289,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;
}
@@ -5469,12 +5316,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;
}
@@ -5502,12 +5344,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;
}
@@ -5536,12 +5373,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;
}
@@ -5584,7 +5416,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");
@@ -5692,12 +5523,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;
}
@@ -5757,12 +5583,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;
}
@@ -5826,12 +5647,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;
}
@@ -5893,12 +5709,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;
}
@@ -5960,12 +5771,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);
}
}
@@ -5993,12 +5799,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);
}
}
@@ -6073,12 +5874,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);
}
}
@@ -6101,12 +5897,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);
}
}
@@ -6146,7 +5937,7 @@ public class ObjectStore implements RawStore, Configurable {
LOG.debug("Done retrieving all objects for listPrincipalAllPartitionGrants");
} finally {
if (!success) {
- rollbackTransaction();
+ rollbackTransaction();
}
}
return mSecurityTabPartList;
@@ -6178,12 +5969,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);
}
}
@@ -6209,12 +5995,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);
}
}
@@ -6292,12 +6073,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);
}
}
@@ -6322,12 +6098,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);
}
}
@@ -6404,12 +6175,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);
}
}
@@ -6463,7 +6229,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);
@@ -6504,12 +6269,7 @@ public class ObjectStore implements RawStore, Configurable {
return -1;
}
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6539,12 +6299,7 @@ public class ObjectStore implements RawStore, Configurable {
return null;
}
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6655,12 +6410,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6748,12 +6498,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6787,12 +6532,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6883,12 +6623,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -6965,12 +6700,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return retVal;
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -7181,7 +6911,6 @@ public class ObjectStore implements RawStore, Configurable {
}
boolean committed = false;
-
try {
openTransaction();
@@ -7506,12 +7235,7 @@ public class ObjectStore implements RawStore, Configurable {
rollbackTransaction();
throw e;
} finally {
- if (!ret) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(ret, query);
}
return ret;
}
@@ -7581,12 +7305,7 @@ public class ObjectStore implements RawStore, Configurable {
rollbackTransaction();
throw e;
} finally {
- if (!ret) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(ret, query);
}
return ret;
}
@@ -7608,12 +7327,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;
@@ -7717,12 +7431,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);
}
}
@@ -7765,12 +7474,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) {
@@ -7798,12 +7502,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;
@@ -7829,12 +7528,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);
}
}
@@ -7946,12 +7640,7 @@ public class ObjectStore implements RawStore, Configurable {
}
return mVerTables.get(0);
} finally {
- if (!committed) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(committed, query);
}
}
@@ -8177,12 +7866,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;
}
@@ -8246,12 +7930,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return funcs;
}
@@ -8282,11 +7961,8 @@ public class ObjectStore implements RawStore, Configurable {
}
return result;
} finally {
- if (query != null) {
- query.closeAll();
- }
if (!commited) {
- rollbackTransaction();
+ rollbackAndCleanup(commited, query);
return null;
}
}
@@ -8316,12 +7992,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);
}
}
@@ -8341,12 +8012,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
}
@@ -8365,12 +8031,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);
}
}
@@ -8587,12 +8248,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return primaryKeys;
}
@@ -8617,12 +8273,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return ret;
}
@@ -8741,12 +8392,7 @@ public class ObjectStore implements RawStore, Configurable {
}
commited = commitTransaction();
} finally {
- if (!commited) {
- rollbackTransaction();
- }
- if (query != null) {
- query.closeAll();
- }
+ rollbackAndCleanup(commited, query);
}
return foreignKeys;
}
@@ -8774,4 +8420,43 @@ 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/c2b5dba7/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());
+ }
}