Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11608 )
Change subject: IMPALA-7597: wraps retries around CatalogMetaProvider ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11608/2//COMMIT_MSG@18 PS2, Line 18: methods of CatalogMetaProvider with retries/logging. When obtaining > I'm confused how this is supposed to work. Don't we need to retry from the good point. most of the top-level "describe" methods use strings as args, not refs. the refs are mostly used for everything under createExecRequest, and there, we're guarding at the top-level. the method to get column stats and the one that looks up all partition info should change as a result. will confirm with tests. http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11608/1/fe/src/main/java/org/apache/impala/catalog/local/RetryableMetaProvider.java@126 PS1, Line 126: while (true) { : try { : return provider_.loadTableNames(dbName); : } catch (InconsistentMetadataFetchException e) { : tracker.handleRetryOrThrow(e); : } > can we factor out while(true) try {} catch {retry} into a helper? I think will try it but only after we're settled on this approach. http://gerrit.cloudera.org:8080/#/c/11608/1/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/1/fe/src/main/java/org/apache/impala/service/Frontend.java@834 PS1, Line 834: ribeResult > It looks like the callers should now understand whether an operation should As discussed, I was trading off looping too many times with retry (consider getCatalogMetrics and all query paths under createExecRequest) vs. letting one of these inconsistent metadata exceptions loose. We spend far more cycles in the latter so opted to not go with an approach that changes the default to loop. -- 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: 2 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: Tue, 09 Oct 2018 20:48:56 +0000 Gerrit-HasComments: Yes
