From 5d0918c28856d38c3de8d2e35a308483ba132aa0 Mon Sep 17 00:00:00 2001 From: Rama Malladi Date: Wed, 27 Jul 2022 18:30:37 +0530 Subject: [PATCH 1/7] RingHash code optimization using a index bucketing/ sharding algorithm --- source/common/upstream/ring_hash_lb.cc | 87 +++++++++++++++++++++----- source/common/upstream/ring_hash_lb.h | 11 +++- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc index 08a668223b416..7285ebe6bce9a 100644 --- a/source/common/upstream/ring_hash_lb.cc +++ b/source/common/upstream/ring_hash_lb.cc @@ -1,4 +1,4 @@ -#include "common/upstream/ring_hash_lb.h" +#include "source/common/upstream/ring_hash_lb.h" #include #include @@ -7,8 +7,8 @@ #include "envoy/config/cluster/v3/cluster.pb.h" -#include "common/common/assert.h" -#include "common/upstream/load_balancer_impl.h" +#include "source/common/common/assert.h" +#include "source/common/upstream/load_balancer_impl.h" #include "absl/container/inlined_vector.h" #include "absl/strings/string_view.h" @@ -58,8 +58,25 @@ HostConstSharedPtr RingHashLoadBalancer::Ring::chooseHost(uint64_t h, uint32_t a // I've generally kept the variable names to make the code easier to compare. // NOTE: The algorithm depends on using signed integers for lowp, midp, and highp. Do not // change them! - int64_t lowp = 0; - int64_t highp = ring_.size(); + int64_t lowp, highp; + + // Algorithm is to shard the indices and lookup the host for a given hash from a bucket + // (instead of a lookup of all hosts). Hence fewer lookups/ accesses and faster execution. +#ifdef BUCKET_ALGORITHM + + // Given a hash 'h', find the bucket index by shifting it to right (by rightShift). + uint64_t bucket_index = h >> rightShift; + // 'lowp' and 'highp' are the lower and upper indices of the bucket. + lowp = ring_bucket_[bucket_index]; + highp = ring_bucket_[bucket_index + 1] - 1; + +#else + + lowp = 0; + highp = ring_.size(); + +#endif + int64_t midp = 0; while (true) { midp = (lowp + highp) / 2; @@ -149,11 +166,10 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho uint64_t max_hashes_per_host = 0; for (const auto& entry : normalized_host_weights) { const auto& host = entry.first; - const std::string& address_string = - use_hostname_for_hashing ? host->hostname() : host->address()->asString(); - ASSERT(!address_string.empty()); + const absl::string_view key_to_hash = hashKey(host, use_hostname_for_hashing); + ASSERT(!key_to_hash.empty()); - hash_key_buffer.assign(address_string.begin(), address_string.end()); + hash_key_buffer.assign(key_to_hash.begin(), key_to_hash.end()); hash_key_buffer.emplace_back('_'); auto offset_start = hash_key_buffer.end(); @@ -173,7 +189,7 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho ? MurmurHash::murmurHash2(hash_key, MurmurHash::STD_HASH_SEED) : HashUtil::xxHash64(hash_key); - ENVOY_LOG(trace, "ring hash: hash_key={} hash={}", hash_key.data(), hash); + ENVOY_LOG(trace, "ring hash: hash_key={} hash={}", hash_key, hash); ring_.push_back({hash, host}); ++i; ++current_hashes; @@ -188,12 +204,55 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho }); if (ENVOY_LOG_CHECK_LEVEL(trace)) { for (const auto& entry : ring_) { - ENVOY_LOG(trace, "ring hash: host={} hash={}", - use_hostname_for_hashing ? entry.host_->hostname() - : entry.host_->address()->asString(), - entry.hash_); + const absl::string_view key_to_hash = hashKey(entry.host_, use_hostname_for_hashing); + ENVOY_LOG(trace, "ring hash: host={} hash={}", key_to_hash, entry.hash_); + } + } + +#ifdef BUCKET_ALGORITHM + /******* BEGIN: code for bucketing algorithm *******/ + + // Find MSB bit of the first hash so we can right shift all the other hashes to create buckets. + int msb = 0; + uint64_t n = ring_[0].hash_; + n = n / 2; + while (n != 0) { + n = n / 2; + msb++; + } + // Arbitrarily choosing MSB + 10 bits to shift to right for creating bucket. The larger the shift + // to right, the fewer the buckets. One can experiments with + 10, 12, 14, ... 20. + msb += 10; + // Save it for later use (when doing lookup of hosts for a given hash). + rightShift = msb; + + // Reserve memory for bucket indices. Worst-case, every hash belongs to a different bucket! + // The ring_bucket_ container stores the start indices of the buckets. + ring_bucket_.reserve(ring_size); + + // Right shift each hash and create buckets of the hosts. + uint64_t ring_index = 0; + uint64_t curr_bucket, prev_bucket = 0; + + // push the first index if the ring_ isn't empty + if(!ring_.empty()) + ring_bucket_.push_back(ring_index); + + for (const auto& entry : ring_) { + curr_bucket = entry.hash_ >> msb; + // If new bucket found, push the index to ring_bucket_ and update curr_bucket. + if(curr_bucket != prev_bucket) + { + prev_bucket = curr_bucket; + ring_bucket_.push_back(ring_index); } + ring_index++; } + // For the last bucket, we need end and hence storing ring_size value. + ring_bucket_.push_back(ring_size); + + /******* END: code for bucketing algorithm *******/ +#endif stats_.size_.set(ring_size); stats_.min_hashes_per_host_.set(min_hashes_per_host); diff --git a/source/common/upstream/ring_hash_lb.h b/source/common/upstream/ring_hash_lb.h index 2f6dc03ab5636..6aa1b656ec59b 100644 --- a/source/common/upstream/ring_hash_lb.h +++ b/source/common/upstream/ring_hash_lb.h @@ -7,8 +7,8 @@ #include "envoy/stats/scope.h" #include "envoy/stats/stats_macros.h" -#include "common/common/logger.h" -#include "common/upstream/thread_aware_lb_impl.h" +#include "source/common/common/logger.h" +#include "source/common/upstream/thread_aware_lb_impl.h" namespace Envoy { namespace Upstream { @@ -66,6 +66,11 @@ class RingHashLoadBalancer : public ThreadAwareLoadBalancerBase, std::vector ring_; +#ifdef BUCKET_ALGORITHM + std::vector ring_bucket_; + uint64_t rightShift; +#endif + RingHashLoadBalancerStats& stats_; }; using RingConstSharedPtr = std::shared_ptr; @@ -87,7 +92,7 @@ class RingHashLoadBalancer : public ThreadAwareLoadBalancerBase, static RingHashLoadBalancerStats generateStats(Stats::Scope& scope); - Stats::ScopePtr scope_; + Stats::ScopeSharedPtr scope_; RingHashLoadBalancerStats stats_; static const uint64_t DefaultMinRingSize = 1024; From 897c9e69fd29d72066a06483cea45d78049864a1 Mon Sep 17 00:00:00 2001 From: Rama Malladi <98832537+RamaMalladiAWS@users.noreply.github.com> Date: Wed, 27 Jul 2022 18:52:39 +0530 Subject: [PATCH 2/7] Update ring_hash_lb.cc --- source/common/upstream/ring_hash_lb.cc | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc index 7285ebe6bce9a..e415d541d6a50 100644 --- a/source/common/upstream/ring_hash_lb.cc +++ b/source/common/upstream/ring_hash_lb.cc @@ -1,4 +1,4 @@ -#include "source/common/upstream/ring_hash_lb.h" +#include "common/upstream/ring_hash_lb.h" #include #include @@ -7,8 +7,8 @@ #include "envoy/config/cluster/v3/cluster.pb.h" -#include "source/common/common/assert.h" -#include "source/common/upstream/load_balancer_impl.h" +#include "common/common/assert.h" +#include "common/upstream/load_balancer_impl.h" #include "absl/container/inlined_vector.h" #include "absl/strings/string_view.h" @@ -166,10 +166,11 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho uint64_t max_hashes_per_host = 0; for (const auto& entry : normalized_host_weights) { const auto& host = entry.first; - const absl::string_view key_to_hash = hashKey(host, use_hostname_for_hashing); - ASSERT(!key_to_hash.empty()); + const std::string& address_string = + use_hostname_for_hashing ? host->hostname() : host->address()->asString(); + ASSERT(!address_string.empty()); - hash_key_buffer.assign(key_to_hash.begin(), key_to_hash.end()); + hash_key_buffer.assign(address_string.begin(), address_string.end()); hash_key_buffer.emplace_back('_'); auto offset_start = hash_key_buffer.end(); @@ -189,7 +190,7 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho ? MurmurHash::murmurHash2(hash_key, MurmurHash::STD_HASH_SEED) : HashUtil::xxHash64(hash_key); - ENVOY_LOG(trace, "ring hash: hash_key={} hash={}", hash_key, hash); + ENVOY_LOG(trace, "ring hash: hash_key={} hash={}", hash_key.data(), hash); ring_.push_back({hash, host}); ++i; ++current_hashes; From 74e0203f1577f06764a22cc550efbbe6f639505a Mon Sep 17 00:00:00 2001 From: Rama Malladi <98832537+RamaMalladiAWS@users.noreply.github.com> Date: Wed, 27 Jul 2022 18:54:59 +0530 Subject: [PATCH 3/7] Update ring_hash_lb.h --- source/common/upstream/ring_hash_lb.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/common/upstream/ring_hash_lb.h b/source/common/upstream/ring_hash_lb.h index 6aa1b656ec59b..d156796d0c76c 100644 --- a/source/common/upstream/ring_hash_lb.h +++ b/source/common/upstream/ring_hash_lb.h @@ -7,8 +7,8 @@ #include "envoy/stats/scope.h" #include "envoy/stats/stats_macros.h" -#include "source/common/common/logger.h" -#include "source/common/upstream/thread_aware_lb_impl.h" +#include "common/common/logger.h" +#include "common/upstream/thread_aware_lb_impl.h" namespace Envoy { namespace Upstream { @@ -92,7 +92,7 @@ class RingHashLoadBalancer : public ThreadAwareLoadBalancerBase, static RingHashLoadBalancerStats generateStats(Stats::Scope& scope); - Stats::ScopeSharedPtr scope_; + Stats::ScopePtr scope_; RingHashLoadBalancerStats stats_; static const uint64_t DefaultMinRingSize = 1024; From 54ae9bb525daf84d67cd7e426e320803354c6251 Mon Sep 17 00:00:00 2001 From: Rama Malladi <98832537+RamaMalladiAWS@users.noreply.github.com> Date: Wed, 27 Jul 2022 18:58:26 +0530 Subject: [PATCH 4/7] Update ring_hash_lb.cc --- source/common/upstream/ring_hash_lb.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc index e415d541d6a50..b6ce09ea7937d 100644 --- a/source/common/upstream/ring_hash_lb.cc +++ b/source/common/upstream/ring_hash_lb.cc @@ -166,7 +166,7 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho uint64_t max_hashes_per_host = 0; for (const auto& entry : normalized_host_weights) { const auto& host = entry.first; - const std::string& address_string = + const std::string& address_string = use_hostname_for_hashing ? host->hostname() : host->address()->asString(); ASSERT(!address_string.empty()); @@ -205,8 +205,10 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho }); if (ENVOY_LOG_CHECK_LEVEL(trace)) { for (const auto& entry : ring_) { - const absl::string_view key_to_hash = hashKey(entry.host_, use_hostname_for_hashing); - ENVOY_LOG(trace, "ring hash: host={} hash={}", key_to_hash, entry.hash_); + ENVOY_LOG(trace, "ring_hash: host={} hash={}", + use_hostname_for_hashing ? entry.host_->hostname() + : entry.host_->address()->asString(), + entry.hash_); } } From 987c227ef7c1be69761e1ad448257ff62004db87 Mon Sep 17 00:00:00 2001 From: Rama Malladi <98832537+RamaMalladiAWS@users.noreply.github.com> Date: Wed, 27 Jul 2022 18:58:57 +0530 Subject: [PATCH 5/7] Update ring_hash_lb.cc --- source/common/upstream/ring_hash_lb.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc index b6ce09ea7937d..3141ea23f8a14 100644 --- a/source/common/upstream/ring_hash_lb.cc +++ b/source/common/upstream/ring_hash_lb.cc @@ -205,7 +205,7 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho }); if (ENVOY_LOG_CHECK_LEVEL(trace)) { for (const auto& entry : ring_) { - ENVOY_LOG(trace, "ring_hash: host={} hash={}", + ENVOY_LOG(trace, "ring hash: host={} hash={}", use_hostname_for_hashing ? entry.host_->hostname() : entry.host_->address()->asString(), entry.hash_); From f19bce24315218794b417300af79df7d6e111379 Mon Sep 17 00:00:00 2001 From: Rama Malladi <98832537+RamaMalladiAWS@users.noreply.github.com> Date: Thu, 28 Jul 2022 10:58:23 +0530 Subject: [PATCH 6/7] Update ring_hash_lb.cc --- source/common/upstream/ring_hash_lb.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc index 3141ea23f8a14..11c23cf38c9fe 100644 --- a/source/common/upstream/ring_hash_lb.cc +++ b/source/common/upstream/ring_hash_lb.cc @@ -223,11 +223,9 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho n = n / 2; msb++; } - // Arbitrarily choosing MSB + 10 bits to shift to right for creating bucket. The larger the shift - // to right, the fewer the buckets. One can experiments with + 10, 12, 14, ... 20. - msb += 10; - // Save it for later use (when doing lookup of hosts for a given hash). - rightShift = msb; + // Arbitrarily choosing MSB + 10 bits to shift hash to right for creating buckets. The larger the shift + // to right, the fewer the buckets. Experiment with values 10, 12, ... 30, the BUCKET_SHIFT parameter. + rightShift += msb; // Reserve memory for bucket indices. Worst-case, every hash belongs to a different bucket! // The ring_bucket_ container stores the start indices of the buckets. @@ -242,7 +240,7 @@ RingHashLoadBalancer::Ring::Ring(const NormalizedHostWeightVector& normalized_ho ring_bucket_.push_back(ring_index); for (const auto& entry : ring_) { - curr_bucket = entry.hash_ >> msb; + curr_bucket = entry.hash_ >> rightShift; // If new bucket found, push the index to ring_bucket_ and update curr_bucket. if(curr_bucket != prev_bucket) { From a623f47dc5ba488e0307c9f1f7acff4d20409fd0 Mon Sep 17 00:00:00 2001 From: Rama Malladi <98832537+RamaMalladiAWS@users.noreply.github.com> Date: Thu, 28 Jul 2022 11:14:06 +0530 Subject: [PATCH 7/7] Update ring_hash_lb.h Introduced BUCKET_SHIFT parameter --- source/common/upstream/ring_hash_lb.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/source/common/upstream/ring_hash_lb.h b/source/common/upstream/ring_hash_lb.h index d156796d0c76c..731b336498d78 100644 --- a/source/common/upstream/ring_hash_lb.h +++ b/source/common/upstream/ring_hash_lb.h @@ -13,6 +13,12 @@ namespace Envoy { namespace Upstream { +#ifdef BUCKET_ALGORITHM +#ifndef BUCKET_SHIFT +#define BUCKET_SHIFT 10 +#endif +#endif + /** * All ring hash load balancer stats. @see stats_macros.h */ @@ -68,7 +74,7 @@ class RingHashLoadBalancer : public ThreadAwareLoadBalancerBase, #ifdef BUCKET_ALGORITHM std::vector ring_bucket_; - uint64_t rightShift; + uint64_t rightShift = BUCKET_SHIFT; #endif RingHashLoadBalancerStats& stats_;