Skip to content

Commit 1422de0

Browse files
JamesNKJamesNK
authored
[2.56.x] Support infinite idle connection timeout values (#2234)
1 parent df13e5a commit 1422de0

File tree

5 files changed

+92
-18
lines changed

5 files changed

+92
-18
lines changed

‎src/Grpc.Net.Client/Balancer/Internal/SocketConnectivitySubchannelTransport.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ public async ValueTask<Stream> GetStreamAsync(BalancerAddress address, Cancellat
332332

333333
var closeSocket = false;
334334

335-
if (DateTime.UtcNow > socketCreatedTime.Value.Add(_socketIdleTimeout))
335+
if (_socketIdleTimeout != Timeout.InfiniteTimeSpan && DateTime.UtcNow > socketCreatedTime.Value.Add(_socketIdleTimeout))
336336
{
337337
SocketConnectivitySubchannelTransportLog.ClosingSocketFromIdleTimeoutOnCreateStream(_logger, _subchannel.Id, address, _socketIdleTimeout);
338338
closeSocket = true;

‎src/Grpc.Net.Client/GrpcChannel.cs‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ private static HttpHandlerContext CalculateHandlerContext(ILogger logger, Uri ad
255255

256256
type = HttpHandlerType.SocketsHttpHandler;
257257
connectTimeout = socketsHttpHandler.ConnectTimeout;
258-
connectionIdleTimeout = socketsHttpHandler.PooledConnectionIdleTimeout;
258+
connectionIdleTimeout = GetConnectionIdleTimeout(socketsHttpHandler);
259259

260260
// Check if the SocketsHttpHandler is being shared by channels.
261261
// It has already been setup by another channel (i.e. ConnectCallback is set) then
@@ -300,6 +300,27 @@ private static HttpHandlerContext CalculateHandlerContext(ILogger logger, Uri ad
300300
}
301301

302302
return new HttpHandlerContext(HttpHandlerType.Custom);
303+
304+
#if NET5_0_OR_GREATER
305+
static TimeSpan? GetConnectionIdleTimeout(SocketsHttpHandler socketsHttpHandler)
306+
{
307+
// Check if either TimeSpan is InfiniteTimeSpan, and return the other one.
308+
if (socketsHttpHandler.PooledConnectionIdleTimeout == Timeout.InfiniteTimeSpan)
309+
{
310+
return socketsHttpHandler.PooledConnectionLifetime;
311+
}
312+
313+
if (socketsHttpHandler.PooledConnectionLifetime == Timeout.InfiniteTimeSpan)
314+
{
315+
return socketsHttpHandler.PooledConnectionIdleTimeout;
316+
}
317+
318+
// Return the bigger TimeSpan.
319+
return socketsHttpHandler.PooledConnectionIdleTimeout > socketsHttpHandler.PooledConnectionLifetime
320+
? socketsHttpHandler.PooledConnectionIdleTimeout
321+
: socketsHttpHandler.PooledConnectionLifetime;
322+
}
323+
#endif
303324
}
304325

305326
#if NET5_0_OR_GREATER

‎test/FunctionalTests/Balancer/ConnectionTests.cs‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,9 @@ async Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext conte
141141
await ExceptionAssert.ThrowsAsync<OperationCanceledException>(() => connectTask).DefaultTimeout();
142142
}
143143

144-
[Test]
145-
public async Task Active_UnaryCall_ConnectionIdleTimeout_SocketRecreated()
144+
[TestCase(0)] // TimeSpan.Zero
145+
[TestCase(1000)] // 1 second
146+
public async Task Active_UnaryCall_ConnectionIdleTimeout_SocketRecreated(int milliseconds)
146147
{
147148
// Ignore errors
148149
SetExpectedErrorsFilter(writeContext =>
@@ -158,7 +159,7 @@ Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext context)
158159
// Arrange
159160
using var endpoint = BalancerHelpers.CreateGrpcEndpoint<HelloRequest, HelloReply>(50051, UnaryMethod, nameof(UnaryMethod), loggerFactory: LoggerFactory);
160161

161-
var connectionIdleTimeout = TimeSpan.FromSeconds(1);
162+
var connectionIdleTimeout = TimeSpan.FromMilliseconds(milliseconds);
162163
var channel = await BalancerHelpers.CreateChannel(
163164
LoggerFactory,
164165
new PickFirstConfig(),
@@ -168,18 +169,52 @@ Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext context)
168169
Logger.LogInformation("Connecting channel.");
169170
await channel.ConnectAsync();
170171

171-
await Task.Delay(connectionIdleTimeout);
172+
// Wait for timeout plus a little extra to avoid issues from imprecise timers.
173+
await Task.Delay(connectionIdleTimeout + TimeSpan.FromMilliseconds(50));
172174

173175
var client = TestClientFactory.Create(channel, endpoint.Method);
174176
var response = await client.UnaryCall(new HelloRequest { Name = "Test!" }).ResponseAsync.DefaultTimeout();
175177

176178
// Assert
177179
Assert.AreEqual("Test!", response.Message);
178180

179-
AssertHasLog(LogLevel.Debug, "ClosingSocketFromIdleTimeoutOnCreateStream", "Subchannel id '1' socket 127.0.0.1:50051 is being closed because it exceeds the idle timeout of 00:00:01.");
181+
AssertHasLog(LogLevel.Debug, "ClosingSocketFromIdleTimeoutOnCreateStream");
180182
AssertHasLog(LogLevel.Trace, "ConnectingOnCreateStream", "Subchannel id '1' doesn't have a connected socket available. Connecting new stream socket for 127.0.0.1:50051.");
181183
}
182184

185+
public async Task Active_UnaryCall_InfiniteConnectionIdleTimeout_SocketNotClosed()
186+
{
187+
SetExpectedErrorsFilter(writeContext =>
188+
{
189+
return true;
190+
});
191+
192+
Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext context)
193+
{
194+
return Task.FromResult(new HelloReply { Message = request.Name });
195+
}
196+
197+
// Arrange
198+
using var endpoint = BalancerHelpers.CreateGrpcEndpoint<HelloRequest, HelloReply>(50051, UnaryMethod, nameof(UnaryMethod), loggerFactory: LoggerFactory);
199+
200+
var channel = await BalancerHelpers.CreateChannel(
201+
LoggerFactory,
202+
new PickFirstConfig(),
203+
new[] { endpoint.Address },
204+
connectionIdleTimeout: Timeout.InfiniteTimeSpan).DefaultTimeout();
205+
206+
Logger.LogInformation("Connecting channel.");
207+
await channel.ConnectAsync();
208+
209+
var client = TestClientFactory.Create(channel, endpoint.Method);
210+
var response = await client.UnaryCall(new HelloRequest { Name = "Test!" }).ResponseAsync.DefaultTimeout();
211+
212+
// Assert
213+
Assert.AreEqual("Test!", response.Message);
214+
215+
Assert.IsFalse(Logs.Any(l => l.EventId.Name == "ClosingSocketFromIdleTimeoutOnCreateStream"), "Shouldn't have a ClosingSocketFromIdleTimeoutOnCreateStream log.");
216+
}
217+
183218
[Test]
184219
public async Task Active_UnaryCall_ServerCloseOnKeepAlive_SocketRecreatedOnRequest()
185220
{

‎test/FunctionalTests/FunctionalTestBase.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,21 +119,21 @@ protected void AssertHasLogRpcConnectionError(StatusCode statusCode, string deta
119119
AssertHasLog(LogLevel.Information, "RpcConnectionError", $"Error status code '{statusCode}' with detail '{detail}' raised.");
120120
}
121121

122-
protected void AssertHasLog(LogLevel logLevel, string name, string message, Func<Exception, bool>? exceptionMatch = null)
122+
protected void AssertHasLog(LogLevel logLevel, string name, string? message = null, Func<Exception, bool>? exceptionMatch = null)
123123
{
124124
if (HasLog(logLevel, name, message, exceptionMatch))
125125
{
126126
return;
127127
}
128128

129-
Assert.Fail($"No match. Log level = {logLevel}, name = {name}, message = '{message}'.");
129+
Assert.Fail($"No match. Log level = {logLevel}, name = {name}, message = '{message ?? "(null)"}'.");
130130
}
131131

132-
protected bool HasLog(LogLevel logLevel, string name, string message, Func<Exception, bool>? exceptionMatch = null)
132+
protected bool HasLog(LogLevel logLevel, string name, string? message = null, Func<Exception, bool>? exceptionMatch = null)
133133
{
134134
return Logs.Any(r =>
135135
{
136-
var match = r.LogLevel == logLevel && r.EventId.Name == name && r.Message == message;
136+
var match = r.LogLevel == logLevel && r.EventId.Name == name && (r.Message == message || message == null);
137137
if (exceptionMatch != null)
138138
{
139139
match = match && r.Exception != null && exceptionMatch(r.Exception);

‎test/Grpc.Net.Client.Tests/GrpcChannelTests.cs‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -222,17 +222,35 @@ public void Build_ConnectTimeout_ReadFromSocketsHttpHandler()
222222
Assert.AreEqual(TimeSpan.FromSeconds(1), channel.ConnectTimeout);
223223
}
224224
225-
[Test]
226-
public void Build_ConnectionIdleTimeout_ReadFromSocketsHttpHandler()
225+
[TestCase(-1, -1, -1)]
226+
[TestCase(0, 0, 0)]
227+
[TestCase(0, -1, 0)]
228+
[TestCase(-1, 0, 0)]
229+
[TestCase(1000, -1, 1000)]
230+
[TestCase(-1, 1000, 1000)]
231+
[TestCase(500, 1000, 1000)]
232+
[TestCase(1000, 500, 1000)]
233+
public void Build_ConnectionIdleTimeout_ReadFromSocketsHttpHandler(
234+
int? pooledConnectionIdleTimeoutMs,
235+
int? pooledConnectionLifetimeMs,
236+
int expectedConnectionIdleTimeoutMs)
227237
{
228-
// Arrange & Act
229-
var channel = GrpcChannel.ForAddress("https://localhost", CreateGrpcChannelOptions(o => o.HttpHandler = new SocketsHttpHandler
238+
// Arrange
239+
var handler = new SocketsHttpHandler();
240+
if (pooledConnectionIdleTimeoutMs != null)
230241
{
231-
PooledConnectionIdleTimeout = TimeSpan.FromSeconds(1)
232-
}));
242+
handler.PooledConnectionIdleTimeout = TimeSpan.FromMilliseconds(pooledConnectionIdleTimeoutMs.Value);
243+
}
244+
if (pooledConnectionLifetimeMs != null)
245+
{
246+
handler.PooledConnectionLifetime = TimeSpan.FromMilliseconds(pooledConnectionLifetimeMs.Value);
247+
}
248+
249+
// Act
250+
var channel = GrpcChannel.ForAddress("https://localhost", CreateGrpcChannelOptions(o => o.HttpHandler = handler));
233251
234252
// Assert
235-
Assert.AreEqual(TimeSpan.FromSeconds(1), channel.ConnectionIdleTimeout);
253+
Assert.AreEqual(TimeSpan.FromMilliseconds(expectedConnectionIdleTimeoutMs), channel.ConnectionIdleTimeout);
236254
}
237255
#endif
238256

0 commit comments

Comments
 (0)