diff --git a/docs/TriggerBinding.md b/docs/TriggerBinding.md index 23c59479..2e3a4313 100644 --- a/docs/TriggerBinding.md +++ b/docs/TriggerBinding.md @@ -119,6 +119,10 @@ The SQL Trigger uses transactions to guarantee that changes are rolled back in t Because of this, and the number of internal state tables that the trigger interacts with, there is a high chance of deadlocks occurring if two functions are attempting to make changes to the tables at the same time. -To avoid this from happening the trigger utilizes the [sp_getapplock](https://learn.microsoft.com/sql/relational-databases/system-stored-procedures/sp-getapplock-transact-sql) statement to ensure that each transaction is processed serially. Before each statement in a transaction, sp_getapplock gets an Exclusive lock on the `_az_func_Trigger` resource - this ensures that for the duration of the transaction it is the only Azure Function that will be accessing any of the tables used. +To avoid this from happening the trigger utilizes the [sp_getapplock](https://learn.microsoft.com/sql/relational-databases/system-stored-procedures/sp-getapplock-transact-sql) statement to ensure that transactions targeting the same table are processed serially. There are two levels of application locks used: -While this helps ensure concurrency safety for Azure Functions, other queries on the system can still cause a deadlock to occur. [This guide](https://learn.microsoft.com/sql/relational-databases/sql-server-deadlocks-guide) can help troubleshoot any issues that occur and provides some suggestions for fixing these issues. You may also utilize the sp_getapplock statement yourself with the `_az_func_Trigger` resource to synchronize requests, although doing so may have a negative impact on the performance of your Functions. +- **Global lock** (`_az_func_Trigger`): Used only during startup for DDL operations that create shared objects (the `az_func` schema and the `GlobalState` table). This ensures that multiple function instances starting simultaneously don't conflict when creating these shared resources. + +- **Per-table lock** (`_az_func_TT_{userTableId}`): Used for all runtime operations. Before each statement in a transaction, `sp_getapplock` gets an Exclusive lock on a resource scoped to the specific table being monitored. This ensures that for the duration of the transaction it is the only Azure Function that will be accessing the state tables for that particular user table, while allowing functions monitoring different tables to process changes in parallel. + +While this helps ensure concurrency safety for Azure Functions, other queries on the system can still cause a deadlock to occur. [This guide](https://learn.microsoft.com/sql/relational-databases/sql-server-deadlocks-guide) can help troubleshoot any issues that occur and provides some suggestions for fixing these issues. You may also utilize the sp_getapplock statement yourself with the `_az_func_TT_{userTableId}` resource (where `{userTableId}` is the SQL Server `OBJECT_ID` of the monitored table) to synchronize requests, although doing so may have a negative impact on the performance of your Functions. diff --git a/src/TriggerBinding/SqlTableChangeMonitor.cs b/src/TriggerBinding/SqlTableChangeMonitor.cs index 7295ca9b..8528ed6f 100644 --- a/src/TriggerBinding/SqlTableChangeMonitor.cs +++ b/src/TriggerBinding/SqlTableChangeMonitor.cs @@ -77,6 +77,11 @@ internal sealed class SqlTableChangeMonitor : IDisposable private readonly IDictionary _telemetryProps; + /// + /// Pre-computed T-SQL statements for acquiring a table-scoped application lock. + /// + private readonly string _tableScopedAppLockStatements; + /// /// Rows that are currently being processed /// @@ -129,6 +134,7 @@ public SqlTableChangeMonitor( this._userTableId = userTableId; this._telemetryProps = telemetryProps ?? new Dictionary(); + this._tableScopedAppLockStatements = GetTableScopedAppLockStatements(userTableId); // TODO: when we move to reading them exclusively from the host options, remove reading from settings.(https://github.com/Azure/azure-functions-sql-extension/issues/961) // Check if there's config settings to override the default max batch size/polling interval values @@ -789,7 +795,7 @@ private static SqlChangeOperation GetChangeOperation(IReadOnlyDictionary GetLeaseLockedOrMaxAttemptRowCountMessage(SqlConnecti // * NULL LeaseExpirationTime OR LeaseExpirationTime <= Current Time // * No attempts remaining (Attempt count = Max attempts) string getLeaseLockedOrMaxAttemptRowCountQuery = $@" - {AppLockStatements} + {this._tableScopedAppLockStatements} DECLARE @last_sync_version bigint; SELECT @last_sync_version = LastSyncVersion @@ -948,7 +954,7 @@ private SqlCommand BuildAcquireLeasesCommand(SqlConnection connection, SqlTransa const string rowDataParameter = "@rowData"; // Create the merge query that will either update the rows that already exist or insert a new one if it doesn't exist string query = $@" - {AppLockStatements} + {this._tableScopedAppLockStatements} WITH {acquireLeasesCte} AS ( SELECT * FROM OPENJSON(@rowData) WITH ({string.Join(",", cteColumnDefinitions)}) ) MERGE INTO {this._bracketedLeasesTableName} @@ -989,7 +995,7 @@ private SqlCommand BuildRenewLeasesCommand(SqlConnection connection, SqlTransact return null; } string renewLeasesQuery = $@" - {AppLockStatements} + {this._tableScopedAppLockStatements} UPDATE {this._bracketedLeasesTableName} SET {LeasesTableLeaseExpirationTimeColumnName} = DATEADD(second, {LeaseIntervalInSeconds}, SYSDATETIME()) @@ -1021,7 +1027,7 @@ private SqlCommand BuildReleaseLeasesCommand(SqlConnection connection, SqlTransa const string rowDataParameter = "@rowData"; string releaseLeasesQuery = -$@"{AppLockStatements} +$@"{this._tableScopedAppLockStatements} WITH {releaseLeasesCte} AS ( SELECT * FROM OPENJSON(@rowData) WITH ({string.Join(",", cteColumnDefinitions)}) ) UPDATE {this._bracketedLeasesTableName} @@ -1053,7 +1059,7 @@ private SqlCommand BuildUpdateTablesPostInvocation(SqlConnection connection, Sql string leasesTableJoinCondition = string.Join(" AND ", this._primaryKeyColumns.Select(col => $"c.{col.name.AsBracketQuotedString()} = l.{col.name.AsBracketQuotedString()}")); string updateTablesPostInvocationQuery = $@" - {AppLockStatements} + {this._tableScopedAppLockStatements} DECLARE @current_last_sync_version bigint; SELECT @current_last_sync_version = LastSyncVersion diff --git a/src/TriggerBinding/SqlTriggerConstants.cs b/src/TriggerBinding/SqlTriggerConstants.cs index 4da60a0e..719dbde1 100644 --- a/src/TriggerBinding/SqlTriggerConstants.cs +++ b/src/TriggerBinding/SqlTriggerConstants.cs @@ -38,12 +38,17 @@ internal static class SqlTriggerConstants public const string ConfigKey_SqlTrigger_MaxChangesPerWorker = "Sql_Trigger_MaxChangesPerWorker"; /// - /// The resource name to use for getting the application lock. We use the same resource name for all instances - /// of the function because there is some shared state across all the functions. + /// The resource name to use for getting the global application lock. This is used for DDL operations + /// during startup that touch shared objects (schema creation, GlobalState table creation). /// - /// A future improvement could be to make unique application locks for each FuncId/TableId combination so that functions - /// working on different tables aren't blocking each other - public const string AppLockResource = "_az_func_Trigger"; + public const string GlobalAppLockResource = "_az_func_Trigger"; + /// + /// The prefix for per-table scoped application locks. The full resource name is formed by appending + /// the user table ID, e.g. "_az_func_TT_12345". This allows functions monitoring different tables + /// to execute their transactions in parallel without blocking each other, while still preventing + /// deadlocks between functions (or scaled-out instances) that monitor the same table. + /// + public const string TableAppLockResourcePrefix = "_az_func_TT_"; /// /// Timeout for acquiring the application lock - 30sec chosen as a reasonable value to ensure we aren't /// hanging infinitely while also giving plenty of time for the blocking transaction to complete. @@ -51,33 +56,47 @@ internal static class SqlTriggerConstants public const int AppLockTimeoutMs = 30000; /// - /// T-SQL statements for getting an application lock. This is used to prevent deadlocks - primarily when multiple instances - /// of a function are running in parallel. - /// - /// The trigger heavily uses transactions to ensure atomic changes, that way if an error occurs during any step of a process we aren't left - /// with an incomplete state. Because of this, locks are placed on rows that are read/modified during the transaction, but the lock isn't - /// applied until the statement itself is executed. Some transactions have many statements executed in a row that touch a number of different - /// tables so it's very easy for two transactions to get in a deadlock depending on the speed they execute their statements and the order they - /// are processed in. - /// - /// So to avoid this we use application locks to ensure that anytime we enter a transaction we first guarantee that we're the only transaction - /// currently making any changes to the tables, which means that we're guaranteed not to have any deadlocks - albeit at the cost of speed. This - /// is acceptable for now, although further investigation could be done into using multiple resources to lock on (such as a different one for each - /// table) to increase the parallelization of the transactions. + /// T-SQL statements for getting the global application lock. This is used only for startup DDL operations + /// that create shared objects (the az_func schema and the GlobalState table). /// /// See the following articles for more information on locking in MSSQL /// https://learn.microsoft.com/sql/relational-databases/sql-server-transaction-locking-and-row-versioning-guide /// https://learn.microsoft.com/sql/t-sql/statements/set-transaction-isolation-level-transact-sql /// https://learn.microsoft.com/sql/relational-databases/system-stored-procedures/sp-getapplock-transact-sql /// - public static readonly string AppLockStatements = $@"DECLARE @result int; - EXEC @result = sp_getapplock @Resource = '{AppLockResource}', + public static readonly string GlobalAppLockStatements = $@"DECLARE @result int; + EXEC @result = sp_getapplock @Resource = '{GlobalAppLockResource}', + @LockMode = 'Exclusive', + @LockTimeout = {AppLockTimeoutMs} + IF @result < 0 + BEGIN + RAISERROR('Unable to acquire exclusive lock on {GlobalAppLockResource}. Result = %d', 16, 1, @result) + END;"; + + /// + /// Generates T-SQL statements for getting a per-table scoped application lock. This is used for all + /// runtime operations that only touch data specific to a single table (GlobalState row, leases table, + /// change tracking queries). By scoping the lock to the table level, functions monitoring different + /// tables can process changes in parallel without blocking each other. + /// + /// The lock is still needed at the table level to prevent deadlocks when multiple scaled-out instances + /// or multiple functions monitor the same table, since they share GlobalState rows and may access + /// overlapping change tracking data. + /// + /// The SQL Server object ID of the user table + /// T-SQL statements that acquire an exclusive app lock scoped to the given table + public static string GetTableScopedAppLockStatements(int userTableId) + { + string resource = $"{TableAppLockResourcePrefix}{userTableId}"; + return $@"DECLARE @result int; + EXEC @result = sp_getapplock @Resource = '{resource}', @LockMode = 'Exclusive', @LockTimeout = {AppLockTimeoutMs} IF @result < 0 BEGIN - RAISERROR('Unable to acquire exclusive lock on {AppLockResource}. Result = %d', 16, 1, @result) + RAISERROR('Unable to acquire exclusive lock on {resource}. Result = %d', 16, 1, @result) END;"; + } /// /// There is already an object named '%.*ls' in the database. diff --git a/src/TriggerBinding/SqlTriggerListener.cs b/src/TriggerBinding/SqlTriggerListener.cs index 52e376ad..c92bbb58 100644 --- a/src/TriggerBinding/SqlTriggerListener.cs +++ b/src/TriggerBinding/SqlTriggerListener.cs @@ -146,7 +146,7 @@ public async Task StartAsync(CancellationToken cancellationToken) createdSchemaDurationMs = await this.CreateSchemaAsync(connection, transaction, cancellationToken); createGlobalStateTableDurationMs = await this.CreateGlobalStateTableAsync(connection, transaction, cancellationToken); insertGlobalStateTableRowDurationMs = await this.InsertGlobalStateTableRowAsync(connection, transaction, userTableId, cancellationToken); - createLeasesTableDurationMs = await this.CreateLeasesTableAsync(connection, transaction, bracketedLeasesTableName, primaryKeyColumns, cancellationToken); + createLeasesTableDurationMs = await this.CreateLeasesTableAsync(connection, transaction, bracketedLeasesTableName, primaryKeyColumns, userTableId, cancellationToken); transaction.Commit(); } @@ -283,7 +283,7 @@ FROM sys.columns AS c private async Task CreateSchemaAsync(SqlConnection connection, SqlTransaction transaction, CancellationToken cancellationToken) { string createSchemaQuery = $@" - {AppLockStatements} + {GlobalAppLockStatements} IF SCHEMA_ID(N'{SchemaName}') IS NULL EXEC ('CREATE SCHEMA {SchemaName}'); @@ -328,7 +328,7 @@ IF SCHEMA_ID(N'{SchemaName}') IS NULL private async Task CreateGlobalStateTableAsync(SqlConnection connection, SqlTransaction transaction, CancellationToken cancellationToken) { string createGlobalStateTableQuery = $@" - {AppLockStatements} + {GlobalAppLockStatements} IF OBJECT_ID(N'{GlobalStateTableName}', 'U') IS NULL CREATE TABLE {GlobalStateTableName} ( @@ -400,8 +400,9 @@ private async Task InsertGlobalStateTableRowAsync(SqlConnection connection } } + string tableScopedAppLockStatements = GetTableScopedAppLockStatements(userTableId); string insertRowGlobalStateTableQuery = $@" - {AppLockStatements} + {tableScopedAppLockStatements} -- For back compatibility copy the lastSyncVersion from _hostIdFunctionId if it exists. IF NOT EXISTS ( SELECT * FROM {GlobalStateTableName} @@ -440,6 +441,7 @@ INSERT INTO {GlobalStateTableName} /// The transaction wrapping this command /// The name of the leases table to create /// The primary keys of the user table this leases table is for + /// The ID of the table being watched /// Cancellation token to pass to the command /// The time taken in ms to execute the command private async Task CreateLeasesTableAsync( @@ -447,6 +449,7 @@ private async Task CreateLeasesTableAsync( SqlTransaction transaction, string leasesTableName, IReadOnlyList<(string name, string type)> primaryKeyColumns, + int userTableId, CancellationToken cancellationToken) { string primaryKeysWithTypes = string.Join(", ", primaryKeyColumns.Select(col => $"{col.name.AsBracketQuotedString()} {col.type}")); @@ -455,8 +458,9 @@ private async Task CreateLeasesTableAsync( // We should only migrate the lease table from the old hostId based one to the newer WEBSITE_SITE_NAME one if // we're actually using the WEBSITE_SITE_NAME one (e.g. leasesTableName is different) bool shouldMigrateOldLeasesTable = !string.IsNullOrEmpty(oldLeasesTableName) && oldLeasesTableName != leasesTableName; + string tableScopedAppLockStatements = GetTableScopedAppLockStatements(userTableId); string createLeasesTableQuery = shouldMigrateOldLeasesTable ? $@" - {AppLockStatements} + {tableScopedAppLockStatements} IF OBJECT_ID(N'{leasesTableName}', 'U') IS NULL BEGIN @@ -479,7 +483,7 @@ INSERT INTO {leasesTableName} End " : $@" - {AppLockStatements} + {tableScopedAppLockStatements} IF OBJECT_ID(N'{leasesTableName}', 'U') IS NULL CREATE TABLE {leasesTableName} ( diff --git a/src/TriggerBinding/SqlTriggerMetricsProvider.cs b/src/TriggerBinding/SqlTriggerMetricsProvider.cs index 4411e31d..db4098c1 100644 --- a/src/TriggerBinding/SqlTriggerMetricsProvider.cs +++ b/src/TriggerBinding/SqlTriggerMetricsProvider.cs @@ -101,8 +101,9 @@ private SqlCommand BuildGetUnprocessedChangesCommand(SqlConnection connection, S { string leasesTableJoinCondition = string.Join(" AND ", primaryKeyColumns.Select(col => $"c.{col.name.AsBracketQuotedString()} = l.{col.name.AsBracketQuotedString()}")); string bracketedLeasesTableName = GetBracketedLeasesTableName(this._userDefinedLeasesTableName, this._userFunctionId, userTableId); + string tableScopedAppLockStatements = GetTableScopedAppLockStatements(userTableId); string getUnprocessedChangesQuery = $@" - {AppLockStatements} + {tableScopedAppLockStatements} DECLARE @last_sync_version bigint; SELECT @last_sync_version = LastSyncVersion diff --git a/test/Unit/TriggerBinding/SqlTriggerConstantsTests.cs b/test/Unit/TriggerBinding/SqlTriggerConstantsTests.cs new file mode 100644 index 00000000..f2229815 --- /dev/null +++ b/test/Unit/TriggerBinding/SqlTriggerConstantsTests.cs @@ -0,0 +1,91 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using Xunit; + +namespace Microsoft.Azure.WebJobs.Extensions.Sql.Tests.Unit +{ + public class SqlTriggerConstantsTests + { + /// + /// Verifies that the global app lock statements use the correct resource name and contain + /// the expected sp_getapplock call with Exclusive mode. + /// + [Fact] + public void GlobalAppLockStatements_ContainsCorrectResource() + { + string statements = SqlTriggerConstants.GlobalAppLockStatements; + Assert.Contains($"@Resource = '{SqlTriggerConstants.GlobalAppLockResource}'", statements); + Assert.Contains("@LockMode = 'Exclusive'", statements); + Assert.Contains($"@LockTimeout = {SqlTriggerConstants.AppLockTimeoutMs}", statements); + Assert.Contains("sp_getapplock", statements); + Assert.Contains("RAISERROR", statements); + } + + /// + /// Verifies that GetTableScopedAppLockStatements generates a lock resource name + /// that includes the table ID with the correct prefix. + /// + [Theory] + [InlineData(12345)] + [InlineData(0)] + [InlineData(-1)] + [InlineData(1845581613)] + [InlineData(int.MaxValue)] + [InlineData(int.MinValue)] + public void GetTableScopedAppLockStatements_ContainsTableIdInResource(int userTableId) + { + string statements = SqlTriggerConstants.GetTableScopedAppLockStatements(userTableId); + string expectedResource = $"{SqlTriggerConstants.TableAppLockResourcePrefix}{userTableId}"; + + Assert.Contains($"@Resource = '{expectedResource}'", statements); + Assert.Contains("@LockMode = 'Exclusive'", statements); + Assert.Contains($"@LockTimeout = {SqlTriggerConstants.AppLockTimeoutMs}", statements); + Assert.Contains("sp_getapplock", statements); + Assert.Contains($"Unable to acquire exclusive lock on {expectedResource}", statements); + } + + /// + /// Verifies that different table IDs produce different lock resource names, + /// ensuring functions monitoring different tables won't block each other. + /// + [Fact] + public void GetTableScopedAppLockStatements_DifferentTableIds_ProduceDifferentResources() + { + string statements1 = SqlTriggerConstants.GetTableScopedAppLockStatements(100); + string statements2 = SqlTriggerConstants.GetTableScopedAppLockStatements(200); + + Assert.NotEqual(statements1, statements2); + Assert.Contains("_az_func_TT_100", statements1); + Assert.Contains("_az_func_TT_200", statements2); + } + + /// + /// Verifies that the same table ID always produces identical lock statements (deterministic). + /// + [Fact] + public void GetTableScopedAppLockStatements_SameTableId_ProducesIdenticalOutput() + { + string statements1 = SqlTriggerConstants.GetTableScopedAppLockStatements(42); + string statements2 = SqlTriggerConstants.GetTableScopedAppLockStatements(42); + + Assert.Equal(statements1, statements2); + } + + /// + /// Verifies that table-scoped lock uses a different resource name than the global lock, + /// ensuring runtime operations don't interfere with startup DDL operations. + /// + [Fact] + public void GetTableScopedAppLockStatements_ResourceDiffersFromGlobalLock() + { + string tableScopedStatements = SqlTriggerConstants.GetTableScopedAppLockStatements(12345); + + // Table-scoped should NOT contain the global resource + Assert.DoesNotContain($"@Resource = '{SqlTriggerConstants.GlobalAppLockResource}'", tableScopedStatements); + + // Global should NOT contain the table-scoped prefix + Assert.DoesNotContain(SqlTriggerConstants.TableAppLockResourcePrefix, SqlTriggerConstants.GlobalAppLockStatements); + } + } +}