Skip to content

Commit

Permalink
optimize srds for route fallback
Browse files Browse the repository at this point in the history
  • Loading branch information
johnlanni committed Nov 25, 2024
1 parent 0cf0aea commit 13aee61
Show file tree
Hide file tree
Showing 17 changed files with 181 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,8 @@ message ScopedRoutes {
// in this message.
ScopedRds scoped_rds = 5;
}

google.protobuf.BoolValue retry_other_scope_when_not_found = 101;
}

message ScopedRds {
Expand Down
12 changes: 8 additions & 4 deletions envoy/router/scopes.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,15 @@ class ScopedConfig : public Envoy::Config::ConfigProvider::Config {
virtual ConfigConstSharedPtr getRouteConfig(const ScopeKeyPtr& scope_key) const PURE;

#if defined(HIGRESS)
virtual ConfigConstSharedPtr
getRouteConfig(const ScopeKeyBuilder* builder, const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info = nullptr) const PURE;
virtual ConfigConstSharedPtr getRouteConfig(const ScopeKeyBuilder* builder,
const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info) const PURE;
virtual ConfigConstSharedPtr getRouteConfig(const ScopeKeyBuilder* builder,
const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info,
std::function<ScopeKeyPtr()>& recompute) const PURE;
virtual ScopeKeyPtr computeScopeKey(const ScopeKeyBuilder*, const Http::HeaderMap&,
const StreamInfo::StreamInfo* = nullptr) const {
const StreamInfo::StreamInfo*) const {
return {};
};
#endif
Expand Down
5 changes: 5 additions & 0 deletions source/common/http/conn_manager_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,11 @@ class ConnectionManagerConfig {
* Zero indicates this behavior is disabled.
*/
virtual std::chrono::seconds keepaliveHeaderTimeout() const PURE;
/**
* @return whether to retry to other scoped routes when the target route is not found in the
* current scope, supported only when using scoped_routes.
*/
virtual bool retryOtherScopeWhenNotFound() const PURE;
#endif
};
} // namespace Http
Expand Down
29 changes: 28 additions & 1 deletion source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,14 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPt
connection_manager_.config_.scopeKeyBuilder().has_value()) {
snapped_scoped_routes_config_ =
connection_manager_.config_.scopedRouteConfigProvider()->config<Router::ScopedConfig>();
#if defined(HIGRESS)
// It is only used to determine whether to remove specific internal headers, but at the cost
// of an additional routing calculation. In our scenario, there is no removal of internal
// headers, so there is no need to calculate the route here.
snapped_route_config_ = std::make_shared<Router::NullConfigImpl>();
#else
snapScopedRouteConfig();
#endif
}
} else {
snapped_route_config_ = connection_manager_.config_.routeConfigProvider()->configCast();
Expand Down Expand Up @@ -1524,9 +1531,10 @@ void ConnectionManagerImpl::startDrainSequence() {

void ConnectionManagerImpl::ActiveStream::snapScopedRouteConfig() {
#if defined(HIGRESS)
snapped_scoped_routes_recompute_ = nullptr;
snapped_route_config_ = snapped_scoped_routes_config_->getRouteConfig(
connection_manager_.config_.scopeKeyBuilder().ptr(), *request_headers_,
&connection()->streamInfo());
&connection()->streamInfo(), snapped_scoped_routes_recompute_);
#else
// NOTE: if a RDS subscription hasn't got a RouteConfiguration back, a Router::NullConfigImpl is
// returned, in that case we let it pass.
Expand Down Expand Up @@ -1653,6 +1661,25 @@ void ConnectionManagerImpl::ActiveStream::refreshCachedRoute(const Router::Route
}
}

#if defined(HIGRESS)
if (connection_manager_.config_.retryOtherScopeWhenNotFound()) {
while (route == nullptr && snapped_scoped_routes_recompute_ != nullptr) {
ASSERT(snapped_scoped_routes_config_ != nullptr);
snapped_route_config_ = snapped_scoped_routes_config_->getRouteConfig(
connection_manager_.config_.scopeKeyBuilder().ptr(), *request_headers_,
&connection()->streamInfo(), snapped_scoped_routes_recompute_);
if (snapped_route_config_ == nullptr) {
break;
}
route = snapped_route_config_->route(cb, *request_headers_, filter_manager_.streamInfo(),
stream_id_);
ENVOY_STREAM_LOG(debug,
"after the route was not found, search again in other scopes and found:{}",
*this, route != nullptr);
}
}
#endif

setRoute(route);
}

