Convenience method to jitter keepalive interval (#1697)

Motivation:

If a large number of clients use keepalive and are brought up at the
same time, their ping intervals could align resulting in servers
receiving large numbers of pings simultaneously.

Modifications:

- Add a convenience method to the client keepalive configuration which
  allows the interval to be jittered while maintaining the invariant
  that the interval must be greater than the timeout.
- Add additional checks on the keepalive invariants

Result:

Users can easily apply jitter to their keepalive configuration.

Sources/GRPC/ConnectionKeepalive.swift

@@ -20,13 +20,21 @@ import NIOCore
 /// The defaults are determined by the gRPC keepalive
 /// [documentation] (https://github.com/grpc/grpc/blob/master/doc/keepalive.md).
 public struct ClientConnectionKeepalive: Hashable, Sendable {
+  private func checkInvariants(line: UInt = #line) {
+    precondition(self.timeout < self.interval, "'timeout' must be less than 'interval'", line: line)
+  }
+
   /// The amount of time to wait before sending a keepalive ping.
-  public var interval: TimeAmount
+  public var interval: TimeAmount {
+    didSet { self.checkInvariants() }
+  }
 
   /// The amount of time to wait for an acknowledgment.
   /// If it does not receive an acknowledgment within this time, it will close the connection
   /// This value must be less than ``interval``.
-  public var timeout: TimeAmount
+  public var timeout: TimeAmount {
+    didSet { self.checkInvariants() }
+  }
 
   /// Send keepalive pings even if there are no calls in flight.
   public var permitWithoutCalls: Bool
@@ -45,23 +53,63 @@ public struct ClientConnectionKeepalive: Hashable, Sendable {
     maximumPingsWithoutData: UInt = 2,
     minimumSentPingIntervalWithoutData: TimeAmount = .minutes(5)
   ) {
-    precondition(timeout < interval, "`timeout` must be less than `interval`")
     self.interval = interval
     self.timeout = timeout
     self.permitWithoutCalls = permitWithoutCalls
     self.maximumPingsWithoutData = maximumPingsWithoutData
     self.minimumSentPingIntervalWithoutData = minimumSentPingIntervalWithoutData
+    self.checkInvariants()
+  }
+}
+
+extension ClientConnectionKeepalive {
+  /// Applies jitter to the ``interval``.
+  ///
+  /// The current ``interval`` will be adjusted by no more than `maxJitter` in either direction,
+  /// that is the ``interval`` may increase or decrease by no more than `maxJitter`. As
+  /// the ``timeout`` must be strictly less than the ``interval``, the lower range of the jittered
+  /// interval is clamped to `max(interval - maxJitter, timeout + .nanoseconds(1)))`.
+  ///
+  /// - Parameter maxJitter: The maximum amount of jitter to apply to the ``interval``, which may
+  ///     be applied in either direction.
+  public mutating func jitterInterval(byAtMost maxJitter: TimeAmount) {
+    // The interval must be larger than the timeout so clamp the lower bound to be greater than
+    // the timeout.
+    let lowerBound = max(self.interval - maxJitter, self.timeout + .nanoseconds(1))
+    let upperBound = self.interval + maxJitter
+    self.interval = .nanoseconds(.random(in: lowerBound.nanoseconds ... upperBound.nanoseconds))
+  }
+
+  /// Returns a new ``ClientConnectionKeepalive`` with a jittered ``interval``.
+  ///
+  /// See also ``jitterInterval(byAtMost:)``.
+  ///
+  /// - Parameter maxJitter: The maximum amount of jitter to apply to the ``interval``, which may
+  ///     be applied in either direction.
+  /// - Returns: A new ``ClientConnectionKeepalive``.
+  public func jitteringInterval(byAtMost maxJitter: TimeAmount) -> Self {
+    var copy = self
+    copy.jitterInterval(byAtMost: maxJitter)
+    return copy
   }
 }
 
 public struct ServerConnectionKeepalive: Hashable {
+  private func checkInvariants(line: UInt = #line) {
+    precondition(self.timeout < self.interval, "'timeout' must be less than 'interval'", line: line)
+  }
+
   /// The amount of time to wait before sending a keepalive ping.
-  public var interval: TimeAmount
+  public var interval: TimeAmount {
+    didSet { self.checkInvariants() }
+  }
 
   /// The amount of time to wait for an acknowledgment.
   /// If it does not receive an acknowledgment within this time, it will close the connection
   /// This value must be less than ``interval``.
-  public var timeout: TimeAmount
+  public var timeout: TimeAmount {
+    didSet { self.checkInvariants() }
+  }
 
   /// Send keepalive pings even if there are no calls in flight.
   public var permitWithoutCalls: Bool
@@ -92,7 +140,6 @@ public struct ServerConnectionKeepalive: Hashable {
     minimumReceivedPingIntervalWithoutData: TimeAmount = .minutes(5),
     maximumPingStrikes: UInt = 2
   ) {
-    precondition(timeout < interval, "`timeout` must be less than `interval`")
     self.interval = interval
     self.timeout = timeout
     self.permitWithoutCalls = permitWithoutCalls
@@ -100,5 +147,38 @@ public struct ServerConnectionKeepalive: Hashable {
     self.minimumSentPingIntervalWithoutData = minimumSentPingIntervalWithoutData
     self.minimumReceivedPingIntervalWithoutData = minimumReceivedPingIntervalWithoutData
     self.maximumPingStrikes = maximumPingStrikes
+    self.checkInvariants()
+  }
+}
+
+extension ServerConnectionKeepalive {
+  /// Applies jitter to the ``interval``.
+  ///
+  /// The current ``interval`` will be adjusted by no more than `maxJitter` in either direction,
+  /// that is the ``interval`` may increase or decrease by no more than `maxJitter`. As
+  /// the ``timeout`` must be strictly less than the ``interval``, the lower range of the jittered
+  /// interval is clamped to `max(interval - maxJitter, timeout + .nanoseconds(1)))`.
+  ///
+  /// - Parameter maxJitter: The maximum amount of jitter to apply to the ``interval``, which may
+  ///     be applied in either direction.
+  public mutating func jitterInterval(byAtMost maxJitter: TimeAmount) {
+    // The interval must be larger than the timeout so clamp the lower bound to be greater than
+    // the timeout.
+    let lowerBound = max(self.interval - maxJitter, self.timeout + .nanoseconds(1))
+    let upperBound = self.interval + maxJitter
+    self.interval = .nanoseconds(.random(in: lowerBound.nanoseconds ... upperBound.nanoseconds))
+  }
+
+  /// Returns a new ``ClientConnectionKeepalive`` with a jittered ``interval``.
+  ///
+  /// See also ``jitterInterval(byAtMost:)``.
+  ///
+  /// - Parameter maxJitter: The maximum amount of jitter to apply to the ``interval``, which may
+  ///     be applied in either direction.
+  /// - Returns: A new ``ClientConnectionKeepalive``.
+  public func jitteringInterval(byAtMost maxJitter: TimeAmount) -> Self {
+    var copy = self
+    copy.jitterInterval(byAtMost: maxJitter)
+    return copy
   }
 }

Tests/GRPCTests/ConnectionManagerTests.swift

@@ -1277,6 +1277,40 @@ extension ConnectionManagerTests {
 
     XCTAssertThrowsError(try multiplexer.wait())
   }
+
+  func testClientKeepaliveJitterWithoutClamping() {
+    let original = ClientConnectionKeepalive(interval: .seconds(2), timeout: .seconds(1))
+    let keepalive = original.jitteringInterval(byAtMost: .milliseconds(500))
+
+    XCTAssertGreaterThanOrEqual(keepalive.interval, .milliseconds(1500))
+    XCTAssertLessThanOrEqual(keepalive.interval, .milliseconds(2500))
+  }
+
+  func testClientKeepaliveJitterClampedToTimeout() {
+    let original = ClientConnectionKeepalive(interval: .seconds(2), timeout: .seconds(1))
+    let keepalive = original.jitteringInterval(byAtMost: .seconds(2))
+
+    // Strictly greater than the timeout of 1 seconds.
+    XCTAssertGreaterThan(keepalive.interval, .seconds(1))
+    XCTAssertLessThanOrEqual(keepalive.interval, .seconds(4))
+  }
+
+  func testServerKeepaliveJitterWithoutClamping() {
+    let original = ServerConnectionKeepalive(interval: .seconds(2), timeout: .seconds(1))
+    let keepalive = original.jitteringInterval(byAtMost: .milliseconds(500))
+
+    XCTAssertGreaterThanOrEqual(keepalive.interval, .milliseconds(1500))
+    XCTAssertLessThanOrEqual(keepalive.interval, .milliseconds(2500))
+  }
+
+  func testServerKeepaliveJitterClampedToTimeout() {
+    let original = ServerConnectionKeepalive(interval: .seconds(2), timeout: .seconds(1))
+    let keepalive = original.jitteringInterval(byAtMost: .seconds(2))
+
+    // Strictly greater than the timeout of 1 seconds.
+    XCTAssertGreaterThan(keepalive.interval, .seconds(1))
+    XCTAssertLessThanOrEqual(keepalive.interval, .seconds(4))
+  }
 }
 
 internal struct Change: Hashable, CustomStringConvertible {