xiangfu0 commented on code in PR #20244: URL: https://github.com/apache/pinot/pull/20244#discussion_r4550856916
########## pinot-tools/src/main/resources/appAssemblerScriptTemplate: ########## @@ -212,7 +215,17 @@ if [ "$(jdk_version)" -gt 11 ]; then --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED \ -Dio.netty.tryReflectionSetAccessible=true \ -Dio.grpc.netty.shaded.io.netty.tryReflectionSetAccessible=true \ - $JAVA_OPTS" + $ALL_JAVA_OPTS" Review Comment: **MAJOR (default change needs sign-off):** Services now require JDK 25, so this affects every tarball launch with an empty `JAVA_OPTS`, not just `pinot-admin`. For example, `start-server`, `start-broker`, `start-controller`, `start-minion` and `start-service-manager` are now capped at `-Xmx1G`. Before, they got the JVM default of 25% of RAM. I checked a rendered script on JDK 25: max heap went from 1992 MB to 1024 MB. The log config also changes from the classpath `log4j2.xml` (cid/MDC pattern) to `conf/log4j2.xml`. Please list these effects in the description and release notes, or move this fix to its own PR. Docker and Helm set `JAVA_OPTS`, so they are not affected. AI-generated review via IntentLab · Model: claude-opus-5-5 (Claude Opus 5.5). ########## pinot-tools/src/main/resources/appAssemblerScriptTemplate: ########## @@ -203,7 +203,10 @@ else ALL_JAVA_OPTS=$JAVA_OPTS fi -if [ "$(jdk_version)" -gt 11 ]; then +JDK_VERSION=$(jdk_version) Review Comment: **MINOR:** `jdk_version` runs `java` from `PATH`, not `$JAVACMD`. If `JAVA_HOME` points to JDK 25 but an older `java` comes first on `PATH`, the JDK 24+ options are skipped without any message. I reproduced this: gRPC still had `hasUnsafe=false`. This is an existing bug, but this option now depends on it. Running `"$JAVACMD" -Xms32M -Xmx32M -version` inside `jdk_version` would fix it. AI-generated review via IntentLab · Model: claude-opus-5-5 (Claude Opus 5.5). -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
