diff --git a/src/etl/LoadBalancer.cpp b/src/etl/LoadBalancer.cpp index e3d2d557b..50f8fb14e 100644 --- a/src/etl/LoadBalancer.cpp +++ b/src/etl/LoadBalancer.cpp @@ -275,7 +275,8 @@ LoadBalancer::forwardToRippled( return std::unexpected{rpc::ClioError::RpcCommandIsMissing}; auto const cmd = boost::json::value_to(request.at("command")); - if (forwardingCache_) { + if (shouldUseCache(isAdmin)) { + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) if (auto cachedResponse = forwardingCache_->get(cmd); cachedResponse) { forwardingCounters_.cacheHit.get() += 1; return *std::move(cachedResponse); @@ -310,8 +311,8 @@ LoadBalancer::forwardToRippled( } if (response) { - if (forwardingCache_ and not response->contains("error")) - forwardingCache_->put(cmd, *response); + if (shouldUseCache(isAdmin) and not response->contains("error")) + forwardingCache_->put(cmd, *response); // NOLINT(bugprone-unchecked-optional-access) return *std::move(response); } @@ -415,4 +416,10 @@ LoadBalancer::chooseForwardingSource() } } +bool +LoadBalancer::shouldUseCache(bool isAdmin) const +{ + return forwardingCache_.has_value() and not isAdmin; +} + } // namespace etl diff --git a/src/etl/LoadBalancer.hpp b/src/etl/LoadBalancer.hpp index 58baa4bd6..30896f68e 100644 --- a/src/etl/LoadBalancer.hpp +++ b/src/etl/LoadBalancer.hpp @@ -254,6 +254,9 @@ private: */ void chooseForwardingSource(); + + bool + shouldUseCache(bool isAdmin) const; }; } // namespace etl diff --git a/tests/unit/etl/LoadBalancerTests.cpp b/tests/unit/etl/LoadBalancerTests.cpp index 2c03848ca..7034ea7dc 100644 --- a/tests/unit/etl/LoadBalancerTests.cpp +++ b/tests/unit/etl/LoadBalancerTests.cpp @@ -809,6 +809,36 @@ TEST_F(LoadBalancerForwardToRippledPrometheusTests, source0Fails) }); } +TEST_F(LoadBalancerForwardToRippledPrometheusTests, adminRequestAlwaysCacheMiss) +{ + configJson_.as_object()["forwarding"] = boost::json::object{{"cache_timeout", 10.}}; + EXPECT_CALL(sourceFactory_, makeSource).Times(2); + auto loadBalancer = makeLoadBalancer(); + + auto const request = boost::json::object{{"command", "server_info"}}; + + auto& cacheMissCounter = makeMock("forwarding_cache_miss_counter", ""); + auto& successDurationCounter = + makeMock("forwarding_duration_milliseconds_counter", "{status=\"success\"}"); + + EXPECT_CALL(cacheMissCounter, add(1)).Times(2); + EXPECT_CALL(successDurationCounter, add(testing::_)).Times(2); + + EXPECT_CALL( + sourceFactory_.sourceAt(0), + forwardToRippled( + request, clientIP_, LoadBalancer::kADMIN_FORWARDING_X_USER_VALUE, testing::_ + ) + ) + .Times(2) + .WillRepeatedly(Return(response_)); + + runSpawn([&](boost::asio::yield_context yield) { + EXPECT_EQ(loadBalancer->forwardToRippled(request, clientIP_, true, yield), response_); + EXPECT_EQ(loadBalancer->forwardToRippled(request, clientIP_, true, yield), response_); + }); +} + struct LoadBalancerForwardToRippledErrorTestBundle { std::string testName; rpc::ClioError firstSourceError; @@ -906,6 +936,59 @@ TEST_F(LoadBalancerForwardToRippledTests, forwardingCacheEnabled) }); } +TEST_F(LoadBalancerForwardToRippledTests, adminRequestBypassesForwardingCache) +{ + configJson_.as_object()["forwarding"] = boost::json::object{{"cache_timeout", 10.}}; + EXPECT_CALL(sourceFactory_, makeSource).Times(2); + auto loadBalancer = makeLoadBalancer(); + + auto const request = boost::json::object{{"command", "server_info"}}; + + EXPECT_CALL( + sourceFactory_.sourceAt(0), + forwardToRippled( + request, clientIP_, LoadBalancer::kADMIN_FORWARDING_X_USER_VALUE, testing::_ + ) + ) + .Times(2) + .WillRepeatedly(Return(response_)); + + runSpawn([&](boost::asio::yield_context yield) { + EXPECT_EQ(loadBalancer->forwardToRippled(request, clientIP_, true, yield), response_); + EXPECT_EQ(loadBalancer->forwardToRippled(request, clientIP_, true, yield), response_); + }); +} + +TEST_F(LoadBalancerForwardToRippledTests, adminResponseNotCachedForSubsequentUserRequest) +{ + configJson_.as_object()["forwarding"] = boost::json::object{{"cache_timeout", 10.}}; + EXPECT_CALL(sourceFactory_, makeSource).Times(2); + auto loadBalancer = makeLoadBalancer(); + + auto const request = boost::json::object{{"command", "server_info"}}; + + EXPECT_CALL( + sourceFactory_.sourceAt(0), + forwardToRippled( + request, clientIP_, LoadBalancer::kADMIN_FORWARDING_X_USER_VALUE, testing::_ + ) + ) + .WillOnce(Return(response_)); + EXPECT_CALL( + sourceFactory_.sourceAt(0), + forwardToRippled( + request, clientIP_, LoadBalancer::kUSER_FORWARDING_X_USER_VALUE, testing::_ + ) + ) + .WillOnce(Return(response_)); + + runSpawn([&](boost::asio::yield_context yield) { + EXPECT_EQ(loadBalancer->forwardToRippled(request, clientIP_, true, yield), response_); + // User request must reach the source, not be served from the admin response + EXPECT_EQ(loadBalancer->forwardToRippled(request, clientIP_, false, yield), response_); + }); +} + TEST_F(LoadBalancerForwardToRippledTests, forwardingCacheDisabledOnLedgerClosedHookCalled) { EXPECT_CALL(sourceFactory_, makeSource).Times(2);