gortiz opened a new pull request, #20244:
URL: https://github.com/apache/pinot/pull/20244

   ## Summary
   
   On JDK 24+, the launcher scripts now add 
`--sun-misc-unsafe-memory-access=allow` and 
`--enable-native-access=ALL-UNNAMED`. Without the first option, Netty 4.2 runs 
without `sun.misc.Unsafe` on JDK 25+. `NettyInspector` gets a new check that 
logs a warning in that case. The launcher also stops dropping the JVM settings 
of the program on JDK 12+.
   
   ## Background
   
   Netty 4.2 disables `sun.misc.Unsafe` by default on JDK 25+ 
(`io.netty.noUnsafe=true by default on Java 25+`). The 
`tryReflectionSetAccessible` properties that the launcher sets have no effect 
on these JDKs. Both Netty copies are affected: the unshaded one and the one 
inside `grpc-netty-shaded`.
   
   Without Unsafe, Netty uses `CleanerJava25`. This cleaner creates and closes 
one shared `Arena` for each pooled chunk. Measured on a laptop, 4 threads, one 
allocate plus free per operation:
   
   | Cleaner | 1 MiB | 8 MiB |
   |---|---|---|
   | `CleanerJava25` (current default on JDK 25 and 27) | 318–482 µs | 598–804 
µs |
   | `DirectCleaner` (with `allow`) | 5–6 µs | 6–7 µs |
   
   Also, `PlatformDependent.allocateMemory` throws `NullPointerException`. 
`ArrowAllocators` uses it for buffers over 2 GB. `PinotDataBuffer` 
(`UnsafePinotBufferFactory`) works with the JDK default `warn`. With `allow`, 
its JEP 471 warning goes away.
   
   `NettyInspector.checkDirectMemory` passes on JDK 25 because reflection 
access works. As a result, nothing in the logs shows that Netty runs without 
Unsafe.
   
   ## Changes
   
   `appAssemblerScriptTemplate`:
   - On JDK 24+, the two options go before the other JVM options. `JAVA_OPTS` 
comes later on the command line, so a value set there overrides them. The last 
value of the option wins (tested on JDK 25 and 27).
   - `--enable-native-access=ALL-UNNAMED` removes the JEP 472 warnings from JNI 
and FFM libraries.
   - The JDK 12+ block replaced `ALL_JAVA_OPTS` with `JAVA_OPTS`. When 
`JAVA_OPTS` was empty, this dropped the `jvmSettings` of the program, for 
example `-Dlog4j2.configurationFile=conf/log4j2.xml` and `-Xms4G` for 
`pinot-admin`. The block now adds to `ALL_JAVA_OPTS`. JDK 11 already used these 
settings.
   - `jdk_version` runs once, not once for each check.
   
   `NettyInspector` / `NettyInstance`:
   - New `checkUnsafe` check. It fails when `PlatformDependent.hasUnsafe()` is 
false, and the message gives the cause and the JVM option to add.
   - `NettyInstance` exposes `hasUnsafe()` and `getUnsafeUnavailabilityCause()`.
   - `CheckResult` exposes `getMessage()` and `getStatus()`.
   
   ## Behavior change
   
   If you start `pinot-admin.sh` on JDK 12+ with an empty `JAVA_OPTS`, it now 
gets `-Xms4G` and the log4j2 configuration file of the distribution. JDK 11 
already behaved this way. If `JAVA_OPTS` is set, as in the Docker images, 
nothing changes.
   
   ## Validation
   
   - New `NettyInspectorTest` (3 tests) passes.
   - I rendered the template into a launch script and started a probe that 
prints `PlatformDependent.hasUnsafe()` for both Netty copies:
   
   | JDK | Before | After |
   |---|---|---|
   | 21 | `hasUnsafe=true` | `hasUnsafe=true`, options unchanged |
   | 25 | `hasUnsafe=false` | `hasUnsafe=true` |
   | 27 | `hasUnsafe=false` | `hasUnsafe=true` |
   
   - `JAVA_OPTS="--sun-misc-unsafe-memory-access=deny"` gives 
`hasUnsafe=false`. This makes sure that a user value overrides the new default.
   - `spotless:check`, `checkstyle:check` and `license:check` pass on 
`pinot-core` and `pinot-tools`.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


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

Reply via email to