Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/13352 )
Change subject: [WIP] IMPALA-8473: publish lineage info via hook ...................................................................... Patch Set 12: (4 comments) A few thoughts on things that can go wrong... http://gerrit.cloudera.org:8080/#/c/13352/12/fe/src/main/java/org/apache/impala/hooks/QueryExecHookManager.java File fe/src/main/java/org/apache/impala/hooks/QueryExecHookManager.java: http://gerrit.cloudera.org:8080/#/c/13352/12/fe/src/main/java/org/apache/impala/hooks/QueryExecHookManager.java@131 PS12, Line 131: throw new InternalException(msg, e); As the class names are specified externally to Impala, this is one of the places that things can go wrong, as classes may not be available in the classpath, or the names are wrong, or... So it may be worth logging something here. And if this does happen, what will be the effect on the system? It Looks to me like maybe the whole Frontend wont start? I suggest that some tests that exercise this error path be added. http://gerrit.cloudera.org:8080/#/c/13352/12/fe/src/main/java/org/apache/impala/hooks/QueryExecHookManager.java@146 PS12, Line 146: throw new InternalException(msg, e); What will happen to the system if this occurs? Should something be logged? http://gerrit.cloudera.org:8080/#/c/13352/12/fe/src/main/java/org/apache/impala/hooks/QueryExecHookManager.java@178 PS12, Line 178: hookExecutor_.submit(() -> hook.postQueryExecute(context)); Suppose the PostExecHook is failing, how will we know? http://gerrit.cloudera.org:8080/#/c/13352/12/fe/src/test/java/org/apache/impala/testutil/AlwaysErrorQueryExecHook.java File fe/src/test/java/org/apache/impala/testutil/AlwaysErrorQueryExecHook.java: http://gerrit.cloudera.org:8080/#/c/13352/12/fe/src/test/java/org/apache/impala/testutil/AlwaysErrorQueryExecHook.java@23 PS12, Line 23: public class AlwaysErrorQueryExecHook implements QueryExecHook { This looks useful! Is this used yet? -- To view, visit http://gerrit.cloudera.org:8080/13352 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23a896537a98bfef07fb27c70e9a87c105cd77a1 Gerrit-Change-Number: 13352 Gerrit-PatchSet: 12 Gerrit-Owner: radford nguyen <radford.ngu...@gmail.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Anonymous Coward (498) Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: radford nguyen <radford.ngu...@gmail.com> Gerrit-Comment-Date: Fri, 17 May 2019 16:54:01 +0000 Gerrit-HasComments: Yes
