Revert "Refactor ExponentialBackoffPolicy"

This reverts commit a98f8afbbe.

There was no expectation across the languages that we would support
other policies for connection retry (changing a parameter would be on
the table, though).

core/src/main/java/io/grpc/ExponentialBackoffPolicy.java

@@ -1,150 +0,0 @@
-/*
- * Copyright 2015, Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *    * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *    * Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following disclaimer
- * in the documentation and/or other materials provided with the
- * distribution.
- *
- *    * Neither the name of Google Inc. nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-package io.grpc;
-
-import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkArgument;
-
-import java.util.Random;
-import java.util.concurrent.TimeUnit;
-
-/**
- * Exponential backoff policy for reconnects.
- */
-public final class ExponentialBackoffPolicy implements BackoffPolicy {
-  /**
-   * Provider tuned for connection retries.
-   * https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md
-   */
-  public static final class Provider implements BackoffPolicy.Provider {
-    @Override
-    public BackoffPolicy get() {
-      return ExponentialBackoffPolicy.builder()
-          .initialBackoffMilis(1, TimeUnit.SECONDS)
-          .maxBackoffMillis(2, TimeUnit.MINUTES)
-          .multiplier(1.6)
-          .jitter(.2)
-          .build();
-    }
-  }
-
-  private final Random random;
-  private final long initialBackoffMillis;
-  private final long maxBackoffMillis;
-  private final double multiplier;
-  private final double jitter;
-
-  private long nextBackoffMillis;
-
-  private ExponentialBackoffPolicy(Random random, long initialBackoffMilis, long maxBackoffMillis,
-                                   double multiplier, double jitter) {
-    this.random = random;
-    this.initialBackoffMillis = initialBackoffMilis;
-    this.maxBackoffMillis = maxBackoffMillis;
-    this.multiplier = multiplier;
-    this.jitter = jitter;
-
-    this.nextBackoffMillis = initialBackoffMillis;
-  }
-
-  @Override
-  public long nextBackoffMillis() {
-    long currentBackoffMillis = nextBackoffMillis;
-    nextBackoffMillis = Math.min((long) (currentBackoffMillis * multiplier), maxBackoffMillis);
-    return currentBackoffMillis
-        + uniformRandom(-jitter * currentBackoffMillis, jitter * currentBackoffMillis);
-  }
-
-  private long uniformRandom(double low, double high) {
-    checkArgument(high >= low);
-    double mag = high - low;
-    return (long) (random.nextDouble() * mag + low);
-  }
-
-  public static Builder builder() {
-    return new Builder();
-  }
-
-  public static final class Builder {
-    private Random random;
-    private long initialBackoffMilis;
-    private long maxBackoffMillis;
-    private double multiplier;
-    private double jitter;
-
-    private Builder() {
-    }
-
-    public Builder random(Random random) {
-      this.random = random;
-      return this;
-    }
-
-    public Builder initialBackoffMilis(long initialBackoff, TimeUnit timeUnit) {
-      this.initialBackoffMilis = timeUnit.toMillis(initialBackoff);
-      return this;
-    }
-
-    public Builder maxBackoffMillis(long maxBackoff, TimeUnit timeUnit) {
-      this.maxBackoffMillis = timeUnit.toMillis(maxBackoff);
-      return this;
-    }
-
-    public Builder multiplier(double multiplier) {
-      this.multiplier = multiplier;
-      return this;
-    }
-
-    public Builder jitter(double jitter) {
-      this.jitter = jitter;
-      return this;
-    }
-
-    /**
-     * Builds {@link BackoffPolicy} instance.
-     *
-     * @return BackoffPolicy object.
-     */
-    public ExponentialBackoffPolicy build() {
-      checkArgument(initialBackoffMilis >= 0, "initialBackoffMilis must be greater or equal zero");
-      checkArgument(maxBackoffMillis > 0, "maxBackoffMillis must be greater than zero");
-      checkArgument(initialBackoffMilis <= maxBackoffMillis, "initialBackoffMilis can't be greater"
-          + "or equal maxBackoffMillis");
-      checkArgument(multiplier > 0, "multiplier must be greater than zero");
-      checkArgument(jitter >= 0, "jitter must be greater or equal zero");
-
-      return new ExponentialBackoffPolicy(firstNonNull(random, new Random()), initialBackoffMilis,
-          maxBackoffMillis, multiplier, jitter);
-    }
-  }
-}
-

