Uploaded image for project: 'Apache RocketMQ'
  1. Apache RocketMQ
  2. ROCKETMQ-161

Update runbroker.sh and runserver.sh to support user defined jvm memory flag

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.1.0-incubating
    • Component/s: None
    • Labels:
      None

      Description

      JVM mem flag is hard coded in runbroker.sh as follows:
      JAVA_OPT="$

      {JAVA_OPT} -server -Xms8g -Xmx8g -Xmn4g"

      If one want to change such flag, he has to change the script, this is not friendly, especially in docker environment.

      Instead, it is able to use an environment variable to handle user defined flag, like:
      if [ -z $BROKER_MEM_OPS ]; then
      BROKER_MEM_OPS = "-Xms8g -Xmx8g -Xmn4g"
      fi
      JAVA_OPT="${JAVA_OPT}

      -server $BROKER_MEM_OPS"

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user dongeforever opened a pull request:

          https://github.com/apache/incubator-rocketmq/pull/87

          ROCKETMQ-161 Update runbroker.sh and runserver.sh to support user defined jvm mem flag

          https://issues.apache.org/jira/browse/ROCKETMQ-161?filter=-1

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/dongeforever/incubator-rocketmq ROCKETMQ-161

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/incubator-rocketmq/pull/87.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 #87


          commit 23e14efd07f0e68b52d139053f0c94043164efda
          Author: dongeforever <zhendongliu92@yeah.net>
          Date: 2017-04-01T12:26:36Z

          ROCKETMQ-161 Update runbroker.sh and runserver.sh to support user defined jvm memory flag


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user dongeforever opened a pull request: https://github.com/apache/incubator-rocketmq/pull/87 ROCKETMQ-161 Update runbroker.sh and runserver.sh to support user defined jvm mem flag https://issues.apache.org/jira/browse/ROCKETMQ-161?filter=-1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongeforever/incubator-rocketmq ROCKETMQ-161 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-rocketmq/pull/87.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 #87 commit 23e14efd07f0e68b52d139053f0c94043164efda Author: dongeforever <zhendongliu92@yeah.net> Date: 2017-04-01T12:26:36Z ROCKETMQ-161 Update runbroker.sh and runserver.sh to support user defined jvm memory flag
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          [![Coverage Status](https://coveralls.io/builds/10884600/badge)](https://coveralls.io/builds/10884600)

          Coverage decreased (-0.1%) to 31.776% when pulling *23e14efd07f0e68b52d139053f0c94043164efda on dongeforever:ROCKETMQ-161* into *45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 [! [Coverage Status] ( https://coveralls.io/builds/10884600/badge)](https://coveralls.io/builds/10884600 ) Coverage decreased (-0.1%) to 31.776% when pulling * 23e14efd07f0e68b52d139053f0c94043164efda on dongeforever: ROCKETMQ-161 * into * 45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          [![Coverage Status](https://coveralls.io/builds/10884600/badge)](https://coveralls.io/builds/10884600)

          Coverage decreased (-0.1%) to 31.776% when pulling *23e14efd07f0e68b52d139053f0c94043164efda on dongeforever:ROCKETMQ-161* into *45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 [! [Coverage Status] ( https://coveralls.io/builds/10884600/badge)](https://coveralls.io/builds/10884600 ) Coverage decreased (-0.1%) to 31.776% when pulling * 23e14efd07f0e68b52d139053f0c94043164efda on dongeforever: ROCKETMQ-161 * into * 45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          [![Coverage Status](https://coveralls.io/builds/10884600/badge)](https://coveralls.io/builds/10884600)

          Coverage decreased (-0.1%) to 31.776% when pulling *23e14efd07f0e68b52d139053f0c94043164efda on dongeforever:ROCKETMQ-161* into *45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 [! [Coverage Status] ( https://coveralls.io/builds/10884600/badge)](https://coveralls.io/builds/10884600 ) Coverage decreased (-0.1%) to 31.776% when pulling * 23e14efd07f0e68b52d139053f0c94043164efda on dongeforever: ROCKETMQ-161 * into * 45a64fd6a8ab66a8b0eb30cbe2dffd59b19274f9 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @shroman please help review and check it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @shroman please help review and check it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vongosling commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @shroman @lizhanhui Could you help us to review this long-time delayed PR

          Show
          githubbot ASF GitHub Bot added a comment - Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @shroman @lizhanhui Could you help us to review this long-time delayed PR
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vesense commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          LGTM +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user vesense commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 LGTM +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @dongeforever Sorry, I don't think this modification is needed. A user will modify this file anyway in order to match his/her system requirements.

          Also, there are even more flags, and if you take this approach, you may have to add more variables for each in future, which will make the script tricky.

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @dongeforever Sorry, I don't think this modification is needed. A user will modify this file anyway in order to match his/her system requirements. Also, there are even more flags, and if you take this approach, you may have to add more variables for each in future, which will make the script tricky.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vsair commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          When running in docker, this would be useful. Users just want to run a docker demo, but the computer could not allocate 8g memory, and there is no way to define the JVM flags in docker's script. OMG!

          Show
          githubbot ASF GitHub Bot added a comment - Github user vsair commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 When running in docker, this would be useful. Users just want to run a docker demo, but the computer could not allocate 8g memory, and there is no way to define the JVM flags in docker's script. OMG!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          Ok, but let's set it through `JVM_OPTS` instead of introducing a variable for that specific setting.

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 Ok, but let's set it through `JVM_OPTS` instead of introducing a variable for that specific setting.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user netroby commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          +1 JVM_OPTS, Do not try to introducing new variable.

          Show
          githubbot ASF GitHub Bot added a comment - Github user netroby commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 +1 JVM_OPTS, Do not try to introducing new variable.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user netroby commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          Or just check JAVA_OPTS have already defined Xmx

          Show
          githubbot ASF GitHub Bot added a comment - Github user netroby commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 Or just check JAVA_OPTS have already defined Xmx
          Show
          githubbot ASF GitHub Bot added a comment - Github user netroby commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 http://stackoverflow.com/questions/229551/string-contains-in-bash
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vsair commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          +1, maybe it's better to merge JAVA_OPTS

          Show
          githubbot ASF GitHub Bot added a comment - Github user vsair commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 +1, maybe it's better to merge JAVA_OPTS
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          The intention is to make user-defined JVM flags to replace the initial ones.
          So just put JAVA_OPT_EXT after JAVA_OPT is enough.
          The later defined JVM parameters will replace the previous ones. I have already tested it.

          @vsair @netroby @shroman how do you think

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 The intention is to make user-defined JVM flags to replace the initial ones. So just put JAVA_OPT_EXT after JAVA_OPT is enough. The later defined JVM parameters will replace the previous ones. I have already tested it. @vsair @netroby @shroman how do you think
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          [![Coverage Status](https://coveralls.io/builds/11179187/badge)](https://coveralls.io/builds/11179187)

          Coverage decreased (-0.04%) to 34.591% when pulling *77e657f1addccedae4c21bc287d04fc2c9db71fd on dongeforever:ROCKETMQ-161* into *42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 [! [Coverage Status] ( https://coveralls.io/builds/11179187/badge)](https://coveralls.io/builds/11179187 ) Coverage decreased (-0.04%) to 34.591% when pulling * 77e657f1addccedae4c21bc287d04fc2c9db71fd on dongeforever: ROCKETMQ-161 * into * 42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          [![Coverage Status](https://coveralls.io/builds/11179187/badge)](https://coveralls.io/builds/11179187)

          Coverage decreased (-0.04%) to 34.591% when pulling *77e657f1addccedae4c21bc287d04fc2c9db71fd on dongeforever:ROCKETMQ-161* into *42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 [! [Coverage Status] ( https://coveralls.io/builds/11179187/badge)](https://coveralls.io/builds/11179187 ) Coverage decreased (-0.04%) to 34.591% when pulling * 77e657f1addccedae4c21bc287d04fc2c9db71fd on dongeforever: ROCKETMQ-161 * into * 42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user coveralls commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          [![Coverage Status](https://coveralls.io/builds/11179187/badge)](https://coveralls.io/builds/11179187)

          Coverage decreased (-0.04%) to 34.591% when pulling *77e657f1addccedae4c21bc287d04fc2c9db71fd on dongeforever:ROCKETMQ-161* into *42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop*.

          Show
          githubbot ASF GitHub Bot added a comment - Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 [! [Coverage Status] ( https://coveralls.io/builds/11179187/badge)](https://coveralls.io/builds/11179187 ) Coverage decreased (-0.04%) to 34.591% when pulling * 77e657f1addccedae4c21bc287d04fc2c9db71fd on dongeforever: ROCKETMQ-161 * into * 42f78c281cbeb5072b04eaf03b1a8059b8d281a7 on apache:develop *.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @dongeforever Hmm, sorry I don't understand yet why `JAVA_OPTS` won't work. Can you please explain how/when it happens?

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @dongeforever Hmm, sorry I don't understand yet why `JAVA_OPTS` won't work. Can you please explain how/when it happens?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @shroman In runbroker.sh and runserver.sh, some default options, such as -Xmx8g, are placed after the JVM_OPTS, so user-defined -Xmx will not take effect.

          And if we put JAVA_OPT_EXT after all default options, it will replace the default ones.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @shroman In runbroker.sh and runserver.sh, some default options, such as -Xmx8g, are placed after the JVM_OPTS, so user-defined -Xmx will not take effect. And if we put JAVA_OPT_EXT after all default options, it will replace the default ones.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shroman commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @dongeforever I see, thank you!
          How about just changing the code to
          ```
          if [ -z "$JVM_OPTS" ] ; then
          #default_options
          JAVA_OPT="$

          {JAVA_OPT}

          -server -Xms8g -Xmx8g -Xmn4g"
          fi
          ```
          ?
          But if you guys think we need `JAVA_OPT_EXT`, please go ahead

          Show
          githubbot ASF GitHub Bot added a comment - Github user shroman commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @dongeforever I see, thank you! How about just changing the code to ``` if [ -z "$JVM_OPTS" ] ; then #default_options JAVA_OPT="$ {JAVA_OPT} -server -Xms8g -Xmx8g -Xmn4g" fi ``` ? But if you guys think we need `JAVA_OPT_EXT`, please go ahead
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          @shroman I insist on using JAVA_OPT_EXT, for that we may not only want to override -Xms -Xmx -Xmn, but also other jvm flags.
          We could use JAVA_OPT_EXT once and for all.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 @shroman I insist on using JAVA_OPT_EXT, for that we may not only want to override -Xms -Xmx -Xmn, but also other jvm flags. We could use JAVA_OPT_EXT once and for all.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lizhanhui commented on the issue:

          https://github.com/apache/incubator-rocketmq/pull/87

          It's OK to introduce an extra variable to override default Java VM options.

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user lizhanhui commented on the issue: https://github.com/apache/incubator-rocketmq/pull/87 It's OK to introduce an extra variable to override default Java VM options. +1
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4d12d11249950468d2830cae31c0f8909ad14395 in incubator-rocketmq's branch refs/heads/develop from dongeforever
          [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=4d12d11 ]

          ROCKETMQ-161 Update runbroker.sh and runserver.sh to support user defined jvm mem flag closes apache/incubator-rocketmq#87

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4d12d11249950468d2830cae31c0f8909ad14395 in incubator-rocketmq's branch refs/heads/develop from dongeforever [ https://git-wip-us.apache.org/repos/asf?p=incubator-rocketmq.git;h=4d12d11 ] ROCKETMQ-161 Update runbroker.sh and runserver.sh to support user defined jvm mem flag closes apache/incubator-rocketmq#87
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dongeforever closed the pull request at:

          https://github.com/apache/incubator-rocketmq/pull/87

          Show
          githubbot ASF GitHub Bot added a comment - Github user dongeforever closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/87
          Hide
          zander dongeforever added a comment -

          Github user dongeforever closed the pull request at:
          https://github.com/apache/incubator-rocketmq/pull/87

          Show
          zander dongeforever added a comment - Github user dongeforever closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/87

            People

            • Assignee:
              zander dongeforever
              Reporter:
              zander dongeforever
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development