Expand Down
3 changes: 3 additions & 0 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,9 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
// route configuration is updated frequently and the request is long-lived.
Router::ConfigConstSharedPtr snapped_route_config_;
Router::ScopedConfigConstSharedPtr snapped_scoped_routes_config_;
#if defined(HIGRESS)
std::function<Router::ScopeKeyPtr()> snapped_scoped_routes_recompute_;
#endif
// This is used to track the route that has been cached in the request. And we will keep this
// route alive until the request is finished.
absl::optional<Router::RouteConstSharedPtr> cached_route_;
Expand Down
34 changes: 24 additions & 10 deletions source/common/router/scoped_config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ HostValueExtractorImpl::computeFragment(const Http::HeaderMap& headers,
if (port_start != absl::string_view::npos) {
host = host.substr(0, port_start);
}
*recompute = [this, host, weak_recompute = ReComputeCbWeakPtr(recompute)]() mutable
-> std::unique_ptr<ScopeKeyFragmentBase> {
return reComputeHelper(std::string(host), weak_recompute, 0);
*recompute = [this, host_str = std::string(host),
weak_recompute = ReComputeCbWeakPtr(
recompute)]() mutable -> std::unique_ptr<ScopeKeyFragmentBase> {
return reComputeHelper(host_str, weak_recompute, 0);
};
return std::make_unique<StringKeyFragment>(host);
}
Expand Down Expand Up @@ -136,13 +137,14 @@ ScopeKeyPtr ScopeKeyBuilderImpl::computeScopeKey(const Http::HeaderMap& headers,
recompute = [&recompute, recompute_cbs]() mutable -> ScopeKeyPtr {
ScopeKey new_key;
for (auto& cb : *recompute_cbs) {
if (*cb == nullptr) {
recompute = nullptr;
return nullptr;
}
auto new_fragment = (*cb)();
if (new_fragment == nullptr) {
return nullptr;
}
if (*cb == nullptr) {
recompute = nullptr;
}
new_key.addFragment(std::move(new_fragment));
}
return std::make_unique<ScopeKey>(std::move(new_key));
Expand Down Expand Up @@ -171,10 +173,14 @@ ScopeKeyPtr ScopedConfigImpl::computeScopeKey(const ScopeKeyBuilder* scope_key_b

Router::ConfigConstSharedPtr
ScopedConfigImpl::getRouteConfig(const ScopeKeyBuilder* scope_key_builder,
const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info) const {
std::function<ScopeKeyPtr()> recompute;
ScopeKeyPtr scope_key = scope_key_builder->computeScopeKey(headers, info, recompute);
const Http::HeaderMap& headers, const StreamInfo::StreamInfo* info,
std::function<ScopeKeyPtr()>& recompute) const {
ScopeKeyPtr scope_key = nullptr;
if (recompute == nullptr) {
scope_key = scope_key_builder->computeScopeKey(headers, info, recompute);
} else {
scope_key = recompute();
}
if (scope_key == nullptr) {
return nullptr;
}
Expand All @@ -189,6 +195,14 @@ ScopedConfigImpl::getRouteConfig(const ScopeKeyBuilder* scope_key_builder,
return nullptr;
}

Router::ConfigConstSharedPtr
ScopedConfigImpl::getRouteConfig(const ScopeKeyBuilder* scope_key_builder,
const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info) const {
std::function<Router::ScopeKeyPtr()> recompute;
return getRouteConfig(scope_key_builder, headers, info, recompute);
}

#endif

bool ScopeKey::operator!=(const ScopeKey& other) const { return !(*this == other); }
Expand Down
9 changes: 9 additions & 0 deletions source/common/router/scoped_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ class ScopedConfigImpl : public ScopedConfig {
Router::ConfigConstSharedPtr getRouteConfig(const ScopeKeyPtr& scope_key) const override;

#if defined(HIGRESS)
Router::ConfigConstSharedPtr
getRouteConfig(const ScopeKeyBuilder* scope_key_builder, const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info,
std::function<ScopeKeyPtr()>& recompute) const override;
Router::ConfigConstSharedPtr getRouteConfig(const ScopeKeyBuilder* scope_key_builder,
const Http::HeaderMap& headers,
const StreamInfo::StreamInfo* info) const override;
Expand All @@ -202,6 +206,11 @@ class NullScopedConfigImpl : public ScopedConfig {
return std::make_shared<const NullConfigImpl>();
}
#if defined(HIGRESS)
Router::ConfigConstSharedPtr getRouteConfig(const ScopeKeyBuilder*, const Http::HeaderMap&,
const StreamInfo::StreamInfo*,
std::function<ScopeKeyPtr()>&) const override {
return std::make_shared<const NullConfigImpl>();
}
Router::ConfigConstSharedPtr getRouteConfig(const ScopeKeyBuilder*, const Http::HeaderMap&,
const StreamInfo::StreamInfo*) const override {
return std::make_shared<const NullConfigImpl>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
config, context_.getServerFactoryContext(), context_.initManager(), stats_prefix_,
scoped_routes_config_provider_manager_);
scope_key_builder_ = Router::ScopedRoutesConfigProviderUtil::createScopeKeyBuilder(config);
retry_other_scope_when_not_found_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config.scoped_routes(), retry_other_scope_when_not_found, true);
break;
case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager::
RouteSpecifierCase::ROUTE_SPECIFIER_NOT_SET:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
}
#if defined(HIGRESS)
std::chrono::seconds keepaliveHeaderTimeout() const override { return keepalive_header_timeout_; }
bool retryOtherScopeWhenNotFound() const override { return retry_other_scope_when_not_found_; }
#endif

private:
Expand Down Expand Up @@ -332,6 +333,9 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
// routes
Router::ScopeKeyBuilderPtr scope_key_builder_;
Config::ConfigProviderPtr scoped_routes_config_provider_;
#if defined(HIGRESS)
bool retry_other_scope_when_not_found_;
#endif
std::chrono::milliseconds drain_timeout_;
bool generate_request_id_;
const bool preserve_external_request_id_;
Expand Down
1 change: 1 addition & 0 deletions source/server/admin/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ class AdminImpl : public Admin,
bool addProxyProtocolConnectionState() const override { return true; }
#if defined(HIGRESS)
std::chrono::seconds keepaliveHeaderTimeout() const override { return {}; }
bool retryOtherScopeWhenNotFound() const override { return false; }
#endif

private:
Expand Down
2 changes: 2 additions & 0 deletions test/common/http/conn_manager_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ class FuzzConfig : public ConnectionManagerConfig {
bool addProxyProtocolConnectionState() const override { return true; }
#if defined(HIGRESS)
std::chrono::seconds keepaliveHeaderTimeout() const override { return keepalive_header_timeout_; }
bool retryOtherScopeWhenNotFound() const override { return retry_other_scope_when_not_found_; }
#endif

const envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager
Expand Down Expand Up @@ -299,6 +300,7 @@ class FuzzConfig : public ConnectionManagerConfig {
std::unique_ptr<HttpConnectionManagerProto::ProxyStatusConfig> proxy_status_config_;
#if defined(HIGRESS)
std::chrono::seconds keepalive_header_timeout_{};
bool retry_other_scope_when_not_found_ = false;
#endif
};

Expand Down
26 changes: 14 additions & 12 deletions test/common/http/conn_manager_impl_test_2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2753,7 +2753,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsRouteNotFound) {
#if defined(HIGRESS)
EXPECT_CALL(*static_cast<const Router::MockScopedConfig*>(
scopedRouteConfigProvider()->config<Router::ScopedConfig>().get()),
getRouteConfig(_, _, _))
getRouteConfig(_, _, _, _))
.Times(2)
.WillRepeatedly(Return(nullptr));
#else
Expand Down Expand Up @@ -2796,7 +2796,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsUpdate) {
#if defined(HIGRESS)
EXPECT_CALL(*static_cast<const Router::MockScopedConfig*>(
scopedRouteConfigProvider()->config<Router::ScopedConfig>().get()),
getRouteConfig(_, _, _))
getRouteConfig(_, _, _, _))
.Times(3)
.WillOnce(Return(nullptr))
.WillOnce(Return(nullptr)) // refreshCachedRoute first time.
Expand Down Expand Up @@ -2870,19 +2870,21 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsCrossScopeReroute) {
#if defined(HIGRESS)
EXPECT_CALL(*static_cast<const Router::MockScopedConfig*>(
scopedRouteConfigProvider()->config<Router::ScopedConfig>().get()),
getRouteConfig(_, _, _))
getRouteConfig(_, _, _, _))
// 1. Snap scoped route config;
// 2. refreshCachedRoute (both in decodeHeaders(headers,end_stream);
// 3. then refreshCachedRoute triggered by decoder_filters_[1]->callbacks_->route().
.Times(3)
.WillRepeatedly(Invoke([&](const Router::ScopeKeyBuilder*, const Http::HeaderMap& headers,
const StreamInfo::StreamInfo*) -> Router::ConfigConstSharedPtr {
auto& test_headers = dynamic_cast<const TestRequestHeaderMapImpl&>(headers);
if (test_headers.get_("scope_key") == "foo") {
return route_config1;
}
return route_config2;
}));
.WillRepeatedly(
Invoke([&](const Router::ScopeKeyBuilder*, const Http::HeaderMap& headers,
const StreamInfo::StreamInfo*,
std::function<Router::ScopeKeyPtr()>&) -> Router::ConfigConstSharedPtr {
auto& test_headers = dynamic_cast<const TestRequestHeaderMapImpl&>(headers);
if (test_headers.get_("scope_key") == "foo") {
return route_config1;
}
return route_config2;
}));
#else
EXPECT_CALL(*static_cast<const Router::MockScopeKeyBuilder*>(scopeKeyBuilder().ptr()),
computeScopeKey(_))
Expand Down Expand Up @@ -2959,7 +2961,7 @@ TEST_F(HttpConnectionManagerImplTest, TestSrdsRouteFound) {
EXPECT_CALL(cluster_manager_, getThreadLocalCluster(_)).WillOnce(Return(fake_cluster1.get()));
#if defined(HIGRESS)
EXPECT_CALL(*scopedRouteConfigProvider()->config<Router::MockScopedConfig>(),
getRouteConfig(_, _, _))
getRouteConfig(_, _, _, _))
// 1. decodeHeaders() snapping route config.
// 2. refreshCachedRoute() later in the same decodeHeaders().
.Times(2);
Expand Down
2 changes: 2 additions & 0 deletions test/common/http/conn_manager_impl_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig {

#if defined(HIGRESS)
std::chrono::seconds keepaliveHeaderTimeout() const override { return keepalive_header_timeout_; }
bool retryOtherScopeWhenNotFound() const override { return retry_other_scope_when_not_found_; }
#endif
// Simple helper to wrapper filter to the factory function.
FilterFactoryCb createDecoderFilterFactoryCb(StreamDecoderFilterSharedPtr filter) {
Expand Down Expand Up @@ -282,6 +283,7 @@ class HttpConnectionManagerImplMixin : public ConnectionManagerConfig {
bool add_proxy_protocol_connection_state_ = true;
#if defined(HIGRESS)
std::chrono::seconds keepalive_header_timeout_{};
bool retry_other_scope_when_not_found_ = true;
#endif

const LocalReply::LocalReplyPtr local_reply_;
Expand Down
71 changes: 71 additions & 0 deletions test/common/router/scoped_rds_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,77 @@ TEST_F(InlineScopedRoutesTest, InlineRouteConfigurations) {
"foo");
}

#if defined(HIGRESS)
TEST_F(InlineScopedRoutesTest, InlineWildcardDomainFallback) {
server_factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
const std::string hcm_config = absl::StrCat(hcm_config_base, R"EOF(
scoped_routes:
name: $0
scope_key_builder:
fragments:
- local_port_value_extractor: {}
- host_value_extractor: {}
scoped_route_configurations_list:
scoped_route_configurations:
- name: foo-scope
route_configuration:
name: foo
virtual_hosts:
- name: bar
domains: ["www.example.com"]
routes:
- match: { path: "/" }
route: { cluster: baz }
key:
fragments:
- string_key: "80"
- string_key: www.example.com
- name: foo2-scope
route_configuration:
name: foo2
virtual_hosts:
- name: bar
domains: ["*.example.com"]
routes:
- match: { path: "/foo" }
route: { cluster: baz }
key:
fragments:
- string_key: "80"
- string_key: "*.example.com"
)EOF");
const auto config =
parseHttpConnectionManagerFromYaml(absl::Substitute(hcm_config, "foo-scoped-routes"));
Envoy::Config::ConfigProviderPtr provider = ScopedRoutesConfigProviderUtil::create(
config, server_factory_context_, context_init_manager_, "foo.", *config_provider_manager_);
Envoy::Router::ScopeKeyBuilderPtr scope_key_builder =
ScopedRoutesConfigProviderUtil::createScopeKeyBuilder(config);
ASSERT_THAT(provider->config<ScopedConfigImpl>(), Not(IsNull()));

NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
auto downstream_connection_info_provider = std::make_shared<Network::ConnectionInfoSetterImpl>(
std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1", 80),
std::make_shared<Network::Address::Ipv4Instance>("127.0.0.2", 1000));
ON_CALL(stream_info, downstreamAddressProvider())
.WillByDefault(ReturnPointee(downstream_connection_info_provider));

std::function<Router::ScopeKeyPtr()> recompute;
Http::TestRequestHeaderMapImpl headers = {{":authority", "www.example.com"},
{":path", "/foo"},
{":method", "GET"},
{":scheme", "http"},
{"x-forwarded-proto", "http"}};
auto route_config = provider->config<ScopedConfigImpl>()->getRouteConfig(
scope_key_builder.get(), headers, &stream_info, recompute);
EXPECT_EQ(route_config->name(), "foo");
EXPECT_EQ(route_config->route(headers, stream_info, 0), nullptr);
route_config = provider->config<ScopedConfigImpl>()->getRouteConfig(
scope_key_builder.get(), headers, &stream_info, recompute);
EXPECT_EQ(route_config->name(), "foo2");
EXPECT_NE(route_config->route(headers, stream_info, 0), nullptr);
}
#endif

TEST_F(InlineScopedRoutesTest, ConfigLoadAndDump) {
server_factory_context_.cluster_manager_.initializeClusters({"baz"}, {});
timeSystem().setSystemTime(std::chrono::milliseconds(1234567891234));
Expand Down
Loading

0 comments on commit 13aee61

Please sign in to comment.