Fix intermittent cpu spike in grpc async client

source/common/grpc/async_client_manager_impl.cc

@@ -1,5 +1,7 @@
 #include "source/common/grpc/async_client_manager_impl.h"
 
+#include <chrono>
+
 #include "envoy/config/core/v3/grpc_service.pb.h"
 #include "envoy/stats/scope.h"
 
@@ -204,13 +206,18 @@ void AsyncClientManagerImpl::RawAsyncClientCache::evictEntriesAndResetEvictionTi
   // Evict all the entries that have expired.
   while (!lru_list_.empty()) {
     MonotonicTime next_expire = lru_list_.back().accessed_time_ + EntryTimeoutInterval;
-    if (now >= next_expire) {
+    std::chrono::seconds time_to_next_expire_sec =
+        std::chrono::duration_cast<std::chrono::seconds>(next_expire - now);
+    // since 'now' and 'next_expire' are in nanoseconds, the following condition is to
+    // check if the difference between them is less than 1 second. If we don't do this, the
+    // timer will be enabled with 0 seconds, which will cause the timer to fire immediately.
+    // This will cause cpu spike.
+    if (time_to_next_expire_sec.count() <= 0) {
       // Erase the expired entry.
       lru_map_.erase(lru_list_.back().config_with_hash_key_);
       lru_list_.pop_back();
     } else {
-      cache_eviction_timer_->enableTimer(
-          std::chrono::duration_cast<std::chrono::seconds>(next_expire - now));
+      cache_eviction_timer_->enableTimer(time_to_next_expire_sec);
       return;
     }
   }

test/common/grpc/async_client_manager_impl_test.cc

@@ -1,3 +1,4 @@
+#include <chrono>
 #include <memory>
 
 #include "envoy/config/core/v3/grpc_service.pb.h"
@@ -111,6 +112,59 @@ TEST_F(RawAsyncClientCacheTest, GetExpiredButNotEvictedCacheEntry) {
   EXPECT_EQ(client_cache_.getCache(config_with_hash_key).get(), nullptr);
 }
 
+class RawAsyncClientCacheTestBusyLoop : public testing::Test {
+public:
+  RawAsyncClientCacheTestBusyLoop() {
+    timer_ = new Event::MockTimer();
+    EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb) {
+      return timer_;
+    }));
+    client_cache_ = std::make_unique<AsyncClientManagerImpl::RawAsyncClientCache>(dispatcher_);
+    EXPECT_CALL(*timer_, enableTimer(testing::Not(std::chrono::milliseconds(0)), _))
+        .Times(testing::AtLeast(1));
+  }
+
+  void waitForMilliSeconds(int ms) {
+    for (int i = 0; i < ms; i++) {
+      time_system_.advanceTimeAndRun(std::chrono::milliseconds(1), dispatcher_,
+                                     Event::Dispatcher::RunType::NonBlock);
+    }
+  }
+
+protected:
+  Event::SimulatedTimeSystem time_system_;
+  NiceMock<Event::MockDispatcher> dispatcher_;
+  Event::MockTimer* timer_;
+  std::unique_ptr<AsyncClientManagerImpl::RawAsyncClientCache> client_cache_;
+};
+
+TEST_F(RawAsyncClientCacheTestBusyLoop, MultipleCacheEntriesEvictionBusyLoop) {
+  envoy::config::core::v3::GrpcService grpc_service;
+  RawAsyncClientSharedPtr foo_client = std::make_shared<MockAsyncClient>();
+  // two entries are added to the cache
+  for (int i = 1; i <= 2; i++) {
+    grpc_service.mutable_envoy_grpc()->set_cluster_name(std::to_string(i));
+    GrpcServiceConfigWithHashKey config_with_hash_key(grpc_service);
+    client_cache_->setCache(config_with_hash_key, foo_client);
+  }
+  // waiting for 49.2 secs to make sure that for the entry which is not accessed, time to expire is
+  // less than 1 second, ~0.8 secs
+  waitForMilliSeconds(49200);
+
+  // Access first cache entry to so that evictEntriesAndResetEvictionTimer() gets called.
+  // Since we are getting first entry, access time of first entry will be updated to current time.
+  grpc_service.mutable_envoy_grpc()->set_cluster_name(std::to_string(1));
+  GrpcServiceConfigWithHashKey config_with_hash_key_1(grpc_service);
+  EXPECT_EQ(client_cache_->getCache(config_with_hash_key_1).get(), foo_client.get());
+
+  // Verifying that though the time to expire for second entry ~0.8 sec, it is considered as expired
+  // to avoid the busy loop which could happen if timer gets enabled with 0(0.8 rounded off to 0)
+  // duration.
+  grpc_service.mutable_envoy_grpc()->set_cluster_name(std::to_string(2));
+  GrpcServiceConfigWithHashKey config_with_hash_key_2(grpc_service);
+  EXPECT_EQ(client_cache_->getCache(config_with_hash_key_2).get(), nullptr);
+}
+
 class AsyncClientManagerImplTest : public testing::Test {
 public:
   AsyncClientManagerImplTest()