Vuk Ercegovac 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 2: (5 comments) comments from initial reading. http://gerrit.cloudera.org:8080/#/c/11280/2/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/11280/2/common/thrift/CatalogObjects.thrift@621 PS2, Line 621: perhaps it makes more sense to put these in CatalogService.thrift? http://gerrit.cloudera.org:8080/#/c/11280/2/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/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@429 PS2, Line 429: delete is delete ignored for this case? http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/Table.java@411 PS2, Line 411: // TODO(todd): this breaks test_ddl.test_alter_set_column_stats. Is this the fix here to make that test work, or is that test in a currently non-working state (I didn't see it xfailed)? http://gerrit.cloudera.org:8080/#/c/11280/2/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/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@699 PS2, Line 699: ineffective how is MAX_VALUE handled? is it ignored as a special case? from the description, it would seem that this impalad would wait until it sees MAX_VALUE minVersion, which seems odd. http://gerrit.cloudera.org:8080/#/c/11280/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@970 PS2, Line 970: TODO(todd): cu this change looks like it covers this todo. -- 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: 2 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Sat, 25 Aug 2018 01:49:29 +0000 Gerrit-HasComments: Yes