yashmayya opened a new pull request, #19102:
URL: https://github.com/apache/pinot/pull/19102
## What this closes
Six modules deliberately compile to Java 11 bytecode with a hard-coded
compiler `release` (not inherited from `${jdk.version}`), so that third-party
plugins built against `pinot-spi` and applications embedding the Java/JDBC
client are not forced onto JDK 25:
`pinot-spi`, `pinot-segment-spi`, `pinot-timeseries/pinot-timeseries-spi`,
`pinot-common`, `pinot-clients/pinot-java-client`,
`pinot-clients/pinot-jdbc-client`.
`--release 11` already guarantees Pinot's **own** code in those modules is
Java-11-clean — it restricts against the Java 11 API signature set, not just
the bytecode version. **The gap is the transitive dependency closure, which
nothing verifies.** Today it happens to hold (arrow-vector 19.0.0 is major 55,
calcite-core 1.42.0 is major 52, helix-core 2.0.1 is major 55), but any routine
dependency bump could ship a Java 17+ jar into the client's closure and nobody
would notice until a user on Java 11 reports `UnsupportedClassVersionError` or
`NoSuchMethodError`. Every existing matrix in `.github/workflows/` is `java: [
25 ]`; no job anywhere loads these artifacts on a Java 11 JVM.
A job that only inspected class-file major versions of Pinot's own jars
would be vacuous, so this one **executes** the clients on a real Java 11 JVM.
## Shape
The build cannot run on Java 11 — the root pom enforces `requireJavaVersion
[25,)`, and Pinot's services have a genuine Java 25 floor (`datasketches-java`
9.0.0 is major 69 and the code uses `java.lang.foreign.MemorySegment`, see
28922369c8 / #19014). `-Djdk.version=11` does not help; that knob only lowers
the compile target while the enforcer checks the *running* JDK. So the job is
necessarily two-JDK: **build with JDK 25, then run the verification under Java
11.**
- `.github/workflows/pinot_java11_client_compatibility.yml` — installs Java
11 first and JDK 25 second (so `JAVA_HOME` is the build JDK), and passes the
Java 11 path via `steps.java11.outputs.path`. I confirmed `path` is a real
output of `actions/setup-java@v5` rather than assuming it; the script
additionally falls back to `JAVA_HOME_11_X64` / `JAVA_HOME_11_ARM64` so it does
not depend on the runner architecture.
- `.github/workflows/scripts/.pinot_java11_client_compat.sh` — resolves the
Java 11 JVM, builds `-pl pinot-java11-client-verifier -am`, then launches the
verifier with the resolved runtime closure.
- `pinot-java11-client-verifier/` — a new module at the reactor root,
alongside the existing `pinot-compatibility-verifier` and
`pinot-dependency-verifier` (both of which are root modules, and the former has
exactly this shape: a runnable `src/main/java` verifier driven by a script in
`.github/workflows/scripts/`). Compiled at `release 11` itself, since the
script launches its classes on the Java 11 JVM. `maven.deploy.skip` keeps a CI
harness out of the published artifacts, and nothing in the reactor depends on
it.
The module depends on both clients, which makes its resolved runtime closure
the union of the two consumer-facing closures.
`maven-dependency-plugin:build-classpath` writes that closure to
`target/runtime-classpath.txt` at `prepare-package`, and the script hands
exactly that to `java -cp`.
## What the 18 checks do
**Closure scan** (`ClasspathClosureScanner`) — walks all ~220 classpath
entries and fails on any class file a Java 11 JVM could not load. This is a
legitimate *complement* to `--release`, not a duplicate: it covers third-party
jars, which `--release` says nothing about. Two categories are correctly not
violations, because a Java 11 JVM never loads them:
- `module-info.class` descriptors, at any version.
- `META-INF/versions/<n>/` entries where `n` > the target release.
The multi-release handling is load-bearing, not defensive:
**`jackson-core-2.22.1` ships major 61 under `META-INF/versions/17/` and major
65 under `versions/21/`, and `jersey-common-2.48` ships major 65** — without it
the job would fail on a dependency every Pinot consumer has.
**Runtime exercises** — these construct and use the artifacts, pulling in
Jackson, Arrow, Calcite, Helix, protobuf, gRPC/Netty, async-http-client and the
JNI compression codecs:
| Check | What it pulls in |
|---|---|
| `spi-schema-deserialization`, `spi-table-config-deserialization` |
pinot-spi + Jackson |
| `segment-spi-data-buffer` | off-heap `PinotDataBuffer` — `Unsafe` /
`sun.misc.Cleaner` reflection, which the class-file scan cannot see |
| `timeseries-spi-plan-serde` | `TimeSeriesPlanSerde` round trip,
`TimeBuckets` |
| `common-data-schema-round-trip`, `common-broker-response-deserialization`
| `DataSchema` binary + JSON, `BrokerResponseNative` over a realistic broker
response |
| `common-response-encoders` | JSON **and Arrow** encode/decode —
arrow-vector + arrow-memory-netty are the likeliest source of a floor bump, and
they only fail when actually exercised |
| `common-calcite-sql-parsing` | `CalciteSqlParser` on a
group-by/order-by/limit query plus a `SET` option |
| `common-helix-segment-metadata` | `SegmentZKMetadata` ↔ `ZNRecord` via
`ZNRecordSerializer`, `ExternalView` |
| `common-grpc-response-decoding` | the gRPC decode path end to end:
protobuf `BrokerResponse` → every compression codec → every encoder, exactly as
`GrpcConnection` unpacks a server response |
| `common-grpc-channel-construction` | `BrokerGrpcQueryClient` → Netty
channel + pooled direct-buffer allocator |
| `java-client-http-transport` | async-http-client / Netty transport
construction |
| `java-client-query-execution`, `java-client-prepared-statement` |
`ConnectionFactory` → `Connection.execute` → `ResultSetGroup` values +
`ExecutionStats`, against a canned transport |
| `jdbc-driver-registration` | `DriverManager` auto-discovery through
`META-INF/services/java.sql.Driver` and `ServiceLoader`, with no explicit
`Class.forName` |
| `jdbc-result-set` | `PinotResultSet` rows, `ResultSetMetaData`, SQL type
mapping |
No live cluster, per the tradeoff below.
**Vacuity guards.** A green job that cannot go red is worse than no job, so
three things are asserted before any of the above counts:
- the JVM really is Java 11 (`Runtime.version().feature()`), because running
on the build JDK would make every other check pass for the wrong reason.
Failing this aborts the run rather than printing 17 meaningless passes.
- at least 100 jars and 20,000 class files were inspected **inside
archives**, counted separately from directory entries — otherwise the
verifier's own `target/classes` would satisfy the guard on its own and make it
unfalsifiable.
- all six Java-11-pinned modules are present on the scanned closure, so
marking one `optional`/`provided` shrinks coverage loudly instead of silently.
## Proving it can fail
Run locally against Temurin 11.0.22. Baseline: 18/18 pass, exit 0, `major
versions: {46=236, 47=1778, 48=104, 49=3104, 50=4391, 51=1612, 52=52314, 53=15,
55=5926}` — no bucket above 55, and the counts make it evident the scan really
looked.
**1. Third-party dependency regression (the actual threat).** Added
`org.apache.datasketches:datasketches-java` — already at 9.0.0 in
`dependencyManagement`, and major 69 precisely because #19014 moved it to Java
25 — to `pinot-java-client`. This is the realistic shape: someone adds a
sketch-backed client helper and silently breaks every Java 11 consumer.
```
scanned 219 archives (69913 class files) ... major versions: {..., 55=5926,
69=440}
FAIL classpath-closure-is-loadable
440 class file(s) on the client runtime closure cannot be loaded by
Java 11. A dependency was
bumped to a release that no longer supports Java 11; pin it back or
drop it from the client
closure. Offenders:
.../datasketches-java-9.0.0.jar!/org/apache/datasketches/common/ArrayOfBooleansSerDe.class
(class file major version 69, needs Java 25)
... and 400 more
```
Script exit 1.
**2. Post-Java-11 API with the pin in place.** Added
`Arrays.stream(_columnNames).toList()` (Java 16+) to `DataSchema.toString()`.
`--release 11` rejects it at **compile** time:
```
DataSchema.java:[226,73] cannot find symbol
symbol: method toList()
location: interface Stream<String>
```
This is the pre-existing guarantee, and it is exactly why the closure scan
is a complement rather than a duplicate — this failure mode cannot reach
runtime while the pin is in place.
**3. Pin removed — proving the *execution* checks go red, not just the
scan.** Kept the `toList()` call and changed `pinot-common` to `release 17`. It
now compiles, and pinot-common ships major 61:
```
scanned 218 archives ... major versions: {..., 55=4738, 61=1188}
FAIL classpath-closure-is-loadable 1188 class file(s) ... cannot be
loaded by Java 11
FAIL common-data-schema-round-trip
java.lang.UnsupportedClassVersionError:
org/apache/pinot/common/utils/DataSchema has been
compiled by a more recent version of the Java Runtime (class file
version 61.0), this version
of the Java Runtime only recognizes class file versions up to 55.0
FAIL common-broker-response-deserialization ... BrokerResponseNative
... 61.0
FAIL common-response-encoders ... ResultTable ... 61.0
FAIL common-calcite-sql-parsing ... CalciteSqlParser ...
61.0
FAIL common-helix-segment-metadata ... SegmentZKMetadata ...
61.0
FAIL common-grpc-response-decoding ... ResultTable ... 61.0
FAIL common-grpc-channel-construction ... BrokerGrpcQueryClient
... 61.0
FAIL java-client-query-execution ...
```
Eight exercise checks fail by name, so the harness catches a real load
failure and not merely a jar inspection.
**4. Mis-wired JDK.** Same classpath on JDK 25:
```
FAIL jvm-is-at-target-feature-version
expected to be running on a Java 11 JVM but this is Java 25 (...);
verifying the clients on
the build JDK would make every other check vacuous
Aborting: the remaining 17 checks would not tell us anything on this JVM.
```
Exit 1. (Arrow also fails on 25 without `--add-opens`, which is consistent
with the deliberate absence of those flags — see below.)
**5. Coverage silently shrinking.** Swapped the `pinot-timeseries-spi` jar
for an identically-contented file under a different name, so its classes still
load but the artifact is unrecognisable:
```
FAIL classpath-closure-is-loadable
these Java 11 pinned modules are not on the verified closure, so
this job is no longer
checking them: [pinot-timeseries-spi]. Either restore the dependency
or update
JAVA11_PINNED_MODULES.
```
All injections reverted; the tree is clean and the baseline run is green
again.
`ClasspathClosureScannerTest` (14 tests, runs in the normal unit-test job —
no Java 11 JVM needed) pins the scanner's fail path, since both of its filters
fail *open* and a regression there would produce a permanently green job rather
than a red one. It covers: major 65 at the jar root → violation;
`module-info.class` at 65 → ignored; `META-INF/versions/17|21/` above target →
ignored; `versions/9/` at or below target → violation; malformed version
directory → ignored; missing `0xCAFEBABE` and truncated entries → not counted;
archive vs directory counters kept apart; corrupt zip → `IOException`;
reporting capped while the total count stays exact; target version honoured at
11/17/21.
## Notes on two deliberate choices
**No live cluster.** A cross-JVM integration test (cluster on JDK 25, client
on Java 11) would be higher fidelity, but much slower and flakier, and it would
put a multi-process cluster on the critical path of what should be a fast
signal on every PR. The canned-transport approach still drives the real
`Connection` / `ResultSetGroup` / `PinotResultSet` code and the real
deserialization paths, which is where a bad bump surfaces. If someone wants the
full-fidelity version later it belongs in a separate opt-in job, not as a
dependency of this smoke.
**No `--add-opens` / `-Dio.netty.tryReflectionSetAccessible=true`.** Those
flags exist in Pinot's launch scripts for JDK 17+; adding them here would paper
over the runtime breakage this job exists to catch. Verified empirically that
nothing on the closure needs them on Java 11 — Arrow logs an
illegal-reflective-access warning and works.
`-Dshade.phase.prop=none` on the build is worth a note: `pinot-java-client`
and `pinot-jdbc-client` each produce a ~148 MB shaded jar, and
`shadedArtifactAttached=true` means those jars never appear on the classpath
this job verifies. Without the flag the job spends minutes building 296 MB it
does not use and then pushes it into `~/.m2`, which `actions/cache` uploads
under a key the other Maven jobs share. `-DskipShade=true` is **not**
sufficient — it only deactivates the `pinot-jdbc-client` profile, while
`pinot-java-client` sets `shade.phase.prop=package` unconditionally. Both
verified by inspecting the produced jars.
## Follow-ups, deliberately not in this PR
Found while analysing this area; left out to keep the change to one concern:
- `.github/workflows/build-pinot-base-docker-image.yml:32` and `:63` still
build base images for `jdk_version: [21, 25]`. `pinot-base-build:21-*` can no
longer build master (the enforcer rejects Java 21) and
`pinot-base-runtime:21-*` would hit the datasketches Java 25 floor at runtime.
`docker/images/pinot-base/pinot-base-build/*.dockerfile` and
`.../pinot-base-runtime/*.dockerfile` also still default `ARG JAVA_VERSION=21`.
`workflow_dispatch`-only, so latent rather than actively broken.
- `pinot-tools/src/main/resources/appAssemblerScriptTemplate:204` guards the
`--add-opens` flags behind `if [ "$(jdk_version)" -gt 11 ]`, which can never be
false now.
- The closure scan is the one check that does not need a Java 11 JVM, so it
could alternatively live in `extra-enforcer-rules`' `enforceBytecodeVersion`
and fire on every `./mvnw install`. That is a project-wide dependency-policy
decision, and keeping it next to the exercise checks means one place reports on
the whole closure.
## Release notes
`none`
--
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]
