Vuk Ercegovac 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) Done http://gerrit.cloudera.org:8080/#/c/11403/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2125 PS3, Line 2125: > spacing Done 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 Done 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 Done 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? Done 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 tha got it... made the message here, which is shown to users less low-level. But I've included these details in the log. 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? Done 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"... Done 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 Done 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? seems to be a python scoping issue. the inner thread updates this so needs a mutable object to do so. if there is a more idiomatic way to achieve this in python (and that works with required versions of python), let me know. for now, added a comment. -- 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 <t...@apache.org> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Sat, 15 Sep 2018 01:38:51 +0000 Gerrit-HasComments: Yes
