Upgrade Pulsar client to 4.2.3#54516
Conversation
This comment has been minimized.
This comment has been minimized.
cescoffier
left a comment
There was a problem hiding this comment.
Before merging we need @ozangunalp review.
|
Most of the CI is failing - so it's is likely incorrect. |
|
I'll check why the CI is failing. |
|
@david-streamlio Thanks for looking into this! I was able to fix the tests with the following change: a471f5b |
This comment has been minimized.
This comment has been minimized.
|
AFAICS, it's also failing on the JVM, not only in native. I will make this PR draft for now until the problems are resolved. |
|
hanks @ozangunalp! Cherry-picked your commit onto the branch. I also kept the small additional commit I pushed just before yours (57e767a), which registers the platform Netty
My guess is your local repro didn't trip that path (macOS / NIO would skip it), so it didn't surface alongside |
78e7e13 to
ff99dfb
Compare
|
@cescoffier, what do you think about registering Netty channels as reflective classes, only for with pulsar extension ? |
|
@david-streamlio it doesn't look very good in my run in here : https://github.com/ozangunalp/quarkus/actions/runs/27146243773/job/80129858734?pr=165 It seems like a segfault related to LightProtoCodec. Are you aware of any changes related to that in Pulsar 4? |
ff99dfb to
95d740e
Compare
|
@ozangunalp Yes — that segfault is a Pulsar 4 change, and it's separate from the reflection issues we already fixed here. Pulsar 4.2.x's LightProtoCodec switched to Unsafe-based zero-copy string encoding. Its static initializer computes STRING_VALUE_OFFSET = UNSAFE.objectFieldOffset(String.value) and BYTE_ARRAY_BASE_OFFSET = UNSAFE.arrayBaseOffset(byte[]), and writeRawString (the frame in your crash, line 403) dereferences those offsets directly. The native build log shows GraalVM's automatic RecomputeFieldValue transform giving up on this code shape ("Could not determine the field where the value … is stored"), so the class gets build-time-initialized and the host-JVM offsets are baked into the image → segfault at runtime against the SubstrateVM heap layout. Fix: initialize LightProtoCodec at run time so the offsets are recomputed correctly — same mechanism we already use for Commands/ConnectionPool (and Commands depends on LightProtoCodec, so they belong together). I've pushed that to the branch; let's see what CI says. On your question to @cescoffier about the Netty channels: the real home for all of this is upstream. My Pulsar PR apache/pulsar#25883 (merged to master / 5.0.0-M1) already embeds the reflection + runtime-init metadata into pulsar-client-original, but it doesn't yet cover LightProtoCodec, DefaultPulsarSslFactory, or the Netty DNS-resolver channels. I'm filing a short follow-up to add those three. Once that lands and gets backported to branch-4.2, Quarkus can drop these downstream registrations entirely — including the Netty channel one, so it wouldn't need to live in the extension long-term. |
|
@david-streamlio thanks for looking into it. I've tested a run previously that fixes the issue with LightProtoCodec : ozangunalp#165 . I think we could also get away with a substitution. |
|
Thanks @ozangunalp! Looks like we landed on the exact same fix — the commit currently at the head of this PR (95d740e) adds On the substitution: I think runtime-init is the better choice here. A @substitution would mean hand-maintaining a copy of writeRawString/the Unsafe-offset logic against Pulsar's internal I'll mark this ready for review so the full native matrix runs and confirms Messaging2 is green. |
|
@ozangunalp did you look at the CI failures? It seems to be a lot more red than usual. |
cescoffier
left a comment
There was a problem hiding this comment.
We need to look at the CI issue before we can merge this.
Also, @ozangunalp has done a very similar work in the Quarkus 4 branch (which uses Netty 4.2).
|
Thanks @cescoffier for the review and @ozangunalp for the feedback — much appreciated. I've pushed two commits addressing both points:
Let me know if there's anything else you'd like changed — otherwise this should be ready for another look. 🙏 |
This comment has been minimized.
This comment has been minimized.
|
Update on the JVM CI failures The native-image issues are resolved (the Native Messaging tests now pass). The remaining failures — Pulsar 4.2.x's It's a regression from the shared Upstream fix submitted: apache/pulsar#26045 — it wraps both DNS-resolver close calls in |
|
@ozangunalp Do you think we can merge this one? |
|
@david-streamlio's fix has been released (today) on upstream Pulsar 4.2.3. Let's rebase this branch on main (Quarkus 4) and bump it to Pulsar 4.2.3. |
The Epoll/KQueue/IOUring channel classes are platform-specific and optional; only register the ones actually present on the classpath instead of registering all of them unconditionally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getAllKnownImplementations requires a more recent Jandex version, which blocks backporting this fix to older branches. Authentication and PulsarSslFactory are interfaces, so getAllKnownImplementors returns the same set without the newer-Jandex requirement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d1987a1 to
05c90f3
Compare
|
Rebased on main and bumped the Pulsar client to 4.2.3, which was released on 2026-07-06 and includes the Since most of the earlier commits from this PR were already merged into main, the branch is now down to three commits:
@ozangunalp @cescoffier this should address the remaining requested changes — ready for re-review whenever CI is approved to run. |
Status for workflow
|
Bumps the Apache Pulsar client from 3.3.0 to 4.2.1.
The 3.3.0 release is no longer covered by Apache Pulsar's security
support policy, and applications that override
pulsar-client.versionto a 4.x release fail to build a native image with:
WithSNISslEngineFactorywas removed from the Pulsar client in 4.x(SNI is now applied directly in
PulsarChannelInitializer); removingthe dangling
addRuntimeInitializedClass(...)registration makes theextension compatible with 4.x.
The rest of the native-image configuration in
SmallRyeReactiveMessagingPulsarProcessorwas reviewed against thePulsar 4.2.1 source and remains correct:
MessageCryptoBc,SecretsSerializer, the OAuth2 protocol classes, the async-http-client hints, and the Protobuf hints are all still present in 4.2.1 with the same package layout. The@SubstituteforClientCredentialsFlow.loadPrivateKeyinio.quarkus.pulsar.runtime.graal.Substitutionsreplaces the entire method body, so the small upstream tweak inside that method (getInputStream()→getContent()) does not affect us.Validated by running
./mvnw verify -f integration-tests/reactive-messaging-pulsar/ -Dnative—all six native-image builds in that module compile cleanly against Pulsar 4.2.1. Could not run the resulting binaries locally on macOS arm64 because the container build produced Linux binaries; runtime smoke tests are left to CI.
Fixes #48776