-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[feat][client] PIP-234: Support shared resources in PulsarAdmin to reduce thread usage #24893
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
Conversation
...ent-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.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.
LGTM. Great work @3pacccccc
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 adds support for sharing resources (event loop groups, timers, and DNS resolvers) across multiple PulsarAdmin client instances to reduce memory footprint and thread usage when creating many clients in the same JVM.
Key changes:
- Added
sharedResourcesparameter toPulsarAdminBuilderandAsyncHttpConnector - Updated
AsyncHttpConnectorto manage shared and created event loop groups - Added new test cases to verify shared resource behavior
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| PulsarAdminBuilder.java | Added API method to accept PulsarClientSharedResources |
| PulsarAdminBuilderImpl.java | Implemented sharedResources method and stored shared resources field |
| PulsarAdminImpl.java | Updated constructors to accept and pass shared resources to connector |
| AsyncHttpConnectorProvider.java | Updated getConnector to accept shared resources parameter |
| AsyncHttpConnector.java | Added logic to handle shared resources, including managing event loops, DNS resolvers, and cleanup |
| PulsarAdminBuilderImplTest.java | Added test cases for shared resources functionality |
| AsyncHttpConnectorTest.java | Updated test constructor calls to include null shared resources parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-admin/src/test/java/org/apache/pulsar/client/admin/internal/PulsarAdminBuilderImplTest.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24893 +/- ##
=============================================
+ Coverage 38.56% 74.23% +35.67%
- Complexity 13262 33877 +20615
=============================================
Files 1856 1913 +57
Lines 145287 149488 +4201
Branches 16877 17371 +494
=============================================
+ Hits 56025 110973 +54948
+ Misses 81696 29647 -52049
- Partials 7566 8868 +1302
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Main Issue: #24796
Motivation
When creating multiple PulsarAdmin clients in the same JVM, each client currently creates its own set of resources (event loops, timers, DNS resolvers). This leads to excessive thread consumption and memory usage when many admin clients are instantiated.
This change allows PulsarAdmin clients to share resources through the
PulsarClientSharedResourcesmechanism, similar to what's already available for PulsarClient.Modifications
sharedResources(PulsarClientSharedResources)method toPulsarAdminBuilderAPIPulsarAdminBuilderImplto accept and propagate shared resources configurationPulsarAdminImplconstructor to pass shared resources to HTTP connectorAsyncHttpConnectorto utilize shared resources:Verifying this change
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: 3pacccccc#29