You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "FrankChen021 (via GitHub)" <gi...@apache.org> on 2026/09/07 04:42:54 UTC

[PR] test: enable compact object headers across test JVMs (druid)

FrankChen021 opened a new pull request, #20271:
URL: https://github.com/apache/druid/pull/20271

   ### Description
   
   Run Druid tests with `-XX:+UseCompactObjectHeaders` to detect regressions under JDK 25 compact object headers before changing runtime defaults.
   
   - Enable the flag in the parent Surefire configuration, inherited by unit and embedded tests, and in the `parallel-test` and processing `benchmark` profiles that override test JVM arguments.
   - Enable it in the embedded Docker-test Failsafe JVM.
   - Set `JAVA_TOOL_OPTIONS` in the CI test image so containerized Druid services and task JVMs, including Kubernetes peon pods, inherit the flag. The Build the Docker image step appends the test-only ENV instruction to the Dockerfile streamed into a single Docker build, before the image is saved and loaded for testing.
   
   Production launch scripts and distribution image defaults are unchanged. Historical Druid images used for backward-compatibility tests retain their existing JVM settings; they may use JDKs that do not support compact headers.
   
   #### Validation
   
   - Passed six `CircularListTest` unit cases and two `SystemTableQueryTest` embedded cases with JDK 25:
     ```sh
     mvn test -pl embedded-tests -am -Dtest='org.apache.druid.collections.CircularListTest,org.apache.druid.testing.embedded.query.SystemTableQueryTest' -Dsurefire.failIfNoSpecifiedTests=false -Pskip-static-checks -Dweb.console.skip=true -T1C
     ```
   - Verified the effective embedded-tests Maven configuration supplies the flag to both Surefire and Docker-test Failsafe, and checked all four test `argLine` definitions.
   - Java 25 Docker smoke checks confirmed direct and shell-launched JVMs report `UseCompactObjectHeaders = true` from the environment. Checked the revised single-build workflow shell syntax, Docker command arguments, and streamed Dockerfile contents.
   - `git diff --check` and workflow shell syntax checks passed.
   - Full unit, embedded, and Druid Docker regression suites remain for CI; the local Docker check used a Java smoke image, not a full Druid cluster.
   
   This PR has:
   
   - [x] been self-reviewed.
   - [x] added a comment explaining the test image configuration.
   
   


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] test: enable compact object headers across test JVMs (druid)

Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #20271:
URL: https://github.com/apache/druid/pull/20271


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] test: enable compact object headers across test JVMs (druid)

Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #20271:
URL: https://github.com/apache/druid/pull/20271#discussion_r3947208972


##########
.github/workflows/docker-tests.yml:
##########
@@ -26,7 +26,12 @@ jobs:
       - name: Set Docker image env var
         run: echo "DRUID_DIST_IMAGE_NAME=apache/druid:docker-tests" >> $GITHUB_ENV
       - name: Build the Docker image
-        run: DOCKER_BUILDKIT=1 docker build -t $DRUID_DIST_IMAGE_NAME -f distribution/docker/Dockerfile .
+        run: |
+          # Enable compact headers in the test image for services and task JVMs, including K3s pods.
+          {
+            cat distribution/docker/Dockerfile
+            printf '\nENV JAVA_TOOL_OPTIONS="-XX:+UseCompactObjectHeaders"\n'
+          } | DOCKER_BUILDKIT=1 docker build -t "$DRUID_DIST_IMAGE_NAME" -f - .

Review Comment:
   Addressed in the latest commit: the Docker image build step now runs `set -o pipefail` before the pipeline, so failures while generating the streamed Dockerfile cannot be hidden by the `docker build` exit status.



##########
pom.xml:
##########
@@ -2048,6 +2048,7 @@
                         <!-- locale settings must be set on the command line before startup -->
                         <!-- set default options -->
                         <argLine>
+                            -XX:+UseCompactObjectHeaders
                             @{jacocoArgLine}
                             ${jdk.strong.encapsulation.argLine}

Review Comment:
   The benchmark-profile concern is intentional and now reflected in the PR description. The compact-header flag is scoped to unit, embedded, and Docker tests; the processing benchmark profile is not part of CI, so it remains unchanged. I removed the stale benchmark-profile wording from the description.



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] test: enable compact object headers across test JVMs (druid)

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #20271:
URL: https://github.com/apache/druid/pull/20271#discussion_r3946614301


##########
pom.xml:
##########
@@ -2048,6 +2048,7 @@
                         <!-- locale settings must be set on the command line before startup -->
                         <!-- set default options -->
                         <argLine>
+                            -XX:+UseCompactObjectHeaders
                             @{jacocoArgLine}
                             ${jdk.strong.encapsulation.argLine}

Review Comment:
   PR description mentions enabling compact headers in the processing `benchmark` profile that overrides test JVM args, but `processing/pom.xml` still has a `benchmark` profile `<argLine>` that does not include `-XX:+UseCompactObjectHeaders`, so benchmark runs would not actually exercise compact headers. Either add the flag there or avoid overriding the parent `argLine` so it inherits this setting.



##########
.github/workflows/docker-tests.yml:
##########
@@ -26,7 +26,12 @@ jobs:
       - name: Set Docker image env var
         run: echo "DRUID_DIST_IMAGE_NAME=apache/druid:docker-tests" >> $GITHUB_ENV
       - name: Build the Docker image
-        run: DOCKER_BUILDKIT=1 docker build -t $DRUID_DIST_IMAGE_NAME -f distribution/docker/Dockerfile .
+        run: |
+          # Enable compact headers in the test image for services and task JVMs, including K3s pods.
+          {
+            cat distribution/docker/Dockerfile
+            printf '\nENV JAVA_TOOL_OPTIONS="-XX:+UseCompactObjectHeaders"\n'
+          } | DOCKER_BUILDKIT=1 docker build -t "$DRUID_DIST_IMAGE_NAME" -f - .

Review Comment:
   The workflow uses a pipeline to stream a modified Dockerfile into `docker build`, but without `set -o pipefail` a failure in `cat distribution/docker/Dockerfile` (or `printf`) would not fail the step because the pipeline exit status is that of `docker build`. Add `set -o pipefail` so missing/invalid Dockerfile input causes the job to fail fast.



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org