You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Kurt Deschler (Code Review)" <ge...@cloudera.org> on 2021/02/24 20:36:43 UTC
[Impala-ASF-CR] IMPALA-10517: Support external use of frontend libraries
Kurt Deschler has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17115
Change subject: IMPALA-10517: Support external use of frontend libraries
......................................................................
IMPALA-10517: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
A flag has been added to FeSupport.loadLibrary() that changes
initialization of specific frontend components. This mode builds upon
logic that is used to initialize the frontend jar for unit tests.
Initialization differs in external mode as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
Co-author: John Sherman <jf...@cloudera.com>
Co-author: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
---
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
8 files changed, 60 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17115/1
--
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: newchange
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 1
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 01:53:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17115
to look at the new patch set (#3).
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
IMPALA-10522: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
A flag has been added to FeSupport.loadLibrary() that changes
initialization of specific frontend components. This mode builds upon
logic that is used to initialize the frontend jar for unit tests.
Initialization differs in external mode as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
Co-author: John Sherman <jf...@cloudera.com>
Co-author: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
---
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
8 files changed, 61 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17115/3
--
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: newpatchset
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 3
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 9: Code-Review+2
--
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: 9
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Mar 2021 19:18:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17115
to look at the new patch set (#4).
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
IMPALA-10522: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
A flag has been added to FeSupport.loadLibrary() that changes
initialization of specific frontend components. This mode builds upon
logic that is used to initialize the frontend jar for unit tests.
Initialization differs in external mode as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
- Disable redirect of stderr/stdout to libfesupport.so glog
- Log messages from libfesupport.so to stderr
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
- Manual verification of minicluster logs
Co-authored-by: John Sherman <jf...@cloudera.com>
Co-authored-by: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
---
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
8 files changed, 72 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17115/4
--
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: newpatchset
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 4
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10517: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17115
to look at the new patch set (#2).
Change subject: IMPALA-10517: Support external use of frontend libraries
......................................................................
IMPALA-10517: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
A flag has been added to FeSupport.loadLibrary() that changes
initialization of specific frontend components. This mode builds upon
logic that is used to initialize the frontend jar for unit tests.
Initialization differs in external mode as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
Co-author: John Sherman <jf...@cloudera.com>
Co-author: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
---
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
8 files changed, 61 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17115/2
--
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: newpatchset
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 2
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8226/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 1
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Feb 2021 20:57:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 9: Verified+1
--
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: 9
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Mar 2021 17:49:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall 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 means to have an ExecEnv that's for an external FE, eg. "Not everything is going to be initialized, just at least the stuff needed to do the FeSupport functions. TODO: separate this out better"
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 aren't getting initialized anymore (frontend_ and default_fs_ below) aren't actually used in any of the FeSupport functionality.
You could store a 'external_fe_' variable and DCHECK on it when those things are accessed, for a little extra safety.
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 - InitForFeSupport() maybe?
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 guess the expected usage is that the external FE could call "loadLibrary(true)" first, and then subsequent calls to "loadLibrary()", eg. in functions in this file like CacheJar(), LookupSymbol(), etc. will get the "externalFE" version of this function (though that won't actually matter, as 'loaded_' would have to be true in those cases)?
Maybe it would be clearer to have a "setExternalFE()" function that the external FE can call, which sets 'externalFE_' and which is required to be called before any calls to loadLibrary(). You could presumably check that 'loaded_' is false in setExternalFE() for a little extra safety.
--
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Mar 2021 19:50:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8241/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 4
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Feb 2021 19:23:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8242/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 5
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Feb 2021 19:40:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17115/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17115/3//COMMIT_MSG@36
PS3, Line 36: Co-author
I think this string should be 'Co-authored-by' based on git documentation https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors
--
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: 3
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Feb 2021 17:13:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17115
to look at the new patch set (#5).
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
IMPALA-10522: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
A flag has been added to FeSupport.loadLibrary() that changes
initialization of specific frontend components. This mode builds upon
logic that is used to initialize the frontend jar for unit tests.
Initialization differs in external mode as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
- Disable redirect of stderr/stdout to libfesupport.so glog
- Log messages from libfesupport.so to stderr
- Use libfesupport.so for JNI symbol look up
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
- Manual verification of minicluster logs
- Ran queries with external frontend
Co-authored-by: John Sherman <jf...@cloudera.com>
Co-authored-by: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
---
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
10 files changed, 82 insertions(+), 41 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17115/5
--
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: newpatchset
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 5
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
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:
Looks like I changed the Change-I by accident. Will fix that.
--
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 01:49:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Thomas Tauber-Marshall, Joe McDonnell, Impala Public Jenkins, John Sherman,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17115
to look at the new patch set (#9).
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
IMPALA-10522: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
Calling FeSupport.setExternalFE() will cause external frontend
initialization mode to be used during FeSupport.loadLibrary(). This
mode builds upon logic that is used to initialize the frontend jar for
unit tests.
Initialization in external frontend mode differs as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
- Disable redirect of stderr/stdout to libfesupport.so glog
- Log messages from libfesupport.so to stderr
- Use libfesupport.so for JNI symbol look up
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
- Manual verification of minicluster logs
- Ran queries with external frontend
Co-authored-by: John Sherman <jf...@cloudera.com>
Co-authored-by: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
12 files changed, 99 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/17115/9
--
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: newpatchset
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 9
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8227/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 2
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Feb 2021 21:00:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
IMPALA-10522: Support external use of frontend libraries
This patch enables the Impala frontend jar and dependent library
libfesupport.so to be used by an external Java frontend.
Calling FeSupport.setExternalFE() will cause external frontend
initialization mode to be used during FeSupport.loadLibrary(). This
mode builds upon logic that is used to initialize the frontend jar for
unit tests.
Initialization in external frontend mode differs as follows:
- Skip instantiating Frontend object and it's dependents
- Skip loading libhdfs
- Skip starting JVM Pause monitor
- Disable Minidumper
- Initialize TimezoneDatabase for external frontends
- Disable redirect of stderr/stdout to libfesupport.so glog
- Log messages from libfesupport.so to stderr
- Use libfesupport.so for JNI symbol look up
Null check were added in places where objects were assumed to be
instantiated but are now skipped during initialization.
Additional change:
1) Add libfesupport.lib path to JAVA_LIBRARY_PATH in test driver
Testing: - Initialized frontend jar from external frontend
- Verified that frontend Java objects can be used externally without
issues
- Verified that exceptions thrown from Impala Java or libfesupport
can be caught or propagated correctly by the external frontend
- Manual verification of minicluster logs
- Ran queries with external frontend
Co-authored-by: John Sherman <jf...@cloudera.com>
Co-authored-by: Aman Sinha <am...@cloudera.com>
Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Reviewed-on: http://gerrit.cloudera.org:8080/17115
Reviewed-by: Thomas Tauber-Marshall <tm...@cloudera.com>
Tested-by: Thomas Tauber-Marshall <tm...@cloudera.com>
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/common/init.cc
M be/src/common/init.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M be/src/util/jni-util.cc
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M testdata/bin/run-hive-server.sh
12 files changed, 99 insertions(+), 46 deletions(-)
Approvals:
Thomas Tauber-Marshall: Looks good to me, approved; Verified
--
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: merged
Gerrit-Change-Id: I4e3a84721ba196ec00773ce2923b19610b90edd9
Gerrit-Change-Number: 17115
Gerrit-PatchSet: 10
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall 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:
I think you forgot to push the updated version of this patch?
--
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 <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 19:28:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10522: Support external use of frontend libraries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10522: Support external use of frontend libraries
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8299/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 9
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 Mar 2021 13:23:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10517: Support external use of frontend libraries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17115 )
Change subject: IMPALA-10517: Support external use of frontend libraries
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17115/1/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/17115/1/be/src/runtime/exec-env.cc@486
PS1, Line 486: // Get the fs.defaultFS value set in core-site.xml and assign it to configured_defaultFs
line too long (92 > 90)
--
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: 1
Gerrit-Owner: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Feb 2021 20:37:38 +0000
Gerrit-HasComments: Yes