bug fix, stats: differentiate panic mode for local and upstream clust…

…ers. (envoyproxy#1631)
envoy-zipkin · Sep 14, 2017 · c806cc1 · c806cc1
1 parent bad711e
commit c806cc1
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 7 deletions.
diff --git a/source/common/upstream/load_balancer_impl.cc b/source/common/upstream/load_balancer_impl.cc
@@ -130,8 +130,7 @@ bool LoadBalancerBase::earlyExitNonZoneRouting() {
   return false;
 }
 
-bool LoadBalancerUtility::isGlobalPanic(const HostSet& host_set, ClusterStats& stats,
-                                        Runtime::Loader& runtime) {
+bool LoadBalancerUtility::isGlobalPanic(const HostSet& host_set, Runtime::Loader& runtime) {
   uint64_t global_panic_threshold =
       std::min<uint64_t>(100, runtime.snapshot().getInteger(RuntimePanicThreshold, 50));
   double healthy_percent = host_set.hosts().size() == 0
@@ -140,7 +139,6 @@ bool LoadBalancerUtility::isGlobalPanic(const HostSet& host_set, ClusterStats& s
 
   // If the % of healthy hosts in the cluster is less than our panic threshold, we use all hosts.
   if (healthy_percent < global_panic_threshold) {
-    stats.lb_healthy_panic_.inc();
     return true;
   }
 
@@ -211,7 +209,8 @@ const std::vector<HostSharedPtr>& LoadBalancerBase::tryChooseLocalZoneHosts() {
 const std::vector<HostSharedPtr>& LoadBalancerBase::hostsToUse() {
   ASSERT(host_set_.healthyHosts().size() <= host_set_.hosts().size());
 
-  if (LoadBalancerUtility::isGlobalPanic(host_set_, stats_, runtime_)) {
+  if (LoadBalancerUtility::isGlobalPanic(host_set_, runtime_)) {
+    stats_.lb_healthy_panic_.inc();
     return host_set_.hosts();
   }
 
@@ -223,7 +222,7 @@ const std::vector<HostSharedPtr>& LoadBalancerBase::hostsToUse() {
     return host_set_.healthyHosts();
   }
 
-  if (LoadBalancerUtility::isGlobalPanic(*local_host_set_, stats_, runtime_)) {
+  if (LoadBalancerUtility::isGlobalPanic(*local_host_set_, runtime_)) {
     stats_.lb_local_cluster_not_ok_.inc();
     return host_set_.healthyHosts();
   }

diff --git a/source/common/upstream/load_balancer_impl.h b/source/common/upstream/load_balancer_impl.h
@@ -20,7 +20,7 @@ class LoadBalancerUtility {
    * majority of hosts are unhealthy we'll be likely in a panic mode. In this case we'll route
    * requests to hosts regardless of whether they are healthy or not.
    */
-  static bool isGlobalPanic(const HostSet& host_set, ClusterStats& stats, Runtime::Loader& runtime);
+  static bool isGlobalPanic(const HostSet& host_set, Runtime::Loader& runtime);
 };
 
 /**

diff --git a/source/common/upstream/ring_hash_lb.cc b/source/common/upstream/ring_hash_lb.cc
@@ -21,7 +21,8 @@ RingHashLoadBalancer::RingHashLoadBalancer(HostSet& host_set, ClusterStats& stat
 }
 
 HostConstSharedPtr RingHashLoadBalancer::chooseHost(const LoadBalancerContext* context) {
-  if (LoadBalancerUtility::isGlobalPanic(host_set_, stats_, runtime_)) {
+  if (LoadBalancerUtility::isGlobalPanic(host_set_, runtime_)) {
+    stats_.lb_healthy_panic_.inc();
     return all_hosts_ring_.chooseHost(context, random_);
   } else {
     return healthy_hosts_ring_.chooseHost(context, random_);

diff --git a/test/common/upstream/load_balancer_impl_test.cc b/test/common/upstream/load_balancer_impl_test.cc
@@ -371,6 +371,7 @@ TEST_F(RoundRobinLoadBalancerTest, NoZoneAwareRoutingLocalEmpty) {
 
   // Local cluster is not OK, we'll do regular routing.
   EXPECT_EQ(cluster_.healthy_hosts_[0], lb_->chooseHost(nullptr));
+  EXPECT_EQ(0U, stats_.lb_healthy_panic_.value());
   EXPECT_EQ(1U, stats_.lb_local_cluster_not_ok_.value());
 }
 

diff --git a/test/common/upstream/ring_hash_lb_test.cc b/test/common/upstream/ring_hash_lb_test.cc
@@ -92,13 +92,15 @@ TEST_F(RingHashLoadBalancerTest, Basic) {
     EXPECT_CALL(random_, random()).WillOnce(Return(10150910876324007730UL));
     EXPECT_EQ(cluster_.hosts_[2], lb_.chooseHost(nullptr));
   }
+  EXPECT_EQ(0UL, stats_.lb_healthy_panic_.value());
 
   cluster_.healthy_hosts_.clear();
   cluster_.runCallbacks({}, {});
   {
     TestLoadBalancerContext context(0);
     EXPECT_EQ(cluster_.hosts_[5], lb_.chooseHost(&context));
   }
+  EXPECT_EQ(1UL, stats_.lb_healthy_panic_.value());
 }
 
 TEST_F(RingHashLoadBalancerTest, UnevenHosts) {