You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/03/31 03:35:39 UTC
[PR] [codex] Raise Pinot build baseline to JDK 17 [pinot]
xiangfu0 opened a new pull request, #18042:
URL: https://github.com/apache/pinot/pull/18042
## Summary
- raise the repo-wide Maven build baseline from Java 11 to Java 17
- remove the remaining child-module compiler overrides that still pinned Java 11
- update CI, Docker defaults, quickstart scripts, and docs so they stop hardcoding JDK 11
- stop the compatibility verifier helper from forcing `-Djdk.version=11` across checked-out revisions
## Root cause
Pinot already compiled successfully on JDK 17 when the build was explicitly forced to `-Djdk.version=17`, but the repository default still targeted Java 11 and several supporting workflows/scripts still hard-pinned JDK 11. That left the project in a state where JDK 17 worked only as an override instead of as the default baseline.
## Developer impact
After this change, building Pinot on JDK 17 works without passing a JDK override flag, and the surrounding CI / Docker / quickstart configuration is aligned with that new baseline.
## How to reproduce the issue
1. Use JDK 17 as `JAVA_HOME`.
2. On `master` before this patch, run `./mvnw -T1C -Ppinot-fastdev -DskipTests -Dskip.integration.tests=true -Dcheckstyle.skip=true -Dspotless.check.skip=true -Dlicense.skip=true compile`.
3. Observe that the repo still defaults to Java 11 and requires `-Djdk.version=17` to exercise a JDK 17 build baseline.
## Validation
- `./mvnw -T1C -Ppinot-fastdev -DskipTests -Dskip.integration.tests=true -Dcheckstyle.skip=true -Dspotless.check.skip=true -Dlicense.skip=true compile`
- `./mvnw -pl "pinot-timeseries/pinot-timeseries-planner,pinot-plugins/pinot-timeseries-lang/pinot-timeseries-m3ql,pinot-clients/pinot-cli" -am spotless:apply`
- `./mvnw -pl "pinot-timeseries/pinot-timeseries-planner,pinot-plugins/pinot-timeseries-lang/pinot-timeseries-m3ql,pinot-clients/pinot-cli" -am checkstyle:check`
- `./mvnw -pl "pinot-timeseries/pinot-timeseries-planner,pinot-plugins/pinot-timeseries-lang/pinot-timeseries-m3ql,pinot-clients/pinot-cli" -am license:format`
- `./mvnw -pl "pinot-timeseries/pinot-timeseries-planner,pinot-plugins/pinot-timeseries-lang/pinot-timeseries-m3ql,pinot-clients/pinot-cli" -am license:check`
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Raise Pinot build baseline to JDK 17 [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #18042:
URL: https://github.com/apache/pinot/pull/18042#issuecomment-4170723667
Why don't we directly upgrade to 21?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [codex] Raise Pinot build baseline to JDK 17 [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18042:
URL: https://github.com/apache/pinot/pull/18042#issuecomment-4159785761
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18042?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 55.48%. Comparing base ([`b07deff`](https://app.codecov.io/gh/apache/pinot/commit/b07deffb584a24ce139b5aba409e8bbbcbc0fbb0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`003b50b`](https://app.codecov.io/gh/apache/pinot/commit/003b50b4f5693f6a323f29f19d087f05ef82ec86?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (b07deff) and HEAD (003b50b). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (b07deff) | HEAD (003b50b) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration1|2|0|
>|integration2|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18042 +/- ##
============================================
- Coverage 63.32% 55.48% -7.85%
+ Complexity 1543 745 -798
============================================
Files 3200 2502 -698
Lines 194209 143147 -51062
Branches 29932 22984 -6948
============================================
- Hits 122986 79424 -43562
+ Misses 61557 56975 -4582
+ Partials 9666 6748 -2918
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <ø> (-7.82%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <ø> (-7.85%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <ø> (-7.84%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.48% <ø> (-0.03%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18042/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18042?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Raise Pinot build baseline to JDK 17 [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #18042:
URL: https://github.com/apache/pinot/pull/18042#issuecomment-4180980462
yes, give another try on jdk 21 :)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Raise Pinot build baseline to JDK 17 [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #18042:
URL: https://github.com/apache/pinot/pull/18042#discussion_r3014173270
##########
docker/images/pinot/docker-build.sh:
##########
@@ -51,11 +51,11 @@ fi
if [[ "$#" -gt 4 ]]
then
- JAVA_VERSION=$5
+ JDK_VERSION=$5
else
- JAVA_VERSION=11
+ JDK_VERSION=17
fi
-echo "Trying to build Pinot docker image from Git URL: [ ${PINOT_GIT_URL} ] on branch: [ ${PINOT_BRANCH} ] and tag it as: [ ${DOCKER_TAG} ]. Kafka Dependencies: [ ${KAFKA_VERSION} ]. Java Version: [ ${JAVA_VERSION} ]."
+echo "Trying to build Pinot docker image from Git URL: [ ${PINOT_GIT_URL} ] on branch: [ ${PINOT_BRANCH} ] and tag it as: [ ${DOCKER_TAG} ]. Kafka Dependencies: [ ${KAFKA_VERSION} ]. JDK Version: [ ${JDK_VERSION} ]."
-docker build --no-cache -t ${DOCKER_TAG} --build-arg PINOT_BRANCH=${PINOT_BRANCH} --build-arg PINOT_GIT_URL=${PINOT_GIT_URL} --build-arg KAFKA_VERSION=${KAFKA_VERSION} --build-arg JAVA_VERSION=${JAVA_VERSION} -f Dockerfile .
+docker build --no-cache -t ${DOCKER_TAG} --build-arg PINOT_BRANCH=${PINOT_BRANCH} --build-arg PINOT_GIT_URL=${PINOT_GIT_URL} --build-arg KAFKA_VERSION=${KAFKA_VERSION} --build-arg JDK_VERSION=${JDK_VERSION} -f Dockerfile .
Review Comment:
`KAFKA_VERSION` is accepted and passed as a `--build-arg`, but the referenced `docker/images/pinot/Dockerfile` doesn’t define or use `ARG KAFKA_VERSION` (only `JDK_VERSION`). This makes the `Kafka Dependencies` argument misleading and effectively dead code; either remove the `KAFKA_VERSION` arg from this script/README or wire it into the Docker build if it’s still required.
##########
.github/copilot-instructions.md:
##########
@@ -9,7 +9,7 @@ This document guides AI coding assistants (Copilot, Cursor, etc.) contributing t
## Libraries and Frameworks
-- Most source code is written in Java 11.
+- Most source code is written in Java 17.
- Code in `pinot-clients` is written in Java 8.
Review Comment:
After raising the repo-wide baseline to Java 17, this doc still states that `pinot-clients` is written in Java 8. Since `maven.compiler.release/source/target` now default to 17 repo-wide, this is ambiguous and may be inaccurate for build/runtime expectations. Please clarify whether `pinot-clients` is still intended to be Java 8 bytecode-compatible (e.g., built with `--release 8`) or update the statement to match the new baseline.
```suggestion
- Code in `pinot-clients` is also built for Java 17.
```
##########
docker/images/pinot/README.md:
##########
@@ -44,11 +44,7 @@ The docker image is tagged as `[Docker Tag]`.
`Kafka Version`: The Kafka Version to build pinot with. Default is `2.0`
-`Java Version`: The Java Build and Runtime image version. Default is `11`
-
-`JDK Version`: The JDK parameter to build pinot, set as part of maven build option: `-Djdk.version=${JDK_VERSION}`. Default is `11`
-
-`OpenJDK Image`: Base image to use for Pinot build and runtime. Default is `openjdk`.
+`JDK Version`: The JDK parameter to build pinot, set as part of maven build option: `-Djdk.version=${JDK_VERSION}`. Default is `17`
Review Comment:
The usage/docs still present `Kafka Version` as a required/meaningful parameter, but `docker/images/pinot/Dockerfile` doesn’t consume `KAFKA_VERSION` at all. Consider either removing `Kafka Version` from the documented interface (and examples) or updating the Docker build to actually use it, so the README matches the implementation.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Raise Pinot build baseline to JDK 17 [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #18042: Raise Pinot build baseline to JDK 17
URL: https://github.com/apache/pinot/pull/18042
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org