Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-1683: Allow REFRESH on a single partition ......................................................................
Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/3385/10/fe/src/main/java/com/cloudera/impala/analysis/ResetMetadataStmt.java File fe/src/main/java/com/cloudera/impala/analysis/ResetMetadataStmt.java: Line 53: public void analyze(Analyzer analyzer) throws AnalysisException { This seems to allow "invalidate metadata <part_spec>" as well. I think its better to add a preconditions check / throw Analysis Exception in such a case. http://gerrit.cloudera.org:8080/#/c/3385/10/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: Line 1175: */ Can you explain a little on the return 'tbl' of this method? Line 1180: long newCatalogVersion = incrementAndGetCatalogVersion(); - I'm not sure on this one but incrementAndGetCatalogVersion() seems to be internally taking catalogLock_.writeLock().lock(). Given you are taking it at L1178 seems to be deadlock. Interestingly there are multiple other places in the code that are doing this. Even there are callers that don't take this lock. So its inconsistent and not related to your patch. - Dimitris/Alex: Can you please take a look at this once. http://gerrit.cloudera.org:8080/#/c/3385/10/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: Line 2752: if (req.isSetPartition_spec()) { Same Preconditions check as my comment in Analyzer (to prevent "invalidate metadata <part_spec>". Reason being we have clients like BDR that uses this Catalog API rather than going via frontend Analyzer. -- To view, visit http://gerrit.cloudera.org:8080/3385 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bf5dd630f2e175be5800187bbf9337c91f82bd8 Gerrit-PatchSet: 10 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
