Make the backoff infinite (#554)

Motivation:

The gRPC backoff continues ad infinitum (it is capped at a maximum
timeout instead of growing exponentially forever). Our backoff
terminates once it reaches the maximum backoff.

Modifications:

- Remove the upper limit on backoff generation so it is produced infinitely.
- Remove a test which is no longer relevant.
- Enable backoff by default

Result:

Client will try connecting until the user calls `close()`

Sources/GRPC/ClientConnection.swift

@@ -467,15 +467,14 @@ extension ClientConnection {
     ///     on debug builds.
     /// - Parameter connectivityStateDelegate: A connectivity state delegate, defaulting to `nil`.
     /// - Parameter tlsConfiguration: TLS configuration, defaulting to `nil`.
-    /// - Parameter connectionBackoff: The connection backoff configuration to use, defaulting
-    ///     to `nil`.
+    /// - Parameter connectionBackoff: The connection backoff configuration to use.
     public init(
       target: ConnectionTarget,
       eventLoopGroup: EventLoopGroup,
       errorDelegate: ClientErrorDelegate? = DebugOnlyLoggingClientErrorDelegate.shared,
       connectivityStateDelegate: ConnectivityStateDelegate? = nil,
       tls: Configuration.TLS? = nil,
-      connectionBackoff: ConnectionBackoff? = nil
+      connectionBackoff: ConnectionBackoff? = ConnectionBackoff()
     ) {
       self.target = target
       self.eventLoopGroup = eventLoopGroup

Sources/GRPC/ConnectionBackoff.swift

@@ -90,16 +90,8 @@ public class ConnectionBackoffIterator: IteratorProtocol {
   /// compute it on-the-fly.
   private var initialElement: Element?
 
-  /// Whether or not we should make another element.
-  private var shouldMakeNextElement: Bool {
-    return self.unjitteredBackoff < self.connectionBackoff.maximumBackoff
-  }
-
   /// Returns the next pair of connection timeout and backoff (in that order) to use should the
   /// connection attempt fail.
-  ///
-  /// The iterator will stop producing values _after_ the unjittered backoff is greater than or
-  /// equal to the maximum backoff set in the configuration used to create this iterator.
   public func next() -> Element? {
     if let initial = self.initialElement {
       self.initialElement = nil
@@ -109,12 +101,8 @@ public class ConnectionBackoffIterator: IteratorProtocol {
     }
   }
 
-  /// Produces the next element to return, or `nil` if no more elements should be made.
-  private func makeNextElement() -> Element? {
-    guard self.shouldMakeNextElement else {
-      return nil
-    }
-
+  /// Produces the next element to return.
+  private func makeNextElement() -> Element {
     let unjittered = self.unjitteredBackoff * self.connectionBackoff.multiplier
     self.unjitteredBackoff = min(unjittered, self.connectionBackoff.maximumBackoff)
 

Tests/GRPCTests/ClientConnectionBackoffTests.swift

@@ -134,13 +134,8 @@ class ClientConnectionBackoffTests: GRPCTestCase {
       target: .hostAndPort("localhost", self.port),
       eventLoopGroup: self.clientGroup,
       connectivityStateDelegate: self.stateDelegate,
-      connectionBackoff: ConnectionBackoff(maximumBackoff: 0.1))
-  }
-
-  func makeClientConnection(
-    _ configuration: ClientConnection.Configuration
-  ) -> ClientConnection {
-    return ClientConnection(configuration: configuration)
+      connectionBackoff: ConnectionBackoff(maximumBackoff: 0.1,
+                                           minimumConnectionTimeout: 0.1))
   }
 
   func testClientConnectionFailsWithNoBackoff() throws {
@@ -149,7 +144,7 @@ class ClientConnectionBackoffTests: GRPCTestCase {
 
     let connectionShutdown = self.expectation(description: "client shutdown")
     self.stateDelegate.shutdownExpectation = connectionShutdown
-    self.client = self.makeClientConnection(configuration)
+    self.client = ClientConnection(configuration: configuration)
 
     self.wait(for: [connectionShutdown], timeout: 1.0)
     XCTAssertEqual(self.stateDelegate.states, [.connecting, .shutdown])
@@ -162,25 +157,20 @@ class ClientConnectionBackoffTests: GRPCTestCase {
     self.stateDelegate.readyExpectation = connectionReady
 
     // Start the client first.
-    self.client = self.makeClientConnection(self.makeClientConfiguration())
+    self.client = ClientConnection(configuration: self.makeClientConfiguration())
 
     self.wait(for: [transientFailure], timeout: 1.0)
+    self.stateDelegate.transientFailureExpectation = nil
+    XCTAssertEqual(self.stateDelegate.clearStates(), [.connecting, .transientFailure])
 
     self.server = self.makeServer()
     let serverStarted = self.expectation(description: "server started")
     self.server.assertSuccess(fulfill: serverStarted)
 
     self.wait(for: [serverStarted, connectionReady], timeout: 2.0, enforceOrder: true)
-    XCTAssertEqual(self.stateDelegate.states, [.connecting, .transientFailure, .connecting, .ready])
-  }
-
-  func testClientEventuallyTimesOut() throws {
-    let connectionShutdown = self.expectation(description: "connection shutdown")
-    self.stateDelegate.shutdownExpectation = connectionShutdown
-    self.client = self.makeClientConnection(self.makeClientConfiguration())
-
-    self.wait(for: [connectionShutdown], timeout: 1.0)
-    XCTAssertEqual(self.stateDelegate.states, [.connecting, .transientFailure, .connecting, .shutdown])
+    // We can have other transient failures and connection attempts while the server starts, we only
+    // care about the last two.
+    XCTAssertEqual(self.stateDelegate.states.suffix(2), [.connecting, .ready])
   }
 
   func testClientReconnectsAutomatically() throws {
@@ -195,7 +185,7 @@ class ClientConnectionBackoffTests: GRPCTestCase {
     self.stateDelegate.readyExpectation = connectionReady
     self.stateDelegate.transientFailureExpectation = transientFailure
 
-    self.client = self.makeClientConnection(configuration)
+    self.client = ClientConnection(configuration: configuration)
 
     // Once the connection is ready we can kill the server.
     self.wait(for: [connectionReady], timeout: 1.0)

Tests/GRPCTests/ClientTLSFailureTests.swift

@@ -58,7 +58,9 @@ class ClientTLSFailureTests: GRPCTestCase {
     return .init(
       target: .hostAndPort("localhost", self.port),
       eventLoopGroup: self.clientEventLoopGroup,
-      tls: tls
+      tls: tls,
+      // No need to retry connecting.
+      connectionBackoff: nil
     )
   }
 

Tests/GRPCTests/ConnectionBackoffTests.swift

@@ -27,7 +27,7 @@ class ConnectionBackoffTests: GRPCTestCase {
     self.backoff.maximumBackoff = 16.0
     self.backoff.minimumConnectionTimeout = 4.2
 
-    let timeoutAndBackoff = Array(self.backoff)
+    let timeoutAndBackoff = self.backoff.prefix(5)
 
     let expectedBackoff: [TimeInterval] = [1.0, 2.0, 4.0, 8.0, 16.0]
     XCTAssertEqual(expectedBackoff, timeoutAndBackoff.map { $0.backoff })
@@ -38,16 +38,15 @@ class ConnectionBackoffTests: GRPCTestCase {
 
   func testBackoffWithNoJitter() {
     self.backoff.jitter = 0.0
-
-    for (i, timeoutAndBackoff) in self.backoff.enumerated() {
+    for (i, backoff) in self.backoff.prefix(100).map({ $0.backoff }).enumerated() {
       let expected = min(pow(self.backoff.initialBackoff * self.backoff.multiplier, Double(i)),
                          self.backoff.maximumBackoff)
-      XCTAssertEqual(expected, timeoutAndBackoff.backoff, accuracy: 1e-6)
+      XCTAssertEqual(expected, backoff, accuracy: 1e-6)
     }
   }
 
   func testBackoffWithJitter() {
-    for (i, timeoutAndBackoff) in self.backoff.enumerated() {
+    for (i, timeoutAndBackoff) in self.backoff.prefix(100).enumerated() {
       let unjittered = min(pow(self.backoff.initialBackoff * self.backoff.multiplier, Double(i)),
                            self.backoff.maximumBackoff)
       let halfJitterRange = self.backoff.jitter * unjittered
@@ -61,13 +60,13 @@ class ConnectionBackoffTests: GRPCTestCase {
     // backoff can still be exceeded if jitter is non-zero.
     self.backoff.jitter = 0.0
 
-    for (_, backoff) in self.backoff {
+    for backoff in self.backoff.prefix(100).map({ $0.backoff }) {
       XCTAssertLessThanOrEqual(backoff, self.backoff.maximumBackoff)
     }
   }
 
   func testConnectionTimeoutAlwaysGreatherThanOrEqualToMinimum() {
-    for (connectionTimeout, _) in self.backoff {
+    for connectionTimeout in self.backoff.prefix(100).map({ $0.timeout }) {
       XCTAssertGreaterThanOrEqual(connectionTimeout, self.backoff.minimumConnectionTimeout)
     }
   }

Tests/GRPCTests/XCTestManifests.swift

@@ -47,7 +47,6 @@ extension ClientConnectionBackoffTests {
     static let __allTests__ClientConnectionBackoffTests = [
         ("testClientConnectionFailsWithNoBackoff", testClientConnectionFailsWithNoBackoff),
         ("testClientEventuallyConnects", testClientEventuallyConnects),
-        ("testClientEventuallyTimesOut", testClientEventuallyTimesOut),
         ("testClientReconnectsAutomatically", testClientReconnectsAutomatically),
     ]
 }