fix(secrets): jittered exponential backoff in SqlServerSecretsStoreMigrator + 8-way concurrent-migrate live test
Flat 100*attempt backoff let concurrent boots retry in lockstep (Auth's twin loop exhausted retries ~1/60 eight-way boots). Backoff is now exponential with equal jitter, via an injectable Random for deterministic tests. Refs scadaproj#18
This commit is contained in:
+33
-4
@@ -13,9 +13,22 @@ namespace ZB.MOM.WW.Secrets.Replicator.SqlServer;
|
||||
/// to run against a database whose recorded version is newer than this build supports.
|
||||
/// </summary>
|
||||
/// <param name="connectionFactory">Factory for connections to the shared secret database.</param>
|
||||
public sealed class SqlServerSecretsStoreMigrator(SecretsSqlServerConnectionFactory connectionFactory)
|
||||
: ISecretsStoreMigrator
|
||||
/// <param name="random">Jitter source for the retry back-off; inject a seeded instance for deterministic tests.</param>
|
||||
public sealed class SqlServerSecretsStoreMigrator(
|
||||
SecretsSqlServerConnectionFactory connectionFactory,
|
||||
Random random) : ISecretsStoreMigrator
|
||||
{
|
||||
/// <summary>
|
||||
/// Initializes the migrator with the process-wide <see cref="Random.Shared"/> as its jitter source.
|
||||
/// </summary>
|
||||
/// <param name="connectionFactory">Factory for connections to the shared secret database.</param>
|
||||
public SqlServerSecretsStoreMigrator(SecretsSqlServerConnectionFactory connectionFactory)
|
||||
: this(connectionFactory, Random.Shared)
|
||||
{
|
||||
}
|
||||
|
||||
private const int BaseBackoffMilliseconds = 100;
|
||||
|
||||
// Every node runs this at every startup, so a virgin database being provisioned by N nodes at
|
||||
// once is the NORMAL case, not a rare race — and an unretried failure aborts that node's whole
|
||||
// host at startup. Two failure shapes come out of that race, and retrying is correct for both
|
||||
@@ -63,13 +76,29 @@ public sealed class SqlServerSecretsStoreMigrator(SecretsSqlServerConnectionFact
|
||||
catch (SqlException ex)
|
||||
when (IsRetryableMigrationError(ex) && attempt < MaxMigrationRetries)
|
||||
{
|
||||
// Staggered back-off so two victims do not retry in lockstep and race again.
|
||||
await Task.Delay(TimeSpan.FromMilliseconds(100 * attempt), cancellationToken)
|
||||
// Jittered back-off so N victims do not retry in lockstep and race again (a flat
|
||||
// 100 * attempt schedule exhausted its retries ~1 in 60 eight-way boots in Auth's
|
||||
// identical loop; scadaproj#18).
|
||||
await Task.Delay(ComputeBackoff(attempt, random), cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Computes the delay before retry number <paramref name="attempt"/> using exponential back-off
|
||||
/// with "equal jitter": half the exponential window is fixed, the other half is random, so the
|
||||
/// result lies in [window/2, window) where window = 100 ms * 2^(attempt - 1).
|
||||
/// </summary>
|
||||
/// <param name="attempt">The 1-based attempt that just failed.</param>
|
||||
/// <param name="random">Jitter source; inject a seeded instance for deterministic tests.</param>
|
||||
/// <returns>The delay to wait before the next attempt.</returns>
|
||||
internal static TimeSpan ComputeBackoff(int attempt, Random random)
|
||||
{
|
||||
double window = BaseBackoffMilliseconds * Math.Pow(2, attempt - 1);
|
||||
return TimeSpan.FromMilliseconds((window / 2) + (random.NextDouble() * window / 2));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Whether any error carried by <paramref name="exception"/> is a retryable provisioning-race
|
||||
/// outcome. Scans the whole <see cref="SqlException.Errors"/> collection rather than only
|
||||
|
||||
+15
-1
@@ -358,6 +358,17 @@ public sealed class SqlServerSecretStoreLiveTests : IAsyncLifetime
|
||||
|
||||
[SkippableFact]
|
||||
public async Task Migrator_converges_when_many_nodes_provision_a_virgin_database_concurrently()
|
||||
=> await ProvisionConcurrentlyAsync(nodes: 4, rounds: 1);
|
||||
|
||||
/// <summary>
|
||||
/// scadaproj#18: the Auth twin of this loop exhausted its retries ~1 in 60 eight-way boots with
|
||||
/// flat back-off; repeated rounds of 8 concurrent migrators against fresh virgin schemas.
|
||||
/// </summary>
|
||||
[SkippableFact]
|
||||
public async Task Migrate_Concurrently_FromEightTasks_AllSucceed()
|
||||
=> await ProvisionConcurrentlyAsync(nodes: 8, rounds: 10);
|
||||
|
||||
private static async Task ProvisionConcurrentlyAsync(int nodes, int rounds)
|
||||
{
|
||||
Skip.IfNot(LiveSqlServer.IsEnabled, LiveSqlServer.SkipReason);
|
||||
|
||||
@@ -367,10 +378,12 @@ public sealed class SqlServerSecretStoreLiveTests : IAsyncLifetime
|
||||
// 'zbsecrets'") — the retry filter then covered only deadlock 1205. Deliberately NOT the
|
||||
// class's shared schema: that one is already migrated by InitializeAsync, and this race
|
||||
// only exists against a virgin database.
|
||||
for (int round = 0; round < rounds; round++)
|
||||
{
|
||||
string schema = LiveSqlServer.NewSchemaName();
|
||||
try
|
||||
{
|
||||
await Task.WhenAll(Enumerable.Range(0, 4).Select(_ => Task.Run(() =>
|
||||
await Task.WhenAll(Enumerable.Range(0, nodes).Select(_ => Task.Run(() =>
|
||||
new SqlServerSecretsStoreMigrator(
|
||||
new SecretsSqlServerConnectionFactory(LiveSqlServer.OptionsFor(schema)))
|
||||
.MigrateAsync(CancellationToken.None))));
|
||||
@@ -385,6 +398,7 @@ public sealed class SqlServerSecretStoreLiveTests : IAsyncLifetime
|
||||
{
|
||||
await LiveSqlServer.DropSchemaAsync(schema);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+36
@@ -37,4 +37,40 @@ public sealed class SqlServerSecretsStoreMigratorTests
|
||||
[InlineData(-2)] // client-side timeout
|
||||
public void Non_race_error_numbers_are_not_retryable(int errorNumber)
|
||||
=> Assert.False(SqlServerSecretsStoreMigrator.IsRetryableMigrationErrorNumber(errorNumber));
|
||||
|
||||
/// <summary>The jittered back-off stays inside [window/2, window) and grows with the attempt.</summary>
|
||||
[Theory]
|
||||
[InlineData(1, 50, 100)]
|
||||
[InlineData(2, 100, 200)]
|
||||
[InlineData(4, 400, 800)]
|
||||
public void Backoff_is_bounded_by_the_exponential_window(int attempt, double minMs, double maxMs)
|
||||
{
|
||||
var random = new Random(12345);
|
||||
for (int i = 0; i < 1000; i++)
|
||||
{
|
||||
double ms = SqlServerSecretsStoreMigrator.ComputeBackoff(attempt, random).TotalMilliseconds;
|
||||
Assert.InRange(ms, minMs, maxMs);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Different draws give different delays (the lockstep-retry defect was a constant schedule).</summary>
|
||||
[Fact]
|
||||
public void Backoff_is_jittered_not_constant()
|
||||
{
|
||||
var random = new Random(42);
|
||||
var delays = Enumerable.Range(0, 8)
|
||||
.Select(_ => SqlServerSecretsStoreMigrator.ComputeBackoff(3, random))
|
||||
.Distinct()
|
||||
.ToList();
|
||||
Assert.True(delays.Count > 1);
|
||||
}
|
||||
|
||||
/// <summary>A fixed jitter source yields an exactly predictable delay.</summary>
|
||||
[Fact]
|
||||
public void Backoff_is_deterministic_for_a_seeded_source()
|
||||
{
|
||||
Assert.Equal(
|
||||
SqlServerSecretsStoreMigrator.ComputeBackoff(3, new Random(7)),
|
||||
SqlServerSecretsStoreMigrator.ComputeBackoff(3, new Random(7)));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user