Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upstream: log metadata used to create subset lbs #2772

Merged
merged 3 commits into from
Mar 22, 2018
Merged

upstream: log metadata used to create subset lbs #2772

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
165944f
upstream: log metadata used to create subset lbs
zuercher Mar 9, 2018
1a2f9bd
Merge branch 'master' into stephan/log-subset-lb-criteria
zuercher Mar 19, 2018
6878b09
test describeMetadata directly
zuercher Mar 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 66 additions & 45 deletions source/common/upstream/subset_lb.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ SubsetLoadBalancer::SubsetLoadBalancer(
const envoy::api::v2::Cluster::CommonLbConfig& common_config)
: lb_type_(lb_type), lb_ring_hash_config_(lb_ring_hash_config), common_config_(common_config),
stats_(stats), runtime_(runtime), random_(random), fallback_policy_(subsets.fallbackPolicy()),
default_subset_(subsets.defaultSubset()), subset_keys_(subsets.subsetKeys()),
original_priority_set_(priority_set), original_local_priority_set_(local_priority_set) {
default_subset_metadata_(subsets.defaultSubset().fields().begin(),
subsets.defaultSubset().fields().end()),
subset_keys_(subsets.subsetKeys()), original_priority_set_(priority_set),
original_local_priority_set_(local_priority_set) {
ASSERT(subsets.isEnabled());

// Create filtered default subset (if necessary) and other subsets based on current hosts.
Expand Down Expand Up @@ -124,19 +126,30 @@ SubsetLoadBalancer::LbSubsetEntryPtr SubsetLoadBalancer::findSubset(
void SubsetLoadBalancer::updateFallbackSubset(uint32_t priority, const HostVector& hosts_added,
const HostVector& hosts_removed) {
if (fallback_policy_ == envoy::api::v2::Cluster::LbSubsetConfig::NO_FALLBACK) {
ENVOY_LOG(debug, "subset lb: fallback load balancer disabled");
return;
}

HostPredicate predicate;

if (fallback_policy_ == envoy::api::v2::Cluster::LbSubsetConfig::ANY_ENDPOINT) {
bool fallback_any = (fallback_policy_ == envoy::api::v2::Cluster::LbSubsetConfig::ANY_ENDPOINT) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: const

default_subset_metadata_.empty();

if (fallback_any) {
predicate = [](const Host&) -> bool { return true; };
} else {
predicate =
std::bind(&SubsetLoadBalancer::hostMatchesDefaultSubset, this, std::placeholders::_1);
predicate = std::bind(&SubsetLoadBalancer::hostMatches, this, default_subset_metadata_,
std::placeholders::_1);
}

if (fallback_subset_ == nullptr) {
if (fallback_any) {
ENVOY_LOG(debug, "subset lb: creating any-endpoint fallback load balancer");
} else {
ENVOY_LOG(debug, "subset lb: creating fallback load balancer for {}",
describeMetadata(default_subset_metadata_));
}

// First update: create the default host subset.
fallback_subset_.reset(new LbSubsetEntry());
fallback_subset_->priority_subset_.reset(new PrioritySubsetImpl(*this, predicate));
Expand All @@ -151,7 +164,7 @@ void SubsetLoadBalancer::updateFallbackSubset(uint32_t priority, const HostVecto
// selects hosts in the subset, and a flag indicating whether any hosts are being added.
void SubsetLoadBalancer::processSubsets(
const HostVector& hosts_added, const HostVector& hosts_removed,
std::function<void(LbSubsetEntryPtr, HostPredicate, bool)> cb) {
std::function<void(LbSubsetEntryPtr, HostPredicate, const SubsetMetadata&, bool)> cb) {
std::unordered_set<LbSubsetEntryPtr> subsets_modified;

std::pair<const HostVector&, bool> steps[] = {{hosts_added, true}, {hosts_removed, false}};
Expand All @@ -176,7 +189,7 @@ void SubsetLoadBalancer::processSubsets(
HostPredicate predicate =
std::bind(&SubsetLoadBalancer::hostMatches, this, kvs, std::placeholders::_1);

cb(entry, predicate, adding_hosts);
cb(entry, predicate, kvs, adding_hosts);
}
}
}
Expand All @@ -189,44 +202,32 @@ void SubsetLoadBalancer::update(uint32_t priority, const HostVector& hosts_added
const HostVector& hosts_removed) {
updateFallbackSubset(priority, hosts_added, hosts_removed);

processSubsets(hosts_added, hosts_removed,
[&](LbSubsetEntryPtr entry, HostPredicate predicate, bool adding_host) {
if (entry->initialized()) {
const bool active_before = entry->active();
entry->priority_subset_->update(priority, hosts_added, hosts_removed,
predicate);

if (active_before && !entry->active()) {
stats_.lb_subsets_active_.dec();
stats_.lb_subsets_removed_.inc();
} else if (!active_before && entry->active()) {
stats_.lb_subsets_active_.inc();
stats_.lb_subsets_created_.inc();
}
} else if (adding_host) {
// Initialize new entry with hosts and update stats. (An uninitialized entry
// with only removed hosts is a degenerate case and we leave the entry
// uninitialized.)
entry->priority_subset_.reset(new PrioritySubsetImpl(*this, predicate));
stats_.lb_subsets_active_.inc();
stats_.lb_subsets_created_.inc();
}
});
}

bool SubsetLoadBalancer::hostMatchesDefaultSubset(const Host& host) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup

const envoy::api::v2::core::Metadata& host_metadata = host.metadata();

for (const auto& it : default_subset_.fields()) {
const ProtobufWkt::Value& host_value = Config::Metadata::metadataValue(
host_metadata, Config::MetadataFilters::get().ENVOY_LB, it.first);

if (!ValueUtil::equal(host_value, it.second)) {
return false;
}
}

return true;
processSubsets(
hosts_added, hosts_removed,
[&](LbSubsetEntryPtr entry, HostPredicate predicate, const SubsetMetadata& kvs,
bool adding_host) {
if (entry->initialized()) {
const bool active_before = entry->active();
entry->priority_subset_->update(priority, hosts_added, hosts_removed, predicate);

if (active_before && !entry->active()) {
stats_.lb_subsets_active_.dec();
stats_.lb_subsets_removed_.inc();
} else if (!active_before && entry->active()) {
stats_.lb_subsets_active_.inc();
stats_.lb_subsets_created_.inc();
}
} else if (adding_host) {
ENVOY_LOG(debug, "subset lb: creating load balancer for {}", describeMetadata(kvs));

// Initialize new entry with hosts and update stats. (An uninitialized entry
// with only removed hosts is a degenerate case and we leave the entry
// uninitialized.)
entry->priority_subset_.reset(new PrioritySubsetImpl(*this, predicate));
stats_.lb_subsets_active_.inc();
stats_.lb_subsets_created_.inc();
}
});
}

bool SubsetLoadBalancer::hostMatches(const SubsetMetadata& kvs, const Host& host) {
Expand Down Expand Up @@ -273,6 +274,26 @@ SubsetLoadBalancer::extractSubsetMetadata(const std::set<std::string>& subset_ke
return kvs;
}

std::string SubsetLoadBalancer::describeMetadata(const SubsetLoadBalancer::SubsetMetadata& kvs) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we test this somehow? I think especially with #2751 this won't even get run in most normal cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test case for this.

if (kvs.empty()) {
return "<no metadata>";
}

std::ostringstream buf;
bool first = true;
for (const auto& it : kvs) {
if (!first) {
buf << ", ";
} else {
first = false;
}

buf << it.first << "=" << MessageUtil::getJsonStringFromMessage(it.second);
}

return buf.str();
}

// Given a vector of key-values (from extractSubsetMetadata), recursively finds the matching
// LbSubsetEntryPtr.
SubsetLoadBalancer::LbSubsetEntryPtr
Expand Down
9 changes: 5 additions & 4 deletions source/common/upstream/subset_lb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable<Logger::Id::ups

void updateFallbackSubset(uint32_t priority, const HostVector& hosts_added,
const HostVector& hosts_removed);
void processSubsets(const HostVector& hosts_added, const HostVector& hosts_removed,
std::function<void(LbSubsetEntryPtr, HostPredicate, bool)> cb);
void processSubsets(
const HostVector& hosts_added, const HostVector& hosts_removed,
std::function<void(LbSubsetEntryPtr, HostPredicate, const SubsetMetadata&, bool)> cb);

HostConstSharedPtr tryChooseHostFromContext(LoadBalancerContext* context, bool& host_chosen);

bool hostMatchesDefaultSubset(const Host& host);
bool hostMatches(const SubsetMetadata& kvs, const Host& host);

LbSubsetEntryPtr
Expand All @@ -126,6 +126,7 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable<Logger::Id::ups
uint32_t idx);

SubsetMetadata extractSubsetMetadata(const std::set<std::string>& subset_keys, const Host& host);
std::string describeMetadata(const SubsetMetadata& kvs);

const LoadBalancerType lb_type_;
const Optional<envoy::api::v2::Cluster::RingHashLbConfig> lb_ring_hash_config_;
Expand All @@ -135,7 +136,7 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable<Logger::Id::ups
Runtime::RandomGenerator& random_;

const envoy::api::v2::Cluster::LbSubsetConfig::LbSubsetFallbackPolicy fallback_policy_;
const ProtobufWkt::Struct default_subset_;
const SubsetMetadata default_subset_metadata_;
const std::vector<std::set<std::string>> subset_keys_;

const PrioritySet& original_priority_set_;
Expand Down