Uploaded image for project: 'Apache NiFi'
  1. Apache NiFi
  2. NIFI-4363

Parameterize heap allocation in NiFi Toolkit scripts

    Details

    • Type: Improvement
    • Status: Patch Available
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 1.3.0
    • Fix Version/s: None
    • Component/s: Tools and Build
    • Labels:
      None

      Description

      Replace hardcoded heap allocation of java in scripts with parameterized values.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          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 <jtswork@gmail.com>
          Date: 2017-09-08T21:33:00Z

          NIFI-4363 Replaced hardcoded heap size options with JAVA_ARGS environment variable.


          Show
          githubbot ASF GitHub Bot added a comment - 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 <jtswork@gmail.com> Date: 2017-09-08T21:33:00Z NIFI-4363 Replaced hardcoded heap size options with JAVA_ARGS environment variable.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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?

          Show
          githubbot ASF GitHub Bot added a comment - 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?
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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!

          Show
          githubbot ASF GitHub Bot added a comment - 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!
          Hide
          githubbot ASF GitHub Bot added a comment -

          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"
          ```

          Show
          githubbot ASF GitHub Bot added a comment - 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" ```

            People

            • Assignee:
              jtstorck Jeff Storck
              Reporter:
              jtstorck Jeff Storck
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development