config: backoff strategy implementation (#3758)

Implements an ExponentialBackOffStrategy that is used when Envoy retries management server connection.
Risk Level: Low
Testing: Added automated tests
Docs Changes: N/A
Release Notes: N/A
Fixes #3737

Signed-off-by: Rama <rama.rao@salesforce.com>

include/envoy/common/BUILD

@@ -37,3 +37,8 @@ envoy_cc_library(
     name = "callback",
     hdrs = ["callback.h"],
 )
+
+envoy_cc_library(
+    name = "backoff_strategy_interface",
+    hdrs = ["backoff_strategy.h"],
+)

include/envoy/common/backoff_strategy.h

@@ -0,0 +1,25 @@
+#pragma once
+
+#include "envoy/common/pure.h"
+
+namespace Envoy {
+/**
+ * Generic interface for all backoff strategy implementations.
+ */
+class BackOffStrategy {
+public:
+  virtual ~BackOffStrategy() {}
+
+  /**
+   * @return the next backoff interval in milli seconds.
+   */
+  virtual uint64_t nextBackOffMs() PURE;
+
+  /**
+   * Resets the intervals so that the back off intervals can start again.
+   */
+  virtual void reset() PURE;
+};
+
+typedef std::unique_ptr<BackOffStrategy> BackOffStrategyPtr;
+} // namespace Envoy

source/common/common/BUILD

@@ -16,6 +16,16 @@ envoy_cc_library(
     deps = [":minimal_logger_lib"],
 )
 
+envoy_cc_library(
+    name = "backoff_lib",
+    srcs = ["backoff_strategy.cc"],
+    hdrs = ["backoff_strategy.h"],
+    deps = [
+        ":assert_lib",
+        "//include/envoy/common:backoff_strategy_interface",
+    ],
+)
+
 envoy_cc_library(
     name = "base64_lib",
     srcs = ["base64.cc"],

source/common/common/backoff_strategy.cc

@@ -0,0 +1,30 @@
+#include "common/common/backoff_strategy.h"
+
+namespace Envoy {
+
+ExponentialBackOffStrategy::ExponentialBackOffStrategy(uint64_t initial_interval,
+                                                       uint64_t max_interval, double multiplier)
+    : initial_interval_(initial_interval), max_interval_(max_interval), multiplier_(multiplier),
+      current_interval_(0) {
+  ASSERT(multiplier_ > 1.0);
+  ASSERT(initial_interval_ <= max_interval_);
+  ASSERT(initial_interval_ * multiplier_ <= max_interval_);
+}
+
+uint64_t ExponentialBackOffStrategy::nextBackOffMs() { return computeNextInterval(); }
+
+void ExponentialBackOffStrategy::reset() { current_interval_ = 0; }
+
+uint64_t ExponentialBackOffStrategy::computeNextInterval() {
+  if (current_interval_ == 0) {
+    current_interval_ = initial_interval_;
+  } else if (current_interval_ >= max_interval_) {
+    current_interval_ = max_interval_;
+  } else {
+    uint64_t new_interval = current_interval_;
+    new_interval = ceil(new_interval * multiplier_);
+    current_interval_ = new_interval > max_interval_ ? max_interval_ : new_interval;
+  }
+  return current_interval_;
+}
+} // namespace Envoy

source/common/common/backoff_strategy.h

@@ -0,0 +1,33 @@
+#pragma once
+
+#include <cstdint>
+#include <memory>
+
+#include "envoy/common/backoff_strategy.h"
+
+#include "common/common/assert.h"
+
+namespace Envoy {
+
+/**
+ * Implementation of BackOffStrategy that increases the back off period for each retry attempt. When
+ * the interval has reached the max interval, it is no longer increased.
+ */
+class ExponentialBackOffStrategy : public BackOffStrategy {
+
+public:
+  ExponentialBackOffStrategy(uint64_t initial_interval, uint64_t max_interval, double multiplier);
+
+  // BackOffStrategy methods
+  uint64_t nextBackOffMs() override;
+  void reset() override;
+
+private:
+  uint64_t computeNextInterval();
+
+  const uint64_t initial_interval_;
+  const uint64_t max_interval_;
+  const double multiplier_;
+  uint64_t current_interval_;
+};
+} // namespace Envoy

source/common/config/BUILD

@@ -142,6 +142,7 @@ envoy_cc_library(
         "//include/envoy/config:subscription_interface",
         "//include/envoy/grpc:async_client_interface",
         "//include/envoy/upstream:cluster_manager_interface",
+        "//source/common/common:backoff_lib",
         "//source/common/common:minimal_logger_lib",
         "//source/common/common:token_bucket_impl_lib",
         "//source/common/protobuf",

source/common/config/grpc_mux_impl.cc

@@ -16,6 +16,8 @@ GrpcMuxImpl::GrpcMuxImpl(const envoy::api::v2::core::Node& node, Grpc::AsyncClie
     : node_(node), async_client_(std::move(async_client)), service_method_(service_method),
       time_source_(time_source) {
   retry_timer_ = dispatcher.createTimer([this]() -> void { establishNewStream(); });
+  backoff_strategy_ptr_ = std::make_unique<ExponentialBackOffStrategy>(
+      RETRY_INITIAL_DELAY_MS, RETRY_MAX_DELAY_MS, MULTIPLIER);
 }
 
 GrpcMuxImpl::~GrpcMuxImpl() {
@@ -29,7 +31,7 @@ GrpcMuxImpl::~GrpcMuxImpl() {
 void GrpcMuxImpl::start() { establishNewStream(); }
 
 void GrpcMuxImpl::setRetryTimer() {
-  retry_timer_->enableTimer(std::chrono::milliseconds(RETRY_DELAY_MS));
+  retry_timer_->enableTimer(std::chrono::milliseconds(backoff_strategy_ptr_->nextBackOffMs()));
 }
 
 void GrpcMuxImpl::establishNewStream() {
@@ -156,6 +158,9 @@ void GrpcMuxImpl::onReceiveInitialMetadata(Http::HeaderMapPtr&& metadata) {
 }
 
 void GrpcMuxImpl::onReceiveMessage(std::unique_ptr<envoy::api::v2::DiscoveryResponse>&& message) {
+  // Reset here so that it starts with fresh backoff interval on next disconnect.
+  backoff_strategy_ptr_->reset();
+
   const std::string& type_url = message->type_url();
   ENVOY_LOG(debug, "Received gRPC message for {} at version {}", type_url, message->version_info());
   if (api_state_.count(type_url) == 0) {

source/common/config/grpc_mux_impl.h

@@ -11,6 +11,7 @@
 #include "envoy/grpc/status.h"
 #include "envoy/upstream/cluster_manager.h"
 
+#include "common/common/backoff_strategy.h"
 #include "common/common/logger.h"
 
 namespace Envoy {
@@ -42,7 +43,9 @@ class GrpcMuxImpl : public GrpcMux,
   void onRemoteClose(Grpc::Status::GrpcStatus status, const std::string& message) override;
 
   // TODO(htuch): Make this configurable or some static.
-  const uint32_t RETRY_DELAY_MS = 5000;
+  const uint32_t RETRY_INITIAL_DELAY_MS = 500;
+  const uint32_t RETRY_MAX_DELAY_MS = 30000; // Do not cross more than 30s
+  const double MULTIPLIER = 2;
 
 private:
   void setRetryTimer();
@@ -101,6 +104,7 @@ class GrpcMuxImpl : public GrpcMux,
   std::list<std::string> subscriptions_;
   Event::TimerPtr retry_timer_;
   MonotonicTimeSource& time_source_;
+  BackOffStrategyPtr backoff_strategy_ptr_;
 };
 
 class NullGrpcMuxImpl : public GrpcMux {

source/common/router/retry_state_impl.cc

@@ -76,6 +76,7 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&
 RetryStateImpl::~RetryStateImpl() { resetRetry(); }
 
 void RetryStateImpl::enableBackoffTimer() {
+  // TODO(ramaraochavali): Implement JitteredExponentialBackOff and refactor this.
   // We use a fully jittered exponential backoff algorithm.
   current_retry_++;
   uint32_t multiplier = (1 << current_retry_) - 1;

test/common/common/BUILD

@@ -10,6 +10,14 @@ load(
 
 envoy_package()
 
+envoy_cc_test(
+    name = "backoff_strategy_test",
+    srcs = ["backoff_strategy_test.cc"],
+    deps = [
+        "//source/common/common:backoff_lib",
+    ],
+)
+
 envoy_cc_test(
     name = "base64_test",
     srcs = ["base64_test.cc"],

test/common/common/backoff_strategy_test.cc

@@ -0,0 +1,57 @@
+#include "common/common/backoff_strategy.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+
+TEST(BackOffStrategyTest, ExponentialBackOffBasicTest) {
+  ExponentialBackOffStrategy exponential_back_off(10, 100, 2);
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(20, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(40, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(80, exponential_back_off.nextBackOffMs());
+}
+
+TEST(BackOffStrategyTest, ExponentialBackOffFractionalMultiplier) {
+  ExponentialBackOffStrategy exponential_back_off(10, 50, 1.5);
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(15, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(23, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(35, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(50, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(50, exponential_back_off.nextBackOffMs());
+}
+
+TEST(BackOffStrategyTest, ExponentialBackOffMaxIntervalReached) {
+  ExponentialBackOffStrategy exponential_back_off(10, 100, 2);
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(20, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(40, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(80, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(100, exponential_back_off.nextBackOffMs()); // Should return Max here
+  EXPECT_EQ(100, exponential_back_off.nextBackOffMs()); // Should return Max here
+}
+
+TEST(BackOffStrategyTest, ExponentialBackOfReset) {
+  ExponentialBackOffStrategy exponential_back_off(10, 100, 2);
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(20, exponential_back_off.nextBackOffMs());
+
+  exponential_back_off.reset();
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs()); // Should start from start
+}
+
+TEST(BackOffStrategyTest, ExponentialBackOfResetAfterMaxReached) {
+  ExponentialBackOffStrategy exponential_back_off(10, 100, 2);
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(20, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(40, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(80, exponential_back_off.nextBackOffMs());
+  EXPECT_EQ(100, exponential_back_off.nextBackOffMs()); // Should return Max here
+
+  exponential_back_off.reset();
+
+  EXPECT_EQ(10, exponential_back_off.nextBackOffMs()); // Should start from start
+}
+
+} // namespace Envoy