Marcell Szabo has posted comments on this change. Change subject: IMPALA-2518: DROP DATABASE CASCADE does not remove cache directives of tables ......................................................................
Patch Set 6: (10 comments) Sorry for the late reply. http://gerrit.cloudera.org:8080/#/c/1966/6/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: Line 1092: * internal cache. Re-throws any Hive Meta Store exceptions encountered during > update comment to reflect additional behavior Done Line 1125: for(String tableName : removedDb.getAllTableNames()) { > formatting Done Line 1183: ! > nit: "." Done Line 1183: * Does not update partition metadata! > but updates table md? Done Line 1188: if (hdfsTable.isMarkedCached()) { > is it possible for this to be set and #clustering-cols to be > 0? if not, a I've looked at HdfsTable for reference, but there doesn't seem to be any checks that indicate that hdfsTable.isMarkedCached() and clustering-cols are mutually exclusive. If they are indeed exclusive, I'd start adding the checks in HdfsTable, so they fail fast on generic metadata load (and not on the rare events when the uncacheTable() is called). Line 1190: HdfsCachingUtil.uncacheTbl(table.getMetaStoreTable()); > the function comment for uncacheTbl() says not to call this for partitioned Done Line 1201: LOG.error("Unable to uncache partition: " + > single line? Done http://gerrit.cloudera.org:8080/#/c/1966/6/fe/src/main/java/com/cloudera/impala/util/HdfsCachingUtil.java File fe/src/main/java/com/cloudera/impala/util/HdfsCachingUtil.java: Line 127: * note: If table has partitions, call uncachePartition() instead of uncacheTbl() > if that's a prerequisite, you should have a checkState() in here. Done Line 129: public static void uncacheTbl(org.apache.hadoop.hive.metastore.api.Table table) > the function name isn't good: you're not uncaching a table, you're simply r Done Line 134: if (id == null) return; > are you saying that if it's partitioned, there won't be a caching directive Under normal circumstances this should be the case. E.g. CatalogOpExecutor.alterTableSetCached skips the default partition for partitioned tables. -- To view, visit http://gerrit.cloudera.org:8080/1966 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I58270e1be49e71a08e12021e7dddab01969d1810 Gerrit-PatchSet: 6 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Marcell Szabo <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Juan Yu <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Marcell Szabo <[email protected]> Gerrit-HasComments: Yes