core/src/main/java/io/grpc/ManagedChannelBuilder.java

@@ -179,12 +179,6 @@ public abstract class ManagedChannelBuilder<T extends ManagedChannelBuilder<T>>
   @ExperimentalApi
   public abstract T compressorRegistry(CompressorRegistry registry);
 
-  /**
-   * Provides a custom {@link BackoffPolicy.Provider} for reconnects.
-   */
-  @ExperimentalApi
-  public abstract T backoffPolicyProvider(BackoffPolicy.Provider backoffPolicyProvider);
-
   /**
    * Builds a channel using the given parameters.
    */

core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java

@@ -37,12 +37,10 @@ import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.MoreExecutors;
 
 import io.grpc.Attributes;
-import io.grpc.BackoffPolicy;
 import io.grpc.ClientInterceptor;
 import io.grpc.CompressorRegistry;
 import io.grpc.DecompressorRegistry;
 import io.grpc.ExperimentalApi;
-import io.grpc.ExponentialBackoffPolicy;
 import io.grpc.LoadBalancer;
 import io.grpc.ManagedChannelBuilder;
 import io.grpc.NameResolver;
@@ -96,9 +94,6 @@ public abstract class AbstractManagedChannelImplBuilder
   @Nullable
   private CompressorRegistry compressorRegistry;
 
-  @Nullable
-  private BackoffPolicy.Provider backoffPolicyProvider;
-
   protected AbstractManagedChannelImplBuilder(String target) {
     this.target = Preconditions.checkNotNull(target);
     this.directServerAddress = null;
@@ -164,13 +159,6 @@ public abstract class AbstractManagedChannelImplBuilder
     return thisT();
   }
 
