[improve][build] PIP-156 Build Pulsar Server on Java 17

[heesung-sn]

Master Issue: #15207

Motivation

This is to upgrade Pulsar Server's Java build version to 17.
The detail motivation is described in the PIP-156, #15207

There will be multiple commits under this PR to complete the tasks under PIP-156.

Modifications

This changes Pulsar Server modules' javac release version to 17 except client (shared) modules
The detail modification is described in the PIP-156, #15207

Verifying this change

• Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

• will add integration tests for different java runtime versions if currently not covered

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

• Dependencies (does it add or upgrade a dependency): (yes / no)
• The public API: (yes / no)
• The schema: (yes / no / don't know)
• The default values of configurations: (yes / no)
• The wire protocol: (yes / no)
• The rest endpoints: (yes / no)
• The admin cli options: (yes / no)
• Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly.

Need to update docs?

• doc-required
  (Your PR needs to update docs and you will update later)

• no-need-doc
  (Please explain why)

• doc
  (Your PR contains doc changes)

• doc-added
  (Docs have been already added)

[nicoloboschi]

We must create a matrix test suite where we run the Pulsar client suite with jdk8 and jdk17 to ensure compatibility

[heesung-sn]

We must create a matrix test suite where we run the Pulsar client suite with jdk8 and jdk17 to ensure compatibility

Yes, that is a part of the plan. I was focusing on the build upgrade and regression checks first. After I confirm that the existing tests pass, I will start to add the Pulsar client jdk8 and jdk17 compatibility test commits here.

[lhotari]

@heesung-sn Just that you know about it, it's usually useful to work on GitHub Actions workflow changes in your own fork before opening a PR to apache/pulsar. Please check the comments in #15385 (comment) .
When running builds in your own fork, you can also get ssh access to the build VMs to investigate issues. ssh access is disabled because of security reasons in apache/pulsar builds.

[lhotari]

There will be multiple commits under this PR to complete the tasks under PIP-156.

I don't think that it's a good goal to complete all changes for PIP-156 in a single PR. Smaller PRs are easier to review and get merged. For example, there's no sense in putting the Kinesis related changes in this PR. That should be solved separately.

[heesung-sn]

Hi @lhotari @nicoloboschi,
I think I resolved your comments in the latest push. Could you review this PR again by any chance?

[nicoloboschi]

Good work!

[lhotari]

@heesung-sn The changes to .asf.yaml will have to be handled separately. To prevent issues where currently in-progress PRs get blocked by the new required checks (CI - Integration - Shade on Java 8, CI - Integration - Shade on Java 11 and CI - Integration - Shade on Java 17), it's better to postpone the changes to .asf.yaml. The changes to .asf.yaml can be made after this PR has been merged for a few days.

#15496 is needed as a preparation for allowing to merge this current PR. That removes the existing required checks CI - Unit - Brokers - JDK 8 and CI - Integration - Shade which are missing after the changes to the workflow. After that is merged, we are ready to merge the changes.

[lhotari]

There's also a need to simplify the pom.xml . Previous the build has been compatible with Java 8 & Java 11 and that has been handled with the jdk11 maven profile. Since Java 17+ will be required for building, it would be useful to remove the jdk11 maven profile solution and instead use a solution that enforces Java 17+ for building (could use https://maven.apache.org/enforcer/maven-enforcer-plugin/). I think it's better to handle this change in a follow-up PR. I propose that we merge this current PR as-is.

@heesung-sn Are you interested in a follow up PR to address the above comment?

[lhotari]

@heesung-sn Are you interested in a follow up PR to address the above comment?

I already created #15505 to address this.

[eolivelli]

@lhotari @heesung-sn

Why are we requiring JDK17 to build ?

I agree that we can ensure that Pulsar works well on JDK17 and that we should run CI on JDK17.
but in many companies it is not allowed to use JDKs that are "too modern."

can we REQUIRE JDK11 instead of JDK17 ?

[lhotari]

Why are we requiring JDK17 to build ?

Initially, the motivation for this was to enable the use of new language features in Pulsar server-side modules, for example, using the Java record types.

I agree that we can ensure that Pulsar works well on JDK17 and that we should run CI on JDK17. but in many companies it is not allowed to use JDKs that are "too modern."

I doubt that it's a problem for Pulsar broker. You must run Pulsar on a recent JDK version in any case, and requiring Java 17 makes sense for the broker.

Pulsar client and Pulsar admin client libraries will continue to support Java 8. For some reason, PIP-156 was specified in a way that Pulsar CLI tools also require Java 17 to run. @heesung-sn Can you explain why Java 17 is required for Pulsar CLI tools?

can we REQUIRE JDK11 instead of JDK17 ?

That's possible, but I don't think that there's a specific reason to stop at JDK11 now that JDK17 is the current Java LTS release.

[BewareMyPower]

Hi, after rebasing to master that includes this PR, I could not build pulsar-client with both JDK8 and JDK11, which are installed by SDKMAN!.

JDK 11:

sdk use java 11.0.2-open
mvn clean install -DskipTests -pl pulsar-client -am

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.9.0:compile (default-compile) on project pulsar-functions-proto: Fatal error compiling: error: release version 17 not supported -> [Help 1]

JDK 8:

sdk use java 8.332.08.1-amzn 
mvn clean install -DskipTests -pl pulsar-client -am

Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.9.0:compile (default-compile) on project pulsar-client-api: Fatal error compiling: invalid flag: --release -> [Help 1]

Could you help take a look? @heesung-sn @lhotari

[BewareMyPower]

I've also tried the patch of #15505 with JDK 8. It failed with

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0:enforce (enforce-maven) on project pulsar: Some Enforcer rules have failed. Look above for specific messages explaining why the rule failed. -> [Help 1]

[merlimat]

@BewareMyPower the goal is to have a pulsar-client jar that is compatible with Java 8, though not necessarily have the build system to work with Java 8

[BewareMyPower]

@merlimat Got it. I saw https://github.com/apache/pulsar/blob/master/README.md#pulsar-runtime-java-version-recommendation just now.

[heesung-sn]

@heesung-sn Are you interested in a follow up PR to address the above comment?

I already created #15505 to address this.

I see this followup change. Thank you for taking care of this.

[heesung-sn]

Why are we requiring JDK17 to build ?

Initially, the motivation for this was to enable the use of new language features in Pulsar server-side modules, for example, using the Java record types.

I agree that we can ensure that Pulsar works well on JDK17 and that we should run CI on JDK17. but in many companies it is not allowed to use JDKs that are "too modern."

I doubt that it's a problem for Pulsar broker. You must run Pulsar on a recent JDK version in any case, and requiring Java 17 makes sense for the broker.

Pulsar client and Pulsar admin client libraries will continue to support Java 8. For some reason, PIP-156 was specified in a way that Pulsar CLI tools also require Java 17 to run. @heesung-sn Can you explain why Java 17 is required for Pulsar CLI tools?

can we REQUIRE JDK11 instead of JDK17 ?

That's possible, but I don't think that there's a specific reason to stop at JDK11 now that JDK17 is the current Java LTS release.

As discussed in the pip-156, #15207, the goal of this upgrade is to enable pulsar to adapt new java features. Regarding CLI java version, because CLI has dependencies on broker modules, it needs to follow broker's java version.

[lhotari]

Regarding CLI java version, because CLI has dependencies on broker modules, it needs to follow broker's java version.

@heesung-sn Why does the CLI depend on broker modules? Would it be possible to break these dependencies so that the CLI could run on older Java versions?

[heesung-sn]

Regarding CLI java version, because CLI has dependencies on broker modules, it needs to follow broker's java version.

@heesung-sn Why does the CLI depend on broker modules? Would it be possible to break these dependencies so that the CLI could run on older Java versions?

I see that many CLI commands directly call broker logics, acting as a "kind-of" broker.

For example, pulsar compact-topic cli command, it internally calls org.apache.pulsar.compaction.CompactorTool.main(), and as you can see CompactorTool class internally depends on many broker classes/modules.

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactorTool.java

Lines 33 to 46 in 6d9ba7b

	import org.apache.bookkeeper.client.BookKeeper;
	import org.apache.bookkeeper.common.util.OrderedScheduler;
	import org.apache.pulsar.broker.BookKeeperClientFactory;
	import org.apache.pulsar.broker.BookKeeperClientFactoryImpl;
	import org.apache.pulsar.broker.ServiceConfiguration;
	import org.apache.pulsar.broker.ServiceConfigurationUtils;
	import org.apache.pulsar.client.api.ClientBuilder;
	import org.apache.pulsar.client.api.PulsarClient;
	import org.apache.pulsar.common.configuration.PulsarConfigurationLoader;
	import org.apache.pulsar.common.util.CmdGenerateDocs;
	import org.apache.pulsar.common.util.netty.EventLoopUtil;
	import org.apache.pulsar.metadata.api.MetadataStoreConfig;
	import org.apache.pulsar.metadata.api.extended.MetadataStoreExtended;
	import org.apache.pulsar.policies.data.loadbalancer.AdvertisedListener;

Specifically, this CompactorTool main func directly calls the broker compaction logic like the below

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactorTool.java

Lines 151 to 156 in 6d9ba7b

	PulsarClient pulsar = clientBuilder.build();
	Compactor compactor = new TwoPhaseCompactor(brokerConfig, pulsar, bk, scheduler);
	long ledgerId = compactor.compact(arguments.topic).get();
	log.info("Compaction of topic {} complete. Compacted to ledger {}", arguments.topic, ledgerId);
	}

