Skip to content

Commit

Permalink
Use optional return type for hostSourceToUse; drop dummy_empty_host_v…
Browse files Browse the repository at this point in the history
…ector

Signed-off-by: James Forcier <jforcier@grubhub.com>
  • Loading branch information
James Forcier committed Aug 23, 2019
1 parent c35f5ab commit 867d7c5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 20 deletions.
32 changes: 18 additions & 14 deletions source/common/upstream/load_balancer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ uint32_t ZoneAwareLoadBalancerBase::tryChooseLocalLocalityHosts(const HostSet& h
return i;
}

ZoneAwareLoadBalancerBase::HostsSource
absl::optional<ZoneAwareLoadBalancerBase::HostsSource>
ZoneAwareLoadBalancerBase::hostSourceToUse(LoadBalancerContext* context) {
auto host_set_and_source = chooseHostSet(context);

Expand All @@ -554,11 +554,11 @@ ZoneAwareLoadBalancerBase::hostSourceToUse(LoadBalancerContext* context) {
if (per_priority_panic_[hosts_source.priority_]) {
stats_.lb_healthy_panic_.inc();
if (disable_cluster_on_panic_) {
hosts_source.source_type_ = HostsSource::SourceType::NoHosts;
return absl::nullopt;
} else {
hosts_source.source_type_ = HostsSource::SourceType::AllHosts;
return hosts_source;
}
return hosts_source;
}

// If we're doing locality weighted balancing, pick locality.
Expand Down Expand Up @@ -594,11 +594,11 @@ ZoneAwareLoadBalancerBase::hostSourceToUse(LoadBalancerContext* context) {
// If the local Envoy instances are in global panic, and we should not disable the cluster, do
// not do locality based routing.
if (disable_cluster_on_panic_) {
hosts_source.source_type_ = HostsSource::SourceType::NoHosts;
return absl::nullopt;
} else {
hosts_source.source_type_ = sourceType(host_availability);
return hosts_source;
}
return hosts_source;
}

hosts_source.source_type_ = localitySourceType(host_availability);
Expand All @@ -619,8 +619,6 @@ const HostVector& ZoneAwareLoadBalancerBase::hostSourceToHosts(HostsSource hosts
return host_set.healthyHostsPerLocality().get()[hosts_source.locality_index_];
case HostsSource::SourceType::LocalityDegradedHosts:
return host_set.degradedHostsPerLocality().get()[hosts_source.locality_index_];
case HostsSource::SourceType::NoHosts:
return dummy_empty_host_vector;
default:
NOT_REACHED_GCOVR_EXCL_LINE;
}
Expand Down Expand Up @@ -707,13 +705,14 @@ void EdfLoadBalancerBase::refresh(uint32_t priority) {
HostsSource(priority, HostsSource::SourceType::LocalityDegradedHosts, locality_index),
host_set->degradedHostsPerLocality().get()[locality_index]);
}
add_hosts_source(HostsSource(priority, HostsSource::SourceType::NoHosts),
dummy_empty_host_vector);
}

HostConstSharedPtr EdfLoadBalancerBase::chooseHostOnce(LoadBalancerContext* context) {
const HostsSource hosts_source = hostSourceToUse(context);
auto scheduler_it = scheduler_.find(hosts_source);
const absl::optional<HostsSource> hosts_source = hostSourceToUse(context);
if (!hosts_source) {
return nullptr;
}
auto scheduler_it = scheduler_.find(*hosts_source);
// We should always have a scheduler for any return value from
// hostSourceToUse() via the construction in refresh();
ASSERT(scheduler_it != scheduler_.end());
Expand All @@ -730,11 +729,11 @@ HostConstSharedPtr EdfLoadBalancerBase::chooseHostOnce(LoadBalancerContext* cont
}
return host;
} else {
const HostVector& hosts_to_use = hostSourceToHosts(hosts_source);
const HostVector& hosts_to_use = hostSourceToHosts(*hosts_source);
if (hosts_to_use.empty()) {
return nullptr;
}
return unweightedHostPick(hosts_to_use, hosts_source);
return unweightedHostPick(hosts_to_use, *hosts_source);
}
}

Expand Down Expand Up @@ -762,7 +761,12 @@ HostConstSharedPtr LeastRequestLoadBalancer::unweightedHostPick(const HostVector
}

HostConstSharedPtr RandomLoadBalancer::chooseHostOnce(LoadBalancerContext* context) {
const HostVector& hosts_to_use = hostSourceToHosts(hostSourceToUse(context));
const absl::optional<HostsSource> hosts_source = hostSourceToUse(context);
if (!hosts_source) {
return nullptr;
}

const HostVector& hosts_to_use = hostSourceToHosts(*hosts_source);
if (hosts_to_use.empty()) {
return nullptr;
}
Expand Down
8 changes: 2 additions & 6 deletions source/common/upstream/load_balancer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,14 @@ class ZoneAwareLoadBalancerBase : public LoadBalancerBase {
LocalityHealthyHosts,
// Degraded hosts for locality @ locality_index.
LocalityDegradedHosts,
// No hosts in the host set.
NoHosts,
};

HostsSource() = default;

HostsSource(uint32_t priority, SourceType source_type)
: priority_(priority), source_type_(source_type) {
ASSERT(source_type == SourceType::AllHosts || source_type == SourceType::HealthyHosts ||
source_type == SourceType::DegradedHosts || source_type == SourceType::NoHosts);
source_type == SourceType::DegradedHosts);
}

HostsSource(uint32_t priority, SourceType source_type, uint32_t locality_index)
Expand Down Expand Up @@ -226,15 +224,13 @@ class ZoneAwareLoadBalancerBase : public LoadBalancerBase {
/**
* Pick the host source to use, doing zone aware routing when the hosts are sufficiently healthy.
*/
HostsSource hostSourceToUse(LoadBalancerContext* context);
absl::optional<HostsSource> hostSourceToUse(LoadBalancerContext* context);

/**
* Index into priority_set via hosts source descriptor.
*/
const HostVector& hostSourceToHosts(HostsSource hosts_source);

const HostVector dummy_empty_host_vector;

private:
enum class LocalityRoutingState {
// Locality based routing is off.
Expand Down

0 comments on commit 867d7c5

Please sign in to comment.