Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around Frontend metadata operations. ...................................................................... Patch Set 5: (6 comments) LGTM. Just a few clarifications. http://gerrit.cloudera.org:8080/#/c/11608/4/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/11608/4/fe/src/main/java/org/apache/impala/service/Frontend.java@681 PS4, Line 681: RI > wasn't entirely sure what's the suggestion here, but I was aiming for consi I meant, log the exception stack trace too. LOG.warn("foo", e) instead of LOG.warn("foo") http://gerrit.cloudera.org:8080/#/c/11608/5/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/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@754 PS5, Line 754: List<? extends FeDb> dbs = getCatalog().getDbs(matcher); mention no need to retry for this call path? http://gerrit.cloudera.org:8080/#/c/11608/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1463 PS5, Line 1463: FeTable table = getCatalog().getTable(request.getTable_name().getDb_name(), this one? http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py File tests/custom_cluster/test_local_catalog.py: http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@173 PS4, Line 173: catalogd_args="--catalog_topic_mode=minimal") > separated these two types of tests into two classes so this test moved up h I see, thanks. http://gerrit.cloudera.org:8080/#/c/11608/4/tests/custom_cluster/test_local_catalog.py@280 PS4, Line 280: catalogd_args="--catalog_topic_mode=minimal") > I didn't want to modify the state (see the alter on L280). That can leak to Ya, i meant to replace that "alter" with "refresh" which bumps the version too, but your call. That makes the check_metadata_retries() simpler. http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py File tests/custom_cluster/test_local_catalog.py: http://gerrit.cloudera.org:8080/#/c/11608/5/tests/custom_cluster/test_local_catalog.py@248 PS5, Line 248: attempt < 100 Is this needed if we join for 30s? -- To view, visit http://gerrit.cloudera.org:8080/11608 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I43a21571d54a7966c5c68bea1fe69dbc62be2a0b Gerrit-Change-Number: 11608 Gerrit-PatchSet: 5 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Thu, 18 Oct 2018 00:35:42 +0000 Gerrit-HasComments: Yes