I think it is possible to break these dependencies, but this requires some serious work. For example, we need to redesign CLI as a pure request/wait client, and there should be a backend async executor service on broker(or on a separate server) that runs the actual CLI logics.

[eolivelli]

In my opinion such tools that use Broker internals are very version specific and they are usually executed from the same package (and machines?) of the broker (I would never use the Compator tool of a older version or from a newer version for instance)
So it is fine to require the same jdk as for the broker.

[michaeljmarshall]

Note that the compactor tool is accessible via the script named bin/pulsar. I would probably view jars activated by that script as server side jars. Note that the pulsar client jar and the pulsar admin client jar are accessible via the CLI using bin/pulsar-client and bin/pulsar-admin, respectively.

[momo-jun]

There are three more occurrences of the JRE/JDK versions (8 or 11) in the following topics. Should they be updated as well?

• deploy-bare-metal.md

• deploy-bare-metal-multi-cluster.md
• getting-started-standalone.md

Thank you for this catch. Yes, I think we need to update these parts too. If you dont mind, I will probably take this as a separate PR for further doc updates. I think we probably start scoping this PR for code change only to avoid the repeated CI test runs.

Hi Heesung, will you like to commit a follow-up PR with the remaining doc changes?

[heesung-sn]

@heesung-sn Just that you know about it, it's usually useful to work on GitHub Actions workflow changes in your own fork before opening a PR to apache/pulsar. Please check the comments in #15385 (comment) . When running builds in your own fork, you can also get ssh access to the build VMs to investigate issues. ssh access is disabled because of security reasons in apache/pulsar builds.

Made a PR for copying the instruction to the contribution page, #15535

[heesung-sn]

Fixed one module: #19991