PIP-121: Introduce ServiceInfoProvider to update service info dynamically#541
PIP-121: Introduce ServiceInfoProvider to update service info dynamically#541BewareMyPower wants to merge 26 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.
d9faf1e to
b91860a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
perf/PerfConsumer.cc:162
--use-tlsis still exposed as a CLI flag, but withClientConfiguration::setUseTls()removed this flag no longer affects behavior (TLS is determined by the URL scheme). Please either translate the URL scheme (e.g. topulsar+ssl://) when the flag is set or fail fast with a clear error to avoid silently running without TLS.
💡 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.
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)