Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIP-156: Build and Run Pulsar Server on Java 17 #15207

Closed
heesung-sn opened this issue Apr 18, 2022 · 16 comments
Closed

PIP-156: Build and Run Pulsar Server on Java 17 #15207

heesung-sn opened this issue Apr 18, 2022 · 16 comments
Assignees
Labels
Milestone

Comments

@heesung-sn
Copy link
Contributor

heesung-sn commented Apr 18, 2022

Motivation

Currently, Pulsar requires Java8 or Java11 to build[1] and run[2], which is quite out-dated, as the latest released Java version is 18[3], as of 4/14/2022 — the Java community observed that the recent Java versions have improved numerous features such as Garbage Collection[4.1][4.2] and Text Blocks[5] . Hence, as a regular maintenance, it has been recommended to upgrade the Pulsar Java version to the latest LTS(Long Term Support) version, 17[3] for both build and runtime.

We will keep the Pulsar Java client target version as-is, to Java8, for the client runtime compatibility — Pulsar client should be able to run on customers’ current java runtime environment.

We will update Pulsar CLIs runtime version to Java17, as CLIs are expected to run on the Admin environment - CLI tools currently require the Pulsar distribution, and therefore it can be assumed that existing users can choose to use Java17 for running CLI tools

[1] https://github.com/apache/pulsar#build-pulsar
[2] https://pulsar.apache.org/docs/en/deploy-bare-metal/#requirements
[3] https://www.java.com/releases/
[4.1] https://blogs.oracle.com/javamagazine/post/understanding-the-jdks-new-superfast-garbage-collectors
[4.2] https://kstefanj.github.io/2021/11/24/gc-progress-8-17.html
[5] https://docs.oracle.com/en/java/javase/15/text-blocks/index.html (Could be useful for Pulsar Function format)

Goal

  • Upgrade the Pulsar server(broker) Java version to 17 for both build and runtime to adapt the latest Java improvements
  • Keep the the pulsar Java client target version to 8 for the current client runtime environment compatibility (client-server shared modules will also stay at Java 8 release)
  • Upgrade Pulsar CLIs runtime version to Java17 (CLIs are expected to run on the Admin env)
  • Revisit Java runtime flags for ZK, BK and Pulsar to reflect improvement from Java 17. For example, default using ZGC(or Shenandoah GC) instead of G1GC.
  • Test Pulsar performance improvement on Java 17

API Changes

  • None

Implementation

High Level Change

Currently,

  • Java8 or Java11 is required to build pulsar
  • Java8 or Java11 is required to run pulsar-broker
  • Java8(>=) is required to run pulsar-client
  • Default GC for ZK, BK and Pulsar is G1GC

As a result,

  • Java17 is required to build pulsar
  • Java17 is required to run pulsar-broker
  • Java8(>=) is required to run pulsar-client
  • Default GC for ZK, BK and Pulsar is ZGC( or Shenandoah GC)

Assumptions

  • We expect Pulsar dependency libraries such as BookKeeper and ZooKeeper to work well on Java17. The compatibility needs to be tested. Any blocking issues will be reported and resolved as sub tasks of this work.
  • We assume that ZGC and Shenandoah GC performs better than G1GC for Pulsar. We will verify this default GC and its java flag change by the performance tests.

Related Community PR

Test Plan

  • Pass the pulsar OSS CI tests
  • Pass the integration tests in the SN Continuous Integration framework
  • Add Java8 Client runtime compatibility integration tests in the CI framework
  • Run OpenMessaging benchmark to compare performance improvement on Java17 with the Java default runtime flag changes

Release Plan

  • Include this update in the Pulsar 2.11 release

Reject Alternatives

  • None
@heesung-sn heesung-sn changed the title Build and Run Pulsar Server on Java 17 [PIP-156] Build and Run Pulsar Server on Java 17 Apr 18, 2022
@heesung-sn heesung-sn changed the title [PIP-156] Build and Run Pulsar Server on Java 17 PIP-156: Build and Run Pulsar Server on Java 17 Apr 18, 2022
@dave2wave
Copy link
Member

Running the broker with jdk17 and Shenandoah GC gives better performance than ZGC.

@heesung-sn
Copy link
Contributor Author

@dave2wave, I think we could definitely test Shenandoah GC too. Let me update this part in the PIP.

@hpvd
Copy link

hpvd commented Apr 19, 2022

a good overview v8 vs v17 with some hints of what might be useful:
https://pretius.com/blog/java-17-features/

@nicoloboschi
Copy link
Contributor

@heesung-sn, thanks for formalizing this plan.
IIUC the tasks to do are:

