PIP-121: Introduce ServiceInfoProvider to update service info dynamically#541
Draft
BewareMyPower wants to merge 20 commits intoapache:mainfrom
Draft
PIP-121: Introduce ServiceInfoProvider to update service info dynamically#541BewareMyPower wants to merge 20 commits intoapache:mainfrom
BewareMyPower wants to merge 20 commits intoapache:mainfrom
Conversation
a9c36c8 to
7b98b25
Compare
b071d12 to
a1746a9
Compare
There was a problem hiding this comment.
Pull request overview
Adds a public API to dynamically switch a Client instance to a different Pulsar cluster at runtime (service URL + auth + TLS trust certs), and refactors internal lookup/connection usage so in-flight components pick up the new cluster after switching.
Changes:
- Introduce
ServiceInfo,Client::updateServiceInfo(...), andClient::getServiceInfo()to support application-managed cluster failover. - Refactor internal components (lookup/schema/partition metadata calls) to route through
ClientImplaccessors guarded by a shared mutex, so lookup service updates are visible everywhere. - Add/extend tests to validate cluster switching and expose token helper for reuse.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/LookupServiceTest.cc | Updates tests to use ClientImpl lookup/schema accessors instead of exposing LookupService directly. |
| tests/ClientTest.cc | Adds testUpdateServiceInfo validating switching between clusters with different auth/TLS settings. |
| tests/AuthTokenTest.cc | Makes getToken() non-static so it can be reused by the new client test. |
| lib/ReaderImpl.cc | Stops passing a fixed LookupService into consumers; relies on client accessors instead. |
| lib/PatternMultiTopicsConsumerImpl.h | Updates ctor signature to remove LookupServicePtr dependency. |
| lib/PatternMultiTopicsConsumerImpl.cc | Routes namespace topic discovery via ClientImpl so lookups reflect cluster switches. |
| lib/PartitionedProducerImpl.h | Removes stored LookupServicePtr member. |
| lib/PartitionedProducerImpl.cc | Fetches partition metadata via ClientImpl accessor to respect cluster switches. |
| lib/MultiTopicsConsumerImpl.h | Removes stored LookupServicePtr member and adjusts ctors accordingly. |
| lib/MultiTopicsConsumerImpl.cc | Fetches partition metadata via ClientImpl accessor; adds client weak-lock checks. |
| lib/ConsumerImpl.h | Adds onClusterSwitching() hook and preserves original start message id from config. |
| lib/ConsumerImpl.cc | Implements onClusterSwitching() to reset some consumer state when switching clusters. |
| lib/ConnectionPool.h | Adds resetForClusterSwitching(...) to close existing conns but keep pool usable. |
| lib/ConnectionPool.cc | Implements pool reset, closing connections with a “switch cluster” flag. |
| lib/ClientImpl.h | Adds shared-mutex guarded lookup/schema/partition APIs + service info update/get. |
| lib/ClientImpl.cc | Implements updateServiceInfo() and getServiceInfo(), rebuilds lookup service on switch. |
| lib/ClientConnection.h | Extends close() signature with switchCluster flag (defaulted). |
| lib/ClientConnection.cc | On cluster-switch close, notifies consumers via onClusterSwitching(). |
| lib/Client.cc | Exposes new public methods and routes schema calls through ClientImpl wrapper. |
| include/pulsar/Client.h | Adds public ServiceInfo struct + Client API for updating/getting service info. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The design is a bit different from Java client's
ServiceUrlProviderbecause the internal classes (e.g.ClientImpl) in C++ client are not exposed unlike the Java client.Differences:
ServiceInfostruct. Otherwise, the interface could be complicated, for example, Java'sAutoClusterFailoverBuilderaccepts 3 methods for this purpose (secondary,secondaryAuthenticationandsecondaryTlsTrustCertsFilePath).ServiceUrlProvider#getServiceUrlis replaced by thegetServiceInfomethod added toClient.initializemethod accepts aonServiceInfoUpdatecallback that is used to update the service info. This can avoid adding a new public method toClientby passing an internal method ofClientImplas the callback.Breaking changes:
ClientConfigurationcan be modified dynamically, remove these public getters because they might return outdated values. Now it should be checked fromClient::getServiceInfo.setUseTlsmethod is removed as well because it should never be set by users. Whether to use TLS is actually determined by the service URL's scheme (e.g.pulsar+ssl://prefix)