From 01c04ac95a076a1f555b60e09e078d4a940e4b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Eggert=20Lundsteen=20Olsen?= Date: Fri, 2 Oct 2026 12:39:28 +0200 Subject: [PATCH] Review 1.3: database choice and discovery take turns (sp_getapplock) The web's database choice and the agent's discovery both update many rows of an instance's insight.ConfiguredDatabases in one transaction, in different orders, which can deadlock. Both now take an application lock per instance first, as the finding saves do. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...InsightRepository.InstanceConfiguration.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/SQLInsight.Repository/InsightRepository.InstanceConfiguration.cs b/src/SQLInsight.Repository/InsightRepository.InstanceConfiguration.cs index f089a9c..8c6a60d 100644 --- a/src/SQLInsight.Repository/InsightRepository.InstanceConfiguration.cs +++ b/src/SQLInsight.Repository/InsightRepository.InstanceConfiguration.cs @@ -240,6 +240,24 @@ public sealed partial class InsightRepository return updated > 0 ? ConfigurationChangeOutcome.Changed : await WhyNotAsync(id, cancellationToken).ConfigureAwait(false); } + /// + /// The web UI (a choice of databases) and the agent (a discovery) both update many rows of one instance's + /// insight.ConfiguredDatabases in one transaction, in different orders; without taking turns they could deadlock. + /// Like the finding saves, they take an application lock first (one per instance; sp_getapplock needs no grant). + /// + private static async Task LockDatabasesOfAsync(System.Data.Common.DbConnection connection, System.Data.Common.DbTransaction transaction, int id) + { + var result = await connection.ExecuteScalarAsync(""" + DECLARE @result int; + EXEC @result = sp_getapplock @Resource = @resource, @LockMode = N'Exclusive', @LockOwner = N'Transaction', @LockTimeout = 60000; + SELECT @result; + """, new { resource = "SQLInsight:ConfiguredDatabases:" + id.ToString(System.Globalization.CultureInfo.InvariantCulture) }, transaction).ConfigureAwait(false); + if (result < 0) + { + throw new TimeoutException($"Kunne ikke få lås på databasevalget (sp_getapplock {result})."); + } + } + private async Task WhyNotAsync(int id, CancellationToken cancellationToken) => await GetConfiguredInstanceAsync(id, cancellationToken).ConfigureAwait(false) is { FromConfigFile: true } ? ConfigurationChangeOutcome.FromConfigFile @@ -264,6 +282,7 @@ public sealed partial class InsightRepository ArgumentNullException.ThrowIfNull(choices); await using var connection = await OpenAsync(cancellationToken).ConfigureAwait(false); await using var transaction = await connection.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); + await LockDatabasesOfAsync(connection, transaction, id).ConfigureAwait(false); var unknown = new List(); foreach (var choice in choices) { @@ -439,6 +458,7 @@ public sealed partial class InsightRepository await using var connection = await OpenAsync(cancellationToken).ConfigureAwait(false); await using var transaction = await connection.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); var at = Utc(now); + await LockDatabasesOfAsync(connection, transaction, id).ConfigureAwait(false); var monitorNew = await connection.ExecuteScalarAsync( "SELECT MonitorNewDatabases FROM insight.ConfiguredInstances WITH (UPDLOCK) WHERE ConfiguredInstanceId = @id;", new { id }, transaction).ConfigureAwait(false) ?? throw new InvalidOperationException($"Instans {id} findes ikke.");