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/03/16 08:41:49 UTC

[GitHub] [pulsar] gaozhangmin opened a new pull request #14711: Support JAVA_HOME in env file when check java version

gaozhangmin opened a new pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711


   ### Motivation
   1、Currently, If we set JAVA_HOME in env file, but `IS_JAVA_8 ` always use `java -version` to check java version.
   It will get wrong version if we set JAVA_HOME, thus, wrong gc parameters are set.
   
   2、pulsar not support BOOKIE_LOG_DIR variable. change to PULSAR_LOG_DIR
   
   ### Modifications
   Check if we have set JAVA_HOME or not. before check java version.
   
   
   
   
   ### Documentation
   
   Check the box below or label this PR directly (if you have committer privilege).
   
   Need to update docs? 
   
   - [ ] `doc-required` 
     
     (If you need help on updating docs, create a doc issue)
     
   - [ ] `no-need-doc` 
     
     (Please explain why)
     
   - [ ] `doc` 
     
     (If this PR contains doc 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

[GitHub] [pulsar] codelipenghui merged pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711


   


-- 
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 #14711: Support JAVA_HOME in env file when check java version

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


   @gaozhangmin:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)


-- 
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] codelipenghui merged pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711


   


-- 
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] lhotari commented on a change in pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711#discussion_r827864956



##########
File path: conf/bkenv.sh
##########
@@ -32,15 +32,21 @@ BOOKIE_CONF=${BOOKIE_CONF:-"$BK_HOME/conf/bookkeeper.conf"}
 # BOOKIE_LOG_CONF=
 
 # Logs location
-# BOOKIE_LOG_DIR=
+# PULSAR_LOG_DIR=
 
 # Memory size options
 BOOKIE_MEM=${BOOKIE_MEM:-${PULSAR_MEM:-"-Xms2g -Xmx2g -XX:MaxDirectMemorySize=2g"}}
 
 # Garbage collection options
 BOOKIE_GC=${BOOKIE_GC:-${PULSAR_GC:-"-XX:+UseG1GC -XX:MaxGCPauseMillis=10 -XX:+ParallelRefProcEnabled -XX:+UnlockExperimentalVMOptions -XX:+DoEscapeAnalysis -XX:ParallelGCThreads=32 -XX:ConcGCThreads=32 -XX:G1NewSizePercent=50 -XX:+DisableExplicitGC"}}
 
-IS_JAVA_8=`java -version 2>&1 |grep version|grep '"1\.8'`
+if test -z "$JAVA_HOME"

Review comment:
       replace `test` with `[` ... `]`
   ```suggestion
   if [ -z "$JAVA_HOME" ]
   ```

##########
File path: conf/pulsar_env.sh
##########
@@ -48,7 +48,12 @@ PULSAR_MEM=${PULSAR_MEM:-"-Xms2g -Xmx2g -XX:MaxDirectMemorySize=4g"}
 PULSAR_GC=${PULSAR_GC:-"-XX:+UseG1GC -XX:MaxGCPauseMillis=10 -XX:+ParallelRefProcEnabled -XX:+UnlockExperimentalVMOptions -XX:+DoEscapeAnalysis -XX:ParallelGCThreads=32 -XX:ConcGCThreads=32 -XX:G1NewSizePercent=50 -XX:+DisableExplicitGC"}
 
 # Garbage collection log.
-IS_JAVA_8=`java -version 2>&1 |grep version|grep '"1\.8'`
+if test -z "$JAVA_HOME"

Review comment:
       replace `test` with `[` ... `]`
   ```suggestion
   if [ -z "$JAVA_HOME" ]
   ```




-- 
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] lhotari commented on a change in pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711#discussion_r827913517



##########
File path: conf/bkenv.sh
##########
@@ -32,15 +32,21 @@ BOOKIE_CONF=${BOOKIE_CONF:-"$BK_HOME/conf/bookkeeper.conf"}
 # BOOKIE_LOG_CONF=
 
 # Logs location
-# BOOKIE_LOG_DIR=
+# PULSAR_LOG_DIR=

Review comment:
       yes, would be better to revert the change since it doesn't look related to JAVA_HOME 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

[GitHub] [pulsar] github-actions[bot] commented on pull request #14711: Support JAVA_HOME in env file when check java version

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


   @gaozhangmin:Thanks for providing doc info!


-- 
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] gaozhangmin commented on pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711#issuecomment-1069172886


   @codelipenghui  @lhotari  PTAL 


-- 
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] gaozhangmin commented on a change in pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711#discussion_r827764095



##########
File path: conf/bkenv.sh
##########
@@ -32,15 +32,21 @@ BOOKIE_CONF=${BOOKIE_CONF:-"$BK_HOME/conf/bookkeeper.conf"}
 # BOOKIE_LOG_CONF=
 
 # Logs location
-# BOOKIE_LOG_DIR=
+# PULSAR_LOG_DIR=

Review comment:
       not related @codelipenghui。 Should I open an new pr?




-- 
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] codelipenghui commented on a change in pull request #14711: Support JAVA_HOME in env file when check java version

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14711:
URL: https://github.com/apache/pulsar/pull/14711#discussion_r827761870



##########
File path: conf/bkenv.sh
##########
@@ -32,15 +32,21 @@ BOOKIE_CONF=${BOOKIE_CONF:-"$BK_HOME/conf/bookkeeper.conf"}
 # BOOKIE_LOG_CONF=
 
 # Logs location
-# BOOKIE_LOG_DIR=
+# PULSAR_LOG_DIR=

Review comment:
       Is it a related change?
   
   Others look good to me.




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