From da424a72da8a0e2c582d0b05f0e88cbbadcfc4b1 Mon Sep 17 00:00:00 2001 From: Dan Rosen Date: Mon, 4 Feb 2019 13:04:55 -0500 Subject: [PATCH 1/3] host weight ranges from 1 to 128 Signed-off-by: Dan Rosen --- include/envoy/upstream/upstream.h | 6 ++++-- source/common/upstream/maglev_lb.cc | 24 ++++++++++-------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 610338d44b223..e280ab7f515f7 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -164,12 +164,14 @@ class Host : virtual public HostDescription { virtual void setOutlierDetector(Outlier::DetectorHostMonitorPtr&& outlier_detector) PURE; /** - * @return the current load balancing weight of the host, in the range 1-100. + * @return the current load balancing weight of the host, in the range 1-128 (see + * envoy.api.v2.endpoint.Endpoint.load_balancing_weight). */ virtual uint32_t weight() const PURE; /** - * Set the current load balancing weight of the host, in the range 1-100. + * Set the current load balancing weight of the host, in the range 1-128 (see + * envoy.api.v2.endpoint.Endpoint.load_balancing_weight). */ virtual void weight(uint32_t new_weight) PURE; diff --git a/source/common/upstream/maglev_lb.cc b/source/common/upstream/maglev_lb.cc index 8e3e91359722d..10c6cbd423964 100644 --- a/source/common/upstream/maglev_lb.cc +++ b/source/common/upstream/maglev_lb.cc @@ -23,7 +23,7 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, } }; - // Compute maximum host weight. If this is zero, we are doing unweighted Maglev. + // Compute maximum host weight. uint32_t max_host_weight = 0; uint32_t total_hosts = 0; for (uint32_t i = 0; i < hosts_per_locality.get().size(); ++i) { @@ -46,8 +46,7 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, const std::string& address = host->address()->asString(); table_build_entries.emplace_back(host, HashUtil::xxHash64(address) % table_size_, (HashUtil::xxHash64(address, 1) % (table_size_ - 1)) + 1, - max_host_weight > 0 ? effective_weight(host->weight(), i) - : 0); + effective_weight(host->weight(), i)); } } @@ -57,18 +56,15 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, while (true) { for (uint64_t i = 0; i < table_build_entries.size(); i++) { TableBuildEntry& entry = table_build_entries[i]; - // Only consider weight if we are doing weighted Maglev. - if (max_host_weight > 0) { - // Counts are in units of max_host_weight. To understand how counts_ and - // weight_ are used below, consider a host with weight equal to - // max_host_weight. This would be picked on every single iteration. If - // it had weight equal to backend_weight_scale / 3, then this would only - // happen every 3 iterations, etc. - if (iteration * entry.weight_ < entry.counts_) { - continue; - } - entry.counts_ += max_host_weight; + // Counts are in units of max_host_weight. To understand how counts_ and + // weight_ are used below, consider a host with weight equal to + // max_host_weight. This would be picked on every single iteration. If + // it had weight equal to backend_weight_scale / 3, then this would only + // happen every 3 iterations, etc. + if (iteration * entry.weight_ < entry.counts_) { + continue; } + entry.counts_ += max_host_weight; uint64_t c = permutation(entry); while (table_[c] != nullptr) { entry.next_++; From 636d1f00859915516624a4c27e8fa3de6335172f Mon Sep 17 00:00:00 2001 From: Dan Rosen Date: Mon, 4 Feb 2019 16:41:52 -0500 Subject: [PATCH 2/3] Provide some sanity checking around host and locality weights. Signed-off-by: Dan Rosen --- source/common/upstream/maglev_lb.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/source/common/upstream/maglev_lb.cc b/source/common/upstream/maglev_lb.cc index 10c6cbd423964..8895d87af6504 100644 --- a/source/common/upstream/maglev_lb.cc +++ b/source/common/upstream/maglev_lb.cc @@ -13,13 +13,21 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, // not good!). ASSERT(Primes::isPrime(table_size)); + // Sanity-check that the locality weights, if provided, line up with the hosts per locality. + if (locality_weights != nullptr) { + ASSERT(locality_weights->size() == hosts_per_locality.get().size()); + } + // Compute host weight combined with locality weight where applicable. const auto effective_weight = [&locality_weights](uint32_t host_weight, uint32_t locality_index) -> uint32_t { - if (locality_weights == nullptr || locality_weights->empty()) { + ASSERT(host_weight != 0); + if (locality_weights == nullptr) { return host_weight; } else { - return host_weight * (*locality_weights)[locality_index]; + auto locality_weight = (*locality_weights)[locality_index]; + ASSERT(locality_weight != 0); + return host_weight * locality_weight; } }; From 26972c516d453359b4a03794935390fe3dc44e56 Mon Sep 17 00:00:00 2001 From: Dan Rosen Date: Tue, 5 Feb 2019 10:34:08 -0500 Subject: [PATCH 3/3] sanity-check that if we're skipping an entry, max weight must be > 1 Signed-off-by: Dan Rosen --- source/common/upstream/maglev_lb.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/upstream/maglev_lb.cc b/source/common/upstream/maglev_lb.cc index 8895d87af6504..292f901aed4c7 100644 --- a/source/common/upstream/maglev_lb.cc +++ b/source/common/upstream/maglev_lb.cc @@ -70,6 +70,7 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, // it had weight equal to backend_weight_scale / 3, then this would only // happen every 3 iterations, etc. if (iteration * entry.weight_ < entry.counts_) { + ASSERT(max_host_weight > 1); continue; } entry.counts_ += max_host_weight;