lhotari commented on code in PR #25274:
URL: https://github.com/apache/pulsar/pull/25274#discussion_r2878197166
##########
conf/pulsar_env.sh:
##########
@@ -89,8 +89,13 @@ if [[ -z "$PULSAR_GC_LOG" ]]; then
fi
fi
-# Extra options to be passed to the jvm
-PULSAR_EXTRA_OPTS="${PULSAR_EXTRA_OPTS:-" -Dpulsar.allocator.exit_on_oom=true
-Dio.netty.recycler.maxCapacityPerThread=4096"}"
+# Extra options to be passed to the jvm, typically used for passing
user-defined JVM OPTS configurations.
+# This PULSAR_EXTRA_OPTS parameter has a higher priority than the predefined
JVM OPTS in the Pulsar bin files.
+# Therefore, in addition to defining extra JVM parameters here,
+# it can also be used to override JVM parameters with the same name in OPTS
+# For example, if IPv6 functionality needs to be enabled, configure:
+# PULSAR_EXTRA_OPTS="${PULSAR_EXTRA_OPTS} -Djava.net.preferIPv4Stack=false"
+PULSAR_EXTRA_OPTS="${PULSAR_EXTRA_OPTS}"
Review Comment:
This can be removed. The documentation for `PULSAR_EXTRA_OPTS` should be in
`bin/pulsar` where it matters the most.
This line isn't useful at all
```
PULSAR_EXTRA_OPTS="${PULSAR_EXTRA_OPTS}"
```
##########
bin/pulsar:
##########
@@ -300,6 +300,14 @@ OPTS="$OPTS --add-opens java.base/java.nio=ALL-UNNAMED
--add-opens java.base/jdk
OPTS="$OPTS --add-opens java.base/jdk.internal.platform=ALL-UNNAMED"
# Required by RocksDB java.lang.System::loadLibrary call
OPTS="$OPTS --enable-native-access=ALL-UNNAMED"
+# These two settings work together to ensure the Pulsar process exits
immediately and predictably
+# if it runs out of either Java heap memory or its internal off-heap memory,
+# as these are unrecoverable errors that require a process restart to clear
the faulty state and restore operation
+OPTS="-XX:+ExitOnOutOfMemoryError -Dpulsar.allocator.exit_on_oom=true $OPTS"
+# Netty tuning
+# These settings are primarily used to modify the Netty allocator
configuration,
+# improving memory utilization and reducing the frequency of requesting
off-heap memory from the OS
Review Comment:
for the `bin/pulsar` script it would be useful to explain at least the unit
used for io.netty.allocator.maxOrder and the impact of it. Unfortunately there
doesn't seem to be Netty documentation that could be referenced directly.
Based on the source code, the default chunk size for the allocator will be
the value of io.netty.allocator.pageSize (default: 8192) <<
io.netty.allocator.maxOrder (default set here: 10).
8192 << 10 = 8192 * 2^10 = 8192 * 1024 = 8388608 = 8 MB
https://github.com/netty/netty/blob/4.1/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L105
Allocations that are larger than chunk size are considered huge allocations
and don't use the pool:
https://github.com/netty/netty/blob/4.1/buffer/src/main/java/io/netty/buffer/PoolArena.java#L141-L142
Summarizing and simplifying this information would be useful.
##########
deployment/terraform-ansible/templates/pulsar_env.sh:
##########
@@ -48,7 +48,7 @@ PULSAR_MEM=" -Xms{{ max_heap_memory }} -Xmx{{ max_heap_memory
}} -XX:MaxDirectMe
PULSAR_GC=" -XX:+UseZGC -XX:+PerfDisableSharedMem -XX:+AlwaysPreTouch"
# Extra options to be passed to the jvm
-PULSAR_EXTRA_OPTS="-Dio.netty.leakDetection.level=disabled
-Dio.netty.recycler.maxCapacityPerThread=4096 ${PULSAR_EXTRA_OPTS}"
+PULSAR_EXTRA_OPTS="-Dio.netty.leakDetection.level=disabled
-Dio.netty.recycler.maxCapacityPerThread=4096 -Dio.netty.allocator.maxOrder=10
${PULSAR_EXTRA_OPTS}"
Review Comment:
This can be removed completely since `pulsar.broker.service` uses
`bin/pulsar broker` to start the broker and settings in `bin/pulsar` are used.
There's no need to set `PULSAR_EXTRA_OPTS`.
##########
tests/integration/src/test/java/org/apache/pulsar/tests/integration/containers/PulsarContainer.java:
##########
@@ -301,7 +301,9 @@ protected void passNettyLeakDetectionSystemProperties() {
protected void initializePulsarExtraOpts() {
appendToEnv("PULSAR_EXTRA_OPTS",
- "-Dpulsar.allocator.exit_on_oom=true
-Dio.netty.recycler.maxCapacityPerThread=4096");
+ "-Dpulsar.allocator.exit_on_oom=true "
+ + "-Dio.netty.recycler.maxCapacityPerThread=4096 "
+ + "-Dio.netty.allocator.maxOrder=10");
Review Comment:
This can be removed. Just leave the empty method `initializePulsarExtraOpts`.
##########
bin/function-localrunner:
##########
@@ -76,8 +76,11 @@ if [[ -z "$PULSAR_GC_LOG" ]]; then
fi
fi
-# Extra options to be passed to the jvm
-PULSAR_EXTRA_OPTS=${PULSAR_EXTRA_OPTS:-" -Dpulsar.allocator.exit_on_oom=true
-Dio.netty.recycler.maxCapacityPerThread=4096"}
+# Extra options to be passed to the jvm，typically used for passing
user-defined JVM OPTS configurations.
+# This PULSAR_EXTRA_OPTS parameter has a higher priority than the predefined
JVM OPTS.
+# Therefore, in addition to defining extra JVM parameters here, it can also be
used to override JVM parameters with the same name in OPTS
+# For example, if IPv6 functionality needs to be enabled, configure:
PULSAR_EXTRA_OPTS="-Djava.net.preferIPv4Stack=false".
+PULSAR_EXTRA_OPTS="${PULSAR_EXTRA_OPTS}"
Review Comment:
this can be removed from the function-localrunner script.
`PULSAR_EXTRA_OPTS="${PULSAR_EXTRA_OPTS}"` is not useful.
The comments in `function-localrunner` aren't 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
