fix: Don't cache responses to admin's requests (#3058)

This commit is contained in:
Sergey Kuznetsov
2026-05-05 17:00:59 +01:00
committed by GitHub
parent a7090f5b19
commit e974e1899f
3 changed files with 96 additions and 3 deletions

View File

@@ -275,7 +275,8 @@ LoadBalancer::forwardToRippled(
return std::unexpected{rpc::ClioError::RpcCommandIsMissing};
auto const cmd = boost::json::value_to<std::string>(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

View File

@@ -254,6 +254,9 @@ private:
*/
void
chooseForwardingSource();
bool
shouldUseCache(bool isAdmin) const;
};
} // namespace etl

View File

@@ -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<CounterInt>("forwarding_cache_miss_counter", "");
auto& successDurationCounter =
makeMock<CounterInt>("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);