Skip to content

Commit 57f7e29

Browse files
Centralize shutdown data-finalization timeout and add forced-shutdown path
Add configurable data-finalization timeout and wire it through CLI, defaults.conf, and GarnetServerOptions so shutdown behavior stays consistent across hosts. - Add --data-finalization-timeout / DataFinalizationTimeoutSeconds (default 15s) - Replace hardcoded 15s finalize CTS in ShutdownAsync with linked caller token plus configured cap; skip persistence on noSave or forced cancellation - Define defaults on GarnetServerOptions (DefaultShutdownTimeoutSeconds, DefaultDataFinalizationTimeoutSeconds) as single source of truth - Windows service: HostOptions.ShutdownTimeout = drain + finalize + 5s margin (replaces fixed 20s buffer); pre-parse both timeout flags - Console host: second Ctrl+C / ProcessExit during shutdown cancels persistence (Redis-like fast path) via shutdownForceCts - Tests: forced cancellation, noSave, and AOF shutdown coverage; fix CS1998 by using void tests with Assert.DoesNotThrowAsync
1 parent 88dc2af commit 57f7e29

8 files changed

Lines changed: 110 additions & 46 deletions

File tree

hosting/Windows/Garnet.worker/Program.cs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,32 @@
33

44
using System;
55
using Garnet;
6+
using Garnet.server;
67
using Microsoft.Extensions.DependencyInjection;
78
using Microsoft.Extensions.Hosting;
89

