Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11280 )
Change subject: IMPALA-7469. Invalidate LocalCatalog cache based on topic updates ...................................................................... Patch Set 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/11280/5/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/11280/5/be/src/catalog/catalog-server.cc@49 PS5, Line 49: impalads > ... all impalad coordinators ... Yea... maybe the names are not so good. I didn't want to use "v1" and "v2" because they felt a little bit internal vs actually descriptive. In theory, yea, the v2 impalad could use the "v1" catalog objects to perform invalidations. But, it's set to only subscribe to the v2 prefix, and making that dynamic seemed like more effort than it's worth. Any suggestions how to resolve this complication? http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1996 PS5, Line 1996: TCatalogObject objectDesc = Preconditions.checkNotNull(req.object_desc, turns out I have to remove this so that the fe tests work, since normally developers will be running in the default mode and the fe tests contact the running catalogd. I think that's OK, though, since I already opened a JIRA so that the impalad will wait on startup until it gets a catalog update, even in 'v2' mode. So, if you misconfigure it, the impalad won't start since it won't get any catalog updates. http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@712 PS5, Line 712: if (obj.type == TCatalogObjectType.CATALOG) { > if this is a statestore-update-only branch, please add a comment. Done http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@737 PS5, Line 737: minVersion > per the interface comment, the return here is to "implement SYNC_DDL". here hmm, I could have sworn I changed this comment, gues si must have forgotten to commit or something. The limitation is actually all INVALIDATE METADATA, because it turns out the impalad also uses this to implement its *local* waiting to make INVALIDATE METADATA synchronous.. Will update comments. http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@738 PS5, Line 738: return > add a reminder that this is ignored for the ddl case. done http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@745 PS5, Line 745: different places > what does this mean? Clarified http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@763 PS5, Line 763: // an issue in practice, so deferring it to later clean-up. > perhaps associate the catalog service id with the object, so that it can be nice idea. I added that to this comment (still want to address separately since it's a rare race with not too bad ramifications) http://gerrit.cloudera.org:8080/#/c/11280/5/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@967 PS5, Line 967: final long version_; > so for the current scheme, if a table comment is modified, its version will yea, unfortunately updating a table comment would mean that we have to refetch everything about the table. The idea of making parittions immutable could avoid having to re-fetch all the partitions, at least. -- To view, visit http://gerrit.cloudera.org:8080/11280 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I615f9e6bd167b36cd8d93da59426dd6813ae4984 Gerrit-Change-Number: 11280 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Sat, 01 Sep 2018 00:40:46 +0000 Gerrit-HasComments: Yes
