yashmayya opened a new pull request, #19103:
URL: https://github.com/apache/pinot/pull/19103
The JDK 25 bump (#19014) left a number of JDK 21 references behind. Most are
cosmetic, but two are functional breakages. Follow-up to #19102.
### Functional fixes
**`docker/images/pinot/docker-build.sh` could not build Pinot at all.** With
no 4th argument it defaulted to `JAVA_VERSION=21`, which sets
`PINOT_BASE_IMAGE_TAG=21-amazoncorretto`. That image ships Corretto 21.0.11,
and the root pom's enforcer requires `[25,)`, so the build failed in
`validate`. Verified locally by running Pinot's exact `requireJavaVersion` rule
inside both base images:
```
apachepinot/pinot-base-build:21-amazoncorretto -> [ERROR] Apache Pinot
requires JDK 25 or newer to build. BUILD FAILURE
apachepinot/pinot-base-build:25-amazoncorretto -> BUILD SUCCESS
```
**`.pinot_base_docker_image_build_and_push.sh` could publish a mislabeled
base image.** It passed `--build-arg JAVA_VERSION=${JDK_VERSION:-21}`, and the
image *contents* come from `JDK_VERSION` while the published *tag* comes from
the independent `TAG` variable. An invocation with `TAG=25-amazoncorretto` and
`JDK_VERSION` unset would have built a JDK 21 image, succeeded, and pushed it
as `pinot-base-runtime:25-amazoncorretto` — a base image whose JRE cannot load
Pinot's `release=25` bytecode. This is `workflow_dispatch`-only so CI always
set the variable, but the fallback made the failure silent and unrecoverable
once pushed. Now hard-fails when unset, matching
`.pinot_compile_and_push_build_image.sh` and
`.pinot_package_single_platform.sh`. Note this `--build-arg` also *overrode*
the dockerfile `ARG` defaults, so fixing those without this would have had no
effect through the only script that builds them.
### Stale configuration
- `build-pinot-base-docker-image.yml`: dropped `21` from both matrices.
**This stops publishing `pinot-base-{build,runtime}:21-*`**, so those tags
freeze and stop receiving OS/JDK CVE patches — worth a line in the release
notes. Nothing references them: every consumer derives its tag from a
JDK-25-only source.
- The four `pinot-base` dockerfiles: `ARG JAVA_VERSION` default 21 → 25.
- Docs stating the wrong baseline: `AGENTS.md` (3 places; its counterpart in
`CLAUDE.md` was already updated, so the two disagreed and `AGENTS.md`
contradicted itself), `.github/copilot-instructions.md`,
`docker/images/pinot/README.md`, and `MseBlock.java`'s class Javadoc.
- `pinot-clients/pinot-cli/README.md` said "Java 21+ on PATH". That one was
wrong rather than conservative: `pinot-cli` compiles at `${jdk.version}` with
no `release=11` override, so the shipped CLI throws
`UnsupportedClassVersionError` on a JDK 21 PATH.
### Dead JDK version guard in the launcher template
`appAssemblerScriptTemplate` gated the `--add-opens` flags behind `if [
"$(jdk_version)" -gt 11 ]`, which can no longer be false. Removing it also
fixes a latent bug: `jdk_version()` probed bare `java` on `PATH` rather than
the `$JAVACMD` actually exec'd, so a stale or missing PATH `java` made the
guard false (or printed `integer expression expected`) and silently dropped the
module-access flags while running on 25 — producing exactly the
`InaccessibleObjectException` those flags exist to prevent. It also forked a
JVM on every script invocation.
Verified behavior-identical: compared the old and new logic across
`{JAVA_OPTS unset, set, set + PLUGINS_INCLUDE}` (byte-identical `ALL_JAVA_OPTS`
in all three), then built `-Pbin-dist` and traced the real generated
`pinot-admin.sh` to confirm the exec line still carries all six `--add-opens`,
both Netty properties and `-Dplugins.dir`. `bin/pinot-admin.sh -help` runs
clean.
### Deliberately not fixed here
Removing the guard exposes that this assignment *overwrites*
`@EXTRA_JVM_ARGUMENTS@` instead of appending, so the per-program `jvmSettings`
in `pinot-tools/pom.xml` — heap sizes, `-Dlog4j2.configurationFile`,
`-Dpinot.admin.system.exit` — are discarded whenever `JAVA_OPTS` is unset. That
is long-standing behavior (the guard was always true, so they were already
being dropped), and appending them would change the effective heap, logging
config and admin exit behavior of every generated script. It needs to be a
deliberate change, not a side effect of a cleanup, so it is left as-is with a
comment. Happy to file a separate issue if that's preferred.
I also kept the now-provably-dead `if [ -z "$JAVA_OPTS" ]` block rather than
deleting it: removing it would drop the `@EXTRA_JVM_ARGUMENTS@` token from the
template entirely and leave the pom's `jvmSettings` with no representation at
all, making that config silently inert and foreclosing the fix above.
Stale JDK 21 mentions in `kb/` and `pinot-perf/README.md` are left for a
separate pass — they describe agent/benchmark guidance rather than build or
runtime configuration.
### Testing
Beyond the enforcer probe and script verification above, built both runtime
base images with the new default ARG (no `--build-arg`) and confirmed the JDK
inside:
```
amazoncorretto -> Corretto-25.0.4.7.1,
JAVA_HOME=/usr/lib/jvm/java-25-amazon-corretto
ms-openjdk -> Microsoft build 25.0.4+7-LTS,
JAVA_HOME=/usr/lib/jvm/msopenjdk-25
```
The `ms-openjdk` files re-declare `ARG JAVA_VERSION` without a value in the
second stage, which correctly inherits the pre-`FROM` default — hence only one
default needed changing there versus two in the corretto files. Confirmed the
new `JDK_VERSION` guard exits 1 before invoking `docker build`. `spotless`,
`checkstyle` and `license:check` pass on `pinot-tools`, `pinot-query-runtime`
and `pinot-cli`; `pinot-query-runtime` test-compiles clean.
I did not rebuild the `pinot-base-build` images locally (the corretto one
compiles Thrift from source); their JDK 25 variants are already published by
CI, and the JDK acquisition steps are identical to the runtime images verified
above.
--
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]
