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/06/20 10:13:16 UTC

[GitHub] [pulsar] Nicklee007 opened a new pull request, #16143: [improve][broker]Specify default nar tmp dir

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

   Master Issue: #15978
   
   relate to PR #15979
   
   ### Motivation
   
   In some Linux os, like Centos or RedHat will auto clean the file in the /tmp dir when 7 days or 30 days the file not be used, In this way, the .class file will loss, so we should change the default nar tmp dir to `$PULSAR_HOME ` will better.
   The PR #15979  help we get the `DEFAULT_NAR_EXTRACTION_DIR` from `nar.extraction.tmpdir`.
   
   ### Modifications
   
   Change the `DEFAULT_NAR_EXTRACTION_DIR`;
   
   - [X ] `doc-required` 
   The `DEFAULT_NAR_EXTRACTION_DIR` change to the `$PULSAR_HOME/tmp`,  user should permit pulsar can write/read the dir.


-- 
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 #16143: [improve][broker]Specify default nar tmp dir

Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun closed pull request #16143: [improve][broker]Specify default nar tmp dir
URL: https://github.com/apache/pulsar/pull/16143


-- 
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] Nicklee007 commented on a diff in pull request #16143: [improve][broker]Specify default nar tmp dir

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


##########
bin/pulsar:
##########
@@ -307,6 +307,9 @@ if [[ -z "$IS_JAVA_8" ]]; then
   OPTS="$OPTS --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED"
 fi
 
+# Specify default nar tmp dir
+OPTS="$OPTS -Dnar.extraction.tmpdir=$PULSAR_HOME/tmp"

Review Comment:
   @eolivelli Relate to PR https://github.com/apache/pulsar/pull/15979, we define `nar.extraction.tmpdir` , the dir only contain some class file which extraction from nar package, use the dir in pulsar can avoid those .class file be clean by os in some linux os;
   Diff linux os has diff behaviours for the /tmp dir clean, Debian will clean when the boot , but as Centos or RedHat will auto clean the file in the /tmp dir when 7 days or 30 days the file not be used, In this way, the .class file will loss.
   In our case, we add the KOP protocol, after few days when we want use the KOP protocol, we find some .class have been cleaned by os.



-- 
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 #16143: [improve][broker]Specify default nar tmp dir

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

   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] tisonkun commented on pull request #16143: [improve][broker]Specify default nar tmp dir

Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #16143:
URL: https://github.com/apache/pulsar/pull/16143#issuecomment-1612218934

   Closing...
   
   I notice that you can configure `DEFAULT_NAR_EXTRACTION_DIR` by passing options already. There are many environments varying in cases so you don't need to change the default.


-- 
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] eolivelli commented on a diff in pull request #16143: [improve][broker]Specify default nar tmp dir

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


##########
bin/pulsar:
##########
@@ -307,6 +307,9 @@ if [[ -z "$IS_JAVA_8" ]]; then
   OPTS="$OPTS --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED"
 fi
 
+# Specify default nar tmp dir
+OPTS="$OPTS -Dnar.extraction.tmpdir=$PULSAR_HOME/tmp"

Review Comment:
   we should make it overridable by a ENV variable.
   for instance in the docker images it is better to not let the nar files be unpacked in /pulsar/tmp
   the same applies for regular non-docker/non-k8s installations, the PULSAR_HOME directory contains "code" and "configuration" it shouldn't contain an arbitrary amount of data)
   



-- 
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] Nicklee007 commented on pull request #16143: [improve][broker]Specify default nar tmp dir

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

   > LGTM
   > 
   > but this should go into 2.12, so I am keeping "request changes" for now
   
   @eolivelli Could you help review this change again, looks like 2.12.0 tag can be add, 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] Nicklee007 commented on pull request #16143: [improve][broker]Specify default nar tmp dir

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

   > LGTM
   > 
   > but this should go into 2.12, so I am keeping "request changes" for now
   
   @eolivelli Could you help review this change again, looks like 2.12.0 tag can be add, 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] Nicklee007 commented on pull request #16143: [improve][broker]Specify default nar tmp dir

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

   > LGTM
   > 
   > but this should go into 2.12, so I am keeping "request changes" for now
   
   @eolivelli  Could you help review this change again, looks like 2.12.0 tag can be add, 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] tisonkun commented on a diff in pull request #16143: [improve][broker]Specify default nar tmp dir

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


##########
bin/pulsar:
##########
@@ -307,6 +307,9 @@ if [[ -z "$IS_JAVA_8" ]]; then
   OPTS="$OPTS --add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED"
 fi
 
+# Specify default nar tmp dir
+OPTS="$OPTS -Dnar.extraction.tmpdir=$PULSAR_HOME/tmp"

Review Comment:
   I agree that this option should be configurable.
   
   Otherwise, you can actually run `OPTS=-Dnar.extraction.tmpdir=/path/to/pulsar/tmp bin/pulsar ...` and achieve the same function.



-- 
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] Nicklee007 commented on pull request #16143: [improve][broker]Specify default nar tmp dir

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

   ping


-- 
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] Nicklee007 commented on pull request #16143: [improve][broker]Specify default nar tmp dir

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

   > LGTM
   > 
   > but this should go into 2.12, so I am keeping "request changes" for now
   
   OK, 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