You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Philip Zeyliger (Code Review)" <ge...@cloudera.org> on 2017/10/19 02:33:28 UTC
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8327
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
IMPALA-6073: Fail on misconfigured CLASSPATH.
Asserts, early, that $CLASSPATH is set and has no wildcards.
Several developers have been tripped up running C++ tests without first
running 'bin/set-classpath.sh'. This causes them to run into HDFS-11851
(getGlobalJNIEnv() may deadlock on exception), wherein, instead of a
relatively clear "Class not found" error, we hang forever.
I considered limiting this to tests, but we clearly need a $CLASSPATH
for the daemons themselves.
Testing:
* Ran a backend test without $CLASSPATH set.
* Started Impala cluster with this change.
Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
---
M be/src/common/init.cc
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/8327/1
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has abandoned this change. ( http://gerrit.cloudera.org:8080/8327 )
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8327 )
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
Patch Set 1: Code-Review-1
> Patch Set 1:
>
> > I think this check seems not to be enough. Let me leave a detailed
> > comment on the JIRA ticket.
>
> It is OK to leave detailed comments here in the code review tool. In fact, it is common.
On JIRA, Kim Jin Chul said:
> CLASSPATH should be set after loading impala-config.sh but the variable may not have sufficient libraries.
I'm -1'ing this for a bit; I think the trick is to figure out if we can get rid of impala-config.sh setting this at all. I'll try it out.
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Oct 2017 21:06:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8327 )
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
Patch Set 1:
Should we abandon if we're not moving forward with this?
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Oct 2018 18:53:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8327 )
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
Patch Set 1:
> I think this check seems not to be enough. Let me leave a detailed
> comment on the JIRA ticket.
It is OK to leave detailed comments here in the code review tool. In fact, it is common.
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2017 10:39:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8327 )
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
Patch Set 1: Code-Review-1
I think this check seems not to be enough. Let me leave a detailed comment on the JIRA ticket.
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Thu, 19 Oct 2017 04:13:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8327 )
Change subject: IMPALA-6073: Fail on misconfigured CLASSPATH.
......................................................................
Patch Set 1:
(3 comments)
Thanks for fixing, lgtm, only minor nits
http://gerrit.cloudera.org:8080/#/c/8327/1/be/src/common/init.cc
File be/src/common/init.cc:
http://gerrit.cloudera.org:8080/#/c/8327/1/be/src/common/init.cc@178
PS1, Line 178: char *classpath = getenv("CLASSPATH");
style: char*
http://gerrit.cloudera.org:8080/#/c/8327/1/be/src/common/init.cc@179
PS1, Line 179: assert(classpath != NULL);
Why not DCHECK and add a message
http://gerrit.cloudera.org:8080/#/c/8327/1/be/src/common/init.cc@182
PS1, Line 182: assert(std::string(classpath).find("*") == std::string::npos);
DCHECK and include an error message
--
To view, visit http://gerrit.cloudera.org:8080/8327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb92fa5cad8bbc29b620bec5904e45ed7aeff13d
Gerrit-Change-Number: 8327
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Oct 2017 03:45:24 +0000
Gerrit-HasComments: Yes