Skip to content

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Sep 25, 2025

Motivation

There "PIP-234: Support using shared thread pool across multiple Pulsar client instance", #19074, which never went forward. The intention is that multiple Pulsar clients could share resources. This is not only needed for thread pools, but also for the Netty DNS resolver cache which is represented by io.netty.resolver.dns.DnsAddressResolverGroup in Netty.

PIP-234 has been discussed in the past in threads

Before PIP-234 becomes a reality, it's useful to have an internal API for sharing Netty's DnsAddressResolverGroup across multiple PulsarClient instances.

There's already an internal API for sharing instances. It's the PulsarClientImpl's Lombok generated builder:

@Builder(builderClassName = "PulsarClientImplBuilder")
private PulsarClientImpl(ClientConfigurationData conf, EventLoopGroup eventLoopGroup, ConnectionPool connectionPool,
Timer timer, ExecutorProvider externalExecutorProvider,
ExecutorProvider internalExecutorProvider,
ScheduledExecutorProvider scheduledExecutorProvider,
ExecutorProvider lookupExecutorProvider) throws PulsarClientException {

This PR adds a class org.apache.pulsar.client.impl.DnsResolverGroupImpl which could later on be abstracted by an interface whenever we get to implement the PIP-234's PulsarClientGroup abstraction.

In addition, this PR contains changes to use a shared DnsResolverGroup for Pulsar broker clients.
There were changes in the past where resource sharing was incrementally added in #12037, #13836, and #13839.

The problem that this could solve is a heavy load on the DNS server when a DNS entry expires and many clients access the same entry. This was something that was already addressed in the past for Pulsar Proxy, #15403 .

Similar problems are present in Flink Pulsar use cases where each Pulsar sink and source creates it's own Pulsar client instance. It would be useful to have PIP-234 available for addressing that with a public Pulsar client API. However, in the mean time, the internal API in this PR could be used as a workaround.

It should also be noted that Kubernetes default ndots 5 configuration adds heavy load on the DNS server.
This article explains the ndots 5 issue. The way to address it for the service url is to add an extra trailing dot to make the DNS name an absolute FQDN. There's also a Pulsar discussion at #24030 with more information about unnecessary DNS lookups. Changing the service url isn't sufficient. There isn't a direct feature to make the pulsar and broker return the address in absolute FQDN dns name format for Pulsar topic lookups. A similar problem exists for the Pulsar Proxy. I'll create a separate PR to address that.

Modifications

  • add internal DnsResolverGroupImpl abstraction for wrapping Netty's DnsAddressResolverGroup and for preparing for PIP-234
  • make it possible to instantiate PulsarClientImpl with a shared DnsResolverGroupImpl instance
  • use a shared DnsResolverGroupImpl instance for Pulsar broker clients

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 4.2.0 milestone Sep 25, 2025
@lhotari lhotari self-assigned this Sep 25, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 25, 2025
@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2025

Codecov Report

❌ Patch coverage is 90.78947% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.20%. Comparing base (46a76e9) to head (28128d4).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../org/apache/pulsar/client/impl/ConnectionPool.java 71.42% 0 Missing and 4 partials ⚠️
...pache/pulsar/client/impl/DnsResolverGroupImpl.java 87.50% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24784       +/-   ##
=============================================
+ Coverage     38.28%   74.20%   +35.91%     
- Complexity    13046    33293    +20247     
=============================================
  Files          1848     1907       +59     
  Lines        144408   148703     +4295     
  Branches      16740    17249      +509     
=============================================
+ Hits          55286   110342    +55056     
+ Misses        81626    29553    -52073     
- Partials       7496     8808     +1312     
Flag Coverage Δ
inttests 26.43% <69.73%> (+0.20%) ⬆️
systests 22.78% <69.73%> (+0.02%) ⬆️
unittests 73.72% <90.78%> (+39.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...n/java/org/apache/pulsar/broker/PulsarService.java 83.79% <100.00%> (+14.36%) ⬆️
...rg/apache/pulsar/client/impl/PulsarClientImpl.java 76.03% <100.00%> (+17.81%) ⬆️
...rg/apache/pulsar/proxy/server/ProxyConnection.java 57.90% <100.00%> (+22.79%) ⬆️
...pache/pulsar/client/impl/DnsResolverGroupImpl.java 87.50% <87.50%> (ø)
.../org/apache/pulsar/client/impl/ConnectionPool.java 76.05% <71.42%> (+16.05%) ⬆️

... and 1400 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lhotari lhotari merged commit 1050f48 into apache:master Sep 25, 2025
96 of 98 checks passed
lhotari added a commit that referenced this pull request Oct 4, 2025
…ss multiple PulsarClient instances (#24784)

(cherry picked from commit 1050f48)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants