Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose clientTimeout option #1322

Closed
wants to merge 3 commits into from
Closed

Expose clientTimeout option #1322

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
695808a
Expose clientTimeout option
vicancy Jun 24, 2021
c151c45
Fix test failure
vicancy Jun 24, 2021
243ed7c
Use a separate connection factory for management sdk
vicancy Jun 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Microsoft.Azure.SignalR.Common/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public static class Periods
public static readonly TimeSpan DefaultServersPingInterval = TimeSpan.FromSeconds(5);
// Depends on DefaultStatusPingInterval, make 1/2 to fast check.
public static readonly TimeSpan DefaultCloseDelayInterval = TimeSpan.FromSeconds(5);

public static readonly TimeSpan DefaultClientCloseTimeout = TimeSpan.FromSeconds(15);

// Custom handshake timeout of SignalR Service
public const int DefaultHandshakeTimeout = 15;
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Azure.SignalR/HubHost/ServiceHubDispatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ internal virtual ServiceConnectionFactory GetServiceConnectionFactory(
connectionDelegate,
_clientConnectionFactory,
_nameProvider,
_serviceEventHandler)
_serviceEventHandler,
_options)
{
ConfigureContext = contextConfig,
ShutdownMode = _options.GracefulShutdown.Mode
};
}

Expand Down
12 changes: 6 additions & 6 deletions src/Microsoft.Azure.SignalR/ServerConnections/ClientConnectionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,30 +225,30 @@ public void TickHeartbeat()
/// <summary>
/// Cancel the outgoing process
/// </summary>
public void CancelOutgoing(int millisecondsDelay = 0)
public void CancelOutgoing(TimeSpan? delay = null)
{
if (millisecondsDelay <= 0)
if (!delay.HasValue)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about inverting the if to make the logic more straight-forward?

{
_abortOutgoingCts.Cancel();
}
else
{
_abortOutgoingCts.CancelAfter(millisecondsDelay);
_abortOutgoingCts.CancelAfter(delay.Value);
}
}

/// <summary>
/// Cancel the application task
/// </summary>
public void CancelApplication(int millisecondsDelay = 0)
public void CancelApplication(TimeSpan? delay = null)
{
if (millisecondsDelay <= 0)
if (!delay.HasValue)
{
_abortApplicationCts.Cancel();
}
else
{
_abortApplicationCts.CancelAfter(millisecondsDelay);
_abortApplicationCts.CancelAfter(delay.Value);
}
}

Expand Down
15 changes: 6 additions & 9 deletions src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ namespace Microsoft.Azure.SignalR
{
internal partial class ServiceConnection : ServiceConnectionBase
{
private const int DefaultCloseTimeoutMilliseconds = 5000;

// Fix issue: https://github.com/Azure/azure-signalr/issues/198
// .NET Framework has restriction about reserved string as the header name like "User-Agent"
private static readonly Dictionary<string, string> CustomHeader = new Dictionary<string, string> { { Constants.AsrsUserAgent, ProductInfo.GetProductInfo() } };
Expand All @@ -30,7 +28,7 @@ internal partial class ServiceConnection : ServiceConnectionBase

private readonly IConnectionFactory _connectionFactory;
private readonly IClientConnectionFactory _clientConnectionFactory;
private readonly int _closeTimeOutMilliseconds;
private readonly TimeSpan? _closeTimeOut;
private readonly IClientConnectionManager _clientConnectionManager;

private readonly ConcurrentDictionary<string, string> _connectionIds =
Expand All @@ -56,14 +54,13 @@ public ServiceConnection(IServiceProtocol serviceProtocol,
IServiceEventHandler serviceEventHandler,
ServiceConnectionType connectionType = ServiceConnectionType.Default,
GracefulShutdownMode mode = GracefulShutdownMode.Off,
int closeTimeOutMilliseconds = DefaultCloseTimeoutMilliseconds
) : base(serviceProtocol, serverId, connectionId, endpoint, serviceMessageHandler, serviceEventHandler, connectionType, loggerFactory?.CreateLogger<ServiceConnection>(), mode)
TimeSpan? closeTimeOut = null) : base(serviceProtocol, serverId, connectionId, endpoint, serviceMessageHandler, serviceEventHandler, connectionType, loggerFactory?.CreateLogger<ServiceConnection>(), mode)
{
_clientConnectionManager = clientConnectionManager;
_connectionFactory = connectionFactory;
_connectionDelegate = connectionDelegate;
_clientConnectionFactory = clientConnectionFactory;
_closeTimeOutMilliseconds = closeTimeOutMilliseconds;
_closeTimeOut = closeTimeOut;
}

protected override Task<ConnectionContext> CreateConnection(string target = null)
Expand Down Expand Up @@ -220,7 +217,7 @@ private async Task ProcessClientConnectionAsync(ClientConnectionContext connecti

// app task completes connection.Transport.Output, which will completes connection.Application.Input and ends the transport
// Transports are written by us and are well behaved, wait for them to drain
connection.CancelOutgoing(_closeTimeOutMilliseconds);
connection.CancelOutgoing(_closeTimeOut);

// transport never throws
await transport;
Expand All @@ -231,7 +228,7 @@ private async Task ProcessClientConnectionAsync(ClientConnectionContext connecti
Log.WaitingForApplication(Logger);

// Wait on the application task to complete
connection.CancelApplication(_closeTimeOutMilliseconds);
connection.CancelApplication(_closeTimeOut);
try
{
await app;
Expand Down Expand Up @@ -440,7 +437,7 @@ private async Task PerformDisconnectAsyncCore(string connectionId)

// lock and wait
// Register the cancellation after timeout
connection.CancelApplication(_closeTimeOutMilliseconds);
connection.CancelApplication(_closeTimeOut);

// wait for the connection's lifetime task to end
await connection.LifetimeTask;
Expand Down
10 changes: 6 additions & 4 deletions src/Microsoft.Azure.SignalR/ServerConnections/ServiceConnectionFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ internal class ServiceConnectionFactory : IServiceConnectionFactory
private readonly IClientConnectionFactory _clientConnectionFactory;
private readonly IServerNameProvider _nameProvider;
private readonly IServiceEventHandler _serviceEventHandler;

public GracefulShutdownMode ShutdownMode { get; set; } = GracefulShutdownMode.Off;
private readonly ServiceOptions _options;

public Action<HttpContext> ConfigureContext { get; set; }

Expand All @@ -29,7 +28,8 @@ public ServiceConnectionFactory(
ConnectionDelegate connectionDelegate,
IClientConnectionFactory clientConnectionFactory,
IServerNameProvider nameProvider,
IServiceEventHandler serviceEventHandler)
IServiceEventHandler serviceEventHandler,
ServiceOptions options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change it to IOptions so that no need to add a WeakConnectionFactory?

{
_serviceProtocol = serviceProtocol;
_clientConnectionManager = clientConnectionManager;
Expand All @@ -39,6 +39,7 @@ public ServiceConnectionFactory(
_clientConnectionFactory = clientConnectionFactory;
_nameProvider = nameProvider;
_serviceEventHandler = serviceEventHandler;
_options = options;
}

public virtual IServiceConnection Create(HubServiceEndpoint endpoint, IServiceMessageHandler serviceMessageHandler, ServiceConnectionType type)
Expand All @@ -56,7 +57,8 @@ public virtual IServiceConnection Create(HubServiceEndpoint endpoint, IServiceMe
serviceMessageHandler,
_serviceEventHandler,
type,
ShutdownMode
_options.GracefulShutdown?.Mode ?? GracefulShutdownMode.Off,
_options.ClientCloseTimeout
)
{
ConfigureContext = ConfigureContext
Expand Down
6 changes: 6 additions & 0 deletions src/Microsoft.Azure.SignalR/ServiceOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,11 @@ public class ServiceOptions : IServiceEndpointOptions
/// Default value is 5, limited to [1, 300].
/// </summary>
public int? MaxPollIntervalInSeconds { get; set; }

/// <summary>
/// After the transport layer for the client ends, if the client hub logic fails to end in this period, it will be terminated.
/// The default value is 15 seconds
/// </summary>
public TimeSpan ClientCloseTimeout { get; set; } = Constants.Periods.DefaultClientCloseTimeout;
}
}
2 changes: 1 addition & 1 deletion test/Microsoft.Azure.SignalR.IntegrationTests/Infrastructure/MockServiceConnectionFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public MockServiceConnectionFactory(
connectionDelegate,
clientConnectionFactory,
nameProvider,
null)
null, new ServiceOptions())
{
_mockService = mockService;
}
Expand Down
6 changes: 3 additions & 3 deletions test/Microsoft.Azure.SignalR.Tests/MultiEndpointServiceConnectionContainerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1268,13 +1268,13 @@ public async Task ServiceConnectionContainerScopeWithPingUpdateTest()
);
var endpoints = sem.GetEndpoints("hub");
var connection1 = new ServiceConnection(protocol, ccm, connectionFactory1, loggerFactory, connectionDelegate, ccf,
"serverId", "server-conn-1", endpoints[0], endpoints[0].ConnectionContainer as IServiceMessageHandler, null, closeTimeOutMilliseconds: 500);
"serverId", "server-conn-1", endpoints[0], endpoints[0].ConnectionContainer as IServiceMessageHandler, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var connection2 = new ServiceConnection(protocol, ccm, connectionFactory2, loggerFactory, connectionDelegate, ccf,
"serverId", "server-conn-2", endpoints[1], endpoints[1].ConnectionContainer as IServiceMessageHandler, null, closeTimeOutMilliseconds: 500);
"serverId", "server-conn-2", endpoints[1], endpoints[1].ConnectionContainer as IServiceMessageHandler, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var connection22 = new ServiceConnection(protocol, ccm, connectionFactory22, loggerFactory, connectionDelegate, ccf,
"serverId", "server-conn-22", endpoints[1], endpoints[1].ConnectionContainer as IServiceMessageHandler, null, closeTimeOutMilliseconds: 500);
"serverId", "server-conn-22", endpoints[1], endpoints[1].ConnectionContainer as IServiceMessageHandler, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var router = new TestEndpointRouter();

Expand Down
10 changes: 5 additions & 5 deletions test/Microsoft.Azure.SignalR.Tests/ServiceConnectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public async Task TestServiceConnectionWithEndlessApplicationTask()
ConnectionDelegate handler = builder.Build();
var connection = new ServiceConnection(protocol, ccm, connectionFactory, loggerFactory, handler, ccf,
"serverId", Guid.NewGuid().ToString("N"),
null, null, null, closeTimeOutMilliseconds: 1000);
null, null, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -278,7 +278,7 @@ public async Task ClientConnectionOutgoingAbortCanEndLifeTime()
ConnectionDelegate handler = builder.Build();
var connection = new ServiceConnection(protocol, ccm, connectionFactory, loggerFactory, handler, ccf,
"serverId", Guid.NewGuid().ToString("N"), null, null, null,
closeTimeOutMilliseconds: 500);
closeTimeOut: TimeSpan.FromMilliseconds(500));

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -336,7 +336,7 @@ public async Task ClientConnectionContextAbortCanSendOutCloseMessage()
ConnectionDelegate handler = builder.Build();

var connection = new ServiceConnection(protocol, ccm, connectionFactory, loggerFactory, handler, ccf,
"serverId", Guid.NewGuid().ToString("N"), null, null, null, closeTimeOutMilliseconds: 500);
"serverId", Guid.NewGuid().ToString("N"), null, null, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -399,7 +399,7 @@ public async Task ClientConnectionWithDiagnosticClientTagTest()
ConnectionDelegate handler = builder.Build();

var connection = new ServiceConnection(protocol, ccm, connectionFactory, loggerFactory, handler, ccf,
"serverId", Guid.NewGuid().ToString("N"), null, null, null, closeTimeOutMilliseconds: 500);
"serverId", Guid.NewGuid().ToString("N"), null, null, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var connectionTask = connection.StartAsync();

Expand Down Expand Up @@ -457,7 +457,7 @@ public async Task ClientConnectionLastWillCanSendOut()
builder.UseConnectionHandler<EndlessConnectionHandler>();
ConnectionDelegate handler = builder.Build();
var connection = new ServiceConnection(protocol, ccm, connectionFactory, loggerFactory, handler, ccf,
"serverId", Guid.NewGuid().ToString("N"), null, null, null, closeTimeOutMilliseconds: 500);
"serverId", Guid.NewGuid().ToString("N"), null, null, null, closeTimeOut: TimeSpan.FromMilliseconds(500));

var connectionTask = connection.StartAsync();

Expand Down