Skip to content

Commit

Permalink
rds: make RouteConfigProvider unique_ptr (#3967)
Browse files Browse the repository at this point in the history 
Signed-off-by: Lizan Zhou <zlizan@google.com>
  • Loading branch information
@lizan @mattklein123
lizan authored and mattklein123 committed Jul 31, 2018
1 parent 62441f9 commit 2012c3e
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 135 deletions.
2 changes: 1 addition & 1 deletion include/envoy/router/rds.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class RouteConfigProvider {
virtual SystemTime lastUpdated() const PURE;
};

typedef std::shared_ptr<RouteConfigProvider> RouteConfigProviderSharedPtr;
typedef std::unique_ptr<RouteConfigProvider> RouteConfigProviderPtr;

} // namespace Router
} // namespace Envoy
34 changes: 10 additions & 24 deletions include/envoy/router/route_config_provider_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,43 +27,29 @@ class RouteConfigProviderManager {
virtual ~RouteConfigProviderManager() {}

/**
* Get a RouteConfigProviderSharedPtr for a route from RDS. Ownership of the RouteConfigProvider
* is shared by all the HttpConnectionManagers who own a RouteConfigProviderSharedPtr. The
* RouteConfigProviderManager holds weak_ptrs to the RouteConfigProviders. Clean up of the weak
* ptrs happen from the destructor of the RouteConfigProvider. This function creates a
* RouteConfigProvider if there isn't one with the same (route_config_name, cluster) already.
* Otherwise, it returns a RouteConfigProviderSharedPtr created from the manager held weak_ptr.
* Get a RouteConfigProviderPtr for a route from RDS. Ownership of the RouteConfigProvider is the
* HttpConnectionManagers who calls this function. The RouteConfigProviderManager holds raw
* pointers to the RouteConfigProviders. Clean up of the pointers happen from the destructor of
* the RouteConfigProvider. This method creates a RouteConfigProvider which may share the
* underlying RDS subscription with the same (route_config_name, cluster).
* @param rds supplies the proto configuration of an RDS-configured RouteConfigProvider.
* @param factory_context is the context to use for the route config provider.
* @param stat_prefix supplies the stat_prefix to use for the provider stats.
*/
virtual RouteConfigProviderSharedPtr getRdsRouteConfigProvider(
virtual RouteConfigProviderPtr createRdsRouteConfigProvider(
const envoy::config::filter::network::http_connection_manager::v2::Rds& rds,
Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix) PURE;

/**
* Get a RouteConfigSharedPtr for a statically defined route. Ownership is as described for
* getRdsRouteConfigProvider above. Unlike getRdsRouteConfigProvider(), this method always creates
* a new RouteConfigProvider.
* getRdsRouteConfigProvider above. This method always create a new RouteConfigProvider.
* @param route_config supplies the RouteConfiguration for this route
* @param runtime supplies the runtime loader.
* @param cm supplies the ClusterManager.
*/
virtual RouteConfigProviderSharedPtr
getStaticRouteConfigProvider(const envoy::api::v2::RouteConfiguration& route_config,
Server::Configuration::FactoryContext& factory_context) PURE;

/**
* @return std::vector<Router::RouteConfigProviderSharedPtr> a list of all the
* dynamic (RDS) RouteConfigProviders currently loaded.
*/
virtual std::vector<RouteConfigProviderSharedPtr> getRdsRouteConfigProviders() PURE;

/**
* @return std::vector<Router::RouteConfigProviderSharedPtr> a list of all the
* static RouteConfigProviders currently loaded.
*/
virtual std::vector<RouteConfigProviderSharedPtr> getStaticRouteConfigProviders() PURE;
virtual RouteConfigProviderPtr
createStaticRouteConfigProvider(const envoy::api::v2::RouteConfiguration& route_config,
Server::Configuration::FactoryContext& factory_context) PURE;
};

} // namespace Router
Expand Down
96 changes: 39 additions & 57 deletions source/common/router/rds_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,37 @@
namespace Envoy {
namespace Router {

RouteConfigProviderSharedPtr RouteConfigProviderUtil::create(
RouteConfigProviderPtr RouteConfigProviderUtil::create(
const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager&
config,
Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix,
RouteConfigProviderManager& route_config_provider_manager) {
switch (config.route_specifier_case()) {
case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::
kRouteConfig:
return route_config_provider_manager.getStaticRouteConfigProvider(config.route_config(),
factory_context);
return route_config_provider_manager.createStaticRouteConfigProvider(config.route_config(),
factory_context);
case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::kRds:
return route_config_provider_manager.getRdsRouteConfigProvider(config.rds(), factory_context,
stat_prefix);
return route_config_provider_manager.createRdsRouteConfigProvider(config.rds(), factory_context,
stat_prefix);
default:
NOT_REACHED_GCOVR_EXCL_LINE;
}
}

StaticRouteConfigProviderImpl::StaticRouteConfigProviderImpl(
const envoy::api::v2::RouteConfiguration& config,
Server::Configuration::FactoryContext& factory_context)
Server::Configuration::FactoryContext& factory_context,
RouteConfigProviderManagerImpl& route_config_provider_manager)
: config_(new ConfigImpl(config, factory_context, true)), route_config_proto_{config},
last_updated_(factory_context.systemTimeSource().currentTime()) {}
last_updated_(factory_context.systemTimeSource().currentTime()),
route_config_provider_manager_(route_config_provider_manager) {
route_config_provider_manager_.static_route_config_providers_.insert(this);
}

StaticRouteConfigProviderImpl::~StaticRouteConfigProviderImpl() {
route_config_provider_manager_.static_route_config_providers_.erase(this);
}

// TODO(htuch): If support for multiple clusters is added per #1170 cluster_name_
// initialization needs to be fixed.
Expand Down Expand Up @@ -189,40 +197,7 @@ RouteConfigProviderManagerImpl::RouteConfigProviderManagerImpl(Server::Admin& ad
RELEASE_ASSERT(config_tracker_entry_, "");
}

std::vector<RouteConfigProviderSharedPtr>
RouteConfigProviderManagerImpl::getRdsRouteConfigProviders() {
std::vector<RouteConfigProviderSharedPtr> ret;
ret.reserve(route_config_subscriptions_.size());
for (const auto& element : route_config_subscriptions_) {
// Because the RouteConfigProviderManager's weak_ptrs only get cleaned up
// in the RdsRouteConfigSubscription destructor, and the single threaded nature
// of this code, locking the weak_ptr will not fail.
auto subscription = element.second.lock();
ASSERT(subscription);
ASSERT(subscription->route_config_providers_.size() > 0);
ret.push_back((*subscription->route_config_providers_.begin())->shared_from_this());
}
return ret;
};

std::vector<RouteConfigProviderSharedPtr>
RouteConfigProviderManagerImpl::getStaticRouteConfigProviders() {
std::vector<RouteConfigProviderSharedPtr> providers_strong;
// Collect non-expired providers.
for (const auto& weak_provider : static_route_config_providers_) {
const auto strong_provider = weak_provider.lock();
if (strong_provider != nullptr) {
providers_strong.push_back(strong_provider);
}
}

// Replace our stored list of weak_ptrs with the filtered list.
static_route_config_providers_.assign(providers_strong.begin(), providers_strong.end());

return providers_strong;
};

Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getRdsRouteConfigProvider(
Router::RouteConfigProviderPtr RouteConfigProviderManagerImpl::createRdsRouteConfigProvider(
const envoy::config::filter::network::http_connection_manager::v2::Rds& rds,
Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix) {

Expand Down Expand Up @@ -252,43 +227,50 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getRdsRoute
}
ASSERT(subscription);

Router::RouteConfigProviderSharedPtr new_provider{
Router::RouteConfigProviderPtr new_provider{
new RdsRouteConfigProviderImpl(std::move(subscription), factory_context)};
return new_provider;
};
}

RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getStaticRouteConfigProvider(
RouteConfigProviderPtr RouteConfigProviderManagerImpl::createStaticRouteConfigProvider(
const envoy::api::v2::RouteConfiguration& route_config,
Server::Configuration::FactoryContext& factory_context) {
auto provider =
std::make_shared<StaticRouteConfigProviderImpl>(std::move(route_config), factory_context);
static_route_config_providers_.push_back(provider);
absl::make_unique<StaticRouteConfigProviderImpl>(route_config, factory_context, *this);
static_route_config_providers_.insert(provider.get());
return provider;
}

ProtobufTypes::MessagePtr RouteConfigProviderManagerImpl::dumpRouteConfigs() {
std::unique_ptr<envoy::admin::v2alpha::RoutesConfigDump>
RouteConfigProviderManagerImpl::dumpRouteConfigs() const {
auto config_dump = std::make_unique<envoy::admin::v2alpha::RoutesConfigDump>();

for (const auto& provider : getRdsRouteConfigProviders()) {
auto config_info = provider->configInfo();
if (config_info) {
for (const auto& element : route_config_subscriptions_) {
// Because the RouteConfigProviderManager's weak_ptrs only get cleaned up
// in the RdsRouteConfigSubscription destructor, and the single threaded nature
// of this code, locking the weak_ptr will not fail.
auto subscription = element.second.lock();
ASSERT(subscription);
ASSERT(subscription->route_config_providers_.size() > 0);

if (subscription->config_info_) {
auto* dynamic_config = config_dump->mutable_dynamic_route_configs()->Add();
dynamic_config->set_version_info(config_info.value().version_);
dynamic_config->mutable_route_config()->MergeFrom(config_info.value().config_);
TimestampUtil::systemClockToTimestamp(provider->lastUpdated(),
*(dynamic_config->mutable_last_updated()));
dynamic_config->set_version_info(subscription->config_info_.value().last_config_version_);
dynamic_config->mutable_route_config()->MergeFrom(subscription->route_config_proto_);
TimestampUtil::systemClockToTimestamp(subscription->last_updated_,
*dynamic_config->mutable_last_updated());
}
}

for (const auto& provider : getStaticRouteConfigProviders()) {
for (const auto& provider : static_route_config_providers_) {
ASSERT(provider->configInfo());
auto* static_config = config_dump->mutable_static_route_configs()->Add();
static_config->mutable_route_config()->MergeFrom(provider->configInfo().value().config_);
TimestampUtil::systemClockToTimestamp(provider->lastUpdated(),
*(static_config->mutable_last_updated()));
*static_config->mutable_last_updated());
}

return ProtobufTypes::MessagePtr{std::move(config_dump)};
return config_dump;
}

} // namespace Router
Expand Down
34 changes: 17 additions & 17 deletions source/common/router/rds_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <unordered_map>
#include <unordered_set>

#include "envoy/admin/v2alpha/config_dump.pb.h"
#include "envoy/api/v2/rds.pb.h"
#include "envoy/api/v2/route/route.pb.h"
#include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h"
Expand Down Expand Up @@ -35,20 +36,24 @@ class RouteConfigProviderUtil {
* @return RouteConfigProviderPtr a new route configuration provider based on the supplied proto
* configuration.
*/
static RouteConfigProviderSharedPtr
static RouteConfigProviderPtr
create(const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager&
config,
Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix,
RouteConfigProviderManager& route_config_provider_manager);
};

class RouteConfigProviderManagerImpl;

/**
* Implementation of RouteConfigProvider that holds a static route configuration.
*/
class StaticRouteConfigProviderImpl : public RouteConfigProvider {
public:
StaticRouteConfigProviderImpl(const envoy::api::v2::RouteConfiguration& config,
Server::Configuration::FactoryContext& factory_context);
Server::Configuration::FactoryContext& factory_context,
RouteConfigProviderManagerImpl& route_config_provider_manager);
~StaticRouteConfigProviderImpl();

// Router::RouteConfigProvider
Router::ConfigConstSharedPtr config() override { return config_; }
Expand All @@ -61,6 +66,7 @@ class StaticRouteConfigProviderImpl : public RouteConfigProvider {
ConfigConstSharedPtr config_;
envoy::api::v2::RouteConfiguration route_config_proto_;
SystemTime last_updated_;
RouteConfigProviderManagerImpl& route_config_provider_manager_;
};

/**
Expand All @@ -80,7 +86,6 @@ struct RdsStats {
ALL_RDS_STATS(GENERATE_COUNTER_STRUCT)
};

class RouteConfigProviderManagerImpl;
class RdsRouteConfigProviderImpl;

/**
Expand Down Expand Up @@ -146,7 +151,6 @@ typedef std::shared_ptr<RdsRouteConfigSubscription> RdsRouteConfigSubscriptionSh
* the subscription.
*/
class RdsRouteConfigProviderImpl : public RouteConfigProvider,
public std::enable_shared_from_this<RdsRouteConfigProviderImpl>,
Logger::Loggable<Logger::Id::router> {
public:
~RdsRouteConfigProviderImpl();
Expand All @@ -172,7 +176,6 @@ class RdsRouteConfigProviderImpl : public RouteConfigProvider,
RdsRouteConfigSubscriptionSharedPtr subscription_;
Server::Configuration::FactoryContext& factory_context_;
ThreadLocal::SlotPtr tls_;
const std::string route_config_name_;

friend class RouteConfigProviderManagerImpl;
};
Expand All @@ -182,32 +185,29 @@ class RouteConfigProviderManagerImpl : public RouteConfigProviderManager,
public:
RouteConfigProviderManagerImpl(Server::Admin& admin);

// RouteConfigProviderManager
std::vector<RouteConfigProviderSharedPtr> getRdsRouteConfigProviders() override;
std::vector<RouteConfigProviderSharedPtr> getStaticRouteConfigProviders() override;
std::unique_ptr<envoy::admin::v2alpha::RoutesConfigDump> dumpRouteConfigs() const;

RouteConfigProviderSharedPtr getRdsRouteConfigProvider(
// RouteConfigProviderManager
RouteConfigProviderPtr createRdsRouteConfigProvider(
const envoy::config::filter::network::http_connection_manager::v2::Rds& rds,
Server::Configuration::FactoryContext& factory_context,
const std::string& stat_prefix) override;

RouteConfigProviderSharedPtr
getStaticRouteConfigProvider(const envoy::api::v2::RouteConfiguration& route_config,
Server::Configuration::FactoryContext& factory_context) override;
RouteConfigProviderPtr
createStaticRouteConfigProvider(const envoy::api::v2::RouteConfiguration& route_config,
Server::Configuration::FactoryContext& factory_context) override;

private:
ProtobufTypes::MessagePtr dumpRouteConfigs();

// TODO(jsedgwick) These two members are prime candidates for the owned-entry list/map
// as in ConfigTracker. I.e. the ProviderImpls would have an EntryOwner for these lists
// Then the lifetime management stuff is centralized and opaque. Plus the copypasta
// in getRdsRouteConfigProviders()/getStaticRouteConfigProviders() goes away.
// Then the lifetime management stuff is centralized and opaque.
std::unordered_map<std::string, std::weak_ptr<RdsRouteConfigSubscription>>
route_config_subscriptions_;
std::vector<std::weak_ptr<RouteConfigProvider>> static_route_config_providers_;
std::unordered_set<RouteConfigProvider*> static_route_config_providers_;
Server::ConfigTracker::EntryOwnerPtr config_tracker_entry_;

friend class RdsRouteConfigSubscription;
friend class StaticRouteConfigProviderImpl;
};

} // namespace Router
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/network/http_connection_manager/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
absl::optional<std::string> user_agent_;
absl::optional<std::chrono::milliseconds> idle_timeout_;
std::chrono::milliseconds stream_idle_timeout_;
Router::RouteConfigProviderSharedPtr route_config_provider_;
Router::RouteConfigProviderPtr route_config_provider_;
std::chrono::milliseconds drain_timeout_;
bool generate_request_id_;
Http::DateProvider& date_provider_;
Expand Down
Loading

0 comments on commit 2012c3e

Please sign in to comment.