910
class Program
1011
{
11-
// Data finalization (AOF commit / checkpoint) uses up to 15 seconds internally (see GarnetServer.FinalizeDataAsync).
12-
// Add this buffer on top of the connection-drain timeout so the host shutdown budget covers the full shutdown sequence.
13-
private const int DataFinalizationBufferSeconds = 20;
12+
// Extra host budget beyond connection drain + data finalization (quiesce, stop listening, etc.).
13+
private const int HostShutdownMarginSeconds = 5;
1414

1515
static void Main(string[] args)
1616
{
17-
// Pre-parse only the shutdown-timeout argument so we can configure both
18-
// the host shutdown budget and the Worker's connection-drain timeout from a single value.
19-
var shutdownTimeoutSeconds = ParseShutdownTimeoutSeconds(args, defaultSeconds: 5);
17+
// Pre-parse shutdown-related arguments so we can configure both the host shutdown budget
18+
// and the Worker's connection-drain timeout before the generic host is built.
19+
var shutdownTimeoutSeconds = ParseIntOption(args, "--shutdown-timeout", "-shutdown-timeout", GarnetServerOptions.DefaultShutdownTimeoutSeconds);
20+
var dataFinalizationTimeoutSeconds = ParseIntOption(args, "--data-finalization-timeout", "-data-finalization-timeout", GarnetServerOptions.DefaultDataFinalizationTimeoutSeconds);
2021
var shutdownTimeout = TimeSpan.FromSeconds(shutdownTimeoutSeconds);
2122

2223
var builder = Host.CreateApplicationBuilder(args);
2324

2425
// Tell the .NET host (and the Windows SCM via WindowsServiceLifetime) how long to wait
25-
// before forcibly killing the process. We add DataFinalizationBufferSeconds so that AOF
26-
// commit / checkpoint can complete after connection draining finishes.
26+
// before forcibly killing the process: drain + data finalization + margin.
2727
builder.Services.Configure<HostOptions>(opts =>
2828
{
29-
opts.ShutdownTimeout = shutdownTimeout + TimeSpan.FromSeconds(DataFinalizationBufferSeconds);
29+
opts.ShutdownTimeout = shutdownTimeout
30+
+ TimeSpan.FromSeconds(dataFinalizationTimeoutSeconds)
31+
+ TimeSpan.FromSeconds(HostShutdownMarginSeconds);
3032
});
3133

3234
builder.Services.AddHostedService(_ => new Worker(args, shutdownTimeout));
@@ -41,15 +43,14 @@ static void Main(string[] args)
4143
}
4244

4345
/// <summary>
44-
/// Scans <paramref name="args"/> for <c>--shutdown-timeout &lt;value&gt;</c> and returns
45-
/// the parsed integer, or <paramref name="defaultSeconds"/> if the argument is absent or invalid.
46-
/// This lightweight pre-parse avoids a full CommandLineParser pass before the host is built.
46+
/// Scans <paramref name="args"/> for <paramref name="longName"/> (or <paramref name="shortName"/>) and returns
47+
/// the parsed positive integer, or <paramref name="defaultSeconds"/> if the argument is absent or invalid.
4748
/// </summary>
48-
private static int ParseShutdownTimeoutSeconds(string[] args, int defaultSeconds)
49+
private static int ParseIntOption(string[] args, string longName, string shortName, int defaultSeconds)
4950
{
5051
for (var i = 0; i < args.Length - 1; i++)
5152
{
52-
if (args[i] is "--shutdown-timeout" or "-shutdown-timeout" &&
53+
if (args[i] is var name && (name == longName || name == shortName) &&
5354
int.TryParse(args[i + 1], out var value) && value > 0)
5455
{
5556
return value;

hosting/Windows/Garnet.worker/Worker.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ public class Worker : BackgroundService
1919
/// <param name="args">Command line arguments forwarded to <see cref="GarnetServer"/>.</param>
2020
/// <param name="shutdownTimeout">
2121
/// How long to wait for active connections to drain during graceful shutdown.
22-
/// Must be less than the host <see cref="Microsoft.Extensions.Hosting.HostOptions.ShutdownTimeout"/>
23-
/// so that data finalization (AOF commit / checkpoint) can also complete within the host budget.
22+
/// Must fit within the host <see cref="Microsoft.Extensions.Hosting.HostOptions.ShutdownTimeout"/>,
23+
/// which also budgets for data finalization and a small margin (see <c>Program.cs</c>).
2424
/// </param>
2525
public Worker(string[] args, TimeSpan shutdownTimeout)
2626
{

libs/host/Configuration/Options.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,10 @@ internal sealed class Options : ICloneable
375375
"The Windows SCM default pre-kill wait is 5 seconds, so values below 5 are not recommended when running as a Windows service.")]
376376
public int ShutdownTimeoutSeconds { get; set; }
377377

378+
[IntRangeValidation(1, int.MaxValue)]
379+
[Option("data-finalization-timeout", Required = false, HelpText = "Timeout in seconds for AOF commit and checkpoint during graceful shutdown data finalization.")]
380+
public int DataFinalizationTimeoutSeconds { get; set; }
381+
378382
[OptionValidation]
379383
[Option("use-azure-storage", Required = false, HelpText = "Use Azure Page Blobs for storage instead of local storage.")]
380384
public bool? UseAzureStorage { get; set; }
@@ -901,6 +905,7 @@ endpoint is IPEndPoint listenEp && clusterAnnounceEndpoint[0] is IPEndPoint anno
901905
ThreadPoolMaxIOCompletionThreads = ThreadPoolMaxIOCompletionThreads,
902906
NetworkConnectionLimit = NetworkConnectionLimit,
903907
ShutdownTimeoutSeconds = ShutdownTimeoutSeconds,
908+
DataFinalizationTimeoutSeconds = DataFinalizationTimeoutSeconds,
904909
DeviceFactoryCreator = deviceType == DeviceType.AzureStorage ? azureFactoryCreator()
905910
: new LocalStorageNamedDeviceFactoryCreator(
906911
deviceType: deviceType,

libs/host/GarnetServer.cs

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,12 @@ static string GetVersion()
7474
/// <summary>
7575
/// Configured shutdown drain timeout in seconds.
7676
/// </summary>
77-
public int ShutdownTimeoutSeconds => opts.ShutdownTimeoutSeconds > 0 ? opts.ShutdownTimeoutSeconds : 5;
77+
public int ShutdownTimeoutSeconds => opts.ShutdownTimeoutSeconds > 0 ? opts.ShutdownTimeoutSeconds : GarnetServerOptions.DefaultShutdownTimeoutSeconds;
78+
79+
/// <summary>
80+
/// Configured data-finalization timeout in seconds (AOF commit / checkpoint during shutdown).
81+
/// </summary>
82+
public int DataFinalizationTimeoutSeconds => opts.DataFinalizationTimeoutSeconds > 0 ? opts.DataFinalizationTimeoutSeconds : GarnetServerOptions.DefaultDataFinalizationTimeoutSeconds;
7883

7984
/// <summary>
8085
/// Create Garnet Server instance using specified command line arguments; use Start to start the server.
@@ -448,11 +453,12 @@ public void Start()
448453
/// </summary>
449454
/// <param name="timeout">Timeout for waiting on active connections (default: configured <see cref="ShutdownTimeoutSeconds"/> value)</param>
450455
/// <param name="noSave">If true, skip data persistence (AOF commit and checkpoint) during shutdown</param>
451-
/// <param name="token">Cancellation token</param>
456+
/// <param name="token">Cancellation token; when cancelled (e.g. second Ctrl+C), connection draining and data finalization are aborted</param>
452457
/// <returns>Task representing the async shutdown operation</returns>
453458
public async Task ShutdownAsync(TimeSpan? timeout = null, bool noSave = false, CancellationToken token = default)
454459
{
455460
var shutdownTimeout = timeout ?? TimeSpan.FromSeconds(ShutdownTimeoutSeconds);
461+
var skipPersistence = noSave;
456462

457463
try
458464
{
@@ -475,9 +481,14 @@ public async Task ShutdownAsync(TimeSpan? timeout = null, bool noSave = false, C
475481
{
476482
await WaitForActiveConnectionsAsync(shutdownTimeout, token).ConfigureAwait(false);
477483
}
484+
catch (OperationCanceledException) when (token.IsCancellationRequested)
485+
{
486+
skipPersistence = true;
487+
logger?.LogWarning("Connection draining was cancelled due to forced shutdown.");
488+
}
478489
catch (OperationCanceledException)
479490
{
480-
logger?.LogWarning("Connection draining was cancelled. Proceeding with data finalization...");
491+
logger?.LogWarning("Connection draining timed out. Proceeding with data finalization...");
481492
}
482493
}
483494
catch (Exception ex)
@@ -486,25 +497,34 @@ public async Task ShutdownAsync(TimeSpan? timeout = null, bool noSave = false, C
486497
}
487498
finally
488499
{
489-
if (!noSave)
500+
if (skipPersistence)
501+
{
502+
logger?.LogInformation("Skipping data persistence during shutdown.");
503+
}
504+
else
490505
{
491506
// Attempt AOF commit or checkpoint as best-effort,
492-
// even if connection draining was cancelled or failed.
493-
// Use a bounded timeout instead of the caller's token to ensure completion.
494-
using var finalizeCts = new CancellationTokenSource(TimeSpan.FromSeconds(15));
507+
// even if connection draining timed out. Honor caller cancellation (forced shutdown)
508+
// and cap total finalize time with DataFinalizationTimeoutSeconds.
509+
using var finalizeCts = CancellationTokenSource.CreateLinkedTokenSource(token);
510+
finalizeCts.CancelAfter(TimeSpan.FromSeconds(DataFinalizationTimeoutSeconds));
495511
try
496512
{
497513
await FinalizeDataAsync(finalizeCts.Token).ConfigureAwait(false);
498514
}
515+
catch (OperationCanceledException) when (token.IsCancellationRequested)
516+
{
517+
logger?.LogWarning("Data finalization was cancelled due to forced shutdown.");
518+
}
519+
catch (OperationCanceledException)
520+
{
521+
logger?.LogWarning("Data finalization timed out after {TimeoutSeconds} seconds.", DataFinalizationTimeoutSeconds);
522+
}
499523
catch (Exception ex)
500524
{
501525
logger?.LogError(ex, "Error during data finalization");
502526
}
503527
}
504-
else
505-
{
506-
logger?.LogInformation("Shutdown with noSave flag - skipping data persistence.");
507-
}
508528
}
509529
}
510530

libs/host/defaults.conf

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,9 @@
272272
/* Timeout in seconds to wait for active connections to drain during graceful shutdown. */
273273
"ShutdownTimeoutSeconds" : 5,
274274

275+
/* Timeout in seconds for AOF commit and checkpoint during graceful shutdown data finalization. */
276+
"DataFinalizationTimeoutSeconds" : 15,
277+
275278
/* Use Azure Page Blobs for storage instead of local storage. */
276279
"UseAzureStorage" : false,
277280

libs/server/Servers/GarnetServerOptions.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,25 @@ public class GarnetServerOptions : ServerOptions
242242
/// </summary>
243243
public bool QuietMode = false;
244244

245+
/// <summary>
246+
/// Default connection-drain timeout in seconds when not configured.
247+
/// </summary>
248+
public const int DefaultShutdownTimeoutSeconds = 5;
249+
250+
/// <summary>
251+
/// Default data-finalization timeout in seconds when not configured.
252+
/// </summary>
253+
public const int DefaultDataFinalizationTimeoutSeconds = 15;
254+
245255
/// <summary>
246256
/// Timeout (in seconds) for waiting on active connections to drain during graceful shutdown.
247257
/// </summary>
248-
public int ShutdownTimeoutSeconds = 5;
258+
public int ShutdownTimeoutSeconds = DefaultShutdownTimeoutSeconds;
259+
260+
/// <summary>
261+
/// Timeout (in seconds) for AOF commit and checkpoint during graceful shutdown data finalization.
262+
/// </summary>
263+
public int DataFinalizationTimeoutSeconds = DefaultDataFinalizationTimeoutSeconds;
249264

250265
/// <summary>
251266
/// SAVE and BGSAVE: We will take a full (index + log) checkpoint when ReadOnlyAddress of log increases by this amount, from the last full checkpoint.

main/GarnetServer/Program.cs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,35 @@ static async Task Main(string[] args)
2222
// Start the server
2323
server.Start();
2424

25-
using var cts = new CancellationTokenSource();
25+
using var runCts = new CancellationTokenSource();
26+
using var shutdownForceCts = new CancellationTokenSource();
27+
var shutdownSignals = 0;
2628

27-
ConsoleCancelEventHandler cancelKeyPressHandler = (sender, e) =>
29+
void OnShutdownSignal()
2830
{
29-
e.Cancel = true; // Prevent the process from terminating immediately
30-
if (!cts.IsCancellationRequested)
31-
cts.Cancel();
32-
};
31+
// First signal: graceful shutdown. Second signal (or ProcessExit while shutting down): fast path.
32+
if (Interlocked.Increment(ref shutdownSignals) >= 2)
33+
shutdownForceCts.Cancel();
3334

34-
EventHandler processExitHandler = (sender, e) =>
35-
{
3635
try
3736
{
38-
if (!cts.IsCancellationRequested)
39-
cts.Cancel();
37+
if (!runCts.IsCancellationRequested)
38+
runCts.Cancel();
4039
}
4140
catch (ObjectDisposedException)
4241
{
4342
// The cancellation source may already be disposed during process teardown.
4443
}
44+
}
45+
46+
ConsoleCancelEventHandler cancelKeyPressHandler = (sender, e) =>
47+
{
48+
e.Cancel = true; // Prevent the process from terminating immediately
49+
OnShutdownSignal();
4550
};
4651

52+
EventHandler processExitHandler = (sender, e) => OnShutdownSignal();
53+
4754
// Signal cancellation on Ctrl+C; avoid blocking the event handler with async work
4855
Console.CancelKeyPress += cancelKeyPressHandler;
4956

@@ -55,15 +62,14 @@ static async Task Main(string[] args)
5562
{
5663
try
5764
{
58-
await Task.Delay(Timeout.Infinite, cts.Token).ConfigureAwait(false);
65+
await Task.Delay(Timeout.Infinite, runCts.Token).ConfigureAwait(false);
5966
}
6067
catch (OperationCanceledException)
6168
{
62-
// Graceful shutdown: drain connections, commit AOF, take checkpoint
69+
// Graceful shutdown on first signal; second signal cancels persistence via shutdownForceCts.
6370
await server.ShutdownAsync(
6471
TimeSpan.FromSeconds(server.ShutdownTimeoutSeconds),
65-
token: CancellationToken.None
66-
).ConfigureAwait(false);
72+
token: shutdownForceCts.Token).ConfigureAwait(false);
6773
}
6874
}
6975
finally

test/standalone/Garnet.test/GarnetServerTcpTests.cs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,24 +167,38 @@ public async Task ShutdownAsyncRespectsTimeout()
167167
}
168168

169169
[Test]
170-
public async Task ShutdownAsyncRespectsCancellation()
170+
public void ShutdownAsyncRespectsCancellation()
171171
{
172172
// Arrange
173173
using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
174174
redis.GetDatabase(0).Ping();
175175

176176
using var cts = new CancellationTokenSource();
177177

178-
// Act - Cancel immediately
178+
// Act - Cancel immediately (forced shutdown: skip data finalization)
179179
cts.Cancel();
180+
var sw = System.Diagnostics.Stopwatch.StartNew();
180181
Assert.DoesNotThrowAsync(async () =>
181182
{
182183
await server.ShutdownAsync(timeout: TimeSpan.FromSeconds(30), token: cts.Token).ConfigureAwait(false);
183184
});
185+
sw.Stop();
186+
187+
// Assert - Should not wait for the full data-finalization timeout
188+
ClassicAssert.Less(sw.ElapsedMilliseconds, 5_000);
189+
}
190+
191+
[Test]
192+
public void ShutdownAsyncNoSaveSkipsPersistence()
193+
{
194+
Assert.DoesNotThrowAsync(async () =>
195+
{
196+
await server.ShutdownAsync(timeout: TimeSpan.FromSeconds(5), noSave: true).ConfigureAwait(false);
197+
});
184198
}
185199

186200
[Test]
187-
public async Task ShutdownAsyncWithAofCommit()
201+
public void ShutdownAsyncWithAofCommit()
188202
{
189203
// Arrange - Create server with AOF enabled
190204
server?.Dispose();
@@ -201,7 +215,7 @@ public async Task ShutdownAsyncWithAofCommit()
201215
db.StringSet($"aof-key-{i}", $"value-{i}");
202216
}
203217

204-
// Act - Shutdown should commit AOF without errors
218+
// Act & Assert - Shutdown should commit AOF without errors
205219
Assert.DoesNotThrowAsync(async () =>
206220
{
207221
await server.ShutdownAsync(timeout: TimeSpan.FromSeconds(5)).ConfigureAwait(false);

0 commit comments

Comments
 (0)