Skip to content

Commit

Permalink
Fix extensible load manager test caused by Pulsar image upgrade
Browse files Browse the repository at this point in the history
### Motivation

The `apachepulsar/pulsar:latest` image was upgraded to 3.2.0 recently,
which includes the PIP-307.

### Modifications

Change the check to verify no new lookup operation after unload. Make
`getLookupCount()` visiable only when `BUILD_TESTS` is ON since this
method is only added for tests.
  • Loading branch information
BewareMyPower committed Feb 6, 2024
1 parent 1f94dd9 commit 5343251
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 5 deletions.
4 changes: 4 additions & 0 deletions lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ set_property(TARGET PULSAR_OBJECT_LIB PROPERTY POSITION_INDEPENDENT_CODE 1)
if (INTEGRATE_VCPKG)
target_link_libraries(PULSAR_OBJECT_LIB PROTO_OBJECTS)
endif ()
if (BUILD_TESTS)
target_compile_definitions(PULSAR_OBJECT_LIB PRIVATE BUILD_FOR_TESTS)
endif ()
target_include_directories(PULSAR_OBJECT_LIB PUBLIC
"${CMAKE_SOURCE_DIR}"
"${CMAKE_SOURCE_DIR}/include"
Expand Down Expand Up @@ -111,6 +114,7 @@ if (BUILD_STATIC_LIB)
endif()
set_property(TARGET pulsarStatic PROPERTY VERSION ${LIBRARY_VERSION})
target_compile_definitions(pulsarStatic PRIVATE PULSAR_STATIC)
target_compile_definitions(pulsarStatic PUBLIC BUILD_FOR_TESTS)
endif()

# When linking statically, install a libpulsar.a that contains all the
Expand Down
9 changes: 7 additions & 2 deletions lib/ClientImpl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ ClientImpl::ClientImpl(const std::string& serviceUrl, const ClientConfiguration&
producerIdGenerator_(0),
consumerIdGenerator_(0),
closingError(ResultOk),
useProxy_(false),
lookupCount_(0L) {
useProxy_(false) {
std::unique_ptr<LoggerFactory> loggerFactory = clientConfiguration_.impl_->takeLogger();
if (loggerFactory) {
LogUtils::setLoggerFactory(std::move(loggerFactory));
Expand Down Expand Up @@ -535,7 +534,9 @@ Future<Result, ClientConnectionPtr> ClientImpl::getConnection(const std::string&
return;
}
useProxy_ = data.proxyThroughServiceUrl;
#ifdef BUILD_FOR_TESTS
lookupCount_++;
#endif
pool_.getConnectionAsync(data.logicalAddress, data.physicalAddress, key)
.addListener([promise](Result result, const ClientConnectionWeakPtr& weakCnx) {
if (result == ResultOk) {
Expand Down Expand Up @@ -666,7 +667,9 @@ void ClientImpl::closeAsync(CloseCallback callback) {
if (*numberOfOpenHandlers == 0 && callback) {
handleClose(ResultOk, numberOfOpenHandlers, callback);
}
#ifdef BUILD_FOR_TESTS
lookupCount_ = 0;
#endif
}

void ClientImpl::handleClose(Result result, SharedInt numberOfOpenHandlers, ResultCallback callback) {
Expand Down Expand Up @@ -754,7 +757,9 @@ void ClientImpl::shutdown() {
partitionListenerExecutorProvider_->close(timeoutProcessor.getLeftTimeout());
timeoutProcessor.tok();
LOG_DEBUG("partitionListenerExecutorProvider_ is closed");
#ifdef BUILD_FOR_TESTS
lookupCount_ = 0;
#endif
}

uint64_t ClientImpl::newProducerId() {
Expand Down
6 changes: 5 additions & 1 deletion lib/ClientImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,9 @@ class ClientImpl : public std::enable_shared_from_this<ClientImpl> {
std::shared_ptr<std::atomic<uint64_t>> getRequestIdGenerator() const { return requestIdGenerator_; }

ConnectionPool& getConnectionPool() noexcept { return pool_; }
#ifdef BUILD_FOR_TESTS
uint64_t getLookupCount() { return lookupCount_; }
#endif

static std::chrono::nanoseconds getOperationTimeout(const ClientConfiguration& clientConfiguration);

Expand Down Expand Up @@ -197,7 +199,9 @@ class ClientImpl : public std::enable_shared_from_this<ClientImpl> {

std::atomic<Result> closingError;
std::atomic<bool> useProxy_;
std::atomic<uint64_t> lookupCount_;
#ifdef BUILD_FOR_TESTS
std::atomic<uint64_t> lookupCount_{0};
#endif

friend class Client;
};
Expand Down
3 changes: 1 addition & 2 deletions tests/extensibleLM/ExtensibleLoadManagerTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,8 @@ TEST(ExtensibleLoadManagerTest, testPubSubWhileUnloading) {
}));
LOG_INFO("after lookup responseData:" << responseDataAfterUnload << ",res:" << res);

// TODO: check lookup counter after pip-307 is released
auto lookupCountAfterUnload = clientImplPtr->getLookupCount();
ASSERT_TRUE(lookupCountBeforeUnload < lookupCountAfterUnload);
ASSERT_EQ(lookupCountBeforeUnload, lookupCountAfterUnload);
break;
}
};
Expand Down

0 comments on commit 5343251

Please sign in to comment.