-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[feat][client] Implement PIP-234 for sharing thread pools and DNS resolver/cache across multiple Pulsar Client instances #24790
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
[feat][client] Implement PIP-234 for sharing thread pools and DNS resolver/cache across multiple Pulsar Client instances #24790
Conversation
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/PulsarClientSharedResources.java
Outdated
Show resolved
Hide resolved
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Outdated
Show resolved
Hide resolved
...lient-api/src/main/java/org/apache/pulsar/client/api/PulsarClientSharedResourcesBuilder.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #24790 +/- ##
=============================================
+ Coverage 38.67% 74.18% +35.51%
- Complexity 13237 33370 +20133
=============================================
Files 1850 1911 +61
Lines 144540 149024 +4484
Branches 16763 17290 +527
=============================================
+ Hits 55895 110556 +54661
+ Misses 81156 29635 -51521
- Partials 7489 8833 +1344
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements PIP-234 to enable sharing of thread pools and DNS resolver/cache across multiple Pulsar Client instances in the same JVM. This addresses resource waste when many client instances create separate infrastructure components, reducing memory usage, thread counts, and DNS lookup load.
- Introduces
PulsarClientSharedResourcesAPI with builder pattern for configuring shared resources - Adds
ClientBuilder.sharedResources()method to attach shared resources to clients - Refactors client initialization to support both shared and per-client resource management
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pulsar-client-api/src/main/java/org/apache/pulsar/client/api/PulsarClientSharedResources.java | Defines the main interface for shared resources with resource types enum |
| pulsar-client-api/src/main/java/org/apache/pulsar/client/api/PulsarClientSharedResourcesBuilder.java | Builder interface with configuration options for different resource types |
| pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java | Adds sharedResources() method to ClientBuilder API |
| pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesImpl.java | Implementation of shared resources with lifecycle management |
| pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java | Builder implementation with resource configuration classes |
| pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientResourcesConfigurer.java | Factory methods for creating various client resources |
| pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java | Updates client builder to support shared resources |
| pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java | Refactors to use resource configurer factory methods |
| pulsar-client/src/main/java/org/apache/pulsar/client/util/ExecutorProvider.java | Adds constructor with daemon thread parameter |
| pulsar-client/src/main/java/org/apache/pulsar/client/util/ScheduledExecutorProvider.java | Adds constructor with daemon thread parameter |
| pulsar-client/src/test/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImplTest.java | Test cases for shared resources functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesImpl.java
Outdated
Show resolved
Hide resolved
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesImpl.java
Show resolved
Hide resolved
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ClientBuilderImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java:1
- The
sharedResourcesfield is declared in the wrong class. This field appears to be intended forClientBuilderImplclass based on the usage pattern, but it's declared inPulsarClientSharedResourcesBuilderImpl.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
Outdated
Show resolved
Hide resolved
pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java:1
- The
serialVersionUIDis declared inClientBuilderImplbut the class doesn't implementSerializable. Either implementSerializableor remove this unused field.
/*
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ControlledClusterFailover.java:1
- The comment mentions 'DNS lookup is performed again' but this logic seems to always force the endpoint to be unresolved. Consider clarifying why this is necessary or if there's a more direct way to ensure fresh DNS resolution.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java
Show resolved
Hide resolved
pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java
Show resolved
Hide resolved
.../src/test/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImplTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java
Show resolved
Hide resolved
...ient/src/main/java/org/apache/pulsar/client/impl/PulsarClientSharedResourcesBuilderImpl.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpLookupService.java
Show resolved
Hide resolved
…olver/cache across multiple Pulsar Client instances (apache#24790)
…flows Fixes: apache#24790 PIP: apache#234 Motivation Authentication implementations such as AuthenticationOAuth2 create their own thread pools, which prevents resource sharing across multiple Pulsar client instances. Since PIP-234 adds support for sharing resources like Netty EventLoopGroup, we need to extend this capability to authentication flows to avoid unnecessary resource duplication. Modifications Added AuthenticationInitContext interface in the public API to pass shared resources to authentication providers Implemented AuthenticationInitContextImpl with service registry for shared resources Extended Authentication interface with new start(AuthenticationInitContext) method (backward compatible) Modified PulsarClientImpl to create context and register shared resources (EventLoopGroup, Timer) Updated AuthenticationOAuth2 to pass context to OAuth2 flows Extended Flow interface in OAuth2 with context-aware initialization Modified FlowBase to use shared EventLoopGroup and Timer from context for AsyncHttpClient Key changes enable OAuth2 flows to reuse: Shared Netty EventLoopGroup Shared Timer Verifying this change This change is already covered by existing tests, such as: OAuth2 authentication tests Client connection tests Added AuthenticationResourceSharingTest for new context functionality Does this pull request potentially affect one of the following parts: The public API (adds AuthenticationInitContext interface and extends Authentication interface) Dependencies (add or upgrade a dependency) The schema The default values of configurations The threading model (enables sharing, doesn't change model) The binary protocol The REST endpoints The admin CLI options The metrics Anything that affects deployment Documentation doc-required Future Work Potential extension to other authentication methods Additional resource types can be added to the context as needed Shared DNS resolver/cache
PIP: #19074
Motivation
When multiple PulsarClient instances run within the same JVM, each client creates separate thread pools, timers, event loops, and DNS resolvers. This approach leads to significant resource waste, including:
This change addresses the resource sharing gap by introducing a public API for sharing core client infrastructure while maintaining isolated connection pools per client instance.
Modifications
This PR implements PIP-234 by adding the following components:
PulsarClientSharedResources- Main interface for managing shared resources across multiple PulsarClient instancesPulsarClientSharedResourcesBuilder- Fluent API for configuring and building shared resources with support for:ClientBuilder.sharedResources()- Method to attach shared resources to a client instanceKey Features
Usage Example
Documentation
docdoc-requireddoc-not-neededdoc-complete