I think we should clarify that we support only jdk17 for all the components and jd8, jdk11 and jdk17 for the java client. (we'll need a matrix job in the CI to test the different jdk versions)

We, at Datastax, have run multiple benchmarks and it looks like the performance are really improved (also using Shenandoah GC). @dave2wave is doing this kind of tests and he can share more details.

My main worry is about connectors. Pulsar builtin connectors are not - all - well tested and some of them are not run by the CI and we don’t know if they will work correctly with jdk17.
There are some tricky issues with JDK17 due to the "new" JDK internals encapsulation rules but we fixed them all in the latest weeks and the Pulsar master actually works well.

We expect Pulsar dependency libraries such as BookKeeper and ZooKeeper to work well on Java17

For BookKeeper, we started a similar work to build and test BookKeeper with JDK17. The work is still not completed but I'm not aware of any issues (even tho, I believe that nobody is using BK with JDK17).

@heesung-sn
Copy link
Contributor Author

@nicoloboschi ,

Thanks, I will send a vote email for this PIP soon and start pull requests.

Yes. I will work on

  1. Update the build to make the target(release) java version to 17, except client modules.
  2. Merge your pull Migrate Docker images and CI to Java 17 #14355
  3. Add integration tests for the different jdk compatibility tests -- I will add more connector tests to CI too.

I will update my test addition details to make sure we don't overlap.

@lhotari
Copy link
Member

lhotari commented Apr 20, 2022

@heesung-sn I would have expected you to first reply on the email thread started by @nicoloboschi in February https://lists.apache.org/thread/c0k8p9vy5wyp9l70mt980gdy10smx6qb .
@nicoloboschi has been working on Java 17 compatibility for months and also brought this up to discussion on the mailing list and in the Pulsar community meetings and this has been discussed there with @merlimat several times. Last discussion was recorded in these community meeting notes: https://lists.apache.org/thread/tq7dsws72zf9r7qzr4l567z9w346ksbm .

@dave2wave
Copy link
Member

@dave2wave, I think we could definitely test Shenandoah GC too. Let me update this part in the PIP.

I have tested this with 1 million 100 byte messages per second using the OpenMessaging Benchmark running openJDK17. I can confirm that Shenandoah GC gives a very flat latency sustaining the average over 15 minute tests.

@lhotari
Copy link
Member

lhotari commented Apr 21, 2022

  • Update the build to make the target(release) java version to 17, except client modules.

It's not just client modules. There are also a few libraries that are shared between the client and the broker. The
maven.compiler.release, maven.compiler.target and maven.compiler.source properties should remain 8 for these modules.

For all other modules, it's fine to set maven.compiler.release, maven.compiler.target and maven.compiler.source to 17.

That has been merged.

  • Add integration tests for the different jdk compatibility tests -- I will add more connector tests to CI too.

When adding these to the CI, please expand the existing GitHub Actions workflow file pulsar-ci.yaml. .

integration-tests:
name: CI - Integration - ${{ matrix.name }}
runs-on: ubuntu-20.04
timeout-minutes: ${{ matrix.timeout || 60 }}
needs: ['changed_files_job', 'pulsar-java-test-image']
env:
PULSAR_TEST_IMAGE_NAME: apachepulsar/java-test-image:latest

It's recommended to use the apachepulsar/java-test-image:latest docker image for most of the new tests since it's fast to build.

It would be useful to refactor the existing Pulsar Java client tests in a way that the same tests cases could be used in an integration test. Otherwise, it will lead to duplication in test logic. Refactoring this could be challenging. One reason is that the current test logic uses inheritance a lot, and that's hard to reuse across different type of tests.

@heesung-sn
Copy link
Contributor Author

To clarify one more thing, we will update Pulsar CLIs runtime version to Java17, as CLIs are expected to run by Admin(I updated the above description for this part).

@lhotari
Copy link
Member

lhotari commented Apr 22, 2022

To clarify one more thing, we will update Pulsar CLIs runtime version to Java17, as CLIs are expected to run by Admin(I updated the above description for this part).

I don't think that "CLIs are expected to be run by Admin" is a sufficient justification. A better explanation is needed. One possibility is to explain that the CLI tools currently require the Pulsar distribution and therefore it can be assumed that existing users can choose to use Java17 for running CLI tools. This is an assumption and it would be better to ask community for feedback.

@heesung-sn
Copy link
Contributor Author

To clarify one more thing, we will update Pulsar CLIs runtime version to Java17, as CLIs are expected to run by Admin(I updated the above description for this part).

I don't think that "CLIs are expected to be run by Admin" is a sufficient justification. A better explanation is needed. One possibility is to explain that the CLI tools currently require the Pulsar distribution and therefore it can be assumed that existing users can choose to use Java17 for running CLI tools. This is an assumption and it would be better to ask community for feedback.

Thank you for elaborating this justification. I added yours in the description.

@heesung-sn
Copy link
Contributor Author

Hi ,
As a follow-up task here, I have a proposal to update pulsar server default GC configs. I tried to summarize the details in my fork PR: heesung-sn#1, and It would be great if I could get some early feedbacks.

@lhotari
Copy link
Member

lhotari commented May 19, 2022

Hi ,
As a follow-up task here, I have a proposal to update pulsar server default GC configs. I tried to summarize the details in my fork PR: heesung-sn#1, and It would be great if I could get some early feedbacks.

@heesung-sn looks good. Great analysis you have there.

@lhotari
Copy link
Member

lhotari commented Jun 7, 2022

@heesung-sn Please review #15670 , that includes a Java 17 optimization, the usage of -Xlog:async .

@heesung-sn
Copy link
Contributor Author

This PIP has been completed(release target 2.11).

@axiopisty
Copy link

Is there somewhere I can see Pulsar's release schedule? When will 2.11 be available?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants