Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13664 )

Change subject: IMPALA-7534. Handle invalidation races in CatalogdMetaProvider
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13664/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/13664/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@427
PS2, Line 427:   private <CacheKeyType, ValueType> ValueType 
loadWithCaching(String itemString,
> I think this could use a method comment since the logic is non-trivial (pro
Ack


http://gerrit.cloudera.org:8080/#/c/13664/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@435
PS2, Line 435:       Object inCache = cache_.get(key, () -> f);
> Just for my understanding, here we are relying on the inherent thread-safet
yea, will add a comment


http://gerrit.cloudera.org:8080/#/c/13664/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@573
PS2, Line 573:             Uninterruptibles.sleepUninterruptibly(10, 
TimeUnit.MILLISECONDS);
> Remove?
woops!


http://gerrit.cloudera.org:8080/#/c/13664/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@687
PS2, Line 687: RuntimeException
> Does this mean the query is not eventually retried?
Do you know if we ever interrupt threads? Otherwise I don't think this 
introduces any actual possibility for an exception.



--
To view, visit http://gerrit.cloudera.org:8080/13664
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
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: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 19:21:32 +0000
Gerrit-HasComments: Yes

Reply via email to