From 7862234f5f6a5b341693f9978773b90d73d6fe25 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 11:09:48 +0800 Subject: [PATCH 01/28] Initial commit --- include/pulsar/Client.h | 15 +++++++++++++++ lib/Client.cc | 7 +++++++ lib/ClientImpl.cc | 8 ++++++++ lib/ClientImpl.h | 4 ++++ 4 files changed, 34 insertions(+) diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index 56130667..95233fe9 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -32,6 +32,7 @@ #include #include +#include #include namespace pulsar { @@ -414,6 +415,20 @@ class PULSAR_PUBLIC Client { void getSchemaInfoAsync(const std::string& topic, int64_t version, std::function callback); + /** + * Update the connection information of the client, including service URL, authentication and TLS trust + * certs file path. + * This method is used to switch the connection to a different Pulsar cluster. All connections will be + * closed and the internal connection info will be updated. + * + * @param serviceUrl the Pulsar endpoint to use (eg: pulsar://localhost:6650) + * @param authentication the authentication information to use for connecting to the Pulsar cluster + * @param tlsTrustCertsFilePath the TLS trust certs file path to use for connecting to the Pulsar cluster + */ + void updateConnectionInfo(const std::string& serviceUrl, + const std::optional& authentication, + const std::optional& tlsTrustCertsFilePath); + private: Client(const std::shared_ptr&); diff --git a/lib/Client.cc b/lib/Client.cc index 39a5948a..60fb62f5 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -197,4 +197,11 @@ void Client::getSchemaInfoAsync(const std::string& topic, int64_t version, ->getSchema(TopicName::get(topic), (version >= 0) ? toBigEndianBytes(version) : "") .addListener(std::move(callback)); } + +void Client::updateConnectionInfo(const std::string& serviceUrl, + const std::optional& authentication, + const std::optional& tlsTrustCertsFilePath) { + impl_->updateConnectionInfo(serviceUrl, authentication, tlsTrustCertsFilePath); +} + } // namespace pulsar diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index eec3b34a..229ee2f3 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -854,4 +854,12 @@ std::chrono::nanoseconds ClientImpl::getOperationTimeout(const ClientConfigurati return clientConfiguration.impl_->operationTimeout; } +void ClientImpl::updateConnectionInfo(const std::string& serviceUrl, + const std::optional& authentication, + const std::optional& tlsTrustCertsFilePath) { + // TODO: + // 1. Reset the `lookupServicePtr_` with the new serviceUrl and auth parameters, and close the old one. + // 2. Close all connections in `pool_` +} + } /* namespace pulsar */ diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 0b4d5969..e89aa98a 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -139,6 +139,10 @@ class ClientImpl : public std::enable_shared_from_this { ConnectionPool& getConnectionPool() noexcept { return pool_; } uint64_t getLookupCount() { return lookupCount_; } + void updateConnectionInfo(const std::string& serviceUrl, + const std::optional& authentication, + const std::optional& tlsTrustCertsFilePath); + static std::chrono::nanoseconds getOperationTimeout(const ClientConfiguration& clientConfiguration); friend class PulsarFriend; From a1746a9d1bf3c37c398845270ec90f36fb06f39e Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 11:33:24 +0800 Subject: [PATCH 02/28] implemented --- lib/ClientImpl.cc | 64 ++++++++++++++++++++++++++++++++----------- lib/ConnectionPool.cc | 15 ++++++++++ lib/ConnectionPool.h | 6 ++++ tests/ClientTest.cc | 47 +++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 16 deletions(-) diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 229ee2f3..9be08415 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -144,11 +144,11 @@ ExecutorServiceProviderPtr ClientImpl::getPartitionListenerExecutorProvider() { } LookupServicePtr ClientImpl::getLookup(const std::string& redirectedClusterURI) { + Lock lock(mutex_); if (redirectedClusterURI.empty()) { return lookupServicePtr_; } - Lock lock(mutex_); auto it = redirectedClusterLookupServicePtrs_.find(redirectedClusterURI); if (it == redirectedClusterLookupServicePtrs_.end()) { auto lookup = createLookup(redirectedClusterURI); @@ -180,7 +180,8 @@ void ClientImpl::createProducerAsync(const std::string& topic, const ProducerCon if (autoDownloadSchema) { auto self = shared_from_this(); - lookupServicePtr_->getSchema(topicName).addListener( + auto lookup = getLookup(); + lookup->getSchema(topicName).addListener( [self, topicName, callback](Result res, const SchemaInfo& topicSchema) { if (res != ResultOk) { callback(res, Producer()); @@ -188,12 +189,12 @@ void ClientImpl::createProducerAsync(const std::string& topic, const ProducerCon } ProducerConfiguration conf; conf.setSchema(topicSchema); - self->lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + self->getLookup()->getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleCreateProducer, self, std::placeholders::_1, std::placeholders::_2, topicName, conf, callback)); }); } else { - lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + getLookup()->getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleCreateProducer, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, conf, callback)); } @@ -266,7 +267,7 @@ void ClientImpl::createReaderAsync(const std::string& topic, const MessageId& st } MessageId msgId(startMessageId); - lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + getLookup()->getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleReaderMetadataLookup, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, msgId, conf, callback)); } @@ -379,7 +380,8 @@ void ClientImpl::subscribeWithRegexAsync(const std::string& regexPattern, const return; } - lookupServicePtr_->getTopicsOfNamespaceAsync(topicNamePtr->getNamespaceName(), mode) + getLookup() + ->getTopicsOfNamespaceAsync(topicNamePtr->getNamespaceName(), mode) .addListener(std::bind(&ClientImpl::createPatternMultiTopicsConsumer, shared_from_this(), std::placeholders::_1, std::placeholders::_2, regexPattern, mode, subscriptionName, conf, callback)); @@ -403,7 +405,7 @@ void ClientImpl::createPatternMultiTopicsConsumer(Result result, const Namespace consumer = std::make_shared(shared_from_this(), regexPattern, mode, *matchTopics, subscriptionName, conf, - lookupServicePtr_, interceptors); + getLookup(), interceptors); consumer->getConsumerCreatedFuture().addListener( std::bind(&ClientImpl::handleConsumerCreated, shared_from_this(), std::placeholders::_1, @@ -450,7 +452,7 @@ void ClientImpl::subscribeAsync(const std::vector& originalTopics, auto interceptors = std::make_shared(conf.getInterceptors()); ConsumerImplBasePtr consumer = std::make_shared( - shared_from_this(), topics, subscriptionName, topicNamePtr, conf, lookupServicePtr_, interceptors); + shared_from_this(), topics, subscriptionName, topicNamePtr, conf, getLookup(), interceptors); consumer->getConsumerCreatedFuture().addListener(std::bind(&ClientImpl::handleConsumerCreated, shared_from_this(), std::placeholders::_1, @@ -480,7 +482,7 @@ void ClientImpl::subscribeAsync(const std::string& topic, const std::string& sub } } - lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + getLookup()->getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleSubscribe, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, subscriptionName, conf, callback)); } @@ -505,7 +507,7 @@ void ClientImpl::handleSubscribe(Result result, const LookupDataResultPtr& parti } consumer = std::make_shared( shared_from_this(), topicName, partitionMetadata->getPartitions(), subscriptionName, conf, - lookupServicePtr_, interceptors); + getLookup(), interceptors); } else { auto consumerImpl = std::make_shared(shared_from_this(), topicName->toString(), subscriptionName, conf, @@ -658,7 +660,7 @@ void ClientImpl::getPartitionsForTopicAsync(const std::string& topic, const GetP return; } } - lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + getLookup()->getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleGetPartitions, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, callback)); } @@ -674,7 +676,7 @@ void ClientImpl::closeAsync(const CloseCallback& callback) { state_ = Closing; memoryLimitController_.close(); - lookupServicePtr_->close(); + getLookup()->close(); for (const auto& it : redirectedClusterLookupServicePtrs_) { it.second->close(); } @@ -776,7 +778,7 @@ void ClientImpl::shutdown() { << " consumers have been shutdown."); } - lookupServicePtr_->close(); + getLookup()->close(); if (!pool_.close()) { // pool_ has already been closed. It means shutdown() has been called before. return; @@ -857,9 +859,39 @@ std::chrono::nanoseconds ClientImpl::getOperationTimeout(const ClientConfigurati void ClientImpl::updateConnectionInfo(const std::string& serviceUrl, const std::optional& authentication, const std::optional& tlsTrustCertsFilePath) { - // TODO: - // 1. Reset the `lookupServicePtr_` with the new serviceUrl and auth parameters, and close the old one. - // 2. Close all connections in `pool_` + LookupServicePtr oldLookupServicePtr; + std::unordered_map oldRedirectedLookupServicePtrs; + + { + Lock lock(mutex_); + if (state_ != Open) { + LOG_ERROR("Client is not open, cannot update connection info"); + return; + } + + if (authentication.has_value()) { + clientConfiguration_.setAuth(*authentication); + } + if (tlsTrustCertsFilePath.has_value()) { + clientConfiguration_.setTlsTrustCertsFilePath(*tlsTrustCertsFilePath); + } + clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl))); + + oldLookupServicePtr = std::move(lookupServicePtr_); + oldRedirectedLookupServicePtrs = std::move(redirectedClusterLookupServicePtrs_); + + lookupServicePtr_ = createLookup(serviceUrl); + redirectedClusterLookupServicePtrs_.clear(); + } + + if (oldLookupServicePtr) { + oldLookupServicePtr->close(); + } + for (const auto& it : oldRedirectedLookupServicePtrs) { + it.second->close(); + } + + pool_.resetConnections(clientConfiguration_.getAuthPtr(), clientConfiguration_); } } /* namespace pulsar */ diff --git a/lib/ConnectionPool.cc b/lib/ConnectionPool.cc index df050b0e..d94d8fe8 100644 --- a/lib/ConnectionPool.cc +++ b/lib/ConnectionPool.cc @@ -67,6 +67,21 @@ bool ConnectionPool::close() { return true; } +void ConnectionPool::resetConnections(const AuthenticationPtr& authentication, + const ClientConfiguration& conf) { + std::unique_lock lock(mutex_); + authentication_ = authentication; + clientConfiguration_ = conf; + + for (auto cnxIt = pool_.begin(); cnxIt != pool_.end(); cnxIt++) { + auto& cnx = cnxIt->second; + if (cnx) { + cnx->close(ResultDisconnected, false); + } + } + pool_.clear(); +} + static const std::string getKey(const std::string& logicalAddress, const std::string& physicalAddress, size_t keySuffix) { std::stringstream ss; diff --git a/lib/ConnectionPool.h b/lib/ConnectionPool.h index 0e3a6d0a..64907b4f 100644 --- a/lib/ConnectionPool.h +++ b/lib/ConnectionPool.h @@ -51,6 +51,12 @@ class PULSAR_PUBLIC ConnectionPool { */ bool close(); + /** + * Close all existing connections and update the authentication and configuration. + * Unlike close(), the pool remains open for new connections. + */ + void resetConnections(const AuthenticationPtr& authentication, const ClientConfiguration& conf); + void remove(const std::string& logicalAddress, const std::string& physicalAddress, size_t keySuffix, ClientConnection* value); diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index dd892686..f9f4b93b 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -506,3 +506,50 @@ TEST(ClientTest, testNoRetry) { ASSERT_TRUE(result.timeMs < 1000) << "consumer: " << result.timeMs << " ms"; } } + +TEST(ClientTest, testUpdateConnectionInfo) { + const std::string cluster1Url = "pulsar://localhost:6650"; + const std::string cluster2Url = "pulsar://localhost:6652"; + const std::string topic1 = "testUpdateConnectionInfo-cluster1-" + std::to_string(time(nullptr)); + const std::string topic2 = "testUpdateConnectionInfo-cluster2-" + std::to_string(time(nullptr)); + + Client client(cluster1Url); + + // Produce and consume on cluster 1 + Producer producer1; + ASSERT_EQ(ResultOk, client.createProducer(topic1, producer1)); + MessageId msgId; + ASSERT_EQ(ResultOk, producer1.send(MessageBuilder().setContent("msg-on-cluster1").build(), msgId)); + producer1.close(); + + // Verify there are connections in the pool + auto connections = PulsarFriend::getConnections(client); + ASSERT_FALSE(connections.empty()); + + // Switch to cluster 2 + client.updateConnectionInfo(cluster2Url, std::nullopt, std::nullopt); + + // Previous connections should have been closed + for (const auto &cnx : connections) { + ASSERT_TRUE(cnx->isClosed()); + } + + // Produce and consume on cluster 2 using the same client + Producer producer2; + ASSERT_EQ(ResultOk, client.createProducer(topic2, producer2)); + ASSERT_EQ(ResultOk, producer2.send(MessageBuilder().setContent("msg-on-cluster2").build(), msgId)); + + Consumer consumer2; + ASSERT_EQ(ResultOk, client.subscribe(topic2, "sub", consumer2)); + Message msg; + ASSERT_EQ(ResultOk, consumer2.receive(msg, 5000)); + ASSERT_EQ("msg-on-cluster2", msg.getDataAsString()); + + // Verify connection pool now has connections to cluster 2 + auto newConnections = PulsarFriend::getConnections(client); + ASSERT_FALSE(newConnections.empty()); + + consumer2.close(); + producer2.close(); + client.close(); +} From 15908f99dedd18e2f8393b15040578a790f00c2f Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 15:08:38 +0800 Subject: [PATCH 03/28] fix auth not cleared if it's not set and improve tests --- lib/ClientImpl.cc | 4 ++ tests/AuthTokenTest.cc | 2 +- tests/ClientTest.cc | 97 +++++++++++++++++++++++++----------------- 3 files changed, 63 insertions(+), 40 deletions(-) diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 9be08415..98fe3c30 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -871,9 +871,13 @@ void ClientImpl::updateConnectionInfo(const std::string& serviceUrl, if (authentication.has_value()) { clientConfiguration_.setAuth(*authentication); + } else { + clientConfiguration_.setAuth(AuthFactory::Disabled()); } if (tlsTrustCertsFilePath.has_value()) { clientConfiguration_.setTlsTrustCertsFilePath(*tlsTrustCertsFilePath); + } else { + clientConfiguration_.setTlsTrustCertsFilePath(""); } clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl))); diff --git a/tests/AuthTokenTest.cc b/tests/AuthTokenTest.cc index 84e8572d..4bfd8085 100644 --- a/tests/AuthTokenTest.cc +++ b/tests/AuthTokenTest.cc @@ -42,7 +42,7 @@ static const std::string serviceUrlHttp = "http://localhost:8080"; static const std::string tokenPath = TOKEN_PATH; -static std::string getToken() { +std::string getToken() { std::ifstream file(tokenPath); std::string str((std::istreambuf_iterator(file)), std::istreambuf_iterator()); return str; diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index f9f4b93b..37376cf2 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -17,13 +17,16 @@ * under the License. */ #include +#include #include +#include #include #include #include #include -#include +#include +#include #include "MockClientImpl.h" #include "PulsarAdminHelper.h" @@ -32,7 +35,6 @@ #include "lib/ClientConnection.h" #include "lib/LogUtils.h" #include "lib/checksum/ChecksumProvider.h" -#include "lib/stats/ProducerStatsImpl.h" DECLARE_LOG_OBJECT() @@ -508,48 +510,65 @@ TEST(ClientTest, testNoRetry) { } TEST(ClientTest, testUpdateConnectionInfo) { - const std::string cluster1Url = "pulsar://localhost:6650"; - const std::string cluster2Url = "pulsar://localhost:6652"; - const std::string topic1 = "testUpdateConnectionInfo-cluster1-" + std::to_string(time(nullptr)); - const std::string topic2 = "testUpdateConnectionInfo-cluster2-" + std::to_string(time(nullptr)); - - Client client(cluster1Url); - - // Produce and consume on cluster 1 - Producer producer1; - ASSERT_EQ(ResultOk, client.createProducer(topic1, producer1)); + extern std::string getToken(); // from AuthToken.cc + + // Access "private/auth" namespace in cluster 1 + auto info1 = + std::make_tuple("pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt); + // Access "private/auth" namespace in cluster 2 + auto info2 = + std::make_tuple("pulsar+ssl://localhost:6653", + AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), + TEST_CONF_DIR "/hn-verification/cacert.pem"); + // Access "public/default" namespace in cluster 1, which doesn't require authentication + auto info3 = std::make_tuple("pulsar://localhost:6650", std::nullopt, std::nullopt); + + Client client{std::get<0>(info1), ClientConfiguration().setAuth(std::get<1>(info1))}; + const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); + Producer producer; + ASSERT_EQ(ResultOk, client.createProducer(topicRequiredAuth, producer)); MessageId msgId; - ASSERT_EQ(ResultOk, producer1.send(MessageBuilder().setContent("msg-on-cluster1").build(), msgId)); - producer1.close(); + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-0").build(), msgId)); - // Verify there are connections in the pool - auto connections = PulsarFriend::getConnections(client); - ASSERT_FALSE(connections.empty()); + // Switch to cluster 2 (started by ./build-support/start-mim-test-service-inside-container.sh) + ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); + client.updateConnectionInfo(std::get<0>(info2), std::get<1>(info2), std::get<2>(info2)); + ASSERT_TRUE(PulsarFriend::getConnections(client).empty()); - // Switch to cluster 2 - client.updateConnectionInfo(cluster2Url, std::nullopt, std::nullopt); + // Now the same will access the same topic in cluster 2 + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-1").build(), msgId)); + ASSERT_EQ(ResultOk, producer.close()); - // Previous connections should have been closed - for (const auto &cnx : connections) { - ASSERT_TRUE(cnx->isClosed()); - } + // Switch back to cluster 1 without any authentication, the previous authentication info configured for + // cluster 2 will be cleared. + client.updateConnectionInfo(std::get<0>(info3), std::get<1>(info3), std::get<2>(info3)); - // Produce and consume on cluster 2 using the same client - Producer producer2; - ASSERT_EQ(ResultOk, client.createProducer(topic2, producer2)); - ASSERT_EQ(ResultOk, producer2.send(MessageBuilder().setContent("msg-on-cluster2").build(), msgId)); + const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); + ASSERT_EQ(ResultOk, client.createProducer(topicNoAuth, producer)); + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-2").build(), msgId)); - Consumer consumer2; - ASSERT_EQ(ResultOk, client.subscribe(topic2, "sub", consumer2)); - Message msg; - ASSERT_EQ(ResultOk, consumer2.receive(msg, 5000)); - ASSERT_EQ("msg-on-cluster2", msg.getDataAsString()); - - // Verify connection pool now has connections to cluster 2 - auto newConnections = PulsarFriend::getConnections(client); - ASSERT_FALSE(newConnections.empty()); - - consumer2.close(); - producer2.close(); client.close(); + + // Verify messages sent to cluster 1 and cluster 2 can be consumed successfully with correct + // authentication info. + auto verify = [](Client &client, const std::string &topic, const std::string &value) { + Reader reader; + ASSERT_EQ(ResultOk, client.createReader(topic, MessageId::earliest(), {}, reader)); + Message msg; + ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); + ASSERT_EQ(value, msg.getDataAsString()); + }; + Client client1{std::get<0>(info1), ClientConfiguration().setAuth(std::get<1>(info1))}; + verify(client1, topicRequiredAuth, "msg-0"); + client1.close(); + + Client client2{ + std::get<0>(info2), + ClientConfiguration().setAuth(std::get<1>(info2)).setTlsTrustCertsFilePath(std::get<2>(info2))}; + verify(client2, topicRequiredAuth, "msg-1"); + client2.close(); + + Client client3{std::get<0>(info3)}; + verify(client3, topicNoAuth, "msg-2"); + client3.close(); } From 8b0899f189df1314b86049f4d339d3b98565d135 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 16:14:55 +0800 Subject: [PATCH 04/28] refactor and add a getServiceInfo method --- include/pulsar/Client.h | 27 ++++++++++++++++++--------- lib/Client.cc | 8 +++----- lib/ClientImpl.cc | 38 +++++++++++++++++++++++++++++--------- lib/ClientImpl.h | 6 +++--- tests/ClientTest.cc | 36 +++++++++++++++++++++--------------- 5 files changed, 74 insertions(+), 41 deletions(-) diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index 95233fe9..8fccbe19 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -43,6 +43,12 @@ typedef std::function TableViewCallback; typedef std::function&)> GetPartitionsCallback; typedef std::function CloseCallback; +struct PULSAR_PUBLIC ServiceInfo { + std::string serviceUrl; + std::optional authentication; + std::optional tlsTrustCertsFilePath; +}; + class ClientImpl; class PulsarFriend; class PulsarWrapper; @@ -416,18 +422,21 @@ class PULSAR_PUBLIC Client { std::function callback); /** - * Update the connection information of the client, including service URL, authentication and TLS trust - * certs file path. + * Update the service information of the client. + * * This method is used to switch the connection to a different Pulsar cluster. All connections will be - * closed and the internal connection info will be updated. + * closed and the internal service info will be updated. * - * @param serviceUrl the Pulsar endpoint to use (eg: pulsar://localhost:6650) - * @param authentication the authentication information to use for connecting to the Pulsar cluster - * @param tlsTrustCertsFilePath the TLS trust certs file path to use for connecting to the Pulsar cluster + * @param serviceInfo the service URL, authentication and TLS trust certs file path to use + */ + void updateServiceInfo(const ServiceInfo& serviceInfo); + + /** + * Get the current service information of the client. + * + * @return the current service information */ - void updateConnectionInfo(const std::string& serviceUrl, - const std::optional& authentication, - const std::optional& tlsTrustCertsFilePath); + ServiceInfo getServiceInfo(); private: Client(const std::shared_ptr&); diff --git a/lib/Client.cc b/lib/Client.cc index 60fb62f5..6ceaa117 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -198,10 +198,8 @@ void Client::getSchemaInfoAsync(const std::string& topic, int64_t version, .addListener(std::move(callback)); } -void Client::updateConnectionInfo(const std::string& serviceUrl, - const std::optional& authentication, - const std::optional& tlsTrustCertsFilePath) { - impl_->updateConnectionInfo(serviceUrl, authentication, tlsTrustCertsFilePath); -} +void Client::updateServiceInfo(const ServiceInfo& serviceInfo) { impl_->updateServiceInfo(serviceInfo); } + +ServiceInfo Client::getServiceInfo() { return impl_->getServiceInfo(); } } // namespace pulsar diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 98fe3c30..2aceafea 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -78,6 +78,15 @@ typedef std::unique_lock Lock; typedef std::vector StringList; +namespace { +std::optional toOptionalAuthentication(const AuthenticationPtr& authentication) { + if (!authentication || authentication->getAuthMethodName() == "none") { + return std::nullopt; + } + return authentication; +} +} // namespace + static LookupServicePtr defaultLookupServiceFactory(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration, ConnectionPool& pool, const AuthenticationPtr& auth) { @@ -101,6 +110,10 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& state_(Open), clientConfiguration_(ClientConfiguration(clientConfiguration) .setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl)))), + serviceInfo_{serviceUrl, toOptionalAuthentication(clientConfiguration.getAuthPtr()), + clientConfiguration.getTlsTrustCertsFilePath().empty() + ? std::nullopt + : std::make_optional(clientConfiguration.getTlsTrustCertsFilePath())}, memoryLimitController_(clientConfiguration.getMemoryLimit()), ioExecutorProvider_(std::make_shared(clientConfiguration_.getIOThreads())), listenerExecutorProvider_( @@ -856,9 +869,7 @@ std::chrono::nanoseconds ClientImpl::getOperationTimeout(const ClientConfigurati return clientConfiguration.impl_->operationTimeout; } -void ClientImpl::updateConnectionInfo(const std::string& serviceUrl, - const std::optional& authentication, - const std::optional& tlsTrustCertsFilePath) { +void ClientImpl::updateServiceInfo(const ServiceInfo& serviceInfo) { LookupServicePtr oldLookupServicePtr; std::unordered_map oldRedirectedLookupServicePtrs; @@ -869,22 +880,26 @@ void ClientImpl::updateConnectionInfo(const std::string& serviceUrl, return; } - if (authentication.has_value()) { - clientConfiguration_.setAuth(*authentication); + if (serviceInfo.authentication.has_value() && *serviceInfo.authentication) { + clientConfiguration_.setAuth(*serviceInfo.authentication); } else { clientConfiguration_.setAuth(AuthFactory::Disabled()); } - if (tlsTrustCertsFilePath.has_value()) { - clientConfiguration_.setTlsTrustCertsFilePath(*tlsTrustCertsFilePath); + if (serviceInfo.tlsTrustCertsFilePath.has_value()) { + clientConfiguration_.setTlsTrustCertsFilePath(*serviceInfo.tlsTrustCertsFilePath); } else { clientConfiguration_.setTlsTrustCertsFilePath(""); } - clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl))); + clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceInfo.serviceUrl))); + serviceInfo_ = {serviceInfo.serviceUrl, toOptionalAuthentication(clientConfiguration_.getAuthPtr()), + clientConfiguration_.getTlsTrustCertsFilePath().empty() + ? std::nullopt + : std::make_optional(clientConfiguration_.getTlsTrustCertsFilePath())}; oldLookupServicePtr = std::move(lookupServicePtr_); oldRedirectedLookupServicePtrs = std::move(redirectedClusterLookupServicePtrs_); - lookupServicePtr_ = createLookup(serviceUrl); + lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); redirectedClusterLookupServicePtrs_.clear(); } @@ -898,4 +913,9 @@ void ClientImpl::updateConnectionInfo(const std::string& serviceUrl, pool_.resetConnections(clientConfiguration_.getAuthPtr(), clientConfiguration_); } +ServiceInfo ClientImpl::getServiceInfo() { + Lock lock(mutex_); + return serviceInfo_; +} + } /* namespace pulsar */ diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index e89aa98a..c96f9aab 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -139,9 +139,8 @@ class ClientImpl : public std::enable_shared_from_this { ConnectionPool& getConnectionPool() noexcept { return pool_; } uint64_t getLookupCount() { return lookupCount_; } - void updateConnectionInfo(const std::string& serviceUrl, - const std::optional& authentication, - const std::optional& tlsTrustCertsFilePath); + void updateServiceInfo(const ServiceInfo& serviceInfo); + ServiceInfo getServiceInfo(); static std::chrono::nanoseconds getOperationTimeout(const ClientConfiguration& clientConfiguration); @@ -196,6 +195,7 @@ class ClientImpl : public std::enable_shared_from_this { State state_; ClientConfiguration clientConfiguration_; + ServiceInfo serviceInfo_; MemoryLimitController memoryLimitController_; ExecutorServiceProviderPtr ioExecutorProvider_; diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 37376cf2..5bc4d89b 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -513,17 +513,16 @@ TEST(ClientTest, testUpdateConnectionInfo) { extern std::string getToken(); // from AuthToken.cc // Access "private/auth" namespace in cluster 1 - auto info1 = - std::make_tuple("pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt); + ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt}; // Access "private/auth" namespace in cluster 2 - auto info2 = - std::make_tuple("pulsar+ssl://localhost:6653", - AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), - TEST_CONF_DIR "/hn-verification/cacert.pem"); + ServiceInfo info2{ + "pulsar+ssl://localhost:6653", + AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), + TEST_CONF_DIR "/hn-verification/cacert.pem"}; // Access "public/default" namespace in cluster 1, which doesn't require authentication - auto info3 = std::make_tuple("pulsar://localhost:6650", std::nullopt, std::nullopt); + ServiceInfo info3{"pulsar://localhost:6650", std::nullopt, std::nullopt}; - Client client{std::get<0>(info1), ClientConfiguration().setAuth(std::get<1>(info1))}; + Client client{info1.serviceUrl, ClientConfiguration().setAuth(*info1.authentication)}; const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); Producer producer; ASSERT_EQ(ResultOk, client.createProducer(topicRequiredAuth, producer)); @@ -532,8 +531,11 @@ TEST(ClientTest, testUpdateConnectionInfo) { // Switch to cluster 2 (started by ./build-support/start-mim-test-service-inside-container.sh) ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); - client.updateConnectionInfo(std::get<0>(info2), std::get<1>(info2), std::get<2>(info2)); + client.updateServiceInfo(info2); ASSERT_TRUE(PulsarFriend::getConnections(client).empty()); + ASSERT_EQ(info2.serviceUrl, client.getServiceInfo().serviceUrl); + ASSERT_EQ(info2.authentication, client.getServiceInfo().authentication); + ASSERT_EQ(info2.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); // Now the same will access the same topic in cluster 2 ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-1").build(), msgId)); @@ -541,7 +543,10 @@ TEST(ClientTest, testUpdateConnectionInfo) { // Switch back to cluster 1 without any authentication, the previous authentication info configured for // cluster 2 will be cleared. - client.updateConnectionInfo(std::get<0>(info3), std::get<1>(info3), std::get<2>(info3)); + client.updateServiceInfo(info3); + ASSERT_EQ(info3.serviceUrl, client.getServiceInfo().serviceUrl); + ASSERT_EQ(info3.authentication, client.getServiceInfo().authentication); + ASSERT_EQ(info3.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); ASSERT_EQ(ResultOk, client.createProducer(topicNoAuth, producer)); @@ -558,17 +563,18 @@ TEST(ClientTest, testUpdateConnectionInfo) { ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); ASSERT_EQ(value, msg.getDataAsString()); }; - Client client1{std::get<0>(info1), ClientConfiguration().setAuth(std::get<1>(info1))}; + Client client1{info1.serviceUrl, ClientConfiguration().setAuth(*info1.authentication)}; verify(client1, topicRequiredAuth, "msg-0"); client1.close(); - Client client2{ - std::get<0>(info2), - ClientConfiguration().setAuth(std::get<1>(info2)).setTlsTrustCertsFilePath(std::get<2>(info2))}; + Client client2{info2.serviceUrl, + ClientConfiguration() + .setAuth(*info2.authentication) + .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath)}; verify(client2, topicRequiredAuth, "msg-1"); client2.close(); - Client client3{std::get<0>(info3)}; + Client client3{info3.serviceUrl}; verify(client3, topicNoAuth, "msg-2"); client3.close(); } From 7ca81369a445c3380145251980c815bdc84ef0a9 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 16:19:59 +0800 Subject: [PATCH 05/28] fix format --- tests/ClientTest.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 5bc4d89b..07128d19 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -515,10 +515,9 @@ TEST(ClientTest, testUpdateConnectionInfo) { // Access "private/auth" namespace in cluster 1 ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt}; // Access "private/auth" namespace in cluster 2 - ServiceInfo info2{ - "pulsar+ssl://localhost:6653", - AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), - TEST_CONF_DIR "/hn-verification/cacert.pem"}; + ServiceInfo info2{"pulsar+ssl://localhost:6653", + AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), + TEST_CONF_DIR "/hn-verification/cacert.pem"}; // Access "public/default" namespace in cluster 1, which doesn't require authentication ServiceInfo info3{"pulsar://localhost:6650", std::nullopt, std::nullopt}; @@ -567,10 +566,9 @@ TEST(ClientTest, testUpdateConnectionInfo) { verify(client1, topicRequiredAuth, "msg-0"); client1.close(); - Client client2{info2.serviceUrl, - ClientConfiguration() - .setAuth(*info2.authentication) - .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath)}; + Client client2{info2.serviceUrl, ClientConfiguration() + .setAuth(*info2.authentication) + .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath)}; verify(client2, topicRequiredAuth, "msg-1"); client2.close(); From e70a0a421f2423b1a79c9e481c0fc1b6329b446e Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 17:06:01 +0800 Subject: [PATCH 06/28] use read-write lock instead --- include/pulsar/Client.h | 2 +- lib/Client.cc | 2 +- lib/ClientImpl.cc | 114 ++++++++++++++++++++-------------------- lib/ClientImpl.h | 5 +- tests/ClientTest.cc | 2 +- 5 files changed, 63 insertions(+), 62 deletions(-) diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index 8fccbe19..f3ee2f66 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -436,7 +436,7 @@ class PULSAR_PUBLIC Client { * * @return the current service information */ - ServiceInfo getServiceInfo(); + ServiceInfo getServiceInfo() const; private: Client(const std::shared_ptr&); diff --git a/lib/Client.cc b/lib/Client.cc index 6ceaa117..45cf2170 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -200,6 +200,6 @@ void Client::getSchemaInfoAsync(const std::string& topic, int64_t version, void Client::updateServiceInfo(const ServiceInfo& serviceInfo) { impl_->updateServiceInfo(serviceInfo); } -ServiceInfo Client::getServiceInfo() { return impl_->getServiceInfo(); } +ServiceInfo Client::getServiceInfo() const { return impl_->getServiceInfo(); } } // namespace pulsar diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 2aceafea..f64908a5 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -24,7 +24,9 @@ #include #include #include +#include #include +#include #include #include "BinaryProtoLookupService.h" @@ -74,8 +76,6 @@ std::string generateRandomName() { return randomName; } -typedef std::unique_lock Lock; - typedef std::vector StringList; namespace { @@ -157,19 +157,26 @@ ExecutorServiceProviderPtr ClientImpl::getPartitionListenerExecutorProvider() { } LookupServicePtr ClientImpl::getLookup(const std::string& redirectedClusterURI) { - Lock lock(mutex_); + std::shared_lock readLock(mutex_); if (redirectedClusterURI.empty()) { return lookupServicePtr_; } - auto it = redirectedClusterLookupServicePtrs_.find(redirectedClusterURI); - if (it == redirectedClusterLookupServicePtrs_.end()) { - auto lookup = createLookup(redirectedClusterURI); - redirectedClusterLookupServicePtrs_.emplace(redirectedClusterURI, lookup); - return lookup; + if (auto it = redirectedClusterLookupServicePtrs_.find(redirectedClusterURI); + it != redirectedClusterLookupServicePtrs_.end()) { + return it->second; } + readLock.unlock(); - return it->second; + std::unique_lock writeLock(mutex_); + // Double check in case another thread acquires the lock and inserts a pair first + if (auto it = redirectedClusterLookupServicePtrs_.find(redirectedClusterURI); + it != redirectedClusterLookupServicePtrs_.end()) { + return it->second; + } + auto lookup = createLookup(redirectedClusterURI); + redirectedClusterLookupServicePtrs_.emplace(redirectedClusterURI, lookup); + return lookup; } void ClientImpl::createProducerAsync(const std::string& topic, const ProducerConfiguration& conf, @@ -179,7 +186,7 @@ void ClientImpl::createProducerAsync(const std::string& topic, const ProducerCon } TopicNamePtr topicName; { - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, Producer()); @@ -267,7 +274,7 @@ void ClientImpl::createReaderAsync(const std::string& topic, const MessageId& st const ReaderConfiguration& conf, const ReaderCallback& callback) { TopicNamePtr topicName; { - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, Reader()); @@ -289,7 +296,7 @@ void ClientImpl::createTableViewAsync(const std::string& topic, const TableViewC const TableViewCallback& callback) { TopicNamePtr topicName; { - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, TableView()); @@ -355,7 +362,7 @@ void ClientImpl::subscribeWithRegexAsync(const std::string& regexPattern, const const SubscribeCallback& callback) { TopicNamePtr topicNamePtr = TopicName::get(regexPattern); - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, Consumer()); @@ -441,7 +448,7 @@ void ClientImpl::subscribeAsync(const std::vector& originalTopics, auto it = std::unique(topics.begin(), topics.end()); auto newSize = std::distance(topics.begin(), it); topics.resize(newSize); - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, Consumer()); @@ -477,7 +484,7 @@ void ClientImpl::subscribeAsync(const std::string& topic, const std::string& sub const ConsumerConfiguration& conf, const SubscribeCallback& callback) { TopicNamePtr topicName; { - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, Consumer()); @@ -662,7 +669,7 @@ void ClientImpl::handleGetPartitions(Result result, const LookupDataResultPtr& p void ClientImpl::getPartitionsForTopicAsync(const std::string& topic, const GetPartitionsCallback& callback) { TopicNamePtr topicName; { - Lock lock(mutex_); + std::shared_lock lock(mutex_); if (state_ != Open) { lock.unlock(); callback(ResultAlreadyClosed, StringList()); @@ -679,7 +686,9 @@ void ClientImpl::getPartitionsForTopicAsync(const std::string& topic, const GetP } void ClientImpl::closeAsync(const CloseCallback& callback) { + std::unique_lock lock(mutex_); if (state_ != Open) { + lock.unlock(); if (callback) { callback(ResultAlreadyClosed); } @@ -689,10 +698,12 @@ void ClientImpl::closeAsync(const CloseCallback& callback) { state_ = Closing; memoryLimitController_.close(); - getLookup()->close(); + lookupServicePtr_->close(); for (const auto& it : redirectedClusterLookupServicePtrs_) { it.second->close(); } + redirectedClusterLookupServicePtrs_.clear(); + lock.unlock(); auto producers = producers_.move(); auto consumers = consumers_.move(); @@ -741,7 +752,7 @@ void ClientImpl::handleClose(Result result, const SharedInt& numberOfOpenHandler --(*numberOfOpenHandlers); } if (*numberOfOpenHandlers == 0) { - Lock lock(mutex_); + std::unique_lock lock(mutex_); if (state_ == Closed) { LOG_DEBUG("Client is already shutting down, possible race condition in handleClose"); return; @@ -821,12 +832,12 @@ void ClientImpl::shutdown() { } uint64_t ClientImpl::newProducerId() { - Lock lock(mutex_); + std::shared_lock lock(mutex_); return producerIdGenerator_++; } uint64_t ClientImpl::newConsumerId() { - Lock lock(mutex_); + std::shared_lock lock(mutex_); return consumerIdGenerator_++; } @@ -870,51 +881,40 @@ std::chrono::nanoseconds ClientImpl::getOperationTimeout(const ClientConfigurati } void ClientImpl::updateServiceInfo(const ServiceInfo& serviceInfo) { - LookupServicePtr oldLookupServicePtr; - std::unordered_map oldRedirectedLookupServicePtrs; - - { - Lock lock(mutex_); - if (state_ != Open) { - LOG_ERROR("Client is not open, cannot update connection info"); - return; - } - - if (serviceInfo.authentication.has_value() && *serviceInfo.authentication) { - clientConfiguration_.setAuth(*serviceInfo.authentication); - } else { - clientConfiguration_.setAuth(AuthFactory::Disabled()); - } - if (serviceInfo.tlsTrustCertsFilePath.has_value()) { - clientConfiguration_.setTlsTrustCertsFilePath(*serviceInfo.tlsTrustCertsFilePath); - } else { - clientConfiguration_.setTlsTrustCertsFilePath(""); - } - clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceInfo.serviceUrl))); - serviceInfo_ = {serviceInfo.serviceUrl, toOptionalAuthentication(clientConfiguration_.getAuthPtr()), - clientConfiguration_.getTlsTrustCertsFilePath().empty() - ? std::nullopt - : std::make_optional(clientConfiguration_.getTlsTrustCertsFilePath())}; - - oldLookupServicePtr = std::move(lookupServicePtr_); - oldRedirectedLookupServicePtrs = std::move(redirectedClusterLookupServicePtrs_); - - lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); - redirectedClusterLookupServicePtrs_.clear(); + std::unique_lock lock(mutex_); + if (state_ != Open) { + LOG_ERROR("Client is not open, cannot update connection info"); + return; } - if (oldLookupServicePtr) { - oldLookupServicePtr->close(); + if (serviceInfo.authentication.has_value() && *serviceInfo.authentication) { + clientConfiguration_.setAuth(*serviceInfo.authentication); + } else { + clientConfiguration_.setAuth(AuthFactory::Disabled()); } - for (const auto& it : oldRedirectedLookupServicePtrs) { - it.second->close(); + if (serviceInfo.tlsTrustCertsFilePath.has_value()) { + clientConfiguration_.setTlsTrustCertsFilePath(*serviceInfo.tlsTrustCertsFilePath); + } else { + clientConfiguration_.setTlsTrustCertsFilePath(""); } + clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceInfo.serviceUrl))); + serviceInfo_ = {serviceInfo.serviceUrl, toOptionalAuthentication(clientConfiguration_.getAuthPtr()), + clientConfiguration_.getTlsTrustCertsFilePath().empty() + ? std::nullopt + : std::make_optional(clientConfiguration_.getTlsTrustCertsFilePath())}; pool_.resetConnections(clientConfiguration_.getAuthPtr(), clientConfiguration_); + + lookupServicePtr_->close(); + for (auto&& it : redirectedClusterLookupServicePtrs_) { + it.second->close(); + } + redirectedClusterLookupServicePtrs_.clear(); + lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); } -ServiceInfo ClientImpl::getServiceInfo() { - Lock lock(mutex_); +ServiceInfo ClientImpl::getServiceInfo() const { + std::shared_lock lock(mutex_); return serviceInfo_; } diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index c96f9aab..26c167f1 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -24,6 +24,7 @@ #include #include #include +#include #include "ConnectionPool.h" #include "Future.h" @@ -140,7 +141,7 @@ class ClientImpl : public std::enable_shared_from_this { uint64_t getLookupCount() { return lookupCount_; } void updateServiceInfo(const ServiceInfo& serviceInfo); - ServiceInfo getServiceInfo(); + ServiceInfo getServiceInfo() const; static std::chrono::nanoseconds getOperationTimeout(const ClientConfiguration& clientConfiguration); @@ -191,7 +192,7 @@ class ClientImpl : public std::enable_shared_from_this { Closed }; - std::mutex mutex_; + mutable std::shared_mutex mutex_; State state_; ClientConfiguration clientConfiguration_; diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 07128d19..31d0d93d 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -509,7 +509,7 @@ TEST(ClientTest, testNoRetry) { } } -TEST(ClientTest, testUpdateConnectionInfo) { +TEST(ClientTest, testUpdateServiceInfo) { extern std::string getToken(); // from AuthToken.cc // Access "private/auth" namespace in cluster 1 From 342505e7dfaf462b0960a2308b93c1466285e814 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 17:19:04 +0800 Subject: [PATCH 07/28] pass by value + std::move for better performance --- include/pulsar/Client.h | 2 +- lib/Client.cc | 2 +- lib/ClientImpl.cc | 2 +- lib/ClientImpl.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index f3ee2f66..15dd0324 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -429,7 +429,7 @@ class PULSAR_PUBLIC Client { * * @param serviceInfo the service URL, authentication and TLS trust certs file path to use */ - void updateServiceInfo(const ServiceInfo& serviceInfo); + void updateServiceInfo(ServiceInfo serviceInfo); /** * Get the current service information of the client. diff --git a/lib/Client.cc b/lib/Client.cc index 45cf2170..311d2ddd 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -198,7 +198,7 @@ void Client::getSchemaInfoAsync(const std::string& topic, int64_t version, .addListener(std::move(callback)); } -void Client::updateServiceInfo(const ServiceInfo& serviceInfo) { impl_->updateServiceInfo(serviceInfo); } +void Client::updateServiceInfo(ServiceInfo serviceInfo) { impl_->updateServiceInfo(std::move(serviceInfo)); } ServiceInfo Client::getServiceInfo() const { return impl_->getServiceInfo(); } diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index f64908a5..7f2ee6c7 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -880,7 +880,7 @@ std::chrono::nanoseconds ClientImpl::getOperationTimeout(const ClientConfigurati return clientConfiguration.impl_->operationTimeout; } -void ClientImpl::updateServiceInfo(const ServiceInfo& serviceInfo) { +void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { std::unique_lock lock(mutex_); if (state_ != Open) { LOG_ERROR("Client is not open, cannot update connection info"); diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 26c167f1..e58d6df6 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -140,7 +140,7 @@ class ClientImpl : public std::enable_shared_from_this { ConnectionPool& getConnectionPool() noexcept { return pool_; } uint64_t getLookupCount() { return lookupCount_; } - void updateServiceInfo(const ServiceInfo& serviceInfo); + void updateServiceInfo(ServiceInfo&& serviceInfo); ServiceInfo getServiceInfo() const; static std::chrono::nanoseconds getOperationTimeout(const ClientConfiguration& clientConfiguration); From 477fb9495e3b496f56ea94ca2e4497bb2d01ec75 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 17:21:08 +0800 Subject: [PATCH 08/28] fix thread safety due to read lock for write operation --- lib/ClientImpl.cc | 10 ++-------- lib/ClientImpl.h | 4 ++-- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 7f2ee6c7..5b71a696 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -831,15 +831,9 @@ void ClientImpl::shutdown() { lookupCount_ = 0; } -uint64_t ClientImpl::newProducerId() { - std::shared_lock lock(mutex_); - return producerIdGenerator_++; -} +uint64_t ClientImpl::newProducerId() { return producerIdGenerator_++; } -uint64_t ClientImpl::newConsumerId() { - std::shared_lock lock(mutex_); - return consumerIdGenerator_++; -} +uint64_t ClientImpl::newConsumerId() { return consumerIdGenerator_++; } uint64_t ClientImpl::newRequestId() { return (*requestIdGenerator_)++; } diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index e58d6df6..11881a2f 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -207,8 +207,8 @@ class ClientImpl : public std::enable_shared_from_this { std::unordered_map redirectedClusterLookupServicePtrs_; ConnectionPool pool_; - uint64_t producerIdGenerator_; - uint64_t consumerIdGenerator_; + std::atomic_uint64_t producerIdGenerator_; + std::atomic_uint64_t consumerIdGenerator_; std::shared_ptr> requestIdGenerator_{std::make_shared>(0)}; SynchronizedHashMap producers_; From 9114edf05e322f1fcc02dc6b6ff42ee1ce613782 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 17:43:58 +0800 Subject: [PATCH 09/28] fix stale lookup service might be refered --- lib/Client.cc | 3 +-- lib/ClientImpl.cc | 37 +++++++++++++-------------- lib/ClientImpl.h | 22 +++++++++++++--- lib/MultiTopicsConsumerImpl.cc | 22 ++++++++++------ lib/MultiTopicsConsumerImpl.h | 7 +---- lib/PartitionedProducerImpl.cc | 8 +++--- lib/PartitionedProducerImpl.h | 3 --- lib/PatternMultiTopicsConsumerImpl.cc | 13 +++++----- lib/PatternMultiTopicsConsumerImpl.h | 1 - lib/ReaderImpl.cc | 1 - tests/LookupServiceTest.cc | 17 +++--------- 11 files changed, 69 insertions(+), 65 deletions(-) diff --git a/lib/Client.cc b/lib/Client.cc index 311d2ddd..ba829b2b 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -193,8 +193,7 @@ uint64_t Client::getNumberOfConsumers() { return impl_->getNumberOfConsumers(); void Client::getSchemaInfoAsync(const std::string& topic, int64_t version, std::function callback) { - impl_->getLookup() - ->getSchema(TopicName::get(topic), (version >= 0) ? toBigEndianBytes(version) : "") + impl_->getSchema(TopicName::get(topic), (version >= 0) ? toBigEndianBytes(version) : "") .addListener(std::move(callback)); } diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 5b71a696..02f017b0 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -200,8 +200,7 @@ void ClientImpl::createProducerAsync(const std::string& topic, const ProducerCon if (autoDownloadSchema) { auto self = shared_from_this(); - auto lookup = getLookup(); - lookup->getSchema(topicName).addListener( + getSchema(topicName).addListener( [self, topicName, callback](Result res, const SchemaInfo& topicSchema) { if (res != ResultOk) { callback(res, Producer()); @@ -209,12 +208,12 @@ void ClientImpl::createProducerAsync(const std::string& topic, const ProducerCon } ProducerConfiguration conf; conf.setSchema(topicSchema); - self->getLookup()->getPartitionMetadataAsync(topicName).addListener( + self->getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleCreateProducer, self, std::placeholders::_1, std::placeholders::_2, topicName, conf, callback)); }); } else { - getLookup()->getPartitionMetadataAsync(topicName).addListener( + getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleCreateProducer, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, conf, callback)); } @@ -287,7 +286,7 @@ void ClientImpl::createReaderAsync(const std::string& topic, const MessageId& st } MessageId msgId(startMessageId); - getLookup()->getPartitionMetadataAsync(topicName).addListener( + getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleReaderMetadataLookup, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, msgId, conf, callback)); } @@ -400,8 +399,7 @@ void ClientImpl::subscribeWithRegexAsync(const std::string& regexPattern, const return; } - getLookup() - ->getTopicsOfNamespaceAsync(topicNamePtr->getNamespaceName(), mode) + getTopicsOfNamespaceAsync(topicNamePtr->getNamespaceName(), mode) .addListener(std::bind(&ClientImpl::createPatternMultiTopicsConsumer, shared_from_this(), std::placeholders::_1, std::placeholders::_2, regexPattern, mode, subscriptionName, conf, callback)); @@ -423,9 +421,8 @@ void ClientImpl::createPatternMultiTopicsConsumer(Result result, const Namespace auto interceptors = std::make_shared(conf.getInterceptors()); - consumer = std::make_shared(shared_from_this(), regexPattern, mode, - *matchTopics, subscriptionName, conf, - getLookup(), interceptors); + consumer = std::make_shared( + shared_from_this(), regexPattern, mode, *matchTopics, subscriptionName, conf, interceptors); consumer->getConsumerCreatedFuture().addListener( std::bind(&ClientImpl::handleConsumerCreated, shared_from_this(), std::placeholders::_1, @@ -472,7 +469,7 @@ void ClientImpl::subscribeAsync(const std::vector& originalTopics, auto interceptors = std::make_shared(conf.getInterceptors()); ConsumerImplBasePtr consumer = std::make_shared( - shared_from_this(), topics, subscriptionName, topicNamePtr, conf, getLookup(), interceptors); + shared_from_this(), topics, subscriptionName, topicNamePtr, conf, interceptors); consumer->getConsumerCreatedFuture().addListener(std::bind(&ClientImpl::handleConsumerCreated, shared_from_this(), std::placeholders::_1, @@ -502,7 +499,7 @@ void ClientImpl::subscribeAsync(const std::string& topic, const std::string& sub } } - getLookup()->getPartitionMetadataAsync(topicName).addListener( + getPartitionMetadataAsync(topicName).addListener( std::bind(&ClientImpl::handleSubscribe, shared_from_this(), std::placeholders::_1, std::placeholders::_2, topicName, subscriptionName, conf, callback)); } @@ -525,9 +522,9 @@ void ClientImpl::handleSubscribe(Result result, const LookupDataResultPtr& parti callback(ResultInvalidConfiguration, Consumer()); return; } - consumer = std::make_shared( - shared_from_this(), topicName, partitionMetadata->getPartitions(), subscriptionName, conf, - getLookup(), interceptors); + consumer = std::make_shared(shared_from_this(), topicName, + partitionMetadata->getPartitions(), + subscriptionName, conf, interceptors); } else { auto consumerImpl = std::make_shared(shared_from_this(), topicName->toString(), subscriptionName, conf, @@ -680,9 +677,9 @@ void ClientImpl::getPartitionsForTopicAsync(const std::string& topic, const GetP return; } } - getLookup()->getPartitionMetadataAsync(topicName).addListener( - std::bind(&ClientImpl::handleGetPartitions, shared_from_this(), std::placeholders::_1, - std::placeholders::_2, topicName, callback)); + getPartitionMetadataAsync(topicName).addListener(std::bind(&ClientImpl::handleGetPartitions, + shared_from_this(), std::placeholders::_1, + std::placeholders::_2, topicName, callback)); } void ClientImpl::closeAsync(const CloseCallback& callback) { @@ -802,7 +799,9 @@ void ClientImpl::shutdown() { << " consumers have been shutdown."); } - getLookup()->close(); + std::shared_lock lock(mutex_); + lookupServicePtr_->close(); + lock.unlock(); if (!pool_.close()) { // pool_ has already been closed. It means shutdown() has been called before. return; diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 11881a2f..2a6a8298 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -29,6 +29,7 @@ #include "ConnectionPool.h" #include "Future.h" #include "LookupDataResult.h" +#include "LookupService.h" #include "MemoryLimitController.h" #include "ProtoApiEnums.h" #include "SynchronizedHashMap.h" @@ -53,8 +54,6 @@ typedef std::weak_ptr ConsumerImplBaseWeakPtr; class ClientConnection; using ClientConnectionPtr = std::shared_ptr; -class LookupService; -using LookupServicePtr = std::shared_ptr; using LookupServiceFactory = std::function; @@ -129,7 +128,6 @@ class ClientImpl : public std::enable_shared_from_this { ExecutorServiceProviderPtr getIOExecutorProvider(); ExecutorServiceProviderPtr getListenerExecutorProvider(); ExecutorServiceProviderPtr getPartitionListenerExecutorProvider(); - LookupServicePtr getLookup(const std::string& redirectedClusterURI = ""); void cleanupProducer(ProducerImplBase* address) { producers_.remove(address); } @@ -143,6 +141,23 @@ class ClientImpl : public std::enable_shared_from_this { void updateServiceInfo(ServiceInfo&& serviceInfo); ServiceInfo getServiceInfo() const; + // Since the underlying `lookupServicePtr_` can be modified by `updateServiceInfo`, we should not expose + // it to other classes, otherwise the update might not be visible. + auto getPartitionMetadataAsync(const TopicNamePtr& topicName) { + std::shared_lock lock(mutex_); + return lookupServicePtr_->getPartitionMetadataAsync(topicName); + } + + auto getTopicsOfNamespaceAsync(const NamespaceNamePtr& nsName, CommandGetTopicsOfNamespace_Mode mode) { + std::shared_lock lock(mutex_); + return lookupServicePtr_->getTopicsOfNamespaceAsync(nsName, mode); + } + + auto getSchema(const TopicNamePtr& topicName, const std::string& version = "") { + std::shared_lock lock(mutex_); + return lookupServicePtr_->getSchema(topicName, version); + } + static std::chrono::nanoseconds getOperationTimeout(const ClientConfiguration& clientConfiguration); friend class PulsarFriend; @@ -182,6 +197,7 @@ class ClientImpl : public std::enable_shared_from_this { const std::string& logicalAddress); LookupServicePtr createLookup(const std::string& serviceUrl); + LookupServicePtr getLookup(const std::string& redirectedClusterURI); static std::string getClientVersion(const ClientConfiguration& clientConfiguration); diff --git a/lib/MultiTopicsConsumerImpl.cc b/lib/MultiTopicsConsumerImpl.cc index 0799eb63..a5699781 100644 --- a/lib/MultiTopicsConsumerImpl.cc +++ b/lib/MultiTopicsConsumerImpl.cc @@ -44,20 +44,19 @@ using std::chrono::seconds; MultiTopicsConsumerImpl::MultiTopicsConsumerImpl(const ClientImplPtr& client, const TopicNamePtr& topicName, int numPartitions, const std::string& subscriptionName, const ConsumerConfiguration& conf, - const LookupServicePtr& lookupServicePtr, const ConsumerInterceptorsPtr& interceptors, Commands::SubscriptionMode subscriptionMode, const optional& startMessageId) : MultiTopicsConsumerImpl(client, {topicName->toString()}, subscriptionName, topicName, conf, - lookupServicePtr, interceptors, subscriptionMode, startMessageId) { + interceptors, subscriptionMode, startMessageId) { topicsPartitions_[topicName->toString()] = numPartitions; } MultiTopicsConsumerImpl::MultiTopicsConsumerImpl( const ClientImplPtr& client, const std::vector& topics, const std::string& subscriptionName, const TopicNamePtr& topicName, const ConsumerConfiguration& conf, - const LookupServicePtr& lookupServicePtr, const ConsumerInterceptorsPtr& interceptors, - Commands::SubscriptionMode subscriptionMode, const optional& startMessageId) + const ConsumerInterceptorsPtr& interceptors, Commands::SubscriptionMode subscriptionMode, + const optional& startMessageId) : ConsumerImplBase(client, topicName ? topicName->toString() : "EmptyTopics", Backoff(milliseconds(100), seconds(60), milliseconds(0)), conf, client->getListenerExecutorProvider()->get()), @@ -66,7 +65,6 @@ MultiTopicsConsumerImpl::MultiTopicsConsumerImpl( conf_(conf), incomingMessages_(conf.getReceiverQueueSize()), messageListener_(conf.getMessageListener()), - lookupServicePtr_(lookupServicePtr), numberTopicPartitions_(std::make_shared>(0)), topics_(topics), subscriptionMode_(subscriptionMode), @@ -93,7 +91,6 @@ MultiTopicsConsumerImpl::MultiTopicsConsumerImpl( if (partitionsUpdateInterval > 0) { partitionsUpdateTimer_ = listenerExecutor_->createDeadlineTimer(); partitionsUpdateInterval_ = seconds(partitionsUpdateInterval); - lookupServicePtr_ = client->getLookup(); } state_ = Pending; @@ -185,7 +182,12 @@ Future MultiTopicsConsumerImpl::subscribeOneTopicAsync(const s auto entry = topicsPartitions_.find(topic); if (entry == topicsPartitions_.end()) { lock.unlock(); - lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + auto client = client_.lock(); + if (!client) { + topicPromise->setFailed(ResultAlreadyClosed); + return topicPromise->getFuture(); + } + client->getPartitionMetadataAsync(topicName).addListener( [this, topicName, topicPromise](Result result, const LookupDataResultPtr& lookupDataResult) { if (result != ResultOk) { LOG_ERROR("Error Checking/Getting Partition Metadata while MultiTopics Subscribing- " @@ -1003,7 +1005,11 @@ void MultiTopicsConsumerImpl::topicPartitionUpdate() { auto topicName = TopicName::get(item.first); auto currentNumPartitions = item.second; auto weakSelf = weak_from_this(); - lookupServicePtr_->getPartitionMetadataAsync(topicName).addListener( + auto client = client_.lock(); + if (!client) { + return; + } + client->getPartitionMetadataAsync(topicName).addListener( [this, weakSelf, topicName, currentNumPartitions](Result result, const LookupDataResultPtr& lookupDataResult) { auto self = weakSelf.lock(); diff --git a/lib/MultiTopicsConsumerImpl.h b/lib/MultiTopicsConsumerImpl.h index dc628652..38a44cdf 100644 --- a/lib/MultiTopicsConsumerImpl.h +++ b/lib/MultiTopicsConsumerImpl.h @@ -46,23 +46,19 @@ class MultiTopicsBrokerConsumerStatsImpl; using MultiTopicsBrokerConsumerStatsPtr = std::shared_ptr; class UnAckedMessageTrackerInterface; using UnAckedMessageTrackerPtr = std::shared_ptr; -class LookupService; -using LookupServicePtr = std::shared_ptr; class MultiTopicsConsumerImpl; class MultiTopicsConsumerImpl : public ConsumerImplBase { public: MultiTopicsConsumerImpl(const ClientImplPtr& client, const TopicNamePtr& topicName, int numPartitions, const std::string& subscriptionName, const ConsumerConfiguration& conf, - const LookupServicePtr& lookupServicePtr, const ConsumerInterceptorsPtr& interceptors, Commands::SubscriptionMode = Commands::SubscriptionModeDurable, const optional& startMessageId = optional{}); MultiTopicsConsumerImpl(const ClientImplPtr& client, const std::vector& topics, const std::string& subscriptionName, const TopicNamePtr& topicName, - const ConsumerConfiguration& conf, const LookupServicePtr& lookupServicePtr_, - const ConsumerInterceptorsPtr& interceptors, + const ConsumerConfiguration& conf, const ConsumerInterceptorsPtr& interceptors, Commands::SubscriptionMode = Commands::SubscriptionModeDurable, const optional& startMessageId = optional{}); @@ -119,7 +115,6 @@ class MultiTopicsConsumerImpl : public ConsumerImplBase { MessageListener messageListener_; DeadlineTimerPtr partitionsUpdateTimer_; TimeDuration partitionsUpdateInterval_; - LookupServicePtr lookupServicePtr_; std::shared_ptr> numberTopicPartitions_; std::atomic failedResult{ResultOk}; Promise multiTopicsConsumerCreatedPromise_; diff --git a/lib/PartitionedProducerImpl.cc b/lib/PartitionedProducerImpl.cc index 4a923666..1aa5c87b 100644 --- a/lib/PartitionedProducerImpl.cc +++ b/lib/PartitionedProducerImpl.cc @@ -23,7 +23,6 @@ #include "ClientImpl.h" #include "ExecutorService.h" #include "LogUtils.h" -#include "LookupService.h" #include "ProducerImpl.h" #include "RoundRobinMessageRouter.h" #include "SinglePartitionMessageRouter.h" @@ -59,7 +58,6 @@ PartitionedProducerImpl::PartitionedProducerImpl(const ClientImplPtr& client, co listenerExecutor_ = client->getListenerExecutorProvider()->get(); partitionsUpdateTimer_ = listenerExecutor_->createDeadlineTimer(); partitionsUpdateInterval_ = std::chrono::seconds(partitionsUpdateInterval); - lookupServicePtr_ = client->getLookup(); } } @@ -433,7 +431,11 @@ void PartitionedProducerImpl::runPartitionUpdateTask() { void PartitionedProducerImpl::getPartitionMetadata() { using namespace std::placeholders; auto weakSelf = weak_from_this(); - lookupServicePtr_->getPartitionMetadataAsync(topicName_) + auto client = client_.lock(); + if (!client) { + return; + } + client->getPartitionMetadataAsync(topicName_) .addListener([weakSelf](Result result, const LookupDataResultPtr& lookupDataResult) { auto self = weakSelf.lock(); if (self) { diff --git a/lib/PartitionedProducerImpl.h b/lib/PartitionedProducerImpl.h index 40f2d34d..94ba7179 100644 --- a/lib/PartitionedProducerImpl.h +++ b/lib/PartitionedProducerImpl.h @@ -38,8 +38,6 @@ using ClientImplPtr = std::shared_ptr; using ClientImplWeakPtr = std::weak_ptr; class ExecutorService; using ExecutorServicePtr = std::shared_ptr; -class LookupService; -using LookupServicePtr = std::shared_ptr; class ProducerImpl; using ProducerImplPtr = std::shared_ptr; class TopicName; @@ -133,7 +131,6 @@ class PartitionedProducerImpl : public ProducerImplBase, ExecutorServicePtr listenerExecutor_; DeadlineTimerPtr partitionsUpdateTimer_; TimeDuration partitionsUpdateInterval_; - LookupServicePtr lookupServicePtr_; ProducerInterceptorsPtr interceptors_; diff --git a/lib/PatternMultiTopicsConsumerImpl.cc b/lib/PatternMultiTopicsConsumerImpl.cc index fd48feed..4b5aab73 100644 --- a/lib/PatternMultiTopicsConsumerImpl.cc +++ b/lib/PatternMultiTopicsConsumerImpl.cc @@ -21,7 +21,6 @@ #include "ClientImpl.h" #include "ExecutorService.h" #include "LogUtils.h" -#include "LookupService.h" DECLARE_LOG_OBJECT() @@ -32,10 +31,8 @@ using std::chrono::seconds; PatternMultiTopicsConsumerImpl::PatternMultiTopicsConsumerImpl( const ClientImplPtr& client, const std::string& pattern, CommandGetTopicsOfNamespace_Mode getTopicsMode, const std::vector& topics, const std::string& subscriptionName, - const ConsumerConfiguration& conf, const LookupServicePtr& lookupServicePtr_, - const ConsumerInterceptorsPtr& interceptors) - : MultiTopicsConsumerImpl(client, topics, subscriptionName, TopicName::get(pattern), conf, - lookupServicePtr_, interceptors), + const ConsumerConfiguration& conf, const ConsumerInterceptorsPtr& interceptors) + : MultiTopicsConsumerImpl(client, topics, subscriptionName, TopicName::get(pattern), conf, interceptors), patternString_(pattern), pattern_(PULSAR_REGEX_NAMESPACE::regex(TopicName::removeDomain(pattern))), getTopicsMode_(getTopicsMode), @@ -84,7 +81,11 @@ void PatternMultiTopicsConsumerImpl::autoDiscoveryTimerTask(const ASIO_ERROR& er // already get namespace from pattern. assert(namespaceName_); - lookupServicePtr_->getTopicsOfNamespaceAsync(namespaceName_, getTopicsMode_) + auto client = client_.lock(); + if (!client) { + return; + } + client->getTopicsOfNamespaceAsync(namespaceName_, getTopicsMode_) .addListener(std::bind(&PatternMultiTopicsConsumerImpl::timerGetTopicsOfNamespace, this, std::placeholders::_1, std::placeholders::_2)); } diff --git a/lib/PatternMultiTopicsConsumerImpl.h b/lib/PatternMultiTopicsConsumerImpl.h index 63527965..796abcc2 100644 --- a/lib/PatternMultiTopicsConsumerImpl.h +++ b/lib/PatternMultiTopicsConsumerImpl.h @@ -52,7 +52,6 @@ class PatternMultiTopicsConsumerImpl : public MultiTopicsConsumerImpl { CommandGetTopicsOfNamespace_Mode getTopicsMode, const std::vector& topics, const std::string& subscriptionName, const ConsumerConfiguration& conf, - const LookupServicePtr& lookupServicePtr_, const ConsumerInterceptorsPtr& interceptors); ~PatternMultiTopicsConsumerImpl() override; diff --git a/lib/ReaderImpl.cc b/lib/ReaderImpl.cc index 7fa7e8b9..754137c5 100644 --- a/lib/ReaderImpl.cc +++ b/lib/ReaderImpl.cc @@ -90,7 +90,6 @@ void ReaderImpl::start(const MessageId& startMessageId, if (partitions_ > 0) { auto consumerImpl = std::make_shared( client_.lock(), TopicName::get(topic_), partitions_, subscription, consumerConf, - client_.lock()->getLookup(), std::make_shared(std::vector()), Commands::SubscriptionModeNonDurable, startMessageId); consumer_ = consumerImpl; diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc index 92aa8204..dc9cfc4e 100644 --- a/tests/LookupServiceTest.cc +++ b/tests/LookupServiceTest.cc @@ -259,7 +259,7 @@ TEST_P(LookupServiceTest, basicGetNamespaceTopics) { ASSERT_EQ(ResultOk, result); // 2. verify getTopicsOfNamespace by regex mode. - auto lookupServicePtr = PulsarFriend::getClientImplPtr(client_)->getLookup(); + auto lookupServicePtr = PulsarFriend::getClientImplPtr(client_); auto verifyGetTopics = [&](CommandGetTopicsOfNamespace_Mode mode, const std::set& expectedTopics) { Future getTopicsFuture = @@ -292,11 +292,8 @@ TEST_P(LookupServiceTest, testGetSchema) { Producer producer; ASSERT_EQ(ResultOk, client_.createProducer(topic, producerConfiguration, producer)); - auto clientImplPtr = PulsarFriend::getClientImplPtr(client_); - auto lookup = clientImplPtr->getLookup(); - SchemaInfo schemaInfo; - auto future = lookup->getSchema(TopicName::get(topic)); + auto future = PulsarFriend::getClientImplPtr(client_)->getSchema(TopicName::get(topic)); ASSERT_EQ(ResultOk, future.get(schemaInfo)); ASSERT_EQ(jsonSchema, schemaInfo.getSchema()); ASSERT_EQ(SchemaType::JSON, schemaInfo.getSchemaType()); @@ -310,11 +307,8 @@ TEST_P(LookupServiceTest, testGetSchemaNotFound) { Producer producer; ASSERT_EQ(ResultOk, client_.createProducer(topic, producer)); - auto clientImplPtr = PulsarFriend::getClientImplPtr(client_); - auto lookup = clientImplPtr->getLookup(); - SchemaInfo schemaInfo; - auto future = lookup->getSchema(TopicName::get(topic)); + auto future = PulsarFriend::getClientImplPtr(client_)->getSchema(TopicName::get(topic)); ASSERT_EQ(ResultTopicNotFound, future.get(schemaInfo)); } @@ -335,11 +329,8 @@ TEST_P(LookupServiceTest, testGetKeyValueSchema) { Producer producer; ASSERT_EQ(ResultOk, client_.createProducer(topic, producerConfiguration, producer)); - auto clientImplPtr = PulsarFriend::getClientImplPtr(client_); - auto lookup = clientImplPtr->getLookup(); - SchemaInfo schemaInfo; - auto future = lookup->getSchema(TopicName::get(topic)); + auto future = PulsarFriend::getClientImplPtr(client_)->getSchema(TopicName::get(topic)); ASSERT_EQ(ResultOk, future.get(schemaInfo)); ASSERT_EQ(keyValueSchema.getSchema(), schemaInfo.getSchema()); ASSERT_EQ(SchemaType::KEY_VALUE, schemaInfo.getSchemaType()); From c6de067e3795b58eed558c03795e385fc8ee16a5 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 21:02:09 +0800 Subject: [PATCH 10/28] fix the reader cannot consume after switching to a new cluster --- lib/ClientConnection.cc | 5 ++++- lib/ClientConnection.h | 3 ++- lib/ClientImpl.cc | 2 +- lib/ConnectionPool.cc | 6 +++--- lib/ConnectionPool.h | 2 +- lib/ConsumerImpl.cc | 13 ++++++++++++- lib/ConsumerImpl.h | 7 +++++++ tests/ClientTest.cc | 25 ++++++++++++++++++++----- 8 files changed, 50 insertions(+), 13 deletions(-) diff --git a/lib/ClientConnection.cc b/lib/ClientConnection.cc index 0a850ed0..41d2f608 100644 --- a/lib/ClientConnection.cc +++ b/lib/ClientConnection.cc @@ -1292,7 +1292,7 @@ void ClientConnection::handleConsumerStatsTimeout(const ASIO_ERROR& ec, startConsumerStatsTimer(consumerStatsRequests); } -void ClientConnection::close(Result result, bool detach) { +void ClientConnection::close(Result result, bool detach, bool switchCluster) { Lock lock(mutex_); if (isClosed()) { return; @@ -1368,6 +1368,9 @@ void ClientConnection::close(Result result, bool detach) { for (ConsumersMap::iterator it = consumers.begin(); it != consumers.end(); ++it) { auto consumer = it->second.lock(); if (consumer) { + if (switchCluster) { + consumer->onClusterSwitching(); + } consumer->handleDisconnection(result, self); } } diff --git a/lib/ClientConnection.h b/lib/ClientConnection.h index b2770006..39e4224d 100644 --- a/lib/ClientConnection.h +++ b/lib/ClientConnection.h @@ -157,10 +157,11 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_thisclose(); for (auto&& it : redirectedClusterLookupServicePtrs_) { diff --git a/lib/ConnectionPool.cc b/lib/ConnectionPool.cc index d94d8fe8..eaa14dc7 100644 --- a/lib/ConnectionPool.cc +++ b/lib/ConnectionPool.cc @@ -67,8 +67,8 @@ bool ConnectionPool::close() { return true; } -void ConnectionPool::resetConnections(const AuthenticationPtr& authentication, - const ClientConfiguration& conf) { +void ConnectionPool::resetForClusterSwitching(const AuthenticationPtr& authentication, + const ClientConfiguration& conf) { std::unique_lock lock(mutex_); authentication_ = authentication; clientConfiguration_ = conf; @@ -76,7 +76,7 @@ void ConnectionPool::resetConnections(const AuthenticationPtr& authentication, for (auto cnxIt = pool_.begin(); cnxIt != pool_.end(); cnxIt++) { auto& cnx = cnxIt->second; if (cnx) { - cnx->close(ResultDisconnected, false); + cnx->close(ResultDisconnected, false, true); } } pool_.clear(); diff --git a/lib/ConnectionPool.h b/lib/ConnectionPool.h index 64907b4f..541cf8ed 100644 --- a/lib/ConnectionPool.h +++ b/lib/ConnectionPool.h @@ -55,7 +55,7 @@ class PULSAR_PUBLIC ConnectionPool { * Close all existing connections and update the authentication and configuration. * Unlike close(), the pool remains open for new connections. */ - void resetConnections(const AuthenticationPtr& authentication, const ClientConfiguration& conf); + void resetForClusterSwitching(const AuthenticationPtr& authentication, const ClientConfiguration& conf); void remove(const std::string& logicalAddress, const std::string& physicalAddress, size_t keySuffix, ClientConnection* value); diff --git a/lib/ConsumerImpl.cc b/lib/ConsumerImpl.cc index 757b6e84..785a8bd7 100644 --- a/lib/ConsumerImpl.cc +++ b/lib/ConsumerImpl.cc @@ -125,7 +125,8 @@ ConsumerImpl::ConsumerImpl(const ClientImplPtr& client, const std::string& topic negativeAcksTracker_(std::make_shared(client, *this, conf)), ackGroupingTrackerPtr_(newAckGroupingTracker(topic, conf, client)), readCompacted_(conf.isReadCompacted()), - startMessageId_(pulsar::getStartMessageId(startMessageId, conf.isStartMessageIdInclusive())), + startMessageIdFromConfig_(pulsar::getStartMessageId(startMessageId, conf.isStartMessageIdInclusive())), + startMessageId_(startMessageIdFromConfig_), maxPendingChunkedMessage_(conf.getMaxPendingChunkedMessage()), autoAckOldestChunkedMessageOnQueueFull_(conf.isAutoAckOldestChunkedMessageOnQueueFull()), expireTimeOfIncompleteChunkedMessageMs_(conf.getExpireTimeOfIncompleteChunkedMessageMs()), @@ -1134,6 +1135,16 @@ void ConsumerImpl::messageProcessed(Message& msg, bool track) { } } +void ConsumerImpl::onClusterSwitching() { + { + LockGuard lock{mutex_}; + incomingMessages_.clear(); + startMessageId_ = startMessageIdFromConfig_; + lastDequedMessageId_ = MessageId::earliest(); + } + ackGroupingTrackerPtr_->flushAndClean(); +} + /** * Clear the internal receiver queue and returns the message id of what was the 1st message in the queue that * was diff --git a/lib/ConsumerImpl.h b/lib/ConsumerImpl.h index 0da82a2d..6f287aa2 100644 --- a/lib/ConsumerImpl.h +++ b/lib/ConsumerImpl.h @@ -162,6 +162,8 @@ class ConsumerImpl : public ConsumerImplBase { void doImmediateAck(const MessageId& msgId, const ResultCallback& callback, CommandAck_AckType ackType); void doImmediateAck(const std::set& msgIds, const ResultCallback& callback); + void onClusterSwitching(); + protected: // overrided methods from HandlerBase Future connectionOpened(const ClientConnectionPtr& cnx) override; @@ -266,6 +268,11 @@ class ConsumerImpl : public ConsumerImplBase { MessageId lastDequedMessageId_{MessageId::earliest()}; MessageId lastMessageIdInBroker_{MessageId::earliest()}; + + // When the consumer switches to a new cluster, we should reset `startMessageId_` to the original value, + // otherwise, the message id of the old cluster might be passed in the Subscribe request on the new + // cluster. + const optional startMessageIdFromConfig_; optional startMessageId_; SeekStatus seekStatus_{SeekStatus::NOT_STARTED}; diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 31d0d93d..93ec3a11 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -522,11 +522,26 @@ TEST(ClientTest, testUpdateServiceInfo) { ServiceInfo info3{"pulsar://localhost:6650", std::nullopt, std::nullopt}; Client client{info1.serviceUrl, ClientConfiguration().setAuth(*info1.authentication)}; + const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); Producer producer; ASSERT_EQ(ResultOk, client.createProducer(topicRequiredAuth, producer)); - MessageId msgId; - ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-0").build(), msgId)); + + Reader reader; + ASSERT_EQ(ResultOk, client.createReader(topicRequiredAuth, MessageId::earliest(), {}, reader)); + + auto sendAndReceive = [&](const std::string &value) { + MessageId msgId; + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent(value).build(), msgId)); + LOG_INFO("Sent " << value << " to " << msgId); + + Message msg; + ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); + LOG_INFO("Read " << msg.getDataAsString() << " from " << msgId); + ASSERT_EQ(value, msg.getDataAsString()); + }; + + sendAndReceive("msg-0"); // Switch to cluster 2 (started by ./build-support/start-mim-test-service-inside-container.sh) ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); @@ -537,8 +552,7 @@ TEST(ClientTest, testUpdateServiceInfo) { ASSERT_EQ(info2.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); // Now the same will access the same topic in cluster 2 - ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-1").build(), msgId)); - ASSERT_EQ(ResultOk, producer.close()); + sendAndReceive("msg-1"); // Switch back to cluster 1 without any authentication, the previous authentication info configured for // cluster 2 will be cleared. @@ -548,8 +562,9 @@ TEST(ClientTest, testUpdateServiceInfo) { ASSERT_EQ(info3.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); + producer.close(); ASSERT_EQ(ResultOk, client.createProducer(topicNoAuth, producer)); - ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-2").build(), msgId)); + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-2").build())); client.close(); From a209112d912af546a0ee582be6e073febd1549df Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 23:03:18 +0800 Subject: [PATCH 11/28] address comments from copilot --- include/pulsar/Client.h | 8 +---- include/pulsar/ServiceInfo.h | 36 +++++++++++++++++++++ lib/ClientConfiguration.cc | 14 ++++----- lib/ClientConfigurationImpl.h | 59 +++++++++++++++++++++++++++++++++-- lib/ClientImpl.cc | 20 +++++------- lib/ConsumerImpl.cc | 3 ++ tests/ClientTest.cc | 2 +- 7 files changed, 112 insertions(+), 30 deletions(-) create mode 100644 include/pulsar/ServiceInfo.h diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index 15dd0324..7e2d10d0 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -29,10 +29,10 @@ #include #include #include +#include #include #include -#include #include namespace pulsar { @@ -43,12 +43,6 @@ typedef std::function TableViewCallback; typedef std::function&)> GetPartitionsCallback; typedef std::function CloseCallback; -struct PULSAR_PUBLIC ServiceInfo { - std::string serviceUrl; - std::optional authentication; - std::optional tlsTrustCertsFilePath; -}; - class ClientImpl; class PulsarFriend; class PulsarWrapper; diff --git a/include/pulsar/ServiceInfo.h b/include/pulsar/ServiceInfo.h new file mode 100644 index 00000000..a0f437bc --- /dev/null +++ b/include/pulsar/ServiceInfo.h @@ -0,0 +1,36 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#ifndef PULSAR_SERVICE_INFO_H_ +#define PULSAR_SERVICE_INFO_H_ + +#include + +#include +#include + +namespace pulsar { + +struct PULSAR_PUBLIC ServiceInfo { + std::string serviceUrl; + std::optional authentication; + std::optional tlsTrustCertsFilePath; +}; + +} // namespace pulsar +#endif diff --git a/lib/ClientConfiguration.cc b/lib/ClientConfiguration.cc index b99c5d25..c3c56b95 100644 --- a/lib/ClientConfiguration.cc +++ b/lib/ClientConfiguration.cc @@ -53,13 +53,13 @@ ClientConfiguration& ClientConfiguration::setConnectionsPerBroker(int connection int ClientConfiguration::getConnectionsPerBroker() const { return impl_->connectionsPerBroker; } ClientConfiguration& ClientConfiguration::setAuth(const AuthenticationPtr& authentication) { - impl_->authenticationPtr = authentication; + impl_->setAuthentication(authentication); return *this; } -Authentication& ClientConfiguration::getAuth() const { return *impl_->authenticationPtr; } +Authentication& ClientConfiguration::getAuth() const { return *impl_->getAuthentication(); } -const AuthenticationPtr& ClientConfiguration::getAuthPtr() const { return impl_->authenticationPtr; } +const AuthenticationPtr& ClientConfiguration::getAuthPtr() const { return impl_->getAuthentication(); } ClientConfiguration& ClientConfiguration::setOperationTimeoutSeconds(int timeout) { impl_->operationTimeout = std::chrono::seconds(timeout); @@ -95,11 +95,11 @@ ClientConfiguration& ClientConfiguration::setMessageListenerThreads(int threads) int ClientConfiguration::getMessageListenerThreads() const { return impl_->messageListenerThreads; } ClientConfiguration& ClientConfiguration::setUseTls(bool useTls) { - impl_->useTls = useTls; + impl_->setUseTls(useTls); return *this; } -bool ClientConfiguration::isUseTls() const { return impl_->useTls; } +bool ClientConfiguration::isUseTls() const { return impl_->isUseTls(); } ClientConfiguration& ClientConfiguration::setValidateHostName(bool validateHostName) { impl_->validateHostName = validateHostName; @@ -127,12 +127,12 @@ const std::string& ClientConfiguration::getTlsCertificateFilePath() const { } ClientConfiguration& ClientConfiguration::setTlsTrustCertsFilePath(const std::string& filePath) { - impl_->tlsTrustCertsFilePath = filePath; + impl_->setTlsTrustCertsFilePath(filePath); return *this; } const std::string& ClientConfiguration::getTlsTrustCertsFilePath() const { - return impl_->tlsTrustCertsFilePath; + return impl_->getTlsTrustCertsFilePath(); } ClientConfiguration& ClientConfiguration::setTlsAllowInsecureConnection(bool allowInsecure) { diff --git a/lib/ClientConfigurationImpl.h b/lib/ClientConfigurationImpl.h index e7a83a19..6207f989 100644 --- a/lib/ClientConfigurationImpl.h +++ b/lib/ClientConfigurationImpl.h @@ -20,13 +20,70 @@ #define LIB_CLIENTCONFIGURATIONIMPL_H_ #include +#include #include +#include + +#include "ServiceNameResolver.h" +#include "ServiceURI.h" namespace pulsar { +// Use struct rather than class here just for ABI compatibility struct ClientConfigurationImpl { + private: + mutable std::shared_mutex mutex; AuthenticationPtr authenticationPtr{AuthFactory::Disabled()}; + std::string tlsTrustCertsFilePath; + bool useTls{false}; + + public: + void updateServiceInfo(const ServiceInfo& serviceInfo) { + std::unique_lock lock(mutex); + if (serviceInfo.authentication.has_value() && *serviceInfo.authentication) { + authenticationPtr = *serviceInfo.authentication; + } else { + authenticationPtr = AuthFactory::Disabled(); + } + if (serviceInfo.tlsTrustCertsFilePath.has_value()) { + tlsTrustCertsFilePath = *serviceInfo.tlsTrustCertsFilePath; + } else { + tlsTrustCertsFilePath = ""; + } + useTls = ServiceNameResolver::useTls(ServiceURI(serviceInfo.serviceUrl)); + } + + auto& getAuthentication() const { + std::shared_lock lock(mutex); + return authenticationPtr; + } + + auto setAuthentication(const AuthenticationPtr& authentication) { + std::unique_lock lock(mutex); + authenticationPtr = authentication; + } + + auto& getTlsTrustCertsFilePath() const { + std::shared_lock lock(mutex); + return tlsTrustCertsFilePath; + } + + auto setTlsTrustCertsFilePath(const std::string& path) { + std::unique_lock lock(mutex); + tlsTrustCertsFilePath = path; + } + + auto isUseTls() const { + std::shared_lock lock(mutex); + return useTls; + } + + auto setUseTls(bool useTls_) { + std::unique_lock lock(mutex); + useTls = useTls_; + } + uint64_t memoryLimit{0ull}; int ioThreads{1}; int connectionsPerBroker{1}; @@ -36,10 +93,8 @@ struct ClientConfigurationImpl { int maxLookupRedirects{20}; int initialBackoffIntervalMs{100}; int maxBackoffIntervalMs{60000}; - bool useTls{false}; std::string tlsPrivateKeyFilePath; std::string tlsCertificateFilePath; - std::string tlsTrustCertsFilePath; bool tlsAllowInsecureConnection{false}; unsigned int statsIntervalInSeconds{600}; // 10 minutes std::unique_ptr loggerFactory; diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 385a9627..91edf43d 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -874,27 +874,17 @@ std::chrono::nanoseconds ClientImpl::getOperationTimeout(const ClientConfigurati } void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { - std::unique_lock lock(mutex_); + std::unique_lock lock{mutex_}; if (state_ != Open) { - LOG_ERROR("Client is not open, cannot update connection info"); + LOG_ERROR("Client is not open, cannot update service info"); return; } - if (serviceInfo.authentication.has_value() && *serviceInfo.authentication) { - clientConfiguration_.setAuth(*serviceInfo.authentication); - } else { - clientConfiguration_.setAuth(AuthFactory::Disabled()); - } - if (serviceInfo.tlsTrustCertsFilePath.has_value()) { - clientConfiguration_.setTlsTrustCertsFilePath(*serviceInfo.tlsTrustCertsFilePath); - } else { - clientConfiguration_.setTlsTrustCertsFilePath(""); - } - clientConfiguration_.setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceInfo.serviceUrl))); serviceInfo_ = {serviceInfo.serviceUrl, toOptionalAuthentication(clientConfiguration_.getAuthPtr()), clientConfiguration_.getTlsTrustCertsFilePath().empty() ? std::nullopt : std::make_optional(clientConfiguration_.getTlsTrustCertsFilePath())}; + clientConfiguration_.impl_->updateServiceInfo(serviceInfo_); pool_.resetForClusterSwitching(clientConfiguration_.getAuthPtr(), clientConfiguration_); @@ -904,6 +894,10 @@ void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { } redirectedClusterLookupServicePtrs_.clear(); lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); + + // TODO: changes on the following two fields are not tested + useProxy_ = false; + lookupCount_ = 0; } ServiceInfo ClientImpl::getServiceInfo() const { diff --git a/lib/ConsumerImpl.cc b/lib/ConsumerImpl.cc index 785a8bd7..71cf1d01 100644 --- a/lib/ConsumerImpl.cc +++ b/lib/ConsumerImpl.cc @@ -1141,6 +1141,9 @@ void ConsumerImpl::onClusterSwitching() { incomingMessages_.clear(); startMessageId_ = startMessageIdFromConfig_; lastDequedMessageId_ = MessageId::earliest(); + lastMessageIdInBroker_ = MessageId::earliest(); + seekStatus_ = SeekStatus::NOT_STARTED; + lastSeekArg_.reset(); } ackGroupingTrackerPtr_->flushAndClean(); } diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 93ec3a11..f6a30154 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -510,7 +510,7 @@ TEST(ClientTest, testNoRetry) { } TEST(ClientTest, testUpdateServiceInfo) { - extern std::string getToken(); // from AuthToken.cc + extern std::string getToken(); // from tests/AuthToken.cc // Access "private/auth" namespace in cluster 1 ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt}; From fc04394d467b8a9224addd47b8497b0c32d1ff60 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 5 Mar 2026 23:07:48 +0800 Subject: [PATCH 12/28] fix libstdc++ header include error --- lib/ClientConfigurationImpl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ClientConfigurationImpl.h b/lib/ClientConfigurationImpl.h index 6207f989..89f82129 100644 --- a/lib/ClientConfigurationImpl.h +++ b/lib/ClientConfigurationImpl.h @@ -23,6 +23,7 @@ #include #include +#include #include #include "ServiceNameResolver.h" From f7e35eaa770d97c8802d83cecdfebb2f7d7153fd Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 17:03:04 +0800 Subject: [PATCH 13/28] Use ServiceInfo as the single source of truth for ClientConnection --- include/pulsar/ClientConfiguration.h | 3 ++ include/pulsar/ServiceInfo.h | 2 +- lib/AtomicSharedPtr.h | 40 ++++++++++++++++++++++++++ lib/ClientConfigurationImpl.h | 4 +-- lib/ClientConnection.cc | 27 +++++++++--------- lib/ClientConnection.h | 6 ++-- lib/ClientImpl.cc | 42 +++++++++++++--------------- lib/ClientImpl.h | 4 ++- lib/ConnectionPool.cc | 21 ++++++-------- lib/ConnectionPool.h | 12 ++++---- tests/ClientTest.cc | 15 ++++++---- tests/LookupServiceTest.cc | 27 ++++++++++++------ 12 files changed, 130 insertions(+), 73 deletions(-) create mode 100644 lib/AtomicSharedPtr.h diff --git a/include/pulsar/ClientConfiguration.h b/include/pulsar/ClientConfiguration.h index 98ccff7f..712f468c 100644 --- a/include/pulsar/ClientConfiguration.h +++ b/include/pulsar/ClientConfiguration.h @@ -76,6 +76,7 @@ class PULSAR_PUBLIC ClientConfiguration { /** * @return the authentication data + * @deprecated the actual authentication data is now stored in `ServiceInfo` */ Authentication& getAuth() const; @@ -213,6 +214,7 @@ class PULSAR_PUBLIC ClientConfiguration { /** * @return whether the TLS encryption is used on the connections + * @deprecated the TLS usage is now determined by the scheme of the `ServiceInfo::serviceUrl` */ bool isUseTls() const; @@ -249,6 +251,7 @@ class PULSAR_PUBLIC ClientConfiguration { /** * @return the path to the trusted TLS certificate file + * @deprecated the trusted TLS certificate file path is now stored in `ServiceInfo` */ const std::string& getTlsTrustCertsFilePath() const; diff --git a/include/pulsar/ServiceInfo.h b/include/pulsar/ServiceInfo.h index a0f437bc..76bb570a 100644 --- a/include/pulsar/ServiceInfo.h +++ b/include/pulsar/ServiceInfo.h @@ -28,7 +28,7 @@ namespace pulsar { struct PULSAR_PUBLIC ServiceInfo { std::string serviceUrl; - std::optional authentication; + AuthenticationPtr authentication; std::optional tlsTrustCertsFilePath; }; diff --git a/lib/AtomicSharedPtr.h b/lib/AtomicSharedPtr.h new file mode 100644 index 00000000..824a1d61 --- /dev/null +++ b/lib/AtomicSharedPtr.h @@ -0,0 +1,40 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#pragma once + +#include +namespace pulsar { + +// C++17 does not have std::atomic>, so we have to manually implement it. +template +class AtomicSharedPtr { + public: + using Pointer = std::shared_ptr; + + explicit AtomicSharedPtr(T&& value) : ptr_(std::make_shared(std::move(value))) {} + + auto load() const { return std::atomic_load(&ptr_); } + + void store(Pointer&& newPtr) { std::atomic_store(&ptr_, std::move(newPtr)); } + + private: + std::shared_ptr ptr_; +}; + +} // namespace pulsar diff --git a/lib/ClientConfigurationImpl.h b/lib/ClientConfigurationImpl.h index 89f82129..b0ad071e 100644 --- a/lib/ClientConfigurationImpl.h +++ b/lib/ClientConfigurationImpl.h @@ -42,8 +42,8 @@ struct ClientConfigurationImpl { public: void updateServiceInfo(const ServiceInfo& serviceInfo) { std::unique_lock lock(mutex); - if (serviceInfo.authentication.has_value() && *serviceInfo.authentication) { - authenticationPtr = *serviceInfo.authentication; + if (serviceInfo.authentication && serviceInfo.authentication->getAuthMethodName() != "none") { + authenticationPtr = serviceInfo.authentication; } else { authenticationPtr = AuthFactory::Disabled(); } diff --git a/lib/ClientConnection.cc b/lib/ClientConnection.cc index 41d2f608..225cb62b 100644 --- a/lib/ClientConnection.cc +++ b/lib/ClientConnection.cc @@ -19,7 +19,9 @@ #include "ClientConnection.h" #include +#include #include +#include #include #include @@ -37,6 +39,8 @@ #include "ProducerImpl.h" #include "PulsarApi.pb.h" #include "ResultUtils.h" +#include "ServiceNameResolver.h" +#include "ServiceURI.h" #include "Url.h" #include "auth/AuthOauth2.h" #include "auth/InitialAuthData.h" @@ -178,13 +182,14 @@ static bool file_exists(const std::string& path) { std::atomic ClientConnection::maxMessageSize_{Commands::DefaultMaxMessageSize}; +// NOTE: we should get the connection info from `serviceInfo` rather than `clientConfiguration` because it can +// be modified dynamically. ClientConnection::ClientConnection(const std::string& logicalAddress, const std::string& physicalAddress, - const ExecutorServicePtr& executor, + ServiceInfo serviceInfo, const ExecutorServicePtr& executor, const ClientConfiguration& clientConfiguration, - const AuthenticationPtr& authentication, const std::string& clientVersion, - ConnectionPool& pool, size_t poolIndex) + const std::string& clientVersion, ConnectionPool& pool, size_t poolIndex) : operationsTimeout_(ClientImpl::getOperationTimeout(clientConfiguration)), - authentication_(authentication), + authentication_(serviceInfo.authentication ? serviceInfo.authentication : AuthFactory::Disabled()), serverProtocolVersion_(proto::ProtocolVersion_MIN), executor_(executor), resolver_(executor_->createTcpResolver()), @@ -204,21 +209,15 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std: pool_(pool), poolIndex_(poolIndex) { LOG_INFO(cnxString_ << "Create ClientConnection, timeout=" << clientConfiguration.getConnectionTimeout()); - if (!authentication_) { - LOG_ERROR("Invalid authentication plugin"); - throw ResultAuthenticationError; - return; - } - auto oauth2Auth = std::dynamic_pointer_cast(authentication_); - if (oauth2Auth) { + if (auto oauth2Auth = std::dynamic_pointer_cast(authentication_)) { // Configure the TLS trust certs file for Oauth2 auto authData = std::dynamic_pointer_cast( std::make_shared(clientConfiguration.getTlsTrustCertsFilePath())); oauth2Auth->getAuthData(authData); } - if (clientConfiguration.isUseTls()) { + if (ServiceNameResolver::useTls(ServiceURI{serviceInfo.serviceUrl})) { ASIO::ssl::context ctx(ASIO::ssl::context::sslv23_client); ctx.set_options(ASIO::ssl::context::default_workarounds | ASIO::ssl::context::no_sslv2 | ASIO::ssl::context::no_sslv3 | ASIO::ssl::context::no_tlsv1 | @@ -239,8 +238,8 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std: } else { ctx.set_verify_mode(ASIO::ssl::context::verify_peer); - const auto& trustCertFilePath = clientConfiguration.getTlsTrustCertsFilePath(); - if (!trustCertFilePath.empty()) { + if (serviceInfo.tlsTrustCertsFilePath) { + const auto& trustCertFilePath = *serviceInfo.tlsTrustCertsFilePath; if (file_exists(trustCertFilePath)) { ctx.load_verify_file(trustCertFilePath); } else { diff --git a/lib/ClientConnection.h b/lib/ClientConnection.h index 39e4224d..ff5d5f5d 100644 --- a/lib/ClientConnection.h +++ b/lib/ClientConnection.h @@ -25,6 +25,8 @@ #include #include #include + +#include "pulsar/ServiceInfo.h" #ifdef USE_ASIO #include #include @@ -141,8 +143,8 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_this +#include #include #include @@ -79,12 +80,17 @@ std::string generateRandomName() { typedef std::vector StringList; namespace { -std::optional toOptionalAuthentication(const AuthenticationPtr& authentication) { +AuthenticationPtr toAuthentication(const AuthenticationPtr& authentication) { if (!authentication || authentication->getAuthMethodName() == "none") { - return std::nullopt; + return AuthFactory::Disabled(); } return authentication; } + +ServiceInfo normalizeServiceInfo(ServiceInfo serviceInfo) { + serviceInfo.authentication = toAuthentication(serviceInfo.authentication); + return serviceInfo; +} } // namespace static LookupServicePtr defaultLookupServiceFactory(const std::string& serviceUrl, @@ -110,17 +116,18 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& state_(Open), clientConfiguration_(ClientConfiguration(clientConfiguration) .setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl)))), - serviceInfo_{serviceUrl, toOptionalAuthentication(clientConfiguration.getAuthPtr()), - clientConfiguration.getTlsTrustCertsFilePath().empty() - ? std::nullopt - : std::make_optional(clientConfiguration.getTlsTrustCertsFilePath())}, + serviceInfo_(normalizeServiceInfo( + ServiceInfo{serviceUrl, clientConfiguration.getAuthPtr(), + clientConfiguration.getTlsTrustCertsFilePath().empty() + ? std::nullopt + : std::make_optional(clientConfiguration.getTlsTrustCertsFilePath())})), memoryLimitController_(clientConfiguration.getMemoryLimit()), ioExecutorProvider_(std::make_shared(clientConfiguration_.getIOThreads())), listenerExecutorProvider_( std::make_shared(clientConfiguration_.getMessageListenerThreads())), partitionListenerExecutorProvider_( std::make_shared(clientConfiguration_.getMessageListenerThreads())), - pool_(clientConfiguration_, ioExecutorProvider_, clientConfiguration_.getAuthPtr(), + pool_(serviceInfo_, clientConfiguration_, ioExecutorProvider_, ClientImpl::getClientVersion(clientConfiguration)), producerIdGenerator_(0), consumerIdGenerator_(0), @@ -880,29 +887,20 @@ void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { return; } - serviceInfo_ = {serviceInfo.serviceUrl, toOptionalAuthentication(clientConfiguration_.getAuthPtr()), - clientConfiguration_.getTlsTrustCertsFilePath().empty() - ? std::nullopt - : std::make_optional(clientConfiguration_.getTlsTrustCertsFilePath())}; - clientConfiguration_.impl_->updateServiceInfo(serviceInfo_); - - pool_.resetForClusterSwitching(clientConfiguration_.getAuthPtr(), clientConfiguration_); - + serviceInfo = normalizeServiceInfo(std::move(serviceInfo)); + serviceInfo_.store(std::make_shared(serviceInfo)); + pool_.closeAllConnectionsForNewCluster(); lookupServicePtr_->close(); + lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); + for (auto&& it : redirectedClusterLookupServicePtrs_) { it.second->close(); } redirectedClusterLookupServicePtrs_.clear(); - lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); - - // TODO: changes on the following two fields are not tested useProxy_ = false; lookupCount_ = 0; } -ServiceInfo ClientImpl::getServiceInfo() const { - std::shared_lock lock(mutex_); - return serviceInfo_; -} +ServiceInfo ClientImpl::getServiceInfo() const { return *(serviceInfo_.load()); } } /* namespace pulsar */ diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 2a6a8298..7ecd5330 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -33,6 +33,8 @@ #include "MemoryLimitController.h" #include "ProtoApiEnums.h" #include "SynchronizedHashMap.h" +#include "lib/AtomicSharedPtr.h" +#include "pulsar/ServiceInfo.h" namespace pulsar { @@ -212,7 +214,7 @@ class ClientImpl : public std::enable_shared_from_this { State state_; ClientConfiguration clientConfiguration_; - ServiceInfo serviceInfo_; + AtomicSharedPtr serviceInfo_; MemoryLimitController memoryLimitController_; ExecutorServiceProviderPtr ioExecutorProvider_; diff --git a/lib/ConnectionPool.cc b/lib/ConnectionPool.cc index eaa14dc7..ca8bd949 100644 --- a/lib/ConnectionPool.cc +++ b/lib/ConnectionPool.cc @@ -38,12 +38,13 @@ DECLARE_LOG_OBJECT() namespace pulsar { -ConnectionPool::ConnectionPool(const ClientConfiguration& conf, +ConnectionPool::ConnectionPool(const AtomicSharedPtr& serviceInfo, + const ClientConfiguration& conf, const ExecutorServiceProviderPtr& executorProvider, - const AuthenticationPtr& authentication, const std::string& clientVersion) - : clientConfiguration_(conf), + const std::string& clientVersion) + : serviceInfo_(serviceInfo), + clientConfiguration_(conf), executorProvider_(executorProvider), - authentication_(authentication), clientVersion_(clientVersion), randomDistribution_(0, conf.getConnectionsPerBroker() - 1), randomEngine_(std::chrono::high_resolution_clock::now().time_since_epoch().count()) {} @@ -67,12 +68,8 @@ bool ConnectionPool::close() { return true; } -void ConnectionPool::resetForClusterSwitching(const AuthenticationPtr& authentication, - const ClientConfiguration& conf) { +void ConnectionPool::closeAllConnectionsForNewCluster() { std::unique_lock lock(mutex_); - authentication_ = authentication; - clientConfiguration_ = conf; - for (auto cnxIt = pool_.begin(); cnxIt != pool_.end(); cnxIt++) { auto& cnx = cnxIt->second; if (cnx) { @@ -122,9 +119,9 @@ Future ConnectionPool::getConnectionAsync(const // No valid or pending connection found in the pool, creating a new one ClientConnectionPtr cnx; try { - cnx.reset(new ClientConnection(logicalAddress, physicalAddress, executorProvider_->get(keySuffix), - clientConfiguration_, authentication_, clientVersion_, *this, - keySuffix)); + cnx.reset(new ClientConnection(logicalAddress, physicalAddress, *serviceInfo_.load(), + executorProvider_->get(keySuffix), clientConfiguration_, + clientVersion_, *this, keySuffix)); } catch (Result result) { Promise promise; promise.setFailed(result); diff --git a/lib/ConnectionPool.h b/lib/ConnectionPool.h index 541cf8ed..27bf1913 100644 --- a/lib/ConnectionPool.h +++ b/lib/ConnectionPool.h @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -31,6 +32,7 @@ #include #include "Future.h" +#include "lib/AtomicSharedPtr.h" namespace pulsar { class ClientConnection; @@ -41,8 +43,8 @@ using ExecutorServiceProviderPtr = std::shared_ptr; class PULSAR_PUBLIC ConnectionPool { public: - ConnectionPool(const ClientConfiguration& conf, const ExecutorServiceProviderPtr& executorProvider, - const AuthenticationPtr& authentication, const std::string& clientVersion); + ConnectionPool(const AtomicSharedPtr& serviceInfo, const ClientConfiguration& conf, + const ExecutorServiceProviderPtr& executorProvider, const std::string& clientVersion); /** * Close the connection pool. @@ -52,10 +54,10 @@ class PULSAR_PUBLIC ConnectionPool { bool close(); /** - * Close all existing connections and update the authentication and configuration. + * Close all existing connections and notify the connection that a new cluster will be used. * Unlike close(), the pool remains open for new connections. */ - void resetForClusterSwitching(const AuthenticationPtr& authentication, const ClientConfiguration& conf); + void closeAllConnectionsForNewCluster(); void remove(const std::string& logicalAddress, const std::string& physicalAddress, size_t keySuffix, ClientConnection* value); @@ -96,9 +98,9 @@ class PULSAR_PUBLIC ConnectionPool { size_t generateRandomIndex() { return randomDistribution_(randomEngine_); } private: + const AtomicSharedPtr& serviceInfo_; ClientConfiguration clientConfiguration_; ExecutorServiceProviderPtr executorProvider_; - AuthenticationPtr authentication_; typedef std::map> PoolMap; PoolMap pool_; const std::string clientVersion_; diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index f6a30154..3e540495 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -511,6 +511,9 @@ TEST(ClientTest, testNoRetry) { TEST(ClientTest, testUpdateServiceInfo) { extern std::string getToken(); // from tests/AuthToken.cc + auto authMethodName = [](const AuthenticationPtr &auth) { + return auth ? auth->getAuthMethodName() : std::string("none"); + }; // Access "private/auth" namespace in cluster 1 ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt}; @@ -519,9 +522,9 @@ TEST(ClientTest, testUpdateServiceInfo) { AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), TEST_CONF_DIR "/hn-verification/cacert.pem"}; // Access "public/default" namespace in cluster 1, which doesn't require authentication - ServiceInfo info3{"pulsar://localhost:6650", std::nullopt, std::nullopt}; + ServiceInfo info3{"pulsar://localhost:6650", AuthFactory::Disabled(), std::nullopt}; - Client client{info1.serviceUrl, ClientConfiguration().setAuth(*info1.authentication)}; + Client client{info1.serviceUrl, ClientConfiguration().setAuth(info1.authentication)}; const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); Producer producer; @@ -548,7 +551,7 @@ TEST(ClientTest, testUpdateServiceInfo) { client.updateServiceInfo(info2); ASSERT_TRUE(PulsarFriend::getConnections(client).empty()); ASSERT_EQ(info2.serviceUrl, client.getServiceInfo().serviceUrl); - ASSERT_EQ(info2.authentication, client.getServiceInfo().authentication); + ASSERT_EQ(authMethodName(info2.authentication), authMethodName(client.getServiceInfo().authentication)); ASSERT_EQ(info2.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); // Now the same will access the same topic in cluster 2 @@ -558,7 +561,7 @@ TEST(ClientTest, testUpdateServiceInfo) { // cluster 2 will be cleared. client.updateServiceInfo(info3); ASSERT_EQ(info3.serviceUrl, client.getServiceInfo().serviceUrl); - ASSERT_EQ(info3.authentication, client.getServiceInfo().authentication); + ASSERT_EQ(authMethodName(info3.authentication), authMethodName(client.getServiceInfo().authentication)); ASSERT_EQ(info3.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); @@ -577,12 +580,12 @@ TEST(ClientTest, testUpdateServiceInfo) { ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); ASSERT_EQ(value, msg.getDataAsString()); }; - Client client1{info1.serviceUrl, ClientConfiguration().setAuth(*info1.authentication)}; + Client client1{info1.serviceUrl, ClientConfiguration().setAuth(info1.authentication)}; verify(client1, topicRequiredAuth, "msg-0"); client1.close(); Client client2{info2.serviceUrl, ClientConfiguration() - .setAuth(*info2.authentication) + .setAuth(info2.authentication) .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath)}; verify(client2, topicRequiredAuth, "msg-1"); client2.close(); diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc index dc9cfc4e..b0179a1f 100644 --- a/tests/LookupServiceTest.cc +++ b/tests/LookupServiceTest.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -30,6 +31,7 @@ #include "HttpHelper.h" #include "PulsarFriend.h" #include "PulsarWrapper.h" +#include "lib/AtomicSharedPtr.h" #include "lib/BinaryProtoLookupService.h" #include "lib/ClientConnection.h" #include "lib/ConnectionPool.h" @@ -79,11 +81,11 @@ using namespace pulsar; TEST(LookupServiceTest, basicLookup) { ExecutorServiceProviderPtr service = std::make_shared(1); - AuthenticationPtr authData = AuthFactory::Disabled(); std::string url = "pulsar://localhost:6650"; ClientConfiguration conf; ExecutorServiceProviderPtr ioExecutorProvider_(std::make_shared(1)); - ConnectionPool pool_(conf, ioExecutorProvider_, authData, ""); + AtomicSharedPtr serviceInfo(ServiceInfo{url, AuthFactory::Disabled(), std::nullopt}); + ConnectionPool pool_(serviceInfo, conf, ioExecutorProvider_, ""); BinaryProtoLookupService lookupService(url, pool_, conf); TopicNamePtr topicName = TopicName::get("topic"); @@ -146,7 +148,9 @@ static void testMultiAddresses(LookupService& lookupService) { } TEST(LookupServiceTest, testMultiAddresses) { - ConnectionPool pool({}, std::make_shared(1), AuthFactory::Disabled(), ""); + AtomicSharedPtr serviceInfo( + ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + ConnectionPool pool(serviceInfo, {}, std::make_shared(1), ""); ClientConfiguration conf; BinaryProtoLookupService binaryLookupService("pulsar://localhost,localhost:9999", pool, conf); testMultiAddresses(binaryLookupService); @@ -158,7 +162,9 @@ TEST(LookupServiceTest, testMultiAddresses) { } TEST(LookupServiceTest, testRetry) { auto executorProvider = std::make_shared(1); - ConnectionPool pool({}, executorProvider, AuthFactory::Disabled(), ""); + AtomicSharedPtr serviceInfo( + ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + ConnectionPool pool(serviceInfo, {}, executorProvider, ""); ClientConfiguration conf; auto lookupService = RetryableLookupService::create( @@ -192,7 +198,9 @@ TEST(LookupServiceTest, testRetry) { TEST(LookupServiceTest, testTimeout) { auto executorProvider = std::make_shared(1); - ConnectionPool pool({}, executorProvider, AuthFactory::Disabled(), ""); + AtomicSharedPtr serviceInfo( + ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + ConnectionPool pool(serviceInfo, {}, executorProvider, ""); ClientConfiguration conf; constexpr int timeoutInSeconds = 2; @@ -467,11 +475,12 @@ class BinaryProtoLookupServiceRedirectTestHelper : public BinaryProtoLookupServi TEST(LookupServiceTest, testRedirectionLimit) { const auto redirect_limit = 5; - AuthenticationPtr authData = AuthFactory::Disabled(); ClientConfiguration conf; conf.setMaxLookupRedirects(redirect_limit); ExecutorServiceProviderPtr ioExecutorProvider_(std::make_shared(1)); - ConnectionPool pool_(conf, ioExecutorProvider_, authData, ""); + AtomicSharedPtr serviceInfo( + ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + ConnectionPool pool_(serviceInfo, conf, ioExecutorProvider_, ""); string url = "pulsar://localhost:6650"; BinaryProtoLookupServiceRedirectTestHelper lookupService(url, pool_, conf); @@ -536,7 +545,9 @@ TEST(LookupServiceTest, testAfterClientShutdown) { TEST(LookupServiceTest, testRetryAfterDestroyed) { auto executorProvider = std::make_shared(1); - ConnectionPool pool({}, executorProvider, AuthFactory::Disabled(), ""); + AtomicSharedPtr serviceInfo( + ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + ConnectionPool pool(serviceInfo, {}, executorProvider, ""); auto internalLookupService = std::make_shared("pulsar://localhost:6650", pool, ClientConfiguration{}); From a01842d7d6ce458b5b3361f48ae690539d6779a5 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 17:18:57 +0800 Subject: [PATCH 14/28] revert changes on ClientConfiguration --- lib/ClientConfiguration.cc | 14 ++++---- lib/ClientConfigurationImpl.h | 60 ++--------------------------------- 2 files changed, 9 insertions(+), 65 deletions(-) diff --git a/lib/ClientConfiguration.cc b/lib/ClientConfiguration.cc index c3c56b95..b99c5d25 100644 --- a/lib/ClientConfiguration.cc +++ b/lib/ClientConfiguration.cc @@ -53,13 +53,13 @@ ClientConfiguration& ClientConfiguration::setConnectionsPerBroker(int connection int ClientConfiguration::getConnectionsPerBroker() const { return impl_->connectionsPerBroker; } ClientConfiguration& ClientConfiguration::setAuth(const AuthenticationPtr& authentication) { - impl_->setAuthentication(authentication); + impl_->authenticationPtr = authentication; return *this; } -Authentication& ClientConfiguration::getAuth() const { return *impl_->getAuthentication(); } +Authentication& ClientConfiguration::getAuth() const { return *impl_->authenticationPtr; } -const AuthenticationPtr& ClientConfiguration::getAuthPtr() const { return impl_->getAuthentication(); } +const AuthenticationPtr& ClientConfiguration::getAuthPtr() const { return impl_->authenticationPtr; } ClientConfiguration& ClientConfiguration::setOperationTimeoutSeconds(int timeout) { impl_->operationTimeout = std::chrono::seconds(timeout); @@ -95,11 +95,11 @@ ClientConfiguration& ClientConfiguration::setMessageListenerThreads(int threads) int ClientConfiguration::getMessageListenerThreads() const { return impl_->messageListenerThreads; } ClientConfiguration& ClientConfiguration::setUseTls(bool useTls) { - impl_->setUseTls(useTls); + impl_->useTls = useTls; return *this; } -bool ClientConfiguration::isUseTls() const { return impl_->isUseTls(); } +bool ClientConfiguration::isUseTls() const { return impl_->useTls; } ClientConfiguration& ClientConfiguration::setValidateHostName(bool validateHostName) { impl_->validateHostName = validateHostName; @@ -127,12 +127,12 @@ const std::string& ClientConfiguration::getTlsCertificateFilePath() const { } ClientConfiguration& ClientConfiguration::setTlsTrustCertsFilePath(const std::string& filePath) { - impl_->setTlsTrustCertsFilePath(filePath); + impl_->tlsTrustCertsFilePath = filePath; return *this; } const std::string& ClientConfiguration::getTlsTrustCertsFilePath() const { - return impl_->getTlsTrustCertsFilePath(); + return impl_->tlsTrustCertsFilePath; } ClientConfiguration& ClientConfiguration::setTlsAllowInsecureConnection(bool allowInsecure) { diff --git a/lib/ClientConfigurationImpl.h b/lib/ClientConfigurationImpl.h index b0ad071e..e7a83a19 100644 --- a/lib/ClientConfigurationImpl.h +++ b/lib/ClientConfigurationImpl.h @@ -20,71 +20,13 @@ #define LIB_CLIENTCONFIGURATIONIMPL_H_ #include -#include #include -#include -#include - -#include "ServiceNameResolver.h" -#include "ServiceURI.h" namespace pulsar { -// Use struct rather than class here just for ABI compatibility struct ClientConfigurationImpl { - private: - mutable std::shared_mutex mutex; AuthenticationPtr authenticationPtr{AuthFactory::Disabled()}; - std::string tlsTrustCertsFilePath; - bool useTls{false}; - - public: - void updateServiceInfo(const ServiceInfo& serviceInfo) { - std::unique_lock lock(mutex); - if (serviceInfo.authentication && serviceInfo.authentication->getAuthMethodName() != "none") { - authenticationPtr = serviceInfo.authentication; - } else { - authenticationPtr = AuthFactory::Disabled(); - } - if (serviceInfo.tlsTrustCertsFilePath.has_value()) { - tlsTrustCertsFilePath = *serviceInfo.tlsTrustCertsFilePath; - } else { - tlsTrustCertsFilePath = ""; - } - useTls = ServiceNameResolver::useTls(ServiceURI(serviceInfo.serviceUrl)); - } - - auto& getAuthentication() const { - std::shared_lock lock(mutex); - return authenticationPtr; - } - - auto setAuthentication(const AuthenticationPtr& authentication) { - std::unique_lock lock(mutex); - authenticationPtr = authentication; - } - - auto& getTlsTrustCertsFilePath() const { - std::shared_lock lock(mutex); - return tlsTrustCertsFilePath; - } - - auto setTlsTrustCertsFilePath(const std::string& path) { - std::unique_lock lock(mutex); - tlsTrustCertsFilePath = path; - } - - auto isUseTls() const { - std::shared_lock lock(mutex); - return useTls; - } - - auto setUseTls(bool useTls_) { - std::unique_lock lock(mutex); - useTls = useTls_; - } - uint64_t memoryLimit{0ull}; int ioThreads{1}; int connectionsPerBroker{1}; @@ -94,8 +36,10 @@ struct ClientConfigurationImpl { int maxLookupRedirects{20}; int initialBackoffIntervalMs{100}; int maxBackoffIntervalMs{60000}; + bool useTls{false}; std::string tlsPrivateKeyFilePath; std::string tlsCertificateFilePath; + std::string tlsTrustCertsFilePath; bool tlsAllowInsecureConnection{false}; unsigned int statsIntervalInSeconds{600}; // 10 minutes std::unique_ptr loggerFactory; From 38a6111bc54c9dd4982f0b1f95cb97713586af11 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 17:28:41 +0800 Subject: [PATCH 15/28] simplify implementations --- include/pulsar/ServiceInfo.h | 25 ++++++++++++++++++++---- lib/ClientConnection.cc | 10 +++++----- lib/ClientConnection.h | 3 +-- lib/ClientImpl.cc | 29 +++++++--------------------- lib/ServiceInfo.cc | 37 ++++++++++++++++++++++++++++++++++++ tests/ClientTest.cc | 30 ++++++++++------------------- 6 files changed, 81 insertions(+), 53 deletions(-) create mode 100644 lib/ServiceInfo.cc diff --git a/include/pulsar/ServiceInfo.h b/include/pulsar/ServiceInfo.h index 76bb570a..8d0291a8 100644 --- a/include/pulsar/ServiceInfo.h +++ b/include/pulsar/ServiceInfo.h @@ -26,10 +26,27 @@ namespace pulsar { -struct PULSAR_PUBLIC ServiceInfo { - std::string serviceUrl; - AuthenticationPtr authentication; - std::optional tlsTrustCertsFilePath; +class PULSAR_PUBLIC ServiceInfo final { + public: + ServiceInfo(std::string serviceUrl, AuthenticationPtr authentication = AuthFactory::Disabled(), + std::optional tlsTrustCertsFilePath = std::nullopt); + + auto& serviceUrl() const noexcept { return serviceUrl_; } + auto useTls() const noexcept { return useTls_; } + auto& authentication() const noexcept { return authentication_; } + auto& tlsTrustCertsFilePath() const noexcept { return tlsTrustCertsFilePath_; } + + bool operator==(const ServiceInfo& other) const noexcept { + return serviceUrl_ == other.serviceUrl_ && useTls_ == other.useTls_ && + authentication_ == other.authentication_ && + tlsTrustCertsFilePath_ == other.tlsTrustCertsFilePath_; + } + + private: + std::string serviceUrl_; + bool useTls_; + AuthenticationPtr authentication_; + std::optional tlsTrustCertsFilePath_; }; } // namespace pulsar diff --git a/lib/ClientConnection.cc b/lib/ClientConnection.cc index 225cb62b..7889b40b 100644 --- a/lib/ClientConnection.cc +++ b/lib/ClientConnection.cc @@ -189,7 +189,7 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std: const ClientConfiguration& clientConfiguration, const std::string& clientVersion, ConnectionPool& pool, size_t poolIndex) : operationsTimeout_(ClientImpl::getOperationTimeout(clientConfiguration)), - authentication_(serviceInfo.authentication ? serviceInfo.authentication : AuthFactory::Disabled()), + authentication_(serviceInfo.authentication()), serverProtocolVersion_(proto::ProtocolVersion_MIN), executor_(executor), resolver_(executor_->createTcpResolver()), @@ -213,11 +213,11 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std: if (auto oauth2Auth = std::dynamic_pointer_cast(authentication_)) { // Configure the TLS trust certs file for Oauth2 auto authData = std::dynamic_pointer_cast( - std::make_shared(clientConfiguration.getTlsTrustCertsFilePath())); + std::make_shared(serviceInfo.tlsTrustCertsFilePath().value_or(""))); oauth2Auth->getAuthData(authData); } - if (ServiceNameResolver::useTls(ServiceURI{serviceInfo.serviceUrl})) { + if (serviceInfo.useTls()) { ASIO::ssl::context ctx(ASIO::ssl::context::sslv23_client); ctx.set_options(ASIO::ssl::context::default_workarounds | ASIO::ssl::context::no_sslv2 | ASIO::ssl::context::no_sslv3 | ASIO::ssl::context::no_tlsv1 | @@ -238,8 +238,8 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std: } else { ctx.set_verify_mode(ASIO::ssl::context::verify_peer); - if (serviceInfo.tlsTrustCertsFilePath) { - const auto& trustCertFilePath = *serviceInfo.tlsTrustCertsFilePath; + if (serviceInfo.tlsTrustCertsFilePath()) { + const auto& trustCertFilePath = *serviceInfo.tlsTrustCertsFilePath(); if (file_exists(trustCertFilePath)) { ctx.load_verify_file(trustCertFilePath); } else { diff --git a/lib/ClientConnection.h b/lib/ClientConnection.h index ff5d5f5d..31b2ceca 100644 --- a/lib/ClientConnection.h +++ b/lib/ClientConnection.h @@ -20,13 +20,12 @@ #define _PULSAR_CLIENT_CONNECTION_HEADER_ #include +#include #include #include #include #include - -#include "pulsar/ServiceInfo.h" #ifdef USE_ASIO #include #include diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 595090dc..ae3cf6e7 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -79,20 +79,6 @@ std::string generateRandomName() { typedef std::vector StringList; -namespace { -AuthenticationPtr toAuthentication(const AuthenticationPtr& authentication) { - if (!authentication || authentication->getAuthMethodName() == "none") { - return AuthFactory::Disabled(); - } - return authentication; -} - -ServiceInfo normalizeServiceInfo(ServiceInfo serviceInfo) { - serviceInfo.authentication = toAuthentication(serviceInfo.authentication); - return serviceInfo; -} -} // namespace - static LookupServicePtr defaultLookupServiceFactory(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration, ConnectionPool& pool, const AuthenticationPtr& auth) { @@ -116,11 +102,10 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& state_(Open), clientConfiguration_(ClientConfiguration(clientConfiguration) .setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl)))), - serviceInfo_(normalizeServiceInfo( - ServiceInfo{serviceUrl, clientConfiguration.getAuthPtr(), - clientConfiguration.getTlsTrustCertsFilePath().empty() - ? std::nullopt - : std::make_optional(clientConfiguration.getTlsTrustCertsFilePath())})), + serviceInfo_(ServiceInfo{serviceUrl, clientConfiguration.getAuthPtr(), + clientConfiguration.getTlsTrustCertsFilePath().empty() + ? std::nullopt + : std::make_optional(clientConfiguration.getTlsTrustCertsFilePath())}), memoryLimitController_(clientConfiguration.getMemoryLimit()), ioExecutorProvider_(std::make_shared(clientConfiguration_.getIOThreads())), listenerExecutorProvider_( @@ -145,8 +130,9 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& ClientImpl::~ClientImpl() { shutdown(); } LookupServicePtr ClientImpl::createLookup(const std::string& serviceUrl) { + const auto serviceInfo = serviceInfo_.load(); auto lookupServicePtr = RetryableLookupService::create( - lookupServiceFactory_(serviceUrl, clientConfiguration_, pool_, clientConfiguration_.getAuthPtr()), + lookupServiceFactory_(serviceUrl, clientConfiguration_, pool_, serviceInfo->authentication()), clientConfiguration_.impl_->operationTimeout, ioExecutorProvider_); return lookupServicePtr; } @@ -887,11 +873,10 @@ void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { return; } - serviceInfo = normalizeServiceInfo(std::move(serviceInfo)); serviceInfo_.store(std::make_shared(serviceInfo)); pool_.closeAllConnectionsForNewCluster(); lookupServicePtr_->close(); - lookupServicePtr_ = createLookup(serviceInfo.serviceUrl); + lookupServicePtr_ = createLookup(serviceInfo.serviceUrl()); for (auto&& it : redirectedClusterLookupServicePtrs_) { it.second->close(); diff --git a/lib/ServiceInfo.cc b/lib/ServiceInfo.cc new file mode 100644 index 00000000..4f3be6d8 --- /dev/null +++ b/lib/ServiceInfo.cc @@ -0,0 +1,37 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#include + +#include + +#include "ServiceNameResolver.h" +#include "ServiceURI.h" + +namespace pulsar { + +ServiceInfo::ServiceInfo(std::string serviceUrl, AuthenticationPtr authentication, + std::optional tlsTrustCertsFilePath) + : serviceUrl_(std::move(serviceUrl)), + useTls_(ServiceNameResolver::useTls(ServiceURI(serviceUrl_))), + authentication_(authentication && authentication->getAuthMethodName() != "none" + ? std::move(authentication) + : AuthFactory::Disabled()), + tlsTrustCertsFilePath_(std::move(tlsTrustCertsFilePath)) {} + +} // namespace pulsar diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 3e540495..c176b76c 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -19,14 +19,11 @@ #include #include #include -#include #include #include #include #include -#include -#include #include "MockClientImpl.h" #include "PulsarAdminHelper.h" @@ -511,20 +508,17 @@ TEST(ClientTest, testNoRetry) { TEST(ClientTest, testUpdateServiceInfo) { extern std::string getToken(); // from tests/AuthToken.cc - auto authMethodName = [](const AuthenticationPtr &auth) { - return auth ? auth->getAuthMethodName() : std::string("none"); - }; // Access "private/auth" namespace in cluster 1 - ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken()), std::nullopt}; + ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken())}; // Access "private/auth" namespace in cluster 2 ServiceInfo info2{"pulsar+ssl://localhost:6653", AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), TEST_CONF_DIR "/hn-verification/cacert.pem"}; // Access "public/default" namespace in cluster 1, which doesn't require authentication - ServiceInfo info3{"pulsar://localhost:6650", AuthFactory::Disabled(), std::nullopt}; + ServiceInfo info3{"pulsar://localhost:6650"}; - Client client{info1.serviceUrl, ClientConfiguration().setAuth(info1.authentication)}; + Client client{info1.serviceUrl(), ClientConfiguration().setAuth(info1.authentication())}; const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); Producer producer; @@ -550,9 +544,7 @@ TEST(ClientTest, testUpdateServiceInfo) { ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); client.updateServiceInfo(info2); ASSERT_TRUE(PulsarFriend::getConnections(client).empty()); - ASSERT_EQ(info2.serviceUrl, client.getServiceInfo().serviceUrl); - ASSERT_EQ(authMethodName(info2.authentication), authMethodName(client.getServiceInfo().authentication)); - ASSERT_EQ(info2.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); + ASSERT_EQ(info2, client.getServiceInfo()); // Now the same will access the same topic in cluster 2 sendAndReceive("msg-1"); @@ -560,9 +552,7 @@ TEST(ClientTest, testUpdateServiceInfo) { // Switch back to cluster 1 without any authentication, the previous authentication info configured for // cluster 2 will be cleared. client.updateServiceInfo(info3); - ASSERT_EQ(info3.serviceUrl, client.getServiceInfo().serviceUrl); - ASSERT_EQ(authMethodName(info3.authentication), authMethodName(client.getServiceInfo().authentication)); - ASSERT_EQ(info3.tlsTrustCertsFilePath, client.getServiceInfo().tlsTrustCertsFilePath); + ASSERT_EQ(info3, client.getServiceInfo()); const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); producer.close(); @@ -580,17 +570,17 @@ TEST(ClientTest, testUpdateServiceInfo) { ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); ASSERT_EQ(value, msg.getDataAsString()); }; - Client client1{info1.serviceUrl, ClientConfiguration().setAuth(info1.authentication)}; + Client client1{info1.serviceUrl(), ClientConfiguration().setAuth(info1.authentication())}; verify(client1, topicRequiredAuth, "msg-0"); client1.close(); - Client client2{info2.serviceUrl, ClientConfiguration() - .setAuth(info2.authentication) - .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath)}; + Client client2{info2.serviceUrl(), ClientConfiguration() + .setAuth(info2.authentication()) + .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath())}; verify(client2, topicRequiredAuth, "msg-1"); client2.close(); - Client client3{info3.serviceUrl}; + Client client3{info3.serviceUrl()}; verify(client3, topicNoAuth, "msg-2"); client3.close(); } From 792fede920503bc2dd4c7b21f8a5784188652e68 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 18:37:46 +0800 Subject: [PATCH 16/28] Pass ServiceInfo to lookup service and fix AuthPluginTest.testInvalidPlugin --- lib/BinaryProtoLookupService.h | 5 +-- lib/ClientConnection.cc | 5 +++ lib/ClientImpl.cc | 23 ++++++------- lib/ClientImpl.h | 17 +++++++--- lib/HTTPLookupService.cc | 11 +++--- lib/HTTPLookupService.h | 4 ++- lib/ServiceInfo.cc | 4 +-- tests/LookupServiceTest.cc | 62 ++++++++++++++++------------------ 8 files changed, 70 insertions(+), 61 deletions(-) diff --git a/lib/BinaryProtoLookupService.h b/lib/BinaryProtoLookupService.h index 948c7f1f..35dcb163 100644 --- a/lib/BinaryProtoLookupService.h +++ b/lib/BinaryProtoLookupService.h @@ -22,6 +22,7 @@ #include #include #include +#include #include @@ -38,9 +39,9 @@ using GetSchemaPromisePtr = std::shared_ptr>; class PULSAR_PUBLIC BinaryProtoLookupService : public LookupService { public: - BinaryProtoLookupService(const std::string& serviceUrl, ConnectionPool& pool, + BinaryProtoLookupService(const ServiceInfo& serviceInfo, ConnectionPool& pool, const ClientConfiguration& clientConfiguration) - : serviceNameResolver_(serviceUrl), + : serviceNameResolver_(serviceInfo.serviceUrl()), cnxPool_(pool), listenerName_(clientConfiguration.getListenerName()), maxLookupRedirects_(clientConfiguration.getMaxLookupRedirects()) {} diff --git a/lib/ClientConnection.cc b/lib/ClientConnection.cc index 7889b40b..bf71b5ea 100644 --- a/lib/ClientConnection.cc +++ b/lib/ClientConnection.cc @@ -209,6 +209,11 @@ ClientConnection::ClientConnection(const std::string& logicalAddress, const std: pool_(pool), poolIndex_(poolIndex) { LOG_INFO(cnxString_ << "Create ClientConnection, timeout=" << clientConfiguration.getConnectionTimeout()); + if (!authentication_) { + LOG_ERROR("Invalid authentication plugin"); + throw ResultAuthenticationError; + return; + } if (auto oauth2Auth = std::dynamic_pointer_cast(authentication_)) { // Configure the TLS trust certs file for Oauth2 diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index ae3cf6e7..18390439 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -19,7 +19,6 @@ #include "ClientImpl.h" #include -#include #include #include @@ -79,16 +78,15 @@ std::string generateRandomName() { typedef std::vector StringList; -static LookupServicePtr defaultLookupServiceFactory(const std::string& serviceUrl, +static LookupServicePtr defaultLookupServiceFactory(const ServiceInfo& serviceInfo, const ClientConfiguration& clientConfiguration, - ConnectionPool& pool, const AuthenticationPtr& auth) { - if (ServiceNameResolver::useHttp(ServiceURI(serviceUrl))) { + ConnectionPool& pool) { + if (ServiceNameResolver::useHttp(ServiceURI(serviceInfo.serviceUrl()))) { LOG_DEBUG("Using HTTP Lookup"); - return std::make_shared(serviceUrl, std::cref(clientConfiguration), - std::cref(auth)); + return std::make_shared(std::cref(serviceInfo), std::cref(clientConfiguration)); } else { LOG_DEBUG("Using Binary Lookup"); - return std::make_shared(serviceUrl, std::ref(pool), + return std::make_shared(std::cref(serviceInfo), std::ref(pool), std::cref(clientConfiguration)); } } @@ -124,15 +122,14 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& if (loggerFactory) { LogUtils::setLoggerFactory(std::move(loggerFactory)); } - lookupServicePtr_ = createLookup(serviceUrl); + lookupServicePtr_ = createLookup(*serviceInfo_.load()); } ClientImpl::~ClientImpl() { shutdown(); } -LookupServicePtr ClientImpl::createLookup(const std::string& serviceUrl) { - const auto serviceInfo = serviceInfo_.load(); +LookupServicePtr ClientImpl::createLookup(ServiceInfo serviceInfo) { auto lookupServicePtr = RetryableLookupService::create( - lookupServiceFactory_(serviceUrl, clientConfiguration_, pool_, serviceInfo->authentication()), + lookupServiceFactory_(std::move(serviceInfo), clientConfiguration_, pool_), clientConfiguration_.impl_->operationTimeout, ioExecutorProvider_); return lookupServicePtr; } @@ -167,7 +164,7 @@ LookupServicePtr ClientImpl::getLookup(const std::string& redirectedClusterURI) it != redirectedClusterLookupServicePtrs_.end()) { return it->second; } - auto lookup = createLookup(redirectedClusterURI); + auto lookup = createRedirectedLookup(redirectedClusterURI); redirectedClusterLookupServicePtrs_.emplace(redirectedClusterURI, lookup); return lookup; } @@ -876,7 +873,7 @@ void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { serviceInfo_.store(std::make_shared(serviceInfo)); pool_.closeAllConnectionsForNewCluster(); lookupServicePtr_->close(); - lookupServicePtr_ = createLookup(serviceInfo.serviceUrl()); + lookupServicePtr_ = createLookup(serviceInfo); for (auto&& it : redirectedClusterLookupServicePtrs_) { it.second->close(); diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 7ecd5330..7b71a387 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -20,6 +20,7 @@ #define LIB_CLIENTIMPL_H_ #include +#include #include #include @@ -34,7 +35,6 @@ #include "ProtoApiEnums.h" #include "SynchronizedHashMap.h" #include "lib/AtomicSharedPtr.h" -#include "pulsar/ServiceInfo.h" namespace pulsar { @@ -56,8 +56,8 @@ typedef std::weak_ptr ConsumerImplBaseWeakPtr; class ClientConnection; using ClientConnectionPtr = std::shared_ptr; -using LookupServiceFactory = std::function; +using LookupServiceFactory = std::function; class ProducerImplBase; using ProducerImplBaseWeakPtr = std::weak_ptr; @@ -198,7 +198,16 @@ class ClientImpl : public std::enable_shared_from_this { const std::string& getPhysicalAddress(const std::string& redirectedClusterURI, const std::string& logicalAddress); - LookupServicePtr createLookup(const std::string& serviceUrl); + // This overload is only used for blue-green migration, where only the service URL is modified, the other + // paramters remain the same + LookupServicePtr createRedirectedLookup(const std::string& redirectedUrl) { + auto serviceInfo = serviceInfo_.load(); + return createLookup( + ServiceInfo{redirectedUrl, serviceInfo->authentication(), serviceInfo->tlsTrustCertsFilePath()}); + } + + LookupServicePtr createLookup(ServiceInfo serviceInfo); + LookupServicePtr getLookup(const std::string& redirectedClusterURI); static std::string getClientVersion(const ClientConfiguration& clientConfiguration); diff --git a/lib/HTTPLookupService.cc b/lib/HTTPLookupService.cc index 79d9e944..b83b9aff 100644 --- a/lib/HTTPLookupService.cc +++ b/lib/HTTPLookupService.cc @@ -47,17 +47,16 @@ const static std::string ADMIN_PATH_V2 = "/admin/v2/"; const static std::string PARTITION_METHOD_NAME = "partitions"; const static int NUMBER_OF_LOOKUP_THREADS = 1; -HTTPLookupService::HTTPLookupService(const std::string &serviceUrl, - const ClientConfiguration &clientConfiguration, - const AuthenticationPtr &authData) +HTTPLookupService::HTTPLookupService(const ServiceInfo &serviceInfo, + const ClientConfiguration &clientConfiguration) : executorProvider_(std::make_shared(NUMBER_OF_LOOKUP_THREADS)), - serviceNameResolver_(serviceUrl), - authenticationPtr_(authData), + serviceNameResolver_(serviceInfo.serviceUrl()), + authenticationPtr_(serviceInfo.authentication()), lookupTimeoutInSeconds_(clientConfiguration.getOperationTimeoutSeconds()), maxLookupRedirects_(clientConfiguration.getMaxLookupRedirects()), tlsPrivateFilePath_(clientConfiguration.getTlsPrivateKeyFilePath()), tlsCertificateFilePath_(clientConfiguration.getTlsCertificateFilePath()), - tlsTrustCertsFilePath_(clientConfiguration.getTlsTrustCertsFilePath()), + tlsTrustCertsFilePath_(serviceInfo.tlsTrustCertsFilePath().value_or("")), isUseTls_(clientConfiguration.isUseTls()), tlsAllowInsecure_(clientConfiguration.isTlsAllowInsecureConnection()), tlsValidateHostname_(clientConfiguration.isValidateHostName()) {} diff --git a/lib/HTTPLookupService.h b/lib/HTTPLookupService.h index d17edd53..61a06155 100644 --- a/lib/HTTPLookupService.h +++ b/lib/HTTPLookupService.h @@ -19,6 +19,8 @@ #ifndef PULSAR_CPP_HTTPLOOKUPSERVICE_H #define PULSAR_CPP_HTTPLOOKUPSERVICE_H +#include + #include #include "ClientImpl.h" @@ -67,7 +69,7 @@ class HTTPLookupService : public LookupService, public std::enable_shared_from_t Result sendHTTPRequest(const std::string& completeUrl, std::string& responseData, long& responseCode); public: - HTTPLookupService(const std::string&, const ClientConfiguration&, const AuthenticationPtr&); + HTTPLookupService(const ServiceInfo& serviceInfo, const ClientConfiguration& config); LookupResultFuture getBroker(const TopicName& topicName) override; diff --git a/lib/ServiceInfo.cc b/lib/ServiceInfo.cc index 4f3be6d8..642b39ab 100644 --- a/lib/ServiceInfo.cc +++ b/lib/ServiceInfo.cc @@ -29,9 +29,7 @@ ServiceInfo::ServiceInfo(std::string serviceUrl, AuthenticationPtr authenticatio std::optional tlsTrustCertsFilePath) : serviceUrl_(std::move(serviceUrl)), useTls_(ServiceNameResolver::useTls(ServiceURI(serviceUrl_))), - authentication_(authentication && authentication->getAuthMethodName() != "none" - ? std::move(authentication) - : AuthFactory::Disabled()), + authentication_(std::move(authentication)), tlsTrustCertsFilePath_(std::move(tlsTrustCertsFilePath)) {} } // namespace pulsar diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc index b0179a1f..db22a34c 100644 --- a/tests/LookupServiceTest.cc +++ b/tests/LookupServiceTest.cc @@ -84,7 +84,7 @@ TEST(LookupServiceTest, basicLookup) { std::string url = "pulsar://localhost:6650"; ClientConfiguration conf; ExecutorServiceProviderPtr ioExecutorProvider_(std::make_shared(1)); - AtomicSharedPtr serviceInfo(ServiceInfo{url, AuthFactory::Disabled(), std::nullopt}); + AtomicSharedPtr serviceInfo(ServiceInfo{url}); ConnectionPool pool_(serviceInfo, conf, ioExecutorProvider_, ""); BinaryProtoLookupService lookupService(url, pool_, conf); @@ -148,28 +148,28 @@ static void testMultiAddresses(LookupService& lookupService) { } TEST(LookupServiceTest, testMultiAddresses) { - AtomicSharedPtr serviceInfo( - ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); ConnectionPool pool(serviceInfo, {}, std::make_shared(1), ""); ClientConfiguration conf; - BinaryProtoLookupService binaryLookupService("pulsar://localhost,localhost:9999", pool, conf); + BinaryProtoLookupService binaryLookupService(ServiceInfo{"pulsar://localhost,localhost:9999"}, pool, + conf); testMultiAddresses(binaryLookupService); // HTTPLookupService calls shared_from_this() internally, we must create a shared pointer to test auto httpLookupServicePtr = std::make_shared( - "http://localhost,localhost:9999", ClientConfiguration{}, AuthFactory::Disabled()); + ServiceInfo{"http://localhost,localhost:9999"}, ClientConfiguration{}); testMultiAddresses(*httpLookupServicePtr); } TEST(LookupServiceTest, testRetry) { auto executorProvider = std::make_shared(1); - AtomicSharedPtr serviceInfo( - ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); ConnectionPool pool(serviceInfo, {}, executorProvider, ""); ClientConfiguration conf; - auto lookupService = RetryableLookupService::create( - std::make_shared("pulsar://localhost:9999,localhost", pool, conf), - std::chrono::seconds(30), executorProvider); + auto lookupService = + RetryableLookupService::create(std::make_shared( + ServiceInfo{"pulsar://localhost:9999,localhost"}, pool, conf), + std::chrono::seconds(30), executorProvider); ServiceNameResolver& serviceNameResolver = lookupService->getServiceNameResolver(); PulsarFriend::setServiceUrlIndex(serviceNameResolver, 0); @@ -198,15 +198,16 @@ TEST(LookupServiceTest, testRetry) { TEST(LookupServiceTest, testTimeout) { auto executorProvider = std::make_shared(1); - AtomicSharedPtr serviceInfo( - ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); ConnectionPool pool(serviceInfo, {}, executorProvider, ""); ClientConfiguration conf; constexpr int timeoutInSeconds = 2; auto lookupService = RetryableLookupService::create( - std::make_shared("pulsar://localhost:9990,localhost:9902,localhost:9904", - pool, conf), + std::make_shared( + ServiceInfo{"pulsar://localhost:9990,localhost:9902,localhost:9904", AuthFactory::Disabled(), + std::nullopt}, + pool, conf), std::chrono::seconds(timeoutInSeconds), executorProvider); auto topicNamePtr = TopicName::get("lookup-service-test-retry"); @@ -463,9 +464,9 @@ INSTANTIATE_TEST_SUITE_P(Pulsar, LookupServiceTest, ::testing::Values(binaryLook class BinaryProtoLookupServiceRedirectTestHelper : public BinaryProtoLookupService { public: - BinaryProtoLookupServiceRedirectTestHelper(const std::string& serviceUrl, ConnectionPool& pool, + BinaryProtoLookupServiceRedirectTestHelper(const ServiceInfo& serviceInfo, ConnectionPool& pool, const ClientConfiguration& clientConfiguration) - : BinaryProtoLookupService(serviceUrl, pool, clientConfiguration) {} + : BinaryProtoLookupService(serviceInfo, pool, clientConfiguration) {} LookupResultFuture findBroker(const std::string& address, bool authoritative, const std::string& topic, size_t redirectCount) { @@ -478,11 +479,10 @@ TEST(LookupServiceTest, testRedirectionLimit) { ClientConfiguration conf; conf.setMaxLookupRedirects(redirect_limit); ExecutorServiceProviderPtr ioExecutorProvider_(std::make_shared(1)); - AtomicSharedPtr serviceInfo( - ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); ConnectionPool pool_(serviceInfo, conf, ioExecutorProvider_, ""); - string url = "pulsar://localhost:6650"; - BinaryProtoLookupServiceRedirectTestHelper lookupService(url, pool_, conf); + const ServiceInfo lookupServiceInfo{"pulsar://localhost:6650"}; + BinaryProtoLookupServiceRedirectTestHelper lookupService(lookupServiceInfo, pool_, conf); const auto topicNamePtr = TopicName::get("topic"); for (auto idx = 0; idx < redirect_limit + 5; ++idx) { @@ -493,8 +493,8 @@ TEST(LookupServiceTest, testRedirectionLimit) { if (idx <= redirect_limit) { ASSERT_EQ(ResultOk, result); - ASSERT_EQ(url, lookupResult.logicalAddress); - ASSERT_EQ(url, lookupResult.physicalAddress); + ASSERT_EQ(lookupServiceInfo.serviceUrl(), lookupResult.logicalAddress); + ASSERT_EQ(lookupServiceInfo.serviceUrl(), lookupResult.physicalAddress); } else { ASSERT_EQ(ResultTooManyLookupRequestException, result); } @@ -522,12 +522,11 @@ class MockLookupService : public BinaryProtoLookupService { }; TEST(LookupServiceTest, testAfterClientShutdown) { - auto client = std::make_shared("pulsar://localhost:6650", ClientConfiguration{}, - [](const std::string& serviceUrl, const ClientConfiguration&, - ConnectionPool& pool, const AuthenticationPtr&) { - return std::make_shared( - serviceUrl, pool, ClientConfiguration{}); - }); + auto client = std::make_shared( + "pulsar://localhost:6650", ClientConfiguration{}, + [](const ServiceInfo& serviceInfo, const ClientConfiguration&, ConnectionPool& pool) { + return std::make_shared(serviceInfo, pool, ClientConfiguration{}); + }); std::promise promise; client->subscribeAsync("lookup-service-test-after-client-shutdown", "sub", ConsumerConfiguration{}, [&promise](Result result, const Consumer&) { promise.set_value(result); }); @@ -545,12 +544,11 @@ TEST(LookupServiceTest, testAfterClientShutdown) { TEST(LookupServiceTest, testRetryAfterDestroyed) { auto executorProvider = std::make_shared(1); - AtomicSharedPtr serviceInfo( - ServiceInfo{binaryLookupUrl, AuthFactory::Disabled(), std::nullopt}); + AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); ConnectionPool pool(serviceInfo, {}, executorProvider, ""); - auto internalLookupService = - std::make_shared("pulsar://localhost:6650", pool, ClientConfiguration{}); + auto internalLookupService = std::make_shared(ServiceInfo{"pulsar://localhost:6650"}, + pool, ClientConfiguration{}); auto lookupService = RetryableLookupService::create(internalLookupService, std::chrono::seconds(30), executorProvider); From 529fa96103ea49b79d386204cf5b9d9c8daf6213 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 19:03:40 +0800 Subject: [PATCH 17/28] remove ambiguous methods from ClientConfiguration --- include/pulsar/ClientConfiguration.h | 32 ++++--------------------- include/pulsar/c/client_configuration.h | 7 ------ lib/ClientConfiguration.cc | 13 ---------- lib/ClientConfigurationImpl.h | 7 ++++++ lib/ClientImpl.cc | 8 ++----- lib/HTTPLookupService.cc | 2 +- lib/c/c_ClientConfiguration.cc | 12 ---------- tests/BasicEndToEndTest.cc | 3 ++- 8 files changed, 17 insertions(+), 67 deletions(-) diff --git a/include/pulsar/ClientConfiguration.h b/include/pulsar/ClientConfiguration.h index 712f468c..b37b7c6a 100644 --- a/include/pulsar/ClientConfiguration.h +++ b/include/pulsar/ClientConfiguration.h @@ -70,16 +70,12 @@ class PULSAR_PUBLIC ClientConfiguration { /** * Set the authentication method to be used with the broker * + * You can get the configured authentication data in `ServiceInfo` returned by `Client::getServiceInfo`. + * * @param authentication the authentication data to use */ ClientConfiguration& setAuth(const AuthenticationPtr& authentication); - /** - * @return the authentication data - * @deprecated the actual authentication data is now stored in `ServiceInfo` - */ - Authentication& getAuth() const; - /** * Set timeout on client operations (subscribe, create producer, close, unsubscribe) * Default is 30 seconds. @@ -203,21 +199,6 @@ class PULSAR_PUBLIC ClientConfiguration { */ ClientConfiguration& setLogger(LoggerFactory* loggerFactory); - /** - * Configure whether to use the TLS encryption on the connections. - * - * The default value is false. - * - * @param useTls - */ - ClientConfiguration& setUseTls(bool useTls); - - /** - * @return whether the TLS encryption is used on the connections - * @deprecated the TLS usage is now determined by the scheme of the `ServiceInfo::serviceUrl` - */ - bool isUseTls() const; - /** * Set the path to the TLS private key file. * @@ -245,16 +226,13 @@ class PULSAR_PUBLIC ClientConfiguration { /** * Set the path to the trusted TLS certificate file. * + * You can get the configured trusted TLS certificate file path in `ServiceInfo` returned by + * `Client::getServiceInfo`. + * * @param tlsTrustCertsFilePath */ ClientConfiguration& setTlsTrustCertsFilePath(const std::string& tlsTrustCertsFilePath); - /** - * @return the path to the trusted TLS certificate file - * @deprecated the trusted TLS certificate file path is now stored in `ServiceInfo` - */ - const std::string& getTlsTrustCertsFilePath() const; - /** * Configure whether the Pulsar client accepts untrusted TLS certificates from brokers. * diff --git a/include/pulsar/c/client_configuration.h b/include/pulsar/c/client_configuration.h index 9e154530..1be7c1f4 100644 --- a/include/pulsar/c/client_configuration.h +++ b/include/pulsar/c/client_configuration.h @@ -147,16 +147,9 @@ PULSAR_PUBLIC void pulsar_client_configuration_set_logger(pulsar_client_configur PULSAR_PUBLIC void pulsar_client_configuration_set_logger_t(pulsar_client_configuration_t *conf, pulsar_logger_t logger); -PULSAR_PUBLIC void pulsar_client_configuration_set_use_tls(pulsar_client_configuration_t *conf, int useTls); - -PULSAR_PUBLIC int pulsar_client_configuration_is_use_tls(pulsar_client_configuration_t *conf); - PULSAR_PUBLIC void pulsar_client_configuration_set_tls_trust_certs_file_path( pulsar_client_configuration_t *conf, const char *tlsTrustCertsFilePath); -PULSAR_PUBLIC const char *pulsar_client_configuration_get_tls_trust_certs_file_path( - pulsar_client_configuration_t *conf); - PULSAR_PUBLIC void pulsar_client_configuration_set_tls_allow_insecure_connection( pulsar_client_configuration_t *conf, int allowInsecure); diff --git a/lib/ClientConfiguration.cc b/lib/ClientConfiguration.cc index b99c5d25..c59dd43b 100644 --- a/lib/ClientConfiguration.cc +++ b/lib/ClientConfiguration.cc @@ -57,8 +57,6 @@ ClientConfiguration& ClientConfiguration::setAuth(const AuthenticationPtr& authe return *this; } -Authentication& ClientConfiguration::getAuth() const { return *impl_->authenticationPtr; } - const AuthenticationPtr& ClientConfiguration::getAuthPtr() const { return impl_->authenticationPtr; } ClientConfiguration& ClientConfiguration::setOperationTimeoutSeconds(int timeout) { @@ -94,13 +92,6 @@ ClientConfiguration& ClientConfiguration::setMessageListenerThreads(int threads) int ClientConfiguration::getMessageListenerThreads() const { return impl_->messageListenerThreads; } -ClientConfiguration& ClientConfiguration::setUseTls(bool useTls) { - impl_->useTls = useTls; - return *this; -} - -bool ClientConfiguration::isUseTls() const { return impl_->useTls; } - ClientConfiguration& ClientConfiguration::setValidateHostName(bool validateHostName) { impl_->validateHostName = validateHostName; return *this; @@ -131,10 +122,6 @@ ClientConfiguration& ClientConfiguration::setTlsTrustCertsFilePath(const std::st return *this; } -const std::string& ClientConfiguration::getTlsTrustCertsFilePath() const { - return impl_->tlsTrustCertsFilePath; -} - ClientConfiguration& ClientConfiguration::setTlsAllowInsecureConnection(bool allowInsecure) { impl_->tlsAllowInsecureConnection = allowInsecure; return *this; diff --git a/lib/ClientConfigurationImpl.h b/lib/ClientConfigurationImpl.h index e7a83a19..45b2aa3b 100644 --- a/lib/ClientConfigurationImpl.h +++ b/lib/ClientConfigurationImpl.h @@ -20,8 +20,10 @@ #define LIB_CLIENTCONFIGURATIONIMPL_H_ #include +#include #include +#include namespace pulsar { @@ -53,6 +55,11 @@ struct ClientConfigurationImpl { ClientConfiguration::ProxyProtocol proxyProtocol; std::unique_ptr takeLogger() { return std::move(loggerFactory); } + + ServiceInfo toServiceInfo(const std::string& serviceUrl) const { + return {serviceUrl, authenticationPtr, + tlsTrustCertsFilePath.empty() ? std::nullopt : std::make_optional(tlsTrustCertsFilePath)}; + } }; } // namespace pulsar diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 18390439..dfb5ee08 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -98,12 +98,8 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& LookupServiceFactory&& lookupServiceFactory) : mutex_(), state_(Open), - clientConfiguration_(ClientConfiguration(clientConfiguration) - .setUseTls(ServiceNameResolver::useTls(ServiceURI(serviceUrl)))), - serviceInfo_(ServiceInfo{serviceUrl, clientConfiguration.getAuthPtr(), - clientConfiguration.getTlsTrustCertsFilePath().empty() - ? std::nullopt - : std::make_optional(clientConfiguration.getTlsTrustCertsFilePath())}), + clientConfiguration_(clientConfiguration), + serviceInfo_(clientConfiguration.impl_->toServiceInfo(serviceUrl)), memoryLimitController_(clientConfiguration.getMemoryLimit()), ioExecutorProvider_(std::make_shared(clientConfiguration_.getIOThreads())), listenerExecutorProvider_( diff --git a/lib/HTTPLookupService.cc b/lib/HTTPLookupService.cc index b83b9aff..0be9713c 100644 --- a/lib/HTTPLookupService.cc +++ b/lib/HTTPLookupService.cc @@ -57,7 +57,7 @@ HTTPLookupService::HTTPLookupService(const ServiceInfo &serviceInfo, tlsPrivateFilePath_(clientConfiguration.getTlsPrivateKeyFilePath()), tlsCertificateFilePath_(clientConfiguration.getTlsCertificateFilePath()), tlsTrustCertsFilePath_(serviceInfo.tlsTrustCertsFilePath().value_or("")), - isUseTls_(clientConfiguration.isUseTls()), + isUseTls_(serviceInfo.useTls()), tlsAllowInsecure_(clientConfiguration.isTlsAllowInsecureConnection()), tlsValidateHostname_(clientConfiguration.isValidateHostName()) {} diff --git a/lib/c/c_ClientConfiguration.cc b/lib/c/c_ClientConfiguration.cc index 483c74ef..6b11a2aa 100644 --- a/lib/c/c_ClientConfiguration.cc +++ b/lib/c/c_ClientConfiguration.cc @@ -115,14 +115,6 @@ void pulsar_client_configuration_set_logger_t(pulsar_client_configuration_t *con conf->conf.setLogger(new PulsarCLoggerFactory(logger)); } -void pulsar_client_configuration_set_use_tls(pulsar_client_configuration_t *conf, int useTls) { - conf->conf.setUseTls(useTls); -} - -int pulsar_client_configuration_is_use_tls(pulsar_client_configuration_t *conf) { - return conf->conf.isUseTls(); -} - void pulsar_client_configuration_set_validate_hostname(pulsar_client_configuration_t *conf, int validateHostName) { conf->conf.setValidateHostName(validateHostName); @@ -155,10 +147,6 @@ void pulsar_client_configuration_set_tls_trust_certs_file_path(pulsar_client_con conf->conf.setTlsTrustCertsFilePath(tlsTrustCertsFilePath); } -const char *pulsar_client_configuration_get_tls_trust_certs_file_path(pulsar_client_configuration_t *conf) { - return conf->conf.getTlsTrustCertsFilePath().c_str(); -} - void pulsar_client_configuration_set_tls_allow_insecure_connection(pulsar_client_configuration_t *conf, int allowInsecure) { conf->conf.setTlsAllowInsecureConnection(allowInsecure); diff --git a/tests/BasicEndToEndTest.cc b/tests/BasicEndToEndTest.cc index 9a02df0c..da5e9785 100644 --- a/tests/BasicEndToEndTest.cc +++ b/tests/BasicEndToEndTest.cc @@ -700,7 +700,8 @@ TEST(BasicEndToEndTest, testConfigurationFile) { ClientConfiguration config2 = config1; AuthenticationDataPtr authData; - ASSERT_EQ(ResultOk, config1.getAuth().getAuthData(authData)); + Client client(lookupUrl, config1); + ASSERT_EQ(ResultOk, client.getServiceInfo().authentication()->getAuthData(authData)); ASSERT_EQ(100, config2.getOperationTimeoutSeconds()); ASSERT_EQ(10, config2.getIOThreads()); ASSERT_EQ(1, config2.getMessageListenerThreads()); From 4adb04a659306649fd57928347b83b7b92d24cb6 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 22:18:00 +0800 Subject: [PATCH 18/28] replace the updateServiceInfo method with the ServiceInfoProvider interface --- include/pulsar/Client.h | 25 ++-- include/pulsar/ServiceInfo.h | 4 + include/pulsar/ServiceInfoProvider.h | 52 ++++++++ lib/AtomicSharedPtr.h | 2 - lib/Client.cc | 17 ++- lib/ClientImpl.cc | 36 +++-- lib/ClientImpl.h | 10 +- lib/DefaultServiceUrlProvider.h | 42 ++++++ tests/ClientTest.cc | 79 ----------- tests/LookupServiceTest.cc | 18 ++- tests/ServiceInfoProviderTest.cc | 188 +++++++++++++++++++++++++++ 11 files changed, 361 insertions(+), 112 deletions(-) create mode 100644 include/pulsar/ServiceInfoProvider.h create mode 100644 lib/DefaultServiceUrlProvider.h create mode 100644 tests/ServiceInfoProviderTest.cc diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index 7e2d10d0..377c9d4f 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -30,9 +30,11 @@ #include #include #include +#include #include #include +#include #include namespace pulsar { @@ -69,6 +71,19 @@ class PULSAR_PUBLIC Client { */ Client(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration); + /** + * Create a Pulsar client object using the specified ServiceInfoProvider. + * + * The ServiceInfoProvider is responsible for providing the service information (such as service URL) + * dynamically. For example, if it detects a primary Pulsar service is down, it can switch to a secondary + * service and update the client with the new service information. + * + * When `close` is called, the client will call `ServiceInfoProvider::close` to guarantee the lifetime of + * the provider is properly managed. + */ + static Client create(std::unique_ptr serviceInfoProvider, + const ClientConfiguration& clientConfiguration); + /** * Create a producer with default configuration * @@ -415,16 +430,6 @@ class PULSAR_PUBLIC Client { void getSchemaInfoAsync(const std::string& topic, int64_t version, std::function callback); - /** - * Update the service information of the client. - * - * This method is used to switch the connection to a different Pulsar cluster. All connections will be - * closed and the internal service info will be updated. - * - * @param serviceInfo the service URL, authentication and TLS trust certs file path to use - */ - void updateServiceInfo(ServiceInfo serviceInfo); - /** * Get the current service information of the client. * diff --git a/include/pulsar/ServiceInfo.h b/include/pulsar/ServiceInfo.h index 8d0291a8..1f63ce38 100644 --- a/include/pulsar/ServiceInfo.h +++ b/include/pulsar/ServiceInfo.h @@ -26,6 +26,10 @@ namespace pulsar { +/** + * ServiceInfo encapsulates the information of a Pulsar service, which is used by the client to connect to the + * service. It includes the service URL, authentication information, and TLS configuration. + */ class PULSAR_PUBLIC ServiceInfo final { public: ServiceInfo(std::string serviceUrl, AuthenticationPtr authentication = AuthFactory::Disabled(), diff --git a/include/pulsar/ServiceInfoProvider.h b/include/pulsar/ServiceInfoProvider.h new file mode 100644 index 00000000..b12c69ae --- /dev/null +++ b/include/pulsar/ServiceInfoProvider.h @@ -0,0 +1,52 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#ifndef PULSAR_SERVICE_INFO_PROVIDER_H_ +#define PULSAR_SERVICE_INFO_PROVIDER_H_ + +#include + +#include + +namespace pulsar { + +class Client; + +class PULSAR_PUBLIC ServiceInfoProvider { + public: + /** + * The destructor will be called when `Client::close()` is invoked, and the provider should stop any + * ongoing work and release the resources in the destructor. + */ + virtual ~ServiceInfoProvider() = default; + + /** + * Initialize the ServiceInfoProvider. + * + * @param client the reference to the client, which could be used by the provider to do some necessary + * @param onServiceInfoUpdate the callback to update `client` with the new `ServiceInfo` + * + * Note: the implementation is responsible to invoke `onServiceInfoUpdate` at least once to provide the + * initial `ServiceInfo` for the client. + */ + virtual void initialize(Client& client, std::function onServiceInfoUpdate) = 0; +}; + +}; // namespace pulsar + +#endif diff --git a/lib/AtomicSharedPtr.h b/lib/AtomicSharedPtr.h index 824a1d61..53a6df22 100644 --- a/lib/AtomicSharedPtr.h +++ b/lib/AtomicSharedPtr.h @@ -27,8 +27,6 @@ class AtomicSharedPtr { public: using Pointer = std::shared_ptr; - explicit AtomicSharedPtr(T&& value) : ptr_(std::make_shared(std::move(value))) {} - auto load() const { return std::atomic_load(&ptr_); } void store(Pointer&& newPtr) { std::atomic_store(&ptr_, std::move(newPtr)); } diff --git a/lib/Client.cc b/lib/Client.cc index ba829b2b..651b1054 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -17,6 +17,7 @@ * under the License. */ #include +#include #include #include @@ -35,11 +36,19 @@ namespace pulsar { Client::Client(const std::shared_ptr& impl) : impl_(impl) {} -Client::Client(const std::string& serviceUrl) - : impl_(std::make_shared(serviceUrl, ClientConfiguration())) {} +Client::Client(const std::string& serviceUrl) : Client(serviceUrl, ClientConfiguration()) {} Client::Client(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration) - : impl_(std::make_shared(serviceUrl, clientConfiguration)) {} + : impl_(std::make_shared(serviceUrl, clientConfiguration)) { + impl_->initialize(*this); +} + +Client Client::create(std::unique_ptr serviceInfoProvider, + const ClientConfiguration& clientConfiguration) { + Client client(std::make_shared(std::move(serviceInfoProvider), clientConfiguration)); + client.impl_->initialize(client); + return client; +} Result Client::createProducer(const std::string& topic, Producer& producer) { return createProducer(topic, ProducerConfiguration(), producer); @@ -197,8 +206,6 @@ void Client::getSchemaInfoAsync(const std::string& topic, int64_t version, .addListener(std::move(callback)); } -void Client::updateServiceInfo(ServiceInfo serviceInfo) { impl_->updateServiceInfo(std::move(serviceInfo)); } - ServiceInfo Client::getServiceInfo() const { return impl_->getServiceInfo(); } } // namespace pulsar diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index dfb5ee08..246457d8 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -34,6 +34,7 @@ #include "Commands.h" #include "ConsumerImpl.h" #include "ConsumerInterceptors.h" +#include "DefaultServiceUrlProvider.h" #include "ExecutorService.h" #include "HTTPLookupService.h" #include "LogUtils.h" @@ -96,10 +97,20 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration, LookupServiceFactory&& lookupServiceFactory) - : mutex_(), + : ClientImpl(std::make_unique(std::cref(serviceUrl), + std::cref(*clientConfiguration.impl_)), + clientConfiguration, std::move(lookupServiceFactory)) {} + +ClientImpl::ClientImpl(std::unique_ptr serviceInfoProvider, + const ClientConfiguration& clientConfiguration) + : ClientImpl(std::move(serviceInfoProvider), clientConfiguration, &defaultLookupServiceFactory) {} + +ClientImpl::ClientImpl(std::unique_ptr serviceInfoProvider, + const ClientConfiguration& clientConfiguration, + LookupServiceFactory&& lookupServiceFactory) + : serviceInfoProvider_(std::move(serviceInfoProvider)), state_(Open), clientConfiguration_(clientConfiguration), - serviceInfo_(clientConfiguration.impl_->toServiceInfo(serviceUrl)), memoryLimitController_(clientConfiguration.getMemoryLimit()), ioExecutorProvider_(std::make_shared(clientConfiguration_.getIOThreads())), listenerExecutorProvider_( @@ -118,11 +129,15 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration& if (loggerFactory) { LogUtils::setLoggerFactory(std::move(loggerFactory)); } - lookupServicePtr_ = createLookup(*serviceInfo_.load()); } ClientImpl::~ClientImpl() { shutdown(); } +void ClientImpl::initialize(Client& client) { + serviceInfoProvider_->initialize( + client, [this](ServiceInfo serviceInfo) { updateServiceInfo(std::move(serviceInfo)); }); +} + LookupServicePtr ClientImpl::createLookup(ServiceInfo serviceInfo) { auto lookupServicePtr = RetryableLookupService::create( lookupServiceFactory_(std::move(serviceInfo), clientConfiguration_, pool_), @@ -669,6 +684,7 @@ void ClientImpl::getPartitionsForTopicAsync(const std::string& topic, const GetP } void ClientImpl::closeAsync(const CloseCallback& callback) { + serviceInfoProvider_.reset(); std::unique_lock lock(mutex_); if (state_ != Open) { lock.unlock(); @@ -745,8 +761,8 @@ void ClientImpl::handleClose(Result result, const SharedInt& numberOfOpenHandler } LOG_DEBUG("Shutting down producers and consumers for client"); - // handleClose() is called in ExecutorService's event loop, while shutdown() tried to wait the event - // loop exits. So here we use another thread to call shutdown(). + // handleClose() is called in ExecutorService's event loop, while shutdown() tried to wait the + // event loop exits. So here we use another thread to call shutdown(). auto self = shared_from_this(); std::thread shutdownTask{[this, self, callback] { shutdown(); @@ -794,9 +810,9 @@ void ClientImpl::shutdown() { } LOG_DEBUG("ConnectionPool is closed"); - // 500ms as the timeout is long enough because ExecutorService::close calls io_service::stop() internally - // and waits until io_service::run() in another thread returns, which should be as soon as possible after - // stop() is called. + // 500ms as the timeout is long enough because ExecutorService::close calls io_service::stop() + // internally and waits until io_service::run() in another thread returns, which should be as soon as + // possible after stop() is called. TimeoutProcessor timeoutProcessor{500}; timeoutProcessor.tik(); @@ -868,7 +884,9 @@ void ClientImpl::updateServiceInfo(ServiceInfo&& serviceInfo) { serviceInfo_.store(std::make_shared(serviceInfo)); pool_.closeAllConnectionsForNewCluster(); - lookupServicePtr_->close(); + if (lookupServicePtr_) { + lookupServicePtr_->close(); + } lookupServicePtr_ = createLookup(serviceInfo); for (auto&& it : redirectedClusterLookupServicePtrs_) { diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 7b71a387..0812ef35 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -21,12 +21,15 @@ #include #include +#include #include #include #include +#include #include +#include "AtomicSharedPtr.h" #include "ConnectionPool.h" #include "Future.h" #include "LookupDataResult.h" @@ -34,7 +37,6 @@ #include "MemoryLimitController.h" #include "ProtoApiEnums.h" #include "SynchronizedHashMap.h" -#include "lib/AtomicSharedPtr.h" namespace pulsar { @@ -73,12 +75,17 @@ std::string generateRandomName(); class ClientImpl : public std::enable_shared_from_this { public: + ClientImpl(std::unique_ptr serviceInfoProvider, + const ClientConfiguration& clientConfiguration); + ClientImpl(std::unique_ptr serviceInfoProvider, + const ClientConfiguration& clientConfiguration, LookupServiceFactory&& lookupServiceFactory); ClientImpl(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration); // only for tests ClientImpl(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration, LookupServiceFactory&& lookupServiceFactory); + void initialize(Client& client); virtual ~ClientImpl(); /** @@ -219,6 +226,7 @@ class ClientImpl : public std::enable_shared_from_this { Closed }; + std::unique_ptr serviceInfoProvider_; mutable std::shared_mutex mutex_; State state_; diff --git a/lib/DefaultServiceUrlProvider.h b/lib/DefaultServiceUrlProvider.h new file mode 100644 index 00000000..775c5747 --- /dev/null +++ b/lib/DefaultServiceUrlProvider.h @@ -0,0 +1,42 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#pragma once + +#include + +#include + +#include "ClientConfigurationImpl.h" + +namespace pulsar { + +class DefaultServiceUrlProvider : public ServiceInfoProvider { + public: + DefaultServiceUrlProvider(const std::string& serviceUrl, const ClientConfigurationImpl& config) + : serviceInfo_(config.toServiceInfo(serviceUrl)) {} + + void initialize(Client& client, std::function onServiceInfoUpdate) override { + onServiceInfoUpdate(serviceInfo_); + } + + private: + ServiceInfo serviceInfo_; +}; + +} // namespace pulsar diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index c176b76c..1cc09d53 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -505,82 +505,3 @@ TEST(ClientTest, testNoRetry) { ASSERT_TRUE(result.timeMs < 1000) << "consumer: " << result.timeMs << " ms"; } } - -TEST(ClientTest, testUpdateServiceInfo) { - extern std::string getToken(); // from tests/AuthToken.cc - - // Access "private/auth" namespace in cluster 1 - ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken())}; - // Access "private/auth" namespace in cluster 2 - ServiceInfo info2{"pulsar+ssl://localhost:6653", - AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), - TEST_CONF_DIR "/hn-verification/cacert.pem"}; - // Access "public/default" namespace in cluster 1, which doesn't require authentication - ServiceInfo info3{"pulsar://localhost:6650"}; - - Client client{info1.serviceUrl(), ClientConfiguration().setAuth(info1.authentication())}; - - const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); - Producer producer; - ASSERT_EQ(ResultOk, client.createProducer(topicRequiredAuth, producer)); - - Reader reader; - ASSERT_EQ(ResultOk, client.createReader(topicRequiredAuth, MessageId::earliest(), {}, reader)); - - auto sendAndReceive = [&](const std::string &value) { - MessageId msgId; - ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent(value).build(), msgId)); - LOG_INFO("Sent " << value << " to " << msgId); - - Message msg; - ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); - LOG_INFO("Read " << msg.getDataAsString() << " from " << msgId); - ASSERT_EQ(value, msg.getDataAsString()); - }; - - sendAndReceive("msg-0"); - - // Switch to cluster 2 (started by ./build-support/start-mim-test-service-inside-container.sh) - ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); - client.updateServiceInfo(info2); - ASSERT_TRUE(PulsarFriend::getConnections(client).empty()); - ASSERT_EQ(info2, client.getServiceInfo()); - - // Now the same will access the same topic in cluster 2 - sendAndReceive("msg-1"); - - // Switch back to cluster 1 without any authentication, the previous authentication info configured for - // cluster 2 will be cleared. - client.updateServiceInfo(info3); - ASSERT_EQ(info3, client.getServiceInfo()); - - const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); - producer.close(); - ASSERT_EQ(ResultOk, client.createProducer(topicNoAuth, producer)); - ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-2").build())); - - client.close(); - - // Verify messages sent to cluster 1 and cluster 2 can be consumed successfully with correct - // authentication info. - auto verify = [](Client &client, const std::string &topic, const std::string &value) { - Reader reader; - ASSERT_EQ(ResultOk, client.createReader(topic, MessageId::earliest(), {}, reader)); - Message msg; - ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); - ASSERT_EQ(value, msg.getDataAsString()); - }; - Client client1{info1.serviceUrl(), ClientConfiguration().setAuth(info1.authentication())}; - verify(client1, topicRequiredAuth, "msg-0"); - client1.close(); - - Client client2{info2.serviceUrl(), ClientConfiguration() - .setAuth(info2.authentication()) - .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath())}; - verify(client2, topicRequiredAuth, "msg-1"); - client2.close(); - - Client client3{info3.serviceUrl()}; - verify(client3, topicNoAuth, "msg-2"); - client3.close(); -} diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc index db22a34c..fc0f8836 100644 --- a/tests/LookupServiceTest.cc +++ b/tests/LookupServiceTest.cc @@ -84,7 +84,8 @@ TEST(LookupServiceTest, basicLookup) { std::string url = "pulsar://localhost:6650"; ClientConfiguration conf; ExecutorServiceProviderPtr ioExecutorProvider_(std::make_shared(1)); - AtomicSharedPtr serviceInfo(ServiceInfo{url}); + AtomicSharedPtr serviceInfo; + serviceInfo.store(std::make_shared(url)); ConnectionPool pool_(serviceInfo, conf, ioExecutorProvider_, ""); BinaryProtoLookupService lookupService(url, pool_, conf); @@ -148,7 +149,8 @@ static void testMultiAddresses(LookupService& lookupService) { } TEST(LookupServiceTest, testMultiAddresses) { - AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); + AtomicSharedPtr serviceInfo; + serviceInfo.store(std::make_shared(binaryLookupUrl)); ConnectionPool pool(serviceInfo, {}, std::make_shared(1), ""); ClientConfiguration conf; BinaryProtoLookupService binaryLookupService(ServiceInfo{"pulsar://localhost,localhost:9999"}, pool, @@ -162,7 +164,8 @@ TEST(LookupServiceTest, testMultiAddresses) { } TEST(LookupServiceTest, testRetry) { auto executorProvider = std::make_shared(1); - AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); + AtomicSharedPtr serviceInfo; + serviceInfo.store(std::make_shared(binaryLookupUrl)); ConnectionPool pool(serviceInfo, {}, executorProvider, ""); ClientConfiguration conf; @@ -198,7 +201,8 @@ TEST(LookupServiceTest, testRetry) { TEST(LookupServiceTest, testTimeout) { auto executorProvider = std::make_shared(1); - AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); + AtomicSharedPtr serviceInfo; + serviceInfo.store(std::make_shared(binaryLookupUrl)); ConnectionPool pool(serviceInfo, {}, executorProvider, ""); ClientConfiguration conf; @@ -479,7 +483,8 @@ TEST(LookupServiceTest, testRedirectionLimit) { ClientConfiguration conf; conf.setMaxLookupRedirects(redirect_limit); ExecutorServiceProviderPtr ioExecutorProvider_(std::make_shared(1)); - AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); + AtomicSharedPtr serviceInfo; + serviceInfo.store(std::make_shared(binaryLookupUrl)); ConnectionPool pool_(serviceInfo, conf, ioExecutorProvider_, ""); const ServiceInfo lookupServiceInfo{"pulsar://localhost:6650"}; BinaryProtoLookupServiceRedirectTestHelper lookupService(lookupServiceInfo, pool_, conf); @@ -544,7 +549,8 @@ TEST(LookupServiceTest, testAfterClientShutdown) { TEST(LookupServiceTest, testRetryAfterDestroyed) { auto executorProvider = std::make_shared(1); - AtomicSharedPtr serviceInfo(ServiceInfo{binaryLookupUrl}); + AtomicSharedPtr serviceInfo; + serviceInfo.store(std::make_shared(binaryLookupUrl)); ConnectionPool pool(serviceInfo, {}, executorProvider, ""); auto internalLookupService = std::make_shared(ServiceInfo{"pulsar://localhost:6650"}, diff --git a/tests/ServiceInfoProviderTest.cc b/tests/ServiceInfoProviderTest.cc new file mode 100644 index 00000000..264e5247 --- /dev/null +++ b/tests/ServiceInfoProviderTest.cc @@ -0,0 +1,188 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "PulsarFriend.h" +#include "WaitUtils.h" +#include "lib/LogUtils.h" + +DECLARE_LOG_OBJECT() + +using namespace pulsar; +using namespace std::chrono_literals; + +class ServiceInfoQueue { + public: + void push(ServiceInfo info) { + { + std::lock_guard lock(mutex_); + queue_.push(std::move(info)); + } + cond_.notify_all(); + } + + ServiceInfo pop() { + std::unique_lock lock(mutex_); + cond_.wait(lock, [this] { return !queue_.empty() || !running_; }); + if (queue_.empty()) { + throw std::runtime_error("Queue is closed"); + } + + ServiceInfo info = std::move(queue_.front()); + queue_.pop(); + return info; + } + + void close() { + running_ = false; + cond_.notify_all(); + } + + private: + mutable std::mutex mutex_; + mutable std::condition_variable cond_; + std::queue queue_; + std::atomic_bool running_{true}; +}; + +class TestServiceInfoProvider : public ServiceInfoProvider { + public: + TestServiceInfoProvider(ServiceInfoQueue &queue) : queue_(queue) {} + + void initialize(Client &, std::function onServiceInfoUpdate) override { + onServiceInfoUpdate(queue_.pop()); + thread_ = std::thread([this, onServiceInfoUpdate] { + try { + while (true) { + ServiceInfo info = queue_.pop(); + onServiceInfoUpdate(std::move(info)); + } + } catch (const std::runtime_error &) { + } + }); + } + + ~TestServiceInfoProvider() override { + queue_.close(); + if (thread_.joinable()) { + thread_.join(); + } + } + + private: + ServiceInfoQueue &queue_; + + std::thread thread_; + mutable std::mutex mutex_; + std::queue newServiceInfo_; +}; + +TEST(ServiceInfoProviderTest, testSwitchCluster) { + extern std::string getToken(); // from tests/AuthToken.cc + // Access "private/auth" namespace in cluster 1 + ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken())}; + // Access "private/auth" namespace in cluster 2 + ServiceInfo info2{"pulsar+ssl://localhost:6653", + AuthTls::create(TEST_CONF_DIR "/client-cert.pem", TEST_CONF_DIR "/client-key.pem"), + TEST_CONF_DIR "/hn-verification/cacert.pem"}; + // Access "public/default" namespace in cluster 1, which doesn't require authentication + ServiceInfo info3{"pulsar://localhost:6650"}; + + ServiceInfoQueue queue; + queue.push(info1); + + auto client = Client::create(std::make_unique(queue), {}); + + const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); + Producer producer; + ASSERT_EQ(ResultOk, client.createProducer(topicRequiredAuth, producer)); + + Reader reader; + ASSERT_EQ(ResultOk, client.createReader(topicRequiredAuth, MessageId::earliest(), {}, reader)); + + auto sendAndReceive = [&](const std::string &value) { + MessageId msgId; + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent(value).build(), msgId)); + LOG_INFO("Sent " << value << " to " << msgId); + + Message msg; + ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); + LOG_INFO("Read " << msg.getDataAsString() << " from " << msgId); + ASSERT_EQ(value, msg.getDataAsString()); + }; + + sendAndReceive("msg-0"); + + // Switch to cluster 2 (started by ./build-support/start-mim-test-service-inside-container.sh) + ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); + queue.push(info2); + ASSERT_TRUE(waitUntil(1s, [&] { + return PulsarFriend::getConnections(client).empty() && client.getServiceInfo() == info2; + })); + + // Now the same will access the same topic in cluster 2 + sendAndReceive("msg-1"); + + // Switch back to cluster 1 without any authentication, the previous authentication info configured for + // cluster 2 will be cleared. + ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); + queue.push(info3); + ASSERT_TRUE(waitUntil(1s, [&] { + return PulsarFriend::getConnections(client).empty() && client.getServiceInfo() == info3; + })); + + const auto topicNoAuth = "testUpdateConnectionInfo-" + std::to_string(time(nullptr)); + producer.close(); + ASSERT_EQ(ResultOk, client.createProducer(topicNoAuth, producer)); + ASSERT_EQ(ResultOk, producer.send(MessageBuilder().setContent("msg-2").build())); + + client.close(); + + // Verify messages sent to cluster 1 and cluster 2 can be consumed successfully with correct + // authentication info. + auto verify = [](Client &client, const std::string &topic, const std::string &value) { + Reader reader; + ASSERT_EQ(ResultOk, client.createReader(topic, MessageId::earliest(), {}, reader)); + Message msg; + ASSERT_EQ(ResultOk, reader.readNext(msg, 3000)); + ASSERT_EQ(value, msg.getDataAsString()); + }; + Client client1{info1.serviceUrl(), ClientConfiguration().setAuth(info1.authentication())}; + verify(client1, topicRequiredAuth, "msg-0"); + client1.close(); + + Client client2{info2.serviceUrl(), ClientConfiguration() + .setAuth(info2.authentication()) + .setTlsTrustCertsFilePath(*info2.tlsTrustCertsFilePath())}; + verify(client2, topicRequiredAuth, "msg-1"); + client2.close(); + + Client client3{info3.serviceUrl()}; + verify(client3, topicNoAuth, "msg-2"); + client3.close(); +} From a238057fd12c8f058e875173e22ee1d4a047136f Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 22:21:04 +0800 Subject: [PATCH 19/28] revert unnecessary change --- lib/ClientImpl.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 246457d8..35e8ade6 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -810,9 +810,9 @@ void ClientImpl::shutdown() { } LOG_DEBUG("ConnectionPool is closed"); - // 500ms as the timeout is long enough because ExecutorService::close calls io_service::stop() - // internally and waits until io_service::run() in another thread returns, which should be as soon as - // possible after stop() is called. + // 500ms as the timeout is long enough because ExecutorService::close calls io_service::stop() internally + // and waits until io_service::run() in another thread returns, which should be as soon as possible after + // stop() is called. TimeoutProcessor timeoutProcessor{500}; timeoutProcessor.tik(); From d8c5b27a90d949178db0bf73ce47c724e7a83f73 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 6 Mar 2026 22:29:16 +0800 Subject: [PATCH 20/28] fix perf build --- perf/PerfConsumer.cc | 1 - perf/PerfProducer.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/perf/PerfConsumer.cc b/perf/PerfConsumer.cc index 7a707a10..ab5d58cc 100644 --- a/perf/PerfConsumer.cc +++ b/perf/PerfConsumer.cc @@ -155,7 +155,6 @@ void handleSubscribe(Result result, Consumer consumer, Latch latch) { void startPerfConsumer(const Arguments& args) { ClientConfiguration conf; - conf.setUseTls(args.isUseTls); conf.setTlsAllowInsecureConnection(args.isTlsAllowInsecureConnection); if (!args.tlsTrustCertsFilePath.empty()) { std::string tlsTrustCertsFilePath(args.tlsTrustCertsFilePath); diff --git a/perf/PerfProducer.cc b/perf/PerfProducer.cc index 17e70cdf..30796b95 100644 --- a/perf/PerfProducer.cc +++ b/perf/PerfProducer.cc @@ -366,7 +366,6 @@ int main(int argc, char** argv) { pulsar::ClientConfiguration conf; conf.setConnectionsPerBroker(args.connectionsPerBroker); conf.setMemoryLimit(args.memoryLimitMb * 1024 * 1024); - conf.setUseTls(args.isUseTls); conf.setTlsAllowInsecureConnection(args.isTlsAllowInsecureConnection); if (!args.tlsTrustCertsFilePath.empty()) { std::string tlsTrustCertsFilePath(args.tlsTrustCertsFilePath); From e5f9cca59514b977d4b6a5a173687ddaed652f02 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Sat, 7 Mar 2026 18:35:00 +0800 Subject: [PATCH 21/28] fix clang-tidy checks and remove Client from the ServiceInfoProvider interface --- include/pulsar/ServiceInfoProvider.h | 5 +---- lib/Client.cc | 4 ++-- lib/ClientConnection.cc | 2 +- lib/ClientConnection.h | 2 +- lib/ClientImpl.cc | 4 ++-- lib/ClientImpl.h | 2 +- lib/DefaultServiceUrlProvider.h | 2 +- tests/ServiceInfoProviderTest.cc | 2 +- 8 files changed, 10 insertions(+), 13 deletions(-) diff --git a/include/pulsar/ServiceInfoProvider.h b/include/pulsar/ServiceInfoProvider.h index b12c69ae..80df84a7 100644 --- a/include/pulsar/ServiceInfoProvider.h +++ b/include/pulsar/ServiceInfoProvider.h @@ -25,8 +25,6 @@ namespace pulsar { -class Client; - class PULSAR_PUBLIC ServiceInfoProvider { public: /** @@ -38,13 +36,12 @@ class PULSAR_PUBLIC ServiceInfoProvider { /** * Initialize the ServiceInfoProvider. * - * @param client the reference to the client, which could be used by the provider to do some necessary * @param onServiceInfoUpdate the callback to update `client` with the new `ServiceInfo` * * Note: the implementation is responsible to invoke `onServiceInfoUpdate` at least once to provide the * initial `ServiceInfo` for the client. */ - virtual void initialize(Client& client, std::function onServiceInfoUpdate) = 0; + virtual void initialize(std::function onServiceInfoUpdate) = 0; }; }; // namespace pulsar diff --git a/lib/Client.cc b/lib/Client.cc index 651b1054..05f3e3ee 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -40,13 +40,13 @@ Client::Client(const std::string& serviceUrl) : Client(serviceUrl, ClientConfigu Client::Client(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration) : impl_(std::make_shared(serviceUrl, clientConfiguration)) { - impl_->initialize(*this); + impl_->initialize(); } Client Client::create(std::unique_ptr serviceInfoProvider, const ClientConfiguration& clientConfiguration) { Client client(std::make_shared(std::move(serviceInfoProvider), clientConfiguration)); - client.impl_->initialize(client); + client.impl_->initialize(); return client; } diff --git a/lib/ClientConnection.cc b/lib/ClientConnection.cc index bf71b5ea..39535469 100644 --- a/lib/ClientConnection.cc +++ b/lib/ClientConnection.cc @@ -185,7 +185,7 @@ std::atomic ClientConnection::maxMessageSize_{Commands::DefaultMaxMessa // NOTE: we should get the connection info from `serviceInfo` rather than `clientConfiguration` because it can // be modified dynamically. ClientConnection::ClientConnection(const std::string& logicalAddress, const std::string& physicalAddress, - ServiceInfo serviceInfo, const ExecutorServicePtr& executor, + const ServiceInfo& serviceInfo, const ExecutorServicePtr& executor, const ClientConfiguration& clientConfiguration, const std::string& clientVersion, ConnectionPool& pool, size_t poolIndex) : operationsTimeout_(ClientImpl::getOperationTimeout(clientConfiguration)), diff --git a/lib/ClientConnection.h b/lib/ClientConnection.h index 31b2ceca..7cc2695d 100644 --- a/lib/ClientConnection.h +++ b/lib/ClientConnection.h @@ -142,7 +142,7 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_this serviceInfoProvider, ClientImpl::~ClientImpl() { shutdown(); } -void ClientImpl::initialize(Client& client) { +void ClientImpl::initialize() { serviceInfoProvider_->initialize( - client, [this](ServiceInfo serviceInfo) { updateServiceInfo(std::move(serviceInfo)); }); + [this](ServiceInfo serviceInfo) { updateServiceInfo(std::move(serviceInfo)); }); } LookupServicePtr ClientImpl::createLookup(ServiceInfo serviceInfo) { diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index 0812ef35..cc9c66db 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -85,7 +85,7 @@ class ClientImpl : public std::enable_shared_from_this { ClientImpl(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration, LookupServiceFactory&& lookupServiceFactory); - void initialize(Client& client); + void initialize(); virtual ~ClientImpl(); /** diff --git a/lib/DefaultServiceUrlProvider.h b/lib/DefaultServiceUrlProvider.h index 775c5747..93c9b091 100644 --- a/lib/DefaultServiceUrlProvider.h +++ b/lib/DefaultServiceUrlProvider.h @@ -31,7 +31,7 @@ class DefaultServiceUrlProvider : public ServiceInfoProvider { DefaultServiceUrlProvider(const std::string& serviceUrl, const ClientConfigurationImpl& config) : serviceInfo_(config.toServiceInfo(serviceUrl)) {} - void initialize(Client& client, std::function onServiceInfoUpdate) override { + void initialize(std::function onServiceInfoUpdate) override { onServiceInfoUpdate(serviceInfo_); } diff --git a/tests/ServiceInfoProviderTest.cc b/tests/ServiceInfoProviderTest.cc index 264e5247..c829ea82 100644 --- a/tests/ServiceInfoProviderTest.cc +++ b/tests/ServiceInfoProviderTest.cc @@ -74,7 +74,7 @@ class TestServiceInfoProvider : public ServiceInfoProvider { public: TestServiceInfoProvider(ServiceInfoQueue &queue) : queue_(queue) {} - void initialize(Client &, std::function onServiceInfoUpdate) override { + void initialize(std::function onServiceInfoUpdate) override { onServiceInfoUpdate(queue_.pop()); thread_ = std::thread([this, onServiceInfoUpdate] { try { From cdb21282382e9fe48fa9601e221c212b38c65c19 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Sun, 8 Mar 2026 23:22:08 +0800 Subject: [PATCH 22/28] fix LookupServiceTest.testAfterClientShutdown --- lib/Client.cc | 10 +++------- tests/LookupServiceTest.cc | 15 ++++++++------- tests/PulsarFriend.h | 2 ++ 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/Client.cc b/lib/Client.cc index 05f3e3ee..48e92dda 100644 --- a/lib/Client.cc +++ b/lib/Client.cc @@ -34,20 +34,16 @@ DECLARE_LOG_OBJECT() namespace pulsar { -Client::Client(const std::shared_ptr& impl) : impl_(impl) {} +Client::Client(const std::shared_ptr& impl) : impl_(impl) { impl_->initialize(); } Client::Client(const std::string& serviceUrl) : Client(serviceUrl, ClientConfiguration()) {} Client::Client(const std::string& serviceUrl, const ClientConfiguration& clientConfiguration) - : impl_(std::make_shared(serviceUrl, clientConfiguration)) { - impl_->initialize(); -} + : Client(std::make_shared(serviceUrl, clientConfiguration)) {} Client Client::create(std::unique_ptr serviceInfoProvider, const ClientConfiguration& clientConfiguration) { - Client client(std::make_shared(std::move(serviceInfoProvider), clientConfiguration)); - client.impl_->initialize(); - return client; + return Client(std::make_shared(std::move(serviceInfoProvider), clientConfiguration)); } Result Client::createProducer(const std::string& topic, Producer& producer) { diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc index fc0f8836..376ed87e 100644 --- a/tests/LookupServiceTest.cc +++ b/tests/LookupServiceTest.cc @@ -527,23 +527,24 @@ class MockLookupService : public BinaryProtoLookupService { }; TEST(LookupServiceTest, testAfterClientShutdown) { - auto client = std::make_shared( + auto client = PulsarFriend::newClientFromImpl(std::make_shared( "pulsar://localhost:6650", ClientConfiguration{}, [](const ServiceInfo& serviceInfo, const ClientConfiguration&, ConnectionPool& pool) { return std::make_shared(serviceInfo, pool, ClientConfiguration{}); - }); + })); + std::promise promise; - client->subscribeAsync("lookup-service-test-after-client-shutdown", "sub", ConsumerConfiguration{}, - [&promise](Result result, const Consumer&) { promise.set_value(result); }); + client.subscribeAsync("lookup-service-test-after-client-shutdown", "sub", ConsumerConfiguration{}, + [&promise](Result result, const Consumer&) { promise.set_value(result); }); // When shutdown is called, there is a pending lookup request due to the 1st lookup is failed in // MockLookupService. Verify shutdown will cancel it and return ResultDisconnected. - client->shutdown(); + client.shutdown(); EXPECT_EQ(ResultDisconnected, promise.get_future().get()); // A new subscribeAsync call will fail immediately in the current thread Result result = ResultOk; - client->subscribeAsync("lookup-service-test-retry-after-destroyed", "sub", ConsumerConfiguration{}, - [&result](Result innerResult, const Consumer&) { result = innerResult; }); + client.subscribeAsync("lookup-service-test-retry-after-destroyed", "sub", ConsumerConfiguration{}, + [&result](Result innerResult, const Consumer&) { result = innerResult; }); EXPECT_EQ(ResultAlreadyClosed, result); } diff --git a/tests/PulsarFriend.h b/tests/PulsarFriend.h index e7084050..09edd6f7 100644 --- a/tests/PulsarFriend.h +++ b/tests/PulsarFriend.h @@ -124,6 +124,8 @@ class PulsarFriend { static std::shared_ptr getClientImplPtr(Client client) { return client.impl_; } + static Client newClientFromImpl(const ClientImplPtr& impl) { return Client{impl}; } + static auto getProducers(const Client& client) -> decltype(ClientImpl::producers_)& { return getClientImplPtr(client)->producers_; } From b2bc7d738fc36e6ab172fd17ed2dc83712cd0124 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Mon, 9 Mar 2026 11:00:07 +0800 Subject: [PATCH 23/28] fix other tests --- include/pulsar/ServiceInfoProvider.h | 7 +++++++ lib/ClientImpl.cc | 11 +++++++++-- lib/DefaultServiceUrlProvider.h | 6 +++--- tests/ServiceInfoProviderTest.cc | 3 ++- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/include/pulsar/ServiceInfoProvider.h b/include/pulsar/ServiceInfoProvider.h index 80df84a7..f854bc17 100644 --- a/include/pulsar/ServiceInfoProvider.h +++ b/include/pulsar/ServiceInfoProvider.h @@ -33,6 +33,13 @@ class PULSAR_PUBLIC ServiceInfoProvider { */ virtual ~ServiceInfoProvider() = default; + /** + * Get the current `ServiceInfo` connection for the client. + * This method is called **only once** internally when the `Client` is being initialized, and the client + * will use the returned `ServiceInfo` to establish the initial connection to the Pulsar cluster. + */ + virtual ServiceInfo getServiceInfo() const = 0; + /** * Initialize the ServiceInfoProvider. * diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index 2a327cac..a84263a1 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -129,13 +129,20 @@ ClientImpl::ClientImpl(std::unique_ptr serviceInfoProvider, if (loggerFactory) { LogUtils::setLoggerFactory(std::move(loggerFactory)); } + + serviceInfo_.store(std::make_shared(serviceInfoProvider_->getServiceInfo())); + lookupServicePtr_ = createLookup(*serviceInfo_.load()); } ClientImpl::~ClientImpl() { shutdown(); } void ClientImpl::initialize() { - serviceInfoProvider_->initialize( - [this](ServiceInfo serviceInfo) { updateServiceInfo(std::move(serviceInfo)); }); + auto weakSelf = weak_from_this(); + serviceInfoProvider_->initialize([weakSelf](ServiceInfo serviceInfo) { + if (auto self = weakSelf.lock()) { + self->updateServiceInfo(std::move(serviceInfo)); + } + }); } LookupServicePtr ClientImpl::createLookup(ServiceInfo serviceInfo) { diff --git a/lib/DefaultServiceUrlProvider.h b/lib/DefaultServiceUrlProvider.h index 93c9b091..ac0baf12 100644 --- a/lib/DefaultServiceUrlProvider.h +++ b/lib/DefaultServiceUrlProvider.h @@ -31,9 +31,9 @@ class DefaultServiceUrlProvider : public ServiceInfoProvider { DefaultServiceUrlProvider(const std::string& serviceUrl, const ClientConfigurationImpl& config) : serviceInfo_(config.toServiceInfo(serviceUrl)) {} - void initialize(std::function onServiceInfoUpdate) override { - onServiceInfoUpdate(serviceInfo_); - } + ServiceInfo getServiceInfo() const override { return serviceInfo_; } + + void initialize(std::function onServiceInfoUpdate) override {} private: ServiceInfo serviceInfo_; diff --git a/tests/ServiceInfoProviderTest.cc b/tests/ServiceInfoProviderTest.cc index c829ea82..9d969366 100644 --- a/tests/ServiceInfoProviderTest.cc +++ b/tests/ServiceInfoProviderTest.cc @@ -74,6 +74,8 @@ class TestServiceInfoProvider : public ServiceInfoProvider { public: TestServiceInfoProvider(ServiceInfoQueue &queue) : queue_(queue) {} + ServiceInfo getServiceInfo() const override { return queue_.pop(); } + void initialize(std::function onServiceInfoUpdate) override { onServiceInfoUpdate(queue_.pop()); thread_ = std::thread([this, onServiceInfoUpdate] { @@ -99,7 +101,6 @@ class TestServiceInfoProvider : public ServiceInfoProvider { std::thread thread_; mutable std::mutex mutex_; - std::queue newServiceInfo_; }; TEST(ServiceInfoProviderTest, testSwitchCluster) { From b91860ae42c15c3a5b58b4cf11ba99f2fe0b1596 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Mon, 9 Mar 2026 11:14:31 +0800 Subject: [PATCH 24/28] revert unnecessary changes --- tests/LookupServiceTest.cc | 14 +++++++------- tests/PulsarFriend.h | 2 -- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/LookupServiceTest.cc b/tests/LookupServiceTest.cc index 376ed87e..53cb76e1 100644 --- a/tests/LookupServiceTest.cc +++ b/tests/LookupServiceTest.cc @@ -527,24 +527,24 @@ class MockLookupService : public BinaryProtoLookupService { }; TEST(LookupServiceTest, testAfterClientShutdown) { - auto client = PulsarFriend::newClientFromImpl(std::make_shared( + auto client = std::make_shared( "pulsar://localhost:6650", ClientConfiguration{}, [](const ServiceInfo& serviceInfo, const ClientConfiguration&, ConnectionPool& pool) { return std::make_shared(serviceInfo, pool, ClientConfiguration{}); - })); + }); std::promise promise; - client.subscribeAsync("lookup-service-test-after-client-shutdown", "sub", ConsumerConfiguration{}, - [&promise](Result result, const Consumer&) { promise.set_value(result); }); + client->subscribeAsync("lookup-service-test-after-client-shutdown", "sub", ConsumerConfiguration{}, + [&promise](Result result, const Consumer&) { promise.set_value(result); }); // When shutdown is called, there is a pending lookup request due to the 1st lookup is failed in // MockLookupService. Verify shutdown will cancel it and return ResultDisconnected. - client.shutdown(); + client->shutdown(); EXPECT_EQ(ResultDisconnected, promise.get_future().get()); // A new subscribeAsync call will fail immediately in the current thread Result result = ResultOk; - client.subscribeAsync("lookup-service-test-retry-after-destroyed", "sub", ConsumerConfiguration{}, - [&result](Result innerResult, const Consumer&) { result = innerResult; }); + client->subscribeAsync("lookup-service-test-retry-after-destroyed", "sub", ConsumerConfiguration{}, + [&result](Result innerResult, const Consumer&) { result = innerResult; }); EXPECT_EQ(ResultAlreadyClosed, result); } diff --git a/tests/PulsarFriend.h b/tests/PulsarFriend.h index 09edd6f7..e7084050 100644 --- a/tests/PulsarFriend.h +++ b/tests/PulsarFriend.h @@ -124,8 +124,6 @@ class PulsarFriend { static std::shared_ptr getClientImplPtr(Client client) { return client.impl_; } - static Client newClientFromImpl(const ClientImplPtr& impl) { return Client{impl}; } - static auto getProducers(const Client& client) -> decltype(ClientImpl::producers_)& { return getClientImplPtr(client)->producers_; } From 1144c2759ae9190cbea22a834288f5fdefe3bf8c Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Mon, 9 Mar 2026 22:18:12 +0800 Subject: [PATCH 25/28] clean up implementation --- include/pulsar/ServiceInfoProvider.h | 12 ++--- lib/AtomicSharedPtr.h | 3 ++ lib/ClientConnection.cc | 2 - lib/ClientImpl.cc | 6 +-- lib/DefaultServiceUrlProvider.h | 2 +- tests/ClientTest.cc | 3 +- tests/ServiceInfoProviderTest.cc | 74 ++++++++++++---------------- 7 files changed, 46 insertions(+), 56 deletions(-) diff --git a/include/pulsar/ServiceInfoProvider.h b/include/pulsar/ServiceInfoProvider.h index f854bc17..4b68fe9b 100644 --- a/include/pulsar/ServiceInfoProvider.h +++ b/include/pulsar/ServiceInfoProvider.h @@ -19,10 +19,9 @@ #ifndef PULSAR_SERVICE_INFO_PROVIDER_H_ #define PULSAR_SERVICE_INFO_PROVIDER_H_ +#include #include -#include - namespace pulsar { class PULSAR_PUBLIC ServiceInfoProvider { @@ -34,11 +33,12 @@ class PULSAR_PUBLIC ServiceInfoProvider { virtual ~ServiceInfoProvider() = default; /** - * Get the current `ServiceInfo` connection for the client. - * This method is called **only once** internally when the `Client` is being initialized, and the client - * will use the returned `ServiceInfo` to establish the initial connection to the Pulsar cluster. + * Get the initial `ServiceInfo` connection for the client. + * This method is called **only once** internally in `Client::create()` to get the initial `ServiceInfo` + * for the client to connect to the Pulsar service. Since it's only called once, it's legal to return a + * moved `ServiceInfo` object to avoid unnecessary copying. */ - virtual ServiceInfo getServiceInfo() const = 0; + virtual ServiceInfo initialServiceInfo() = 0; /** * Initialize the ServiceInfoProvider. diff --git a/lib/AtomicSharedPtr.h b/lib/AtomicSharedPtr.h index 53a6df22..30dea074 100644 --- a/lib/AtomicSharedPtr.h +++ b/lib/AtomicSharedPtr.h @@ -27,6 +27,9 @@ class AtomicSharedPtr { public: using Pointer = std::shared_ptr; + AtomicSharedPtr() = default; + explicit AtomicSharedPtr(T value) : ptr_(std::make_shared(std::move(value))) {} + auto load() const { return std::atomic_load(&ptr_); } void store(Pointer&& newPtr) { std::atomic_store(&ptr_, std::move(newPtr)); } diff --git a/lib/ClientConnection.cc b/lib/ClientConnection.cc index 39535469..43b545f9 100644 --- a/lib/ClientConnection.cc +++ b/lib/ClientConnection.cc @@ -182,8 +182,6 @@ static bool file_exists(const std::string& path) { std::atomic ClientConnection::maxMessageSize_{Commands::DefaultMaxMessageSize}; -// NOTE: we should get the connection info from `serviceInfo` rather than `clientConfiguration` because it can -// be modified dynamically. ClientConnection::ClientConnection(const std::string& logicalAddress, const std::string& physicalAddress, const ServiceInfo& serviceInfo, const ExecutorServicePtr& executor, const ClientConfiguration& clientConfiguration, diff --git a/lib/ClientImpl.cc b/lib/ClientImpl.cc index a84263a1..95424699 100644 --- a/lib/ClientImpl.cc +++ b/lib/ClientImpl.cc @@ -111,6 +111,7 @@ ClientImpl::ClientImpl(std::unique_ptr serviceInfoProvider, : serviceInfoProvider_(std::move(serviceInfoProvider)), state_(Open), clientConfiguration_(clientConfiguration), + serviceInfo_(serviceInfoProvider_->initialServiceInfo()), memoryLimitController_(clientConfiguration.getMemoryLimit()), ioExecutorProvider_(std::make_shared(clientConfiguration_.getIOThreads())), listenerExecutorProvider_( @@ -130,7 +131,6 @@ ClientImpl::ClientImpl(std::unique_ptr serviceInfoProvider, LogUtils::setLoggerFactory(std::move(loggerFactory)); } - serviceInfo_.store(std::make_shared(serviceInfoProvider_->getServiceInfo())); lookupServicePtr_ = createLookup(*serviceInfo_.load()); } @@ -768,8 +768,8 @@ void ClientImpl::handleClose(Result result, const SharedInt& numberOfOpenHandler } LOG_DEBUG("Shutting down producers and consumers for client"); - // handleClose() is called in ExecutorService's event loop, while shutdown() tried to wait the - // event loop exits. So here we use another thread to call shutdown(). + // handleClose() is called in ExecutorService's event loop, while shutdown() tried to wait the event + // loop exits. So here we use another thread to call shutdown(). auto self = shared_from_this(); std::thread shutdownTask{[this, self, callback] { shutdown(); diff --git a/lib/DefaultServiceUrlProvider.h b/lib/DefaultServiceUrlProvider.h index ac0baf12..917a7575 100644 --- a/lib/DefaultServiceUrlProvider.h +++ b/lib/DefaultServiceUrlProvider.h @@ -31,7 +31,7 @@ class DefaultServiceUrlProvider : public ServiceInfoProvider { DefaultServiceUrlProvider(const std::string& serviceUrl, const ClientConfigurationImpl& config) : serviceInfo_(config.toServiceInfo(serviceUrl)) {} - ServiceInfo getServiceInfo() const override { return serviceInfo_; } + ServiceInfo initialServiceInfo() override { return std::move(serviceInfo_); } void initialize(std::function onServiceInfoUpdate) override {} diff --git a/tests/ClientTest.cc b/tests/ClientTest.cc index 1cc09d53..dd892686 100644 --- a/tests/ClientTest.cc +++ b/tests/ClientTest.cc @@ -17,13 +17,13 @@ * under the License. */ #include -#include #include #include #include #include #include +#include #include "MockClientImpl.h" #include "PulsarAdminHelper.h" @@ -32,6 +32,7 @@ #include "lib/ClientConnection.h" #include "lib/LogUtils.h" #include "lib/checksum/ChecksumProvider.h" +#include "lib/stats/ProducerStatsImpl.h" DECLARE_LOG_OBJECT() diff --git a/tests/ServiceInfoProviderTest.cc b/tests/ServiceInfoProviderTest.cc index 9d969366..aedc24d4 100644 --- a/tests/ServiceInfoProviderTest.cc +++ b/tests/ServiceInfoProviderTest.cc @@ -20,11 +20,9 @@ #include #include -#include #include #include -#include -#include +#include #include #include "PulsarFriend.h" @@ -36,70 +34,62 @@ DECLARE_LOG_OBJECT() using namespace pulsar; using namespace std::chrono_literals; -class ServiceInfoQueue { +class ServiceInfoHolder { public: - void push(ServiceInfo info) { - { - std::lock_guard lock(mutex_); - queue_.push(std::move(info)); - } - cond_.notify_all(); - } + ServiceInfoHolder(ServiceInfo info) : serviceInfo_(std::move(info)) {} - ServiceInfo pop() { - std::unique_lock lock(mutex_); - cond_.wait(lock, [this] { return !queue_.empty() || !running_; }); - if (queue_.empty()) { - throw std::runtime_error("Queue is closed"); + std::optional getUpdatedValue() { + std::lock_guard lock(mutex_); + if (!owned_) { + return std::nullopt; } - - ServiceInfo info = std::move(queue_.front()); - queue_.pop(); - return info; + owned_ = false; + return std::move(serviceInfo_); } - void close() { - running_ = false; - cond_.notify_all(); + void updateValue(ServiceInfo info) { + std::lock_guard lock(mutex_); + serviceInfo_ = std::move(info); + owned_ = true; } private: + ServiceInfo serviceInfo_; + bool owned_{true}; + mutable std::mutex mutex_; - mutable std::condition_variable cond_; - std::queue queue_; - std::atomic_bool running_{true}; }; class TestServiceInfoProvider : public ServiceInfoProvider { public: - TestServiceInfoProvider(ServiceInfoQueue &queue) : queue_(queue) {} + TestServiceInfoProvider(ServiceInfoHolder &serviceInfo) : serviceInfo_(serviceInfo) {} - ServiceInfo getServiceInfo() const override { return queue_.pop(); } + ServiceInfo initialServiceInfo() override { return serviceInfo_.getUpdatedValue().value(); } void initialize(std::function onServiceInfoUpdate) override { - onServiceInfoUpdate(queue_.pop()); thread_ = std::thread([this, onServiceInfoUpdate] { - try { - while (true) { - ServiceInfo info = queue_.pop(); - onServiceInfoUpdate(std::move(info)); + while (running_) { + auto updatedValue = serviceInfo_.getUpdatedValue(); + if (updatedValue) { + onServiceInfoUpdate(std::move(*updatedValue)); } - } catch (const std::runtime_error &) { + // Use a tight wait loop for tests + std::this_thread::sleep_for(10ms); } }); } ~TestServiceInfoProvider() override { - queue_.close(); + running_ = false; if (thread_.joinable()) { thread_.join(); } } private: - ServiceInfoQueue &queue_; - std::thread thread_; + ServiceInfoHolder &serviceInfo_; + std::atomic_bool running_{true}; mutable std::mutex mutex_; }; @@ -114,10 +104,8 @@ TEST(ServiceInfoProviderTest, testSwitchCluster) { // Access "public/default" namespace in cluster 1, which doesn't require authentication ServiceInfo info3{"pulsar://localhost:6650"}; - ServiceInfoQueue queue; - queue.push(info1); - - auto client = Client::create(std::make_unique(queue), {}); + ServiceInfoHolder serviceInfo{info1}; + auto client = Client::create(std::make_unique(serviceInfo), {}); const auto topicRequiredAuth = "private/auth/testUpdateConnectionInfo-" + std::to_string(time(nullptr)); Producer producer; @@ -141,7 +129,7 @@ TEST(ServiceInfoProviderTest, testSwitchCluster) { // Switch to cluster 2 (started by ./build-support/start-mim-test-service-inside-container.sh) ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); - queue.push(info2); + serviceInfo.updateValue(info2); ASSERT_TRUE(waitUntil(1s, [&] { return PulsarFriend::getConnections(client).empty() && client.getServiceInfo() == info2; })); @@ -152,7 +140,7 @@ TEST(ServiceInfoProviderTest, testSwitchCluster) { // Switch back to cluster 1 without any authentication, the previous authentication info configured for // cluster 2 will be cleared. ASSERT_FALSE(PulsarFriend::getConnections(client).empty()); - queue.push(info3); + serviceInfo.updateValue(info3); ASSERT_TRUE(waitUntil(1s, [&] { return PulsarFriend::getConnections(client).empty() && client.getServiceInfo() == info3; })); From 5285efdae95c4ef9bf0541f27442a3547aa50c94 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 12 Mar 2026 17:17:22 +0800 Subject: [PATCH 26/28] address comments --- include/pulsar/Client.h | 5 +++-- include/pulsar/ServiceInfoProvider.h | 18 ++++++++++++------ lib/ClientConnection.h | 4 +++- lib/ClientImpl.cc | 6 +++--- lib/ClientImpl.h | 2 +- ...Provider.h => DefaultServiceInfoProvider.h} | 4 ++-- perf/PerfConsumer.cc | 4 ---- perf/PerfProducer.cc | 4 ---- tests/ServiceInfoProviderTest.cc | 2 +- 9 files changed, 25 insertions(+), 24 deletions(-) rename lib/{DefaultServiceUrlProvider.h => DefaultServiceInfoProvider.h} (88%) diff --git a/include/pulsar/Client.h b/include/pulsar/Client.h index 377c9d4f..e9813e3d 100644 --- a/include/pulsar/Client.h +++ b/include/pulsar/Client.h @@ -78,8 +78,9 @@ class PULSAR_PUBLIC Client { * dynamically. For example, if it detects a primary Pulsar service is down, it can switch to a secondary * service and update the client with the new service information. * - * When `close` is called, the client will call `ServiceInfoProvider::close` to guarantee the lifetime of - * the provider is properly managed. + * The Client instance takes ownership of the given ServiceInfoProvider. The provider will be destroyed + * as part of the client's shutdown lifecycle, for example when `Client::close()` or + * `Client::closeAsync()` is called, ensuring that its lifetime is properly managed. */ static Client create(std::unique_ptr serviceInfoProvider, const ClientConfiguration& clientConfiguration); diff --git a/include/pulsar/ServiceInfoProvider.h b/include/pulsar/ServiceInfoProvider.h index 4b68fe9b..1b518da5 100644 --- a/include/pulsar/ServiceInfoProvider.h +++ b/include/pulsar/ServiceInfoProvider.h @@ -19,9 +19,10 @@ #ifndef PULSAR_SERVICE_INFO_PROVIDER_H_ #define PULSAR_SERVICE_INFO_PROVIDER_H_ -#include #include +#include + namespace pulsar { class PULSAR_PUBLIC ServiceInfoProvider { @@ -35,18 +36,23 @@ class PULSAR_PUBLIC ServiceInfoProvider { /** * Get the initial `ServiceInfo` connection for the client. * This method is called **only once** internally in `Client::create()` to get the initial `ServiceInfo` - * for the client to connect to the Pulsar service. Since it's only called once, it's legal to return a - * moved `ServiceInfo` object to avoid unnecessary copying. + * for the client to connect to the Pulsar service, typically before {@link initialize} is invoked. + * Since it's only called once, it's legal to return a moved `ServiceInfo` object to avoid unnecessary + * copying. */ virtual ServiceInfo initialServiceInfo() = 0; /** * Initialize the ServiceInfoProvider. * - * @param onServiceInfoUpdate the callback to update `client` with the new `ServiceInfo` + * After the client has obtained the initial `ServiceInfo` via {@link initialServiceInfo}, this method is + * called to allow the provider to start any background work (for example, service discovery or watching + * configuration changes) and to report subsequent updates to the service information. + * + * @param onServiceInfoUpdate the callback to deliver updated `ServiceInfo` values to the client after + * the initial connection has been established * - * Note: the implementation is responsible to invoke `onServiceInfoUpdate` at least once to provide the - * initial `ServiceInfo` for the client. + * Implementations may choose not to invoke `onServiceInfoUpdate` if the `ServiceInfo` never changes. */ virtual void initialize(std::function onServiceInfoUpdate) = 0; }; diff --git a/lib/ClientConnection.h b/lib/ClientConnection.h index 7cc2695d..8779778e 100644 --- a/lib/ClientConnection.h +++ b/lib/ClientConnection.h @@ -157,7 +157,9 @@ class PULSAR_PUBLIC ClientConnection : public std::enable_shared_from_this(std::cref(serviceUrl), - std::cref(*clientConfiguration.impl_)), + : ClientImpl(std::make_unique(std::cref(serviceUrl), + std::cref(*clientConfiguration.impl_)), clientConfiguration, std::move(lookupServiceFactory)) {} ClientImpl::ClientImpl(std::unique_ptr serviceInfoProvider, diff --git a/lib/ClientImpl.h b/lib/ClientImpl.h index cc9c66db..7772b15b 100644 --- a/lib/ClientImpl.h +++ b/lib/ClientImpl.h @@ -206,7 +206,7 @@ class ClientImpl : public std::enable_shared_from_this { const std::string& logicalAddress); // This overload is only used for blue-green migration, where only the service URL is modified, the other - // paramters remain the same + // parameters remain the same LookupServicePtr createRedirectedLookup(const std::string& redirectedUrl) { auto serviceInfo = serviceInfo_.load(); return createLookup( diff --git a/lib/DefaultServiceUrlProvider.h b/lib/DefaultServiceInfoProvider.h similarity index 88% rename from lib/DefaultServiceUrlProvider.h rename to lib/DefaultServiceInfoProvider.h index 917a7575..6479bf9a 100644 --- a/lib/DefaultServiceUrlProvider.h +++ b/lib/DefaultServiceInfoProvider.h @@ -26,9 +26,9 @@ namespace pulsar { -class DefaultServiceUrlProvider : public ServiceInfoProvider { +class DefaultServiceInfoProvider : public ServiceInfoProvider { public: - DefaultServiceUrlProvider(const std::string& serviceUrl, const ClientConfigurationImpl& config) + DefaultServiceInfoProvider(const std::string& serviceUrl, const ClientConfigurationImpl& config) : serviceInfo_(config.toServiceInfo(serviceUrl)) {} ServiceInfo initialServiceInfo() override { return std::move(serviceInfo_); } diff --git a/perf/PerfConsumer.cc b/perf/PerfConsumer.cc index ab5d58cc..88c4f5da 100644 --- a/perf/PerfConsumer.cc +++ b/perf/PerfConsumer.cc @@ -57,7 +57,6 @@ static int64_t currentTimeMillis() { struct Arguments { std::string authParams; std::string authPlugin; - bool isUseTls; bool isTlsAllowInsecureConnection; std::string tlsTrustCertsFilePath; std::string topic; @@ -261,9 +260,6 @@ int main(int argc, char** argv) { ("auth-plugin,a", po::value(&args.authPlugin)->default_value(""), "Authentication plugin class library path") // - ("use-tls,b", po::value(&args.isUseTls)->default_value(false), - "Whether tls connection is used") // - ("allow-insecure,d", po::value(&args.isTlsAllowInsecureConnection)->default_value(true), "Whether insecure tls connection is allowed") // diff --git a/perf/PerfProducer.cc b/perf/PerfProducer.cc index 30796b95..784c651a 100644 --- a/perf/PerfProducer.cc +++ b/perf/PerfProducer.cc @@ -47,7 +47,6 @@ typedef std::shared_ptr RateLimiterPtr; struct Arguments { std::string authParams; std::string authPlugin; - bool isUseTls; bool isTlsAllowInsecureConnection; std::string tlsTrustCertsFilePath; std::string topic; @@ -223,9 +222,6 @@ int main(int argc, char** argv) { ("auth-plugin,a", po::value(&args.authPlugin)->default_value(""), "Authentication plugin class library path") // - ("use-tls,b", po::value(&args.isUseTls)->default_value(false), - "Whether tls connection is used") // - ("allow-insecure,d", po::value(&args.isTlsAllowInsecureConnection)->default_value(true), "Whether insecure tls connection is allowed") // diff --git a/tests/ServiceInfoProviderTest.cc b/tests/ServiceInfoProviderTest.cc index aedc24d4..82f5f6f7 100644 --- a/tests/ServiceInfoProviderTest.cc +++ b/tests/ServiceInfoProviderTest.cc @@ -94,7 +94,7 @@ class TestServiceInfoProvider : public ServiceInfoProvider { }; TEST(ServiceInfoProviderTest, testSwitchCluster) { - extern std::string getToken(); // from tests/AuthToken.cc + extern std::string getToken(); // from tests/AuthTokenTest.cc // Access "private/auth" namespace in cluster 1 ServiceInfo info1{"pulsar://localhost:6650", AuthToken::createWithToken(getToken())}; // Access "private/auth" namespace in cluster 2 From b9c2d546b060e631b4406b4a1cfae3ce518f0b0c Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 12 Mar 2026 19:57:11 +0800 Subject: [PATCH 27/28] fix tests --- lib/ConnectionPool.cc | 24 ++++-------------------- lib/ConnectionPool.h | 7 +++++++ 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/lib/ConnectionPool.cc b/lib/ConnectionPool.cc index bc2c8393..6465ff77 100644 --- a/lib/ConnectionPool.cc +++ b/lib/ConnectionPool.cc @@ -55,19 +55,8 @@ bool ConnectionPool::close() { return false; } - std::vector connectionsToClose; - // ClientConnection::close() will remove the connection from the pool, which is not allowed when iterating - // over a map, so we store the connections to close in a vector first and don't iterate the pool when - // closing the connections. - std::unique_lock lock(mutex_); - connectionsToClose.reserve(pool_.size()); - for (auto&& kv : pool_) { - connectionsToClose.emplace_back(kv.second); - } - pool_.clear(); - lock.unlock(); - - for (auto&& cnx : connectionsToClose) { + for (auto&& kv : releaseConnections()) { + auto& cnx = kv.second; if (cnx) { // Close with a fatal error to not let client retry auto& future = cnx->close(ResultAlreadyClosed); @@ -96,14 +85,9 @@ bool ConnectionPool::close() { } void ConnectionPool::closeAllConnectionsForNewCluster() { - std::unique_lock lock(mutex_); - for (auto cnxIt = pool_.begin(); cnxIt != pool_.end(); cnxIt++) { - auto& cnx = cnxIt->second; - if (cnx) { - cnx->close(ResultDisconnected, true); - } + for (auto&& kv : releaseConnections()) { + kv.second->close(ResultDisconnected, true); } - pool_.clear(); } static const std::string getKey(const std::string& logicalAddress, const std::string& physicalAddress, diff --git a/lib/ConnectionPool.h b/lib/ConnectionPool.h index 27bf1913..f828ac69 100644 --- a/lib/ConnectionPool.h +++ b/lib/ConnectionPool.h @@ -110,6 +110,13 @@ class PULSAR_PUBLIC ConnectionPool { std::uniform_int_distribution<> randomDistribution_; std::mt19937 randomEngine_; + auto releaseConnections() { + decltype(pool_) pool; + std::lock_guard lock{mutex_}; + pool.swap(pool_); + return pool; + } + friend class PulsarFriend; }; } // namespace pulsar From 696d616491f64100610ebe49b3940e9eafb40735 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Fri, 13 Mar 2026 12:48:15 +0800 Subject: [PATCH 28/28] reset redirectedClusterURI --- lib/ConsumerImpl.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ConsumerImpl.cc b/lib/ConsumerImpl.cc index 71cf1d01..026f146d 100644 --- a/lib/ConsumerImpl.cc +++ b/lib/ConsumerImpl.cc @@ -1145,6 +1145,7 @@ void ConsumerImpl::onClusterSwitching() { seekStatus_ = SeekStatus::NOT_STARTED; lastSeekArg_.reset(); } + setRedirectedClusterURI(""); ackGroupingTrackerPtr_->flushAndClean(); }