You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by jtstorck <gi...@git.apache.org> on 2017/09/08 21:49:16 UTC
[GitHub] nifi pull request #2136: NIFI-4363 Replaced hardcoded heap size options with...
GitHub user jtstorck opened a pull request:
https://github.com/apache/nifi/pull/2136
NIFI-4363 Replaced hardcoded heap size options with JAVA_ARGS environ…
…ment variable.
Hardcoded heap size settings have been preserved as defaults in the JAVA_ARGS variable if it is not available in the scope of the script.
To test in *nix and OS X, temporarily use "set -x" in a script to echo commands to the terminal.
Example:
```sh
$ JAVA_ARGS="-Xms1m -Xmx24m" ./encrypt-config.sh
```
will run:
```sh
/usr/bin/java -cp '/Users/jstorck/git-repos/nifi/nifi-toolkit/nifi-toolkit-assembly/target/nifi-toolkit-1.4.0-SNAPSHOT-bin/nifi-toolkit-1.4.0-SNAPSHOT/classpath:/Users/jstorck/git-repos/nifi/nifi-toolkit/nifi-toolkit-assembly/target/nifi-toolkit-1.4.0-SNAPSHOT-bin/nifi-toolkit-1.4.0-SNAPSHOT/lib/*' -Xms1m -Xmx24m org.apache.nifi.properties.ConfigEncryptionTool
```
Without specifying JAVA_ARGS:
```sh
$ ./encrypt-config.sh
```
will run:
```sh
/usr/bin/java -cp '/Users/jstorck/git-repos/nifi/nifi-toolkit/nifi-toolkit-assembly/target/nifi-toolkit-1.4.0-SNAPSHOT-bin/nifi-toolkit-1.4.0-SNAPSHOT/classpath:/Users/jstorck/git-repos/nifi/nifi-toolkit/nifi-toolkit-assembly/target/nifi-toolkit-1.4.0-SNAPSHOT-bin/nifi-toolkit-1.4.0-SNAPSHOT/lib/*' -Xms128m -Xmx256m org.apache.nifi.properties.ConfigEncryptionTool
```
To test in Windows, temporarily remove @echo off from a batch file to echo commands to the terminal.
Example:
```bat
C:\Users\Administrator\Documents>set JAVA_ARGS=-Xmx24m
C:\Users\Administrator\Documents>encrypt-config.bat
```
will run:
```bat
C:\Users\Administrator\Documents>cmd.exe /C ""java" -cp C:\Users\Administrator\Documents\..\classpath;C:\Users\Administrator\Documents\..\lib\* -Xmx24m org.apache.nifi.properties.ConfigEncryptionTool ""
```
Without setting JAVA_ARGS:
```bat
C:\Users\Administrator\Documents>encrypt-config.bat
```
will run:
```bat
C:\Users\Administrator\Documents>cmd.exe /C ""java" -cp C:\Users\Administrator\Documents\..\classpath;C:\Users\Administrator\Documents\..\lib\* -Xms128m -Xmx256m org.apache.nifi.properties.ConfigEncryptionTool ""```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jtstorck/nifi NIFI-4363
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2136.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2136
----
commit 889379bddbf7fd0d8e7fa1ca0acad5c31395258c
Author: Jeff Storck <jt...@gmail.com>
Date: 2017-09-08T21:33:00Z
NIFI-4363 Replaced hardcoded heap size options with JAVA_ARGS environment variable.
----
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/2136
`JAVA_OPTS` or `JVM_ARGS` would be fine, though I'm still okay with `JAVA_ARGS` since technically they are arguments to java itself. I used `JAVA_ARGS` because that's what the .bat scripts were using, and I was trying to establish parity between the .sh and .bat scripts. If everything else looks good to you, you can make the change before committing, or I can push an update to the PR.
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2136
@jtstorck Thanks for cherry-picking! Merged to master.
---
[GitHub] nifi pull request #2136: NIFI-4363 Replaced hardcoded heap size options with...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2136
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2136
Trivial thing, but would `JAVA_OPTS` be more standard naming instead of `JAVA_ARGS`? When I see args, I'd imagine arguments those passed to Java main method. How do you think?
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/2136
@ijokarumawak I cherry-picked your commit and updated my PR.
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2136
Thanks @yuri1969 . @jtstorck I found other batch files already use `JAVA_ARGS` such as `run-nifi.bat`, but those usages are enclosed within the batch file itself.
So, I think we can still establish parity between .sh and .bat scripts when those existing bat scripts exposes the config to user land, by using `JAVA_OPTS`. The reason I care about naming here is, because the name is exposed to user code base.
I've created a commit based on yours.
https://github.com/ijokarumawak/nifi/commit/8fca2515d3ca422e6669f7e8a6d111eb0a218205
If it looks good, please cherry-pick it, or just updating your PR is fine for me, too.
Other part, I'm +1 for this PR, thanks!
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by yuri1969 <gi...@git.apache.org>.
Github user yuri1969 commented on the issue:
https://github.com/apache/nifi/pull/2136
In my opinion, `JAVA_OPTS` suits the `-X*` or `-D*` options better.
---
[GitHub] nifi issue #2136: NIFI-4363 Replaced hardcoded heap size options with JAVA_A...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2136
JFYI, when I tested on Windows, I had to wrap a `set` command as follows to properly set a value containing whitespace:
```
set "JAVA_OPTS=-Xms1m -Xmx24m"
```
---