pr feedback

Change-Id: Ie0fde53cd17b1feeaaffd15382e936a46d84c5c1

Author: nicholsl

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/NoOpChannelPrimer.java

@@ -30,12 +30,12 @@ static NoOpChannelPrimer create() {
   private NoOpChannelPrimer() {}
 
   @Override
-  public void primeChannel(ManagedChannel managedChannel) {
+  public void primeChannel(ManagedChannel channel) {
     // No op
   }
 
   @Override
-  public SettableApiFuture<PingAndWarmResponse> sendPrimeRequestsAsync(ManagedChannel var1) {
+  public SettableApiFuture<PingAndWarmResponse> sendPrimeRequestsAsync(ManagedChannel channel) {
     SettableApiFuture future = SettableApiFuture.create();
     future.set(PingAndWarmResponse.getDefaultInstance());
     return future;

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableChannelPool.java

@@ -451,14 +451,19 @@ static class Entry {
 
     private final AtomicInteger maxOutstanding = new AtomicInteger();
 
+    /**
+     * Queue storing the last 5 minutes of probe results
+     */
     @VisibleForTesting
     final ConcurrentLinkedQueue<ProbeResult> probeHistory = new ConcurrentLinkedQueue<>();
 
-    // we keep both so that we don't have to check size() on the ConcurrentLinkedQueue all the time
-    AtomicInteger failedProbesInWindow = new AtomicInteger();
-    AtomicInteger successfulProbesInWindow = new AtomicInteger();
+    /**
+     * Keep both # of failed and # of successful probes so that we don't have to check size() on the ConcurrentLinkedQueue all the time
+     */
+    final AtomicInteger failedProbesInWindow = new AtomicInteger();
+    final AtomicInteger successfulProbesInWindow = new AtomicInteger();
 
-    // Flag that the channel should be closed once all of the outstanding RPC complete.
+    // Flag that the channel should be closed once all the outstanding RPCs complete.
     private final AtomicBoolean shutdownRequested = new AtomicBoolean();
     // Flag that the channel has been closed.
     private final AtomicBoolean shutdownInitiated = new AtomicBoolean();

google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/ChannelPoolHealthChecker.java

@@ -36,13 +36,13 @@
 import javax.annotation.Nullable;
 
 /** Class that manages the health checking in the BigtableChannelPool */
-public class ChannelPoolHealthChecker {
+class ChannelPoolHealthChecker {
 
   // Configuration constants
   // Window_Duration is the duration over which we keep probe results
   private static final Duration WINDOW_DURATION = Duration.ofMinutes(5);
   // Interval at which we probe channel health
-  static final Duration PROBE_INTERVAL = Duration.ofSeconds(30);
+  private static final Duration PROBE_INTERVAL = Duration.ofSeconds(30);
   // Timeout deadline for a probe
   @VisibleForTesting static final Duration PROBE_DEADLINE = Duration.ofMillis(500);
   // Minimum interval between new idle channel evictions
@@ -121,9 +121,6 @@ void runProbes() {
       if (channelPrimer instanceof BigtableChannelPrimer) {
         BigtableChannelPrimer primer = (BigtableChannelPrimer) channelPrimer;
         probeFuture = primer.sendPrimeRequestsAsync(entry.getManagedChannel());
-      } else if (channelPrimer instanceof NoOpChannelPrimer) {
-        NoOpChannelPrimer primer = (NoOpChannelPrimer) channelPrimer;
-        probeFuture = primer.sendPrimeRequestsAsync(entry.getManagedChannel());
       } else {
         continue;
       }
@@ -156,7 +153,7 @@ void addProbeResult(Entry entry, ProbeResult result) {
   }
 
   @VisibleForTesting
-  void pruneHistoryFor(Entry entry) {
+  void pruneHistory(Entry entry) {
     Instant windowStart = clock.instant().minus(WINDOW_DURATION);
     while (!entry.probeHistory.isEmpty()
         && entry.probeHistory.peek().startTime().isBefore(windowStart)) {
@@ -186,14 +183,14 @@ boolean isEntryHealthy(Entry entry) {
   /**
    * Finds a channel that is an outlier in terms of health.
    *
-   * @return Entry
+   * @return the entry to be evicted. Returns null if nothing to evict.
    */
   @Nullable
   @VisibleForTesting
   Entry findOutlierEntry() {
     List<Entry> unhealthyEntries =
         this.entrySupplier.get().stream()
-            .peek(this::pruneHistoryFor)
+            .peek(this::pruneHistory)
             .filter(entry -> !isEntryHealthy(entry))
             .collect(Collectors.toList());
 

google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/gaxx/grpc/ChannelPoolHealthCheckerTest.java

@@ -118,7 +118,7 @@ public void testPruning_removesOldProbesAndCounters() {
 
     Instant newTime = mockClock.instant().plus(Duration.ofMinutes(6));
     Mockito.when(mockClock.instant()).thenReturn(newTime);
-    healthChecker.pruneHistoryFor(entry); // Manually call for direct testing
+    healthChecker.pruneHistory(entry); // Manually call for direct testing
 
     assertThat(entry.probeHistory).isEmpty();
     assertThat(entry.failedProbesInWindow.get()).isEqualTo(0);