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

Reply via email to