You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/15 09:50:10 UTC

[GitHub] [pulsar] wangjialing218 opened a new pull request, #16623: rename pulsar env variables for pulsar tools

wangjialing218 opened a new pull request, #16623:
URL: https://github.com/apache/pulsar/pull/16623

   ### Motivation
   
   As #4105, we could config pulsar in container environments through environment variables.
   Current, both broker and pulsar tools (admin, client, perf) use `PULSAR_MEM` to config jvm memory.
   Consider we create a pod for broker through kubernetes, we set `containers.resources.limits.memory` to 25Gi, set env `PULSAR_MEM` to `-Xms10g -Xmx10g -XX:MaxDirectMemorySize=10g`, broker will start with this jvm memory config.
   When we run pulsar-admin or pulsar-client in this pod, they will start with broker's jvm memory config since `PULSAR_MEM` was already set, that will cause pod's total memory exceed the limit.
   
   ### Modifications
   Rename `PULSAR_MEM` to `PULSAR_TOOL_MEM` for pulsar tools, we could set `PULSAR_TOOL_MEM` through environment variables for pulsar tools, or just use the default jvm memory setting.
   
   ### Verifying this change
   
   This change is a trivial rework
   
   ### Documentation
     
   - [x] `doc-not-needed` 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] tisonkun closed pull request #16623: rename pulsar env variables for pulsar tools

Posted by GitBox <gi...@apache.org>.
tisonkun closed pull request #16623: rename pulsar env variables for pulsar tools
URL: https://github.com/apache/pulsar/pull/16623


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] github-actions[bot] commented on pull request #16623: rename pulsar env variables for pulsar tools

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #16623:
URL: https://github.com/apache/pulsar/pull/16623#issuecomment-1217382756

   The pr had no activity for 30 days, mark with Stale label.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] wangjialing218 commented on a diff in pull request #16623: rename pulsar env variables for pulsar tools

Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on code in PR #16623:
URL: https://github.com/apache/pulsar/pull/16623#discussion_r922927959


##########
conf/pulsar_tools_env.sh:
##########
@@ -42,13 +42,13 @@
 # PULSAR_GLOBAL_ZK_CONF=
 
 # Extra options to be passed to the jvm
-PULSAR_MEM=${PULSAR_MEM:-"-Xmx128m -XX:MaxDirectMemorySize=128m"}
+PULSAR_TOOL_MEM=${PULSAR_MEM:-"-Xmx128m -XX:MaxDirectMemorySize=128m"}

Review Comment:
   Done, thx



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] dave2wave commented on a diff in pull request #16623: rename pulsar env variables for pulsar tools

Posted by GitBox <gi...@apache.org>.
dave2wave commented on code in PR #16623:
URL: https://github.com/apache/pulsar/pull/16623#discussion_r922716979


##########
conf/pulsar_tools_env.sh:
##########
@@ -42,13 +42,13 @@
 # PULSAR_GLOBAL_ZK_CONF=
 
 # Extra options to be passed to the jvm
-PULSAR_MEM=${PULSAR_MEM:-"-Xmx128m -XX:MaxDirectMemorySize=128m"}
+PULSAR_TOOL_MEM=${PULSAR_MEM:-"-Xmx128m -XX:MaxDirectMemorySize=128m"}

Review Comment:
   1. This will set PULSAR_TOOL_MEM to PULSAR_MEM. It will ignore PULSAR_TOOL_MEM if it is set in the environment.
   2. In a proper override you do need to consider that legacy users will be surprised by this behavior so if PULSAR_TOOL_MEM is not set then check PULSAR_MEM
   
   This same comment applies to PULSAR_TOOL_GC



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] tisonkun commented on pull request #16623: rename pulsar env variables for pulsar tools

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #16623:
URL: https://github.com/apache/pulsar/pull/16623#issuecomment-1345283189

   Close as no consensus. I suggest you start a thread on the dev@ mailing list for such breaking changes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org