Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11403 )

Change subject: IMPALA-7530, IMPALA-7509. Re-plan queries if they fetch 
inconsistent metadata
......................................................................


Patch Set 3:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/11403/3/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/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2098
PS3, Line 2098: .
Mention that appropriate LookupStats is set (error conditions)


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2125
PS3, Line 2125:
spacing


http://gerrit.cloudera.org:8080/#/c/11403/3/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/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@312
PS3, Line 312: case DB_NOT_FOUND:
             :     case FUNCTION_NOT_FOUND:
             :     case TABLE_NOT_FOUND:
indentation


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@317
PS3, Line 317: Could not load
Instead say, it is not found? Could not load sounds like it is present but 
couldn't be loaded for somereason


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@322
PS3, Line 322:
Preconditions lookup_stats == OK?


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@333
PS3, Line 333: (
Can we include the information of the object that changed. Also mention that it 
changed? (The message here doesn't make much sense for those reading)


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java@171
PS3, Line 171:
Add some detail of why this is present?


http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/service/Frontend.java@1073
PS3, Line 1073:  "
Can we add "attempt xx of 10"...


http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:

http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py@158
PS3, Line 158:     try:
Add a docstring with an overview


http://gerrit.cloudera.org:8080/#/c/11403/3/tests/custom_cluster/test_local_catalog.py@180
PS3, Line 180: replans_seen = [0]
just curious, why is this a list?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17389954780fa22d7866224c17e128458fffa545
Gerrit-Change-Number: 11403
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Comment-Date: Fri, 14 Sep 2018 19:02:22 +0000
Gerrit-HasComments: Yes

Reply via email to