fix(secrets-sqlserver): retry the virgin-DB concurrent migrator race; release family 0.5.1

The ScadaBridge central shared-store live gate proved SqlServerSecretsStoreMigrator's "safe to run concurrently" claim false on a virgin database: Serializable isolation does not serialize the IF NOT EXISTS metadata probes against another session's in-flight CREATE, so of two central nodes booting together the loser died with SqlException 2714 ("There is already an object named 'zbsecrets'") — the retry filter covered only deadlock 1205. Losing such a race means the object now exists (the desired end state), so 2714 (schema/table), 1913 (index) and 2627 (schema_version MERGE-insert PK) are now retryable alongside 1205; the retry re-runs the idempotent IF NOT EXISTS path. Classification scans the whole SqlException.Errors collection (2714 arrives stacked with 2759), the retry cap keeps genuine conflicts failing closed, and 2705 is deliberately excluded (unreachable — columns live in a single CREATE TABLE statement). Predicate extracted as an internal static pinned by offline tests; live suite gains an env-gated 4-concurrent-migrators virgin-database test. Six packages pushed to the Gitea feed at 0.5.1. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
2026-08-07 11:35:13 -04:00
parent e62b4a5baf
commit 31ca9407a6
5 changed files with 175 additions and 13 deletions
@@ -355,6 +355,37 @@ public sealed class SqlServerSecretStoreLiveTests : IAsyncLifetime

        Assert.NotNull(await _store.GetAsync(new SecretName("survives/migration"), CancellationToken.None));
    }
+
+    [SkippableFact]
+    public async Task Migrator_converges_when_many_nodes_provision_a_virgin_database_concurrently()
+    {
+        Skip.IfNot(LiveSqlServer.IsEnabled, LiveSqlServer.SkipReason);
+
+        // The ScadaBridge central shared-store live gate (2026-08-07): an ordinary compose up
+        // recreated both central nodes at once, both ran the boot migrator against the virgin
+        // database, and the loser died with SqlException 2714 ("There is already an object named
+        // '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.
+        string schema = LiveSqlServer.NewSchemaName();
+        try
+        {
+            await Task.WhenAll(Enumerable.Range(0, 4).Select(_ => Task.Run(() =>
+                new SqlServerSecretsStoreMigrator(
+                        new SecretsSqlServerConnectionFactory(LiveSqlServer.OptionsFor(schema)))
+                    .MigrateAsync(CancellationToken.None))));
+
+            // Every "node" survived; prove the converged schema is actually usable.
+            var store = new SqlServerSecretStore(
+                new SecretsSqlServerConnectionFactory(LiveSqlServer.OptionsFor(schema)));
+            await store.UpsertAsync(MakeSecret("concurrent/provision"), CancellationToken.None);
+            Assert.NotNull(await store.GetAsync(new SecretName("concurrent/provision"), CancellationToken.None));
+        }
+        finally
+        {
+            await LiveSqlServer.DropSchemaAsync(schema);
+        }
+    }
 }

 /// <summary>
@@ -0,0 +1,40 @@
+namespace ZB.MOM.WW.Secrets.Replicator.SqlServer.Tests;
+
+/// <summary>
+/// Offline pins for the migrator's retry classification. A <see cref="Microsoft.Data.SqlClient.SqlException"/>
+/// cannot be constructed normally, so the error-number predicate is extracted as an internal static
+/// and pinned here by number; the concurrent-provision behaviour itself is exercised by the
+/// env-gated live suite.
+/// </summary>
+public sealed class SqlServerSecretsStoreMigratorTests
+{
+    /// <summary>
+    /// The provisioning-race outcomes the ScadaBridge central shared-store live gate (2026-08-07)
+    /// proved reachable — or that the multi-statement DDL makes reachable — when N nodes provision
+    /// one virgin database at once. Losing such a race means the object now exists, which is the
+    /// desired end state, so a retry of the idempotent IF NOT EXISTS path must be taken.
+    /// </summary>
+    [Theory]
+    [InlineData(1205)] // deadlock victim (pre-0.5.1 was the ONLY retried number)
+    [InlineData(2714)] // "There is already an object named ..." — lost CREATE SCHEMA/TABLE (the live-gate defect)
+    [InlineData(1913)] // "index ... already exists" — lost CREATE INDEX
+    [InlineData(2627)] // PK violation — lost schema_version MERGE-insert
+    public void Provisioning_race_error_numbers_are_retryable(int errorNumber)
+        => Assert.True(SqlServerSecretsStoreMigrator.IsRetryableMigrationErrorNumber(errorNumber));
+
+    /// <summary>
+    /// Errors that reproduce identically on retry must propagate so the node fails closed instead
+    /// of spinning through the retry budget against a genuine misconfiguration.
+    /// </summary>
+    [Theory]
+    [InlineData(2705)] // duplicate column — unreachable from the race (single CREATE TABLE statement)
+    [InlineData(208)] // invalid object name
+    [InlineData(229)] // permission denied
+    [InlineData(2759)] // "CREATE SCHEMA failed due to previous errors" — retryable only via its companion 2714
+    [InlineData(4060)] // cannot open database
+    [InlineData(18456)] // login failed
+    [InlineData(0)]
+    [InlineData(-2)] // client-side timeout
+    public void Non_race_error_numbers_are_not_retryable(int errorNumber)
+        => Assert.False(SqlServerSecretsStoreMigrator.IsRetryableMigrationErrorNumber(errorNumber));
+}