impl(bigtable): simplify adding channels in dynamic channel pool

Author: scotthart

google/cloud/bigtable/internal/dynamic_channel_pool.h

@@ -241,22 +241,6 @@ class DynamicChannelPool
     return channel;
   }
 
-  struct ChannelAddVisitor {
-    std::size_t pool_size;
-    explicit ChannelAddVisitor(std::size_t pool_size) : pool_size(pool_size) {}
-    std::size_t operator()(
-        typename bigtable::experimental::DynamicChannelPoolSizingPolicy::
-            DiscreteChannels const& c) const {
-      return c.number;
-    }
-    std::size_t operator()(
-        typename bigtable::experimental::DynamicChannelPoolSizingPolicy::
-            PercentageOfPoolSize const& c) const {
-      return static_cast<std::size_t>(
-          std::floor(static_cast<double>(pool_size) * c.percentage));
-    }
-  };
-
   // Determines the number of channels to add and reserves the channel ids to
   // be used. Lastly, it calls CompletionQueue::RunAsync with a callback that
   // executes AddChannels with the reserved ids.
@@ -271,8 +255,7 @@ class DynamicChannelPool
     } else {
       num_channels_to_add =
           std::min(sizing_policy_.maximum_channel_pool_size - channels_.size(),
-                   absl::visit(ChannelAddVisitor(channels_.size()),
-                               sizing_policy_.channels_to_add_per_resize));
+                   std::size_t(1));
     }
     num_pending_channels_ += num_channels_to_add;
     std::vector<int> new_channel_ids;

google/cloud/bigtable/internal/dynamic_channel_pool_test.cc

@@ -371,7 +371,7 @@ TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolUndersized) {
   }
 }
 
-TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNearMax) {
+TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolAtMax) {
   auto instance_name =
       bigtable::InstanceResource(Project("my-project"), "my-instance")
           .FullName();
@@ -386,58 +386,7 @@ TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNearMax) {
       std::make_shared<MockBigtableStub>(), 0));
   DynamicChannelPoolSizingPolicy sizing_policy;
   sizing_policy.minimum_channel_pool_size = 2;
-  sizing_policy.maximum_channel_pool_size = 3;
-  sizing_policy.channels_to_add_per_resize =
-      DynamicChannelPoolSizingPolicy::DiscreteChannels(3);
-
-  MockFunction<StatusOr<std::shared_ptr<ChannelUsage<BigtableStub>>>(
-      std::uint32_t, std::string const&, StubManager::Priming)>
-      stub_factory_fn;
-
-  // Pool creation should set the pool size increase cooldown timer.
-  EXPECT_CALL(*mock_cq_impl_, MakeRelativeTimer)
-      .WillOnce([&](std::chrono::nanoseconds ns) {
-        EXPECT_THAT(ns.count(),
-                    Eq(std::chrono::nanoseconds(
-                           sizing_policy.pool_size_decrease_cooldown_interval)
-                           .count()));
-        return make_ready_future(StatusOr(std::chrono::system_clock::now()));
-      });
-
-  auto pool = DynamicChannelPool<BigtableStub>::Create(
-      instance_name, CompletionQueue(mock_cq_impl_), channels, refresh_state,
-      stub_factory_fn.AsStdFunction(), sizing_policy);
-  DynamicChannelPoolTestWrapper wrapper(pool);
-
-  EXPECT_CALL(*mock_cq_impl_, RunAsync).Times(1);
-  auto test_fn = [](std::vector<int> const& new_channel_ids) {
-    EXPECT_THAT(new_channel_ids, ::testing::ElementsAreArray({2}));
-  };
-
-  {
-    auto lk = wrapper.CreateLock();
-    wrapper.ScheduleAddChannels(lk, test_fn);
-  }
-}
-
-TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNotNearMax) {
-  auto instance_name =
-      bigtable::InstanceResource(Project("my-project"), "my-instance")
-          .FullName();
-  auto refresh_state = std::make_shared<ConnectionRefreshState>(
-      fake_cq_impl_, std::chrono::milliseconds(1),
-      std::chrono::milliseconds(10));
-
-  std::vector<std::shared_ptr<ChannelUsage<BigtableStub>>> channels;
-  channels.push_back(std::make_shared<ChannelUsage<BigtableStub>>(
-      std::make_shared<MockBigtableStub>(), 0));
-  channels.push_back(std::make_shared<ChannelUsage<BigtableStub>>(
-      std::make_shared<MockBigtableStub>(), 0));
-  DynamicChannelPoolSizingPolicy sizing_policy;
-  sizing_policy.minimum_channel_pool_size = 2;
-  sizing_policy.maximum_channel_pool_size = 10;
-  sizing_policy.channels_to_add_per_resize =
-      DynamicChannelPoolSizingPolicy::DiscreteChannels(3);
+  sizing_policy.maximum_channel_pool_size = 2;
 
   MockFunction<StatusOr<std::shared_ptr<ChannelUsage<BigtableStub>>>(
       std::uint32_t, std::string const&, StubManager::Priming)>
@@ -460,7 +409,7 @@ TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNotNearMax) {
 
   EXPECT_CALL(*mock_cq_impl_, RunAsync).Times(1);
   auto test_fn = [](std::vector<int> const& new_channel_ids) {
-    EXPECT_THAT(new_channel_ids, ::testing::ElementsAreArray({2, 3, 4}));
+    EXPECT_THAT(new_channel_ids, IsEmpty());
   };
 
   {
@@ -469,7 +418,7 @@ TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNotNearMax) {
   }
 }
 
-TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNotNearMaxPercentage) {
+TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolBelowMax) {
   auto instance_name =
       bigtable::InstanceResource(Project("my-project"), "my-instance")
           .FullName();
@@ -485,8 +434,6 @@ TEST_F(DynamicChannelPoolTest, ScheduleAddChannelsPoolNotNearMaxPercentage) {
   DynamicChannelPoolSizingPolicy sizing_policy;
   sizing_policy.minimum_channel_pool_size = 2;
   sizing_policy.maximum_channel_pool_size = 10;
-  sizing_policy.channels_to_add_per_resize =
-      DynamicChannelPoolSizingPolicy::PercentageOfPoolSize(0.5);
 
   MockFunction<StatusOr<std::shared_ptr<ChannelUsage<BigtableStub>>>(
       std::uint32_t, std::string const&, StubManager::Priming)>

google/cloud/bigtable/options.h

@@ -42,13 +42,11 @@
 #include "google/cloud/bigtable/instance_resource.h"
 #include "google/cloud/bigtable/internal/endpoint_options.h"
 #include "google/cloud/bigtable/retry_policy.h"
-#include "google/cloud/bigtable/rpc_retry_policy.h"
 #include "google/cloud/bigtable/version.h"
 #include "google/cloud/backoff_policy.h"
 #include "google/cloud/options.h"
 #include <chrono>
 #include <string>
-#include <variant>
 
 namespace google {
 namespace cloud {
@@ -202,18 +200,6 @@ struct DynamicChannelPoolSizingPolicy {
   std::chrono::milliseconds pool_size_decrease_cooldown_interval =
       std::chrono::seconds(120);
 
-  struct DiscreteChannels {
-    explicit DiscreteChannels(int number = 0) : number(number) {}
-    int number;
-  };
-  struct PercentageOfPoolSize {
-    explicit PercentageOfPoolSize(double percentage = 0.0)
-        : percentage(percentage) {}
-    double percentage;
-  };
-  std::variant<DiscreteChannels, PercentageOfPoolSize>
-      channels_to_add_per_resize = DiscreteChannels{1};
-
   // If the average number of outstanding RPCs is below this threshold,
   // the pool size will be decreased.
   int minimum_average_outstanding_rpcs_per_channel = 1;