[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..

IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands

JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options.
Impala has several flags that specify shell commands for Impala to run,
such as
- s3a_access_key_cmd
- s3a_secret_key_cmd
- ssl_private_key_password_cmd
- webserver_private_key_password_cmd

When debugging the JVM inside the Impala process, it is useful to
specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port.
However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed
to these shell commands. If any of these shell commands run Java, then
that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus
try to bind to the same port. The Impala process JVM is already bound to
that port, so this will fail.

This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running
these shell commands.

Testing:
- Added a new test for os-util.cc

Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Reviewed-on: http://gerrit.cloudera.org:8080/12005
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/rpc/thrift-server.cc
M be/src/runtime/hdfs-fs-cache.cc
M be/src/util/CMakeLists.txt
A be/src/util/os-util-test.cc
M be/src/util/os-util.cc
M be/src/util/os-util.h
M be/src/util/webserver.cc
7 files changed, 86 insertions(+), 8 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 6: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Fri, 30 Nov 2018 01:00:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1471/ : 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/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 21:16:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 6: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 21:08:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3505/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 21:08:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 5: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 20:49:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc
File be/src/util/os-util.cc:

http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@100
PS4, Line 100:   for (const auto& env: unset_environment) {
> unused?
Done


http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@103
PS4, Line 103:   string new_cmd = Substitute("$0$1", unset_cmd, cmd);
> unused?
Done



--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 20:42:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..

IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands

JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options.
Impala has several flags that specify shell commands for Impala to run,
such as
- s3a_access_key_cmd
- s3a_secret_key_cmd
- ssl_private_key_password_cmd
- webserver_private_key_password_cmd

When debugging the JVM inside the Impala process, it is useful to
specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port.
However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed
to these shell commands. If any of these shell commands run Java, then
that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus
try to bind to the same port. The Impala process JVM is already bound to
that port, so this will fail.

This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running
these shell commands.

Testing:
- Added a new test for os-util.cc

Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
---
M be/src/rpc/thrift-server.cc
M be/src/runtime/hdfs-fs-cache.cc
M be/src/util/CMakeLists.txt
A be/src/util/os-util-test.cc
M be/src/util/os-util.cc
M be/src/util/os-util.h
M be/src/util/webserver.cc
7 files changed, 86 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/12005/5
--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc
File be/src/util/os-util.cc:

http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@100
PS4, Line 100:   int i = 0;
unused?


http://gerrit.cloudera.org:8080/#/c/12005/4/be/src/util/os-util.cc@103
PS4, Line 103: i++;
unused?



--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 20:38:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1470/ : 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/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 20:36:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 4: Code-Review+1

(1 comment)

Carry Phil's +1

http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc
File be/src/util/os-util.cc:

http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc@102
PS3, Line 102: unset_cmd += "unset " + env + "; ";
> It's a nit, but you can just do:
Good idea. Done.



--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 20:06:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..

IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands

JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options.
Impala has several flags that specify shell commands for Impala to run,
such as
- s3a_access_key_cmd
- s3a_secret_key_cmd
- ssl_private_key_password_cmd
- webserver_private_key_password_cmd

When debugging the JVM inside the Impala process, it is useful to
specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port.
However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed
to these shell commands. If any of these shell commands run Java, then
that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus
try to bind to the same port. The Impala process JVM is already bound to
that port, so this will fail.

This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running
these shell commands.

Testing:
- Added a new test for os-util.cc

Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
---
M be/src/rpc/thrift-server.cc
M be/src/runtime/hdfs-fs-cache.cc
M be/src/util/CMakeLists.txt
A be/src/util/os-util-test.cc
M be/src/util/os-util.cc
M be/src/util/os-util.h
M be/src/util/webserver.cc
7 files changed, 88 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/12005/4
--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-29 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc
File be/src/util/os-util.cc:

http://gerrit.cloudera.org:8080/#/c/12005/3/be/src/util/os-util.cc@102
PS3, Line 102: unset_cmd += string(((i > 0) ? "; " : "")) + "unset " + env;
It's a nit, but you can just do:

for (...) {
  unset_cmd += "unset " + env + ";"
}

and then you don't have to worry about the "i>0" check, which makes life a 
little bit easier to think about. Likewise, ignore the unset_environment.size() 
check and just always do the substitution.



--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 29 Nov 2018 19:30:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1458/ : 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/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 28 Nov 2018 22:34:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-28 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 3:

> Patch Set 1:
>
> I'm vaguely uncomfortable with the non-genericness of the solution here. If 
> we wanted to expose generic "unset" options, we could model that as a 
> set, and act accordingly. I think defaulting to clearing out 
> JAVA_TOOL_OPTIONS is pretty sneaky. You also would need to think about shell 
> escaping and so on (or verifying that these are actually variables.)
>
> In practice, maybe just change:
>
> RunShellProcess(FLAGS_s3a_access_key_cmd, _access_key_, true)
>
> to
>
> RunShellProcess("unset JAVA_TOOL_OPTIONS; " + FLAGS_s3a_access_key_cmd, 
> _access_key_, true)
> (or use string::substitute or whatever)
>
> in the three or so places you need it?

I updated the code to make RunShellProcess more generic. I prefer putting 
RunShellProcess so that I can easily unit test it. Furthermore, this makes 
RunShellProcess somewhat similar (I know it's not quite the same) to Python's 
Popen with env parameter.


--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 28 Nov 2018 22:03:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-28 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..

IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands

JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options.
Impala has several flags that specify shell commands for Impala to run,
such as
- s3a_access_key_cmd
- s3a_secret_key_cmd
- ssl_private_key_password_cmd
- webserver_private_key_password_cmd

When debugging the JVM inside the Impala process, it is useful to
specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port.
However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed
to these shell commands. If any of these shell commands run Java, then
that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus
try to bind to the same port. The Impala process JVM is already bound to
that port, so this will fail.

This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running
these shell commands.

Testing:
- Added a new test for os-util.cc

Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
---
M be/src/rpc/thrift-server.cc
M be/src/runtime/hdfs-fs-cache.cc
M be/src/util/CMakeLists.txt
A be/src/util/os-util-test.cc
M be/src/util/os-util.cc
M be/src/util/os-util.h
M be/src/util/webserver.cc
7 files changed, 89 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/12005/3
--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-28 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1456/ : 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/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 28 Nov 2018 21:14:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-28 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12005 )

Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..


Patch Set 1:

I'm vaguely uncomfortable with the non-genericness of the solution here. If we 
wanted to expose generic "unset" options, we could model that as a set, 
and act accordingly. I think defaulting to clearing out JAVA_TOOL_OPTIONS is 
pretty sneaky. You also would need to think about shell escaping and so on (or 
verifying that these are actually variables.)

In practice, maybe just change:

RunShellProcess(FLAGS_s3a_access_key_cmd, _access_key_, true)

to

RunShellProcess("unset JAVA_TOOL_OPTIONS; " + FLAGS_s3a_access_key_cmd, 
_access_key_, true)
(or use string::substitute or whatever)

in the three or so places you need it?


--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 28 Nov 2018 21:02:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6293: Unset JAVA TOOL OPTIONS variable from shell commands

2018-11-28 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12005


Change subject: IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell 
commands
..

IMPALA-6293: Unset JAVA_TOOL_OPTIONS variable from shell commands

JAVA_TOOL_OPTIONS is a variable used by JVM to specify the JVM options.
Impala has several flags that specify shell commands for Impala to run,
such as
- s3a_access_key_cmd
- s3a_secret_key_cmd
- ssl_private_key_password_cmd
- webserver_private_key_password_cmd

When debugging the JVM inside the Impala process, it is useful to
specify JAVA_TOOL_OPTIONS to run the Java debugger on a particular port.
However, JAVA_TOOL_OPTIONS remains in the environment, so it is passed
to these shell commands. If any of these shell commands run Java, then
that JVM will attempt to use the JAVA_TOOL_OPTIONS specified and thus
try to bind to the same port. The Impala process JVM is already bound to
that port, so this will fail.

This patch fixes the issue by unsetting JAVA_TOOL_OPTIONS before running
these shell commands.

Testing:
- Added a new test for os-util.

Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
---
M be/src/util/CMakeLists.txt
A be/src/util/os-util-test.cc
M be/src/util/os-util.cc
M be/src/util/os-util.h
4 files changed, 64 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/12005/1
--
To view, visit http://gerrit.cloudera.org:8080/12005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I96a84a27d3bf7b02f04e70562acfd67386b93183
Gerrit-Change-Number: 12005
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya