Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.h File be/src/runtime/exec-env.h: http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.h@82 PS8, Line 82: class ExecEnv { > Might be nice to flesh out the class comment here a little about what it me Done http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/runtime/exec-env.cc@253 PS8, Line 253: frontend_(external_fe ? nullptr : new Frontend()), > Its a bit tricky to trace through and prove that the variables here that ar All of the local references were inside null checks. I added DCHECK to the frontend() method to protect external callers. http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/17115/8/be/src/service/fe-support.cc@83 PS8, Line 83: InitForFeTests > This function name is unfortunate now. Not sure what a better options is - Done http://gerrit.cloudera.org:8080/#/c/17115/8/fe/src/main/java/org/apache/impala/service/FeSupport.java File fe/src/main/java/org/apache/impala/service/FeSupport.java: http://gerrit.cloudera.org:8080/#/c/17115/8/fe/src/main/java/org/apache/impala/service/FeSupport.java@489 PS8, Line 489: public static void loadLibrary() { > I'm a little confused by this function and the 'externalFE_' variable - I g Done -- To view, visit http://gerrit.cloudera.org:8080/17115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9 Gerrit-Change-Number: 17115 Gerrit-PatchSet: 8 Gerrit-Owner: Kurt Deschler <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: John Sherman <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Comment-Date: Tue, 02 Mar 2021 01:53:09 +0000 Gerrit-HasComments: Yes
