Increase the client idle timeout, remove the default server idle timeout (#968)

Motivation:

We usually follow the gRPC core library for default values for various
configuration settings. However, the gRPC core library documentation
around connection idle timeouts is inconsistent at best. It suggests
that both client and server will idle connections after 5 minutes (by
default).

Other documentation suggests that the client will idle connections after
30 minutes by default and that the server will never shed idle
connections by default. The code in gRPC core library mostly agrees with
these values. The exception is for the client where an as yet unresolved
bug has led to the client idle timeout being disabled by default.

Modifications:

- Increase the default client idle timeout to 30 minutes
- Change the server default idle timeout to indefinite

Result:

Our default configuration is closer to the gRPC core lib.

Sources/GRPC/ClientConnection.swift

@@ -560,7 +560,7 @@ extension ClientConnection {
     /// - Parameter tls: TLS configuration, defaulting to `nil`.
     /// - Parameter connectionBackoff: The connection backoff configuration to use.
     /// - Parameter connectionKeepalive: The keepalive configuration to use.
-    /// - Parameter connectionIdleTimeout: The amount of time to wait before closing the connection, defaulting to 5 minutes.
+    /// - Parameter connectionIdleTimeout: The amount of time to wait before closing the connection, defaulting to 30 minutes.
     /// - Parameter callStartBehavior: The behavior used to determine when a call should start in
     ///     relation to its underlying connection. Defaults to `waitsForConnectivity`.
     /// - Parameter httpTargetWindowSize: The HTTP/2 flow control target window size.
@@ -577,7 +577,7 @@ extension ClientConnection {
       tls: Configuration.TLS? = nil,
       connectionBackoff: ConnectionBackoff? = ConnectionBackoff(),
       connectionKeepalive: ClientConnectionKeepalive = ClientConnectionKeepalive(),
-      connectionIdleTimeout: TimeAmount = .minutes(5),
+      connectionIdleTimeout: TimeAmount = .minutes(30),
       callStartBehavior: CallStartBehavior = .waitsForConnectivity,
       httpTargetWindowSize: Int = 65535,
       backgroundActivityLogger: Logger = Logger(

Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift

@@ -151,7 +151,7 @@ extension ClientConnection.Builder {
   /// The amount of time to wait before closing the connection. The idle timeout will start only
   /// if there are no RPCs in progress and will be cancelled as soon as any RPCs start. If a
   /// connection becomes idle, starting a new RPC will automatically create a new connection.
-  /// Defaults to 5 minutes if not set.
+  /// Defaults to 30 minutes if not set.
   @discardableResult
   public func withConnectionIdleTimeout(_ timeout: TimeAmount) -> Self {
     self.configuration.connectionIdleTimeout = timeout

Sources/GRPC/GRPCIdleHandler.swift

@@ -56,7 +56,7 @@ internal class GRPCIdleHandler: ChannelInboundHandler {
     case closed
   }
 
-  init(mode: Mode, logger: Logger, idleTimeout: TimeAmount = .minutes(5)) {
+  init(mode: Mode, logger: Logger, idleTimeout: TimeAmount) {
     self.mode = mode
     self.idleTimeout = idleTimeout
     self.logger = logger
@@ -199,7 +199,7 @@ internal class GRPCIdleHandler: ChannelInboundHandler {
   }
 
   private func scheduleIdleTimeout(context: ChannelHandlerContext) {
-    guard self.activeStreams == 0 else {
+    guard self.activeStreams == 0, self.idleTimeout.nanoseconds != .max else {
       return
     }
 

Sources/GRPC/Server.swift

@@ -286,7 +286,8 @@ extension Server {
     ///   - errorDelegate: The error delegate, defaulting to a logging delegate.
     ///   - tls: TLS configuration, defaulting to `nil`.
     ///   - connectionKeepalive: The keepalive configuration to use.
-    ///   - connectionIdleTimeout: The amount of time to wait before closing the connection, defaulting to 5 minutes.
+    ///   - connectionIdleTimeout: The amount of time to wait before closing the connection, this is
+    ///       indefinite by default.
     ///   - messageEncoding: Message compression configuration, defaulting to no compression.
     ///   - httpTargetWindowSize: The HTTP/2 flow control target window size.
     ///   - logger: A logger. Defaults to a no-op logger.
@@ -299,7 +300,7 @@ extension Server {
       errorDelegate: ServerErrorDelegate? = nil,
       tls: TLS? = nil,
       connectionKeepalive: ServerConnectionKeepalive = ServerConnectionKeepalive(),
-      connectionIdleTimeout: TimeAmount = .minutes(5),
+      connectionIdleTimeout: TimeAmount = .nanoseconds(.max),
       messageEncoding: ServerMessageEncoding = .disabled,
       httpTargetWindowSize: Int = 65535,
       logger: Logger = Logger(label: "io.grpc", factory: { _ in SwiftLogNoOpLogHandler() }),

Sources/GRPC/ServerBuilder.swift

@@ -89,8 +89,8 @@ extension Server.Builder {
 
 extension Server.Builder {
   /// The amount of time to wait before closing connections. The idle timeout will start only
-  /// if there are no RPCs in progress and will be cancelled as soon as any RPCs start. Defaults to
-  /// 5 minutes if not set.
+  /// if there are no RPCs in progress and will be cancelled as soon as any RPCs start. Unless a
+  /// an idle timeout it set connections will not be idled by default.
   @discardableResult
   public func withConnectionIdleTimeout(_ timeout: TimeAmount) -> Self {
     self.configuration.connectionIdleTimeout = timeout

Tests/GRPCTests/ConnectionManagerTests.swift

@@ -126,7 +126,11 @@ extension ConnectionManagerTests {
 
     // Setup the real channel and activate it.
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -169,7 +173,11 @@ extension ConnectionManagerTests {
 
     // Setup the channel.
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -219,7 +227,11 @@ extension ConnectionManagerTests {
 
     // Setup the channel.
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -282,7 +294,11 @@ extension ConnectionManagerTests {
 
     // Setup the channel.
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -343,7 +359,11 @@ extension ConnectionManagerTests {
 
     // Setup the actual channel and complete the promise.
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -450,7 +470,11 @@ extension ConnectionManagerTests {
 
     // Prepare the channel
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -514,7 +538,11 @@ extension ConnectionManagerTests {
 
     // Prepare the channel
     let firstChannel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(firstChannel)
@@ -578,7 +606,11 @@ extension ConnectionManagerTests {
 
     // Prepare the first channel
     let firstChannel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     firstChannelPromise.succeed(firstChannel)
@@ -615,7 +647,11 @@ extension ConnectionManagerTests {
 
     // Prepare the second channel
     let secondChannel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     secondChannelPromise.succeed(secondChannel)
@@ -656,7 +692,11 @@ extension ConnectionManagerTests {
 
     // Setup the channel.
     let channel = EmbeddedChannel(
-      handler: GRPCIdleHandler(mode: .client(manager), logger: self.logger),
+      handler: GRPCIdleHandler(
+        mode: .client(manager),
+        logger: self.logger,
+        idleTimeout: .minutes(5)
+      ),
       loop: self.loop
     )
     channelPromise.succeed(channel)
@@ -804,7 +844,7 @@ extension ConnectionManagerTests {
     let channel = EmbeddedChannel(loop: self.loop)
     XCTAssertNoThrow(try channel.pipeline.addHandlers([
       CloseDroppingHandler(),
-      GRPCIdleHandler(mode: .client(manager), logger: manager.logger),
+      GRPCIdleHandler(mode: .client(manager), logger: manager.logger, idleTimeout: .minutes(5)),
     ]).wait())
     channelPromise.succeed(channel)
     self.loop.run()
@@ -860,7 +900,7 @@ extension ConnectionManagerTests {
     // Setup the real channel and activate it.
     let channel = EmbeddedChannel(loop: self.loop)
     XCTAssertNoThrow(try channel.pipeline.addHandlers([
-      GRPCIdleHandler(mode: .client(manager), logger: manager.logger),
+      GRPCIdleHandler(mode: .client(manager), logger: manager.logger, idleTimeout: .minutes(5)),
     ]).wait())
     channelPromise.succeed(channel)
     self.loop.run()
@@ -911,7 +951,7 @@ extension ConnectionManagerTests {
     // Setup the actual channel and activate it.
     let channel = EmbeddedChannel(loop: self.loop)
     XCTAssertNoThrow(try channel.pipeline.addHandlers([
-      GRPCIdleHandler(mode: .client(manager), logger: manager.logger),
+      GRPCIdleHandler(mode: .client(manager), logger: manager.logger, idleTimeout: .minutes(5)),
     ]).wait())
     channelPromise.succeed(channel)
     self.loop.run()