> On June 29, 2015, 4:55 p.m., Chaoyu Tang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line > > 3485 > > <https://reviews.apache.org/r/35803/diff/3/?file=993090#file993090line3485> > > > > Should listPrincipalGlobalGrants be passed in a queryWrapper as well? > > Aihua Xu wrote: > listPrincipalGlobalGrants has handled the releasing the resource by > itself, so we don't need to pass that in. > > Chaoyu Tang wrote: > In local copy of code, the query in this method is not self closed (line > 4395) > {code} > @SuppressWarnings("unchecked") > @Override > public List<MGlobalPrivilege> listPrincipalGlobalGrants(String > principalName, PrincipalType principalType) { > boolean commited = false; > List<MGlobalPrivilege> userNameDbPriv = null; > try { > openTransaction(); > if (principalName != null) { > Query query = pm.newQuery(MGlobalPrivilege.class, > "principalName == t1 && principalType == t2 "); > query.declareParameters( > "java.lang.String t1, java.lang.String t2"); > userNameDbPriv = (List<MGlobalPrivilege>) query > .executeWithArray(principalName, principalType.toString()); > pm.retrieveAll(userNameDbPriv); > } > commited = commitTransaction(); > } finally { > if (!commited) { > rollbackTransaction(); > } > } > return userNameDbPriv; > } > {code}
My patch will close the query. See line 4493. > On June 29, 2015, 4:55 p.m., Chaoyu Tang wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line > > 3477 > > <https://reviews.apache.org/r/35803/diff/3/?file=993090#file993090line3477> > > > > Do we have to use a list of queryWrappers with each wrapper for a > > different listXXX call? If so, we need open eight queries for this > > removeRole methods. Could we use only one and reuse it? > > Aihua Xu wrote: > Originally we were already using eight queries and I was not trying to > change that logic in this patch. And also we can't reuse the same query since > we need to release the resource for each of the query anyway. > > Chaoyu Tang wrote: > What I meant reuse is that to use only one queryWrapper and close it > after each call to listXXX, so there will be at most one open query (cursor) > at a time in this method and help the scalability. Yeah. We can do that. - Aihua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35803/#review89749 ----------------------------------------------------------- On June 26, 2015, 5:23 p.m., Aihua Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35803/ > ----------------------------------------------------------- > > (Updated June 26, 2015, 5:23 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > HIVE-10895 ObjectStore does not close Query objects in some calls, causing a > potential leak in some metastore db resources > > > Diffs > ----- > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 417ecc8 > metastore/src/java/org/apache/hadoop/hive/metastore/tools/HiveMetaTool.java > d0ff329 > metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/35803/diff/ > > > Testing > ------- > > Testing has been done. > > > Thanks, > > Aihua Xu > >