diff --git a/src/rmq/rmqa/rmqa_rabbitcontextimpl.cpp b/src/rmq/rmqa/rmqa_rabbitcontextimpl.cpp index 3ffc5b6..9196923 100644 --- a/src/rmq/rmqa/rmqa_rabbitcontextimpl.cpp +++ b/src/rmq/rmqa/rmqa_rabbitcontextimpl.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -191,16 +192,21 @@ RabbitContextImpl::RabbitContextImpl( , d_producerTracing(options.producerTracing()) { - const bool isHostHealthMonitoringEnabled = - options.hostHealthConfig().has_value(); + // Host health monitoring runs only when a config has been selected. An + // explicit opt-out (HostHealthAwarenessOff) and an unset selection both + // leave no config, so no monitor is created -- preserving the behaviour + // from before an opt-out could be expressed: monitoring ran whenever a + // config was attached. get_if returns a pointer into the selection owned by + // `options` (which outlives this constructor), or null for the opt-out and + // unset alternatives. + const rmqt::HostHealthConfig* hostHealthConfig = + bsl::get_if(&options.hostHealthSelection()); + const bool isHostHealthMonitoringEnabled = hostHealthConfig != 0; // Host health monitoring enabled if (isHostHealthMonitoringEnabled) { - const rmqt::HostHealthConfig& hostHealthConfig = - *options.hostHealthConfig(); - d_hostHealthMonitor = bsl::make_shared( - hostHealthConfig, d_metricPublisher.get()); + *hostHealthConfig, d_metricPublisher.get()); d_hostHealthMonitor->start(d_eventLoop->timerFactory()); } diff --git a/src/rmq/rmqa/rmqa_rabbitcontextoptions.cpp b/src/rmq/rmqa/rmqa_rabbitcontextoptions.cpp index 8c1810a..ddceb29 100644 --- a/src/rmq/rmqa/rmqa_rabbitcontextoptions.cpp +++ b/src/rmq/rmqa/rmqa_rabbitcontextoptions.cpp @@ -62,18 +62,18 @@ void defaultErrorCallback(const bsl::string& err, int rc) } // namespace RabbitContextOptions::RabbitContextOptions() -: d_threadpool() -, d_onError(bdlf::BindUtil::bind(&defaultErrorCallback, +: d_onError(bdlf::BindUtil::bind(&defaultErrorCallback, bdlf::PlaceHolders::_1, bdlf::PlaceHolders::_2)) +, d_hostHealthSelection(HostHealthAwarenessUnset()) +, d_threadpool() , d_metricPublisher() -, d_clientProperties() , d_messageProcessingTimeout(DEFAULT_MESSAGE_PROCESSING_TIMEOUT) -, d_tunables() , d_connectionErrorThreshold() , d_connectionEstablishmentTimeout() +, d_clientProperties() +, d_tunables() , d_shuffleConnectionEndpoints() -, d_hostHealthConfig() { populateUsefulInformation(&d_clientProperties); } @@ -174,7 +174,15 @@ RabbitContextOptions& RabbitContextOptions::setShuffleConnectionEndpoints( RabbitContextOptions& RabbitContextOptions::setHostHealthConfig( const rmqt::HostHealthConfig& hostHealthConfig) { - d_hostHealthConfig = hostHealthConfig; + // A config is one alternative of the selection; delegate so the variant + // setter remains the single writer of d_hostHealthSelection. + return setHostHealthSelection(hostHealthConfig); +} + +RabbitContextOptions& RabbitContextOptions::setHostHealthSelection( + const HostHealthSelection& selection) +{ + d_hostHealthSelection = selection; return *this; } diff --git a/src/rmq/rmqa/rmqa_rabbitcontextoptions.h b/src/rmq/rmqa/rmqa_rabbitcontextoptions.h index d382e7f..20a1d6b 100644 --- a/src/rmq/rmqa/rmqa_rabbitcontextoptions.h +++ b/src/rmq/rmqa/rmqa_rabbitcontextoptions.h @@ -28,7 +28,9 @@ #include #include +#include #include +#include #include namespace BloombergLP { @@ -44,6 +46,30 @@ class RabbitContextOptions { public: typedef bsl::set Tunables; + /// \brief Tag type expressing no host health preference: it neither opts in + /// nor opts out, leaving the choice to enclosing configuration layers. This + /// is the alternative held by a default-constructed \c HostHealthSelection + /// (the default). See \c setHostHealthSelection. + struct HostHealthAwarenessUnset {}; + + /// \brief Tag type selecting an explicit opt-out from host health + /// awareness; see \c setHostHealthSelection. Selecting it is distinct from + /// leaving the host health selection unset (\c HostHealthAwarenessUnset), + /// which expresses no preference and lets enclosing configuration layers + /// apply their own policy. + struct HostHealthAwarenessOff {}; + + /// \brief A caller's host health selection: exactly one of three mutually + /// exclusive alternatives -- \c HostHealthAwarenessUnset (no preference; + /// the default), \c rmqt::HostHealthConfig (opt-in; monitoring runs), or + /// \c HostHealthAwarenessOff (explicit opt-out). A default-constructed + /// selection holds \c HostHealthAwarenessUnset. See + /// \c setHostHealthSelection. + typedef bsl::variant + HostHealthSelection; + /// \brief By Default RabbitContext will /// 1) Create it's own threadpool for /// calling back to client code e.g. consuming messages, confirming @@ -177,9 +203,25 @@ class RabbitContextOptions { /// config is not set, \c consumeOnlyFromHealthyHost has no effect. /// /// \param hostHealthConfig configuration for host health monitoring + /// + /// \note This is a convenience equivalent to \c setHostHealthSelection with + /// a \c HostHealthConfig: selecting a config is what causes the monitor to + /// run, and it replaces any prior opt-out selection. Leaving the selection + /// unset (the default) expresses no preference. RabbitContextOptions& setHostHealthConfig(const rmqt::HostHealthConfig& hostHealthConfig); + /// \brief Set the caller's host health selection: a \c HostHealthConfig to + /// opt in (monitoring runs) or \c HostHealthAwarenessOff to explicitly + /// opt out (no monitor is created even if a config could otherwise be + /// attached). The alternatives are mutually exclusive -- this replaces any + /// previously set config or opt-out. Not calling this at all leaves the + /// selection unset (the default), which expresses no preference and lets + /// enclosing configuration layers apply their own policy. + /// \param selection the host health selection + RabbitContextOptions& + setHostHealthSelection(const HostHealthSelection& selection); + bdlmt::ThreadPool* threadpool() const { return d_threadpool; } const bsl::shared_ptr& metricPublisher() const @@ -227,12 +269,15 @@ class RabbitContextOptions { return d_shuffleConnectionEndpoints; } - /// \brief Get the host health config. If not set, host health monitoring is - /// disabled. By default, host health monitoring is disabled. - /// \return The host health config - const bsl::optional& hostHealthConfig() const + /// \brief Get the caller's host health selection. The returned variant + /// holds \c HostHealthAwarenessUnset when no preference has been expressed + /// (the default), a \c rmqt::HostHealthConfig when opted in (via + /// \c setHostHealthSelection or \c setHostHealthConfig), or a + /// \c HostHealthAwarenessOff when explicitly opted out. + /// \return The host health selection + const HostHealthSelection& hostHealthSelection() const { - return d_hostHealthConfig; + return d_hostHealthSelection; } #ifdef USES_LIBRMQ_EXPERIMENTAL_FEATURES @@ -240,19 +285,22 @@ class RabbitContextOptions { #endif private: + // Members are ordered to minimize padding (see + // clang-analyzer-optin.performance.Padding), not by logical grouping. The + // constructor's initializer list mirrors this order to avoid -Wreorder. static const int DEFAULT_MESSAGE_PROCESSING_TIMEOUT = 60; - bdlmt::ThreadPool* d_threadpool; rmqt::ErrorCallback d_onError; + HostHealthSelection d_hostHealthSelection; + bdlmt::ThreadPool* d_threadpool; bsl::shared_ptr d_metricPublisher; - rmqt::FieldTable d_clientProperties; bsls::TimeInterval d_messageProcessingTimeout; - rmqt::Tunables d_tunables; - bsl::optional d_connectionErrorThreshold; - bsl::optional d_connectionEstablishmentTimeout; bsl::shared_ptr d_consumerTracing; bsl::shared_ptr d_producerTracing; + bsl::optional d_connectionErrorThreshold; + bsl::optional d_connectionEstablishmentTimeout; + rmqt::FieldTable d_clientProperties; + rmqt::Tunables d_tunables; bsl::optional d_shuffleConnectionEndpoints; - bsl::optional d_hostHealthConfig; }; } // namespace rmqa diff --git a/src/tests/rmqa/rmqa_rabbitcontextimpl.t.cpp b/src/tests/rmqa/rmqa_rabbitcontextimpl.t.cpp index 3817f5b..4f4381d 100644 --- a/src/tests/rmqa/rmqa_rabbitcontextimpl.t.cpp +++ b/src/tests/rmqa/rmqa_rabbitcontextimpl.t.cpp @@ -31,6 +31,7 @@ #include #include +#include #include using namespace BloombergLP; @@ -97,10 +98,47 @@ class RabbitContextImplTests : public Test { TEST_F(RabbitContextImplTests, ItsAlive) { + // d_options has a host health config selected, so the monitor is created -- + // timerFactory() is called three times (monitor start, connection factory, + // connection watchdog). createExpectations(); rmqa::RabbitContextImpl context(getMockEventLoop(), d_options); } +TEST_F(RabbitContextImplTests, LegacyUserWithConfigStillMonitors) +{ + // Backward-compatibility guarantee: a caller who enabled monitoring before + // an explicit opt-out could be expressed did so by attaching a host health + // config, and did not opt out. They must keep being monitored -- the + // monitor is still created (three timerFactory() calls). + ASSERT_TRUE(bsl::holds_alternative( + d_options.hostHealthSelection())); + + createExpectations(); + rmqa::RabbitContextImpl context(getMockEventLoop(), d_options); +} + +TEST_F(RabbitContextImplTests, HostHealthMonitoringOptOutSuppressesMonitor) +{ + // Even though d_options had a host health config attached, an explicit + // opt-out selection replaces it and prevents the monitor from being + // created. The monitor is what makes the third timerFactory() call (its + // start()), so without it timerFactory() is called only twice (connection + // factory + watchdog). + d_options.setHostHealthSelection( + rmqa::RabbitContextOptions::HostHealthAwarenessOff()); + ASSERT_FALSE(bsl::holds_alternative( + d_options.hostHealthSelection())); + + EXPECT_CALL(*d_mockEventLoop, resolver(false)).Times(1); + EXPECT_CALL(*d_mockEventLoop, timerFactory()) + .Times(2) + .WillRepeatedly(Return(d_mockTimerFactory)); + EXPECT_CALL(*d_mockEventLoop, waitForEventLoopExit(_)).Times(1); + + rmqa::RabbitContextImpl context(getMockEventLoop(), d_options); +} + TEST_F(RabbitContextImplTests, ErrorWhenProvideNullEndpoint) { createExpectations(); diff --git a/src/tests/rmqa/rmqa_rabbitcontextoptions.t.cpp b/src/tests/rmqa/rmqa_rabbitcontextoptions.t.cpp index eabd469..8214611 100644 --- a/src/tests/rmqa/rmqa_rabbitcontextoptions.t.cpp +++ b/src/tests/rmqa/rmqa_rabbitcontextoptions.t.cpp @@ -21,7 +21,9 @@ #include #include +#include +#include #include using namespace BloombergLP; @@ -31,6 +33,33 @@ using namespace ::testing; namespace { bool alwaysHealthy() { return true; } +// gmock matchers over a RabbitContextOptions' host health selection, so +// assertions read EXPECT_THAT(options, isOptOut()) / Not(isConfig()) etc. `arg` +// is the matched RabbitContextOptions. + +/// Matches when the selection is an explicit opt-out. +MATCHER(isOptOut, "holds an explicit host health opt-out") +{ + return bsl::holds_alternative< + rmqa::RabbitContextOptions::HostHealthAwarenessOff>( + arg.hostHealthSelection()); +} + +/// Matches when no host health preference is expressed (the default). +MATCHER(isUnset, "expresses no host health preference") +{ + return bsl::holds_alternative< + rmqa::RabbitContextOptions::HostHealthAwarenessUnset>( + arg.hostHealthSelection()); +} + +/// Matches when a host health config is selected (opt-in). +MATCHER(isConfig, "holds a host health config") +{ + return bsl::holds_alternative( + arg.hostHealthSelection()); +} + /// `bsls_asserttest`'s `BSLS_ASSERTTEST_ASSERT_{PASS,FAIL}` macros report their /// outcome by calling a driver-supplied `ASSERT(bool)` (which its header allows /// to be a function, not just a macro). Route that into a gtest expectation. @@ -43,7 +72,10 @@ TEST(RabbitContextOptions, Defaults) rmqa::RabbitContextOptions t; EXPECT_FALSE(t.metricPublisher()); EXPECT_FALSE(t.threadpool()); - EXPECT_FALSE(t.hostHealthConfig().has_value()); + EXPECT_THAT(t, Not(isConfig())); + // No host health selection is expressed by default: neither a config nor + // an explicit opt-out. + EXPECT_THAT(t, isUnset()); t.errorCallback()("heres an error", -1); } @@ -52,7 +84,7 @@ TEST(RabbitContextOptions, SetHostHealthConfig) rmqa::RabbitContextOptions options; // Initially not set - EXPECT_FALSE(options.hostHealthConfig().has_value()); + EXPECT_THAT(options, Not(isConfig())); // Create a health checker function rmqt::HostHealthConfig config(alwaysHealthy); @@ -61,7 +93,55 @@ TEST(RabbitContextOptions, SetHostHealthConfig) options.setHostHealthConfig(config); // Now it should be set - EXPECT_TRUE(options.hostHealthConfig().has_value()); + EXPECT_THAT(options, isConfig()); +} + +TEST(RabbitContextOptions, SetHostHealthSelectionOptOut) +{ + rmqa::RabbitContextOptions options; + + // Initially no selection is expressed (distinct from an explicit opt-out). + EXPECT_THAT(options, Not(isConfig())); + EXPECT_THAT(options, isUnset()); + + // Explicit opt-out. + options.setHostHealthSelection( + RabbitContextOptions::HostHealthAwarenessOff()); + EXPECT_THAT(options, isOptOut()); + EXPECT_THAT(options, Not(isConfig())); +} + +TEST(RabbitContextOptions, SetHostHealthSelectionConfig) +{ + rmqa::RabbitContextOptions options; + + // Selecting a config via the variant setter is equivalent to + // setHostHealthConfig: the config is retrievable and it is not an opt-out. + options.setHostHealthSelection(rmqt::HostHealthConfig(alwaysHealthy)); + EXPECT_THAT(options, isConfig()); + EXPECT_THAT(options, Not(isOptOut())); +} + +TEST(RabbitContextOptions, HostHealthSelectionIsMutuallyExclusive) +{ + // A config and an explicit opt-out are two alternatives of a single + // selection, so setting one replaces the other -- the contradictory + // "config attached AND opted out" state cannot be expressed. + rmqa::RabbitContextOptions options; + + // Opt-out then attach a config: the config wins. + options.setHostHealthSelection( + RabbitContextOptions::HostHealthAwarenessOff()); + options.setHostHealthConfig(rmqt::HostHealthConfig(alwaysHealthy)); + EXPECT_THAT(options, isConfig()); + EXPECT_THAT(options, Not(isOptOut())); + + // Attach a config then opt out: the opt-out wins. + options.setHostHealthConfig(rmqt::HostHealthConfig(alwaysHealthy)); + options.setHostHealthSelection( + RabbitContextOptions::HostHealthAwarenessOff()); + EXPECT_THAT(options, isOptOut()); + EXPECT_THAT(options, Not(isConfig())); } TEST(RabbitContextOptions, SetConnectionEstablishmentTimeoutAcceptsValid)