-  @Override
-  @ExperimentalApi
-  public final T backoffPolicyProvider(BackoffPolicy.Provider backoffPolicyProvider) {
-    this.backoffPolicyProvider = backoffPolicyProvider;
-    return thisT();
-  }
-
   private T thisT() {
     @SuppressWarnings("unchecked")
     T thisT = (T) this;
@@ -204,7 +192,8 @@ public abstract class AbstractManagedChannelImplBuilder
         buildTransportFactory(), authorityOverride);
     return new ManagedChannelImpl(
         target,
-        firstNonNull(backoffPolicyProvider, new ExponentialBackoffPolicy.Provider()),
+        // TODO(carl-mastrangelo): Allow clients to pass this in
+        new ExponentialBackoffPolicy.Provider(),
         firstNonNull(nameResolverFactory, NameResolverRegistry.getDefaultRegistry()),
         getNameResolverParams(),
         firstNonNull(loadBalancerFactory, SimpleLoadBalancerFactory.getInstance()),

core/src/main/java/io/grpc/internal/BackoffPolicy.java

@@ -29,12 +29,12 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-package io.grpc;
+package io.grpc.internal;
 
 /**
  * Determines how long to wait before doing some action (typically a retry, or a reconnect).
  */
-public interface BackoffPolicy {
+interface BackoffPolicy {
   interface Provider {
     BackoffPolicy get();
   }

core/src/main/java/io/grpc/internal/ExponentialBackoffPolicy.java

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2015, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc.internal;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Retry Policy for Transport reconnection.  Initial parameters from
+ * https://github.com/grpc/grpc/blob/master/doc/connection-backoff.md
+ *
+ * <p>TODO(carl-mastrangelo): add unit tests for this class
+ */
+final class ExponentialBackoffPolicy implements BackoffPolicy {
+  static final class Provider implements BackoffPolicy.Provider {
+    @Override
+    public BackoffPolicy get() {
+      return new ExponentialBackoffPolicy();
+    }
+  }
+
+  private Random random = new Random();
+  private long initialBackoffMillis = TimeUnit.SECONDS.toMillis(1);
+  private long maxBackoffMillis = TimeUnit.MINUTES.toMillis(2);
+  private double multiplier = 1.6;
+  private double jitter = .2;
+
+  private long nextBackoffMillis = initialBackoffMillis;
+
+  @Override
+  public long nextBackoffMillis() {
+    long currentBackoffMillis = nextBackoffMillis;
+    nextBackoffMillis = Math.min((long) (currentBackoffMillis * multiplier), maxBackoffMillis);
+    return currentBackoffMillis
+        + uniformRandom(-jitter * currentBackoffMillis, jitter * currentBackoffMillis);
+  }
+
+  private long uniformRandom(double low, double high) {
+    checkArgument(high >= low);
+    double mag = high - low;
+    return (long) (random.nextDouble() * mag + low);
+  }
+
+  /*
+   * No guice and no flags means we get to implement these setters for testing ourselves.  Do not
+   * call these from non-test code.
+   */
+
+  @VisibleForTesting
+  ExponentialBackoffPolicy setRandom(Random random) {
+    this.random = random;
+    return this;
+  }
+
+  @VisibleForTesting
+  ExponentialBackoffPolicy setInitialBackoffMillis(long initialBackoffMillis) {
+    this.initialBackoffMillis = initialBackoffMillis;
+    return this;
+  }
+
+  @VisibleForTesting
+  ExponentialBackoffPolicy setMaxBackoffMillis(long maxBackoffMillis) {
+    this.maxBackoffMillis = maxBackoffMillis;
+    return this;
+  }
+
+  @VisibleForTesting
+  ExponentialBackoffPolicy setMultiplier(double multiplier) {
+    this.multiplier = multiplier;
+    return this;
+  }
+
+  @VisibleForTesting
+  ExponentialBackoffPolicy setJitter(double jitter) {
+    this.jitter = jitter;
+    return this;
+  }
+}
+

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

@@ -38,7 +38,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Supplier;
 
 import io.grpc.Attributes;
-import io.grpc.BackoffPolicy;
 import io.grpc.CallOptions;
 import io.grpc.Channel;
 import io.grpc.ClientCall;

core/src/main/java/io/grpc/internal/TransportSet.java

@@ -35,7 +35,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 
-import io.grpc.BackoffPolicy;
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.LoadBalancer;
 import io.grpc.Status;

core/src/test/java/io/grpc/internal/ExponentialBackoffPolicyTest.java

@@ -29,7 +29,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-package io.grpc;
+package io.grpc.internal;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -39,13 +39,13 @@ import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 import java.util.Random;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Test for {@link ExponentialBackoffPolicy}.
  */
 @RunWith(JUnit4.class)
 public class ExponentialBackoffPolicyTest {
+  private ExponentialBackoffPolicy policy = new ExponentialBackoffPolicy();
   private Random notRandom = new Random() {
     @Override
     public double nextDouble() {
@@ -56,26 +56,18 @@ public class ExponentialBackoffPolicyTest {
   @Test
   public void maxDelayReached() {
     long maxBackoffMillis = 120 * 1000;
-
+    policy.setMaxBackoffMillis(maxBackoffMillis)
-    ExponentialBackoffPolicy policy = ExponentialBackoffPolicy.builder()
+        .setJitter(0)
-        .random(notRandom)
+        .setRandom(notRandom);
-        .initialBackoffMilis(1, TimeUnit.SECONDS)
-        .maxBackoffMillis(maxBackoffMillis, TimeUnit.MILLISECONDS)
-        .multiplier(1.6)
-        .jitter(0)
-        .build();
-
     for (int i = 0; i < 50; i++) {
       if (maxBackoffMillis == policy.nextBackoffMillis()) {
         return; // Success
       }
     }
-
     assertEquals("max delay not reached", maxBackoffMillis, policy.nextBackoffMillis());
   }
 
-  @Test
+  @Test public void canProvide() {
-  public void canProvide() {
     assertNotNull(new ExponentialBackoffPolicy.Provider().get());
   }
 }

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -54,7 +54,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import io.grpc.Attributes;
-import io.grpc.BackoffPolicy;
 import io.grpc.CallOptions;
 import io.grpc.Channel;
 import io.grpc.ClientCall;

core/src/test/java/io/grpc/internal/ManagedChannelImplTransportManagerTest.java

@@ -46,7 +46,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import io.grpc.Attributes;
-import io.grpc.BackoffPolicy;
 import io.grpc.ClientInterceptor;
 import io.grpc.CompressorRegistry;
 import io.grpc.DecompressorRegistry;

core/src/test/java/io/grpc/internal/TransportSetTest.java

@@ -47,7 +47,6 @@ import static org.mockito.Mockito.when;
 
 import com.google.common.base.Stopwatch;
 
-import io.grpc.BackoffPolicy;
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.IntegerMarshaller;
 import io.grpc.LoadBalancer;