[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.

[Philip Zeyliger (Code Review)]

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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Mon, 30 Oct 2017 21:06:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.

[Jim Apple (Code Review)]

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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Thu, 19 Oct 2017 10:39:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.

[Kim Jin Chul (Code Review)]

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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Comment-Date: Thu, 19 Oct 2017 04:13:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.

[Alex Behm (Code Review)]

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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Thu, 19 Oct 2017 03:45:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6073: Fail on misconfigured CLASSPATH.

[Philip Zeyliger (Code Review)]

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