Replace single global app lock with per-table scoped locks (#984)

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.

src/TriggerBinding/SqlTableChangeMonitor.cs

@@ -77,6 +77,11 @@ internal sealed class SqlTableChangeMonitor<T> : IDisposable
 
         private readonly IDictionary<TelemetryPropertyName, string> _telemetryProps;
 
+        /// <summary>
+        /// Pre-computed T-SQL statements for acquiring a table-scoped application lock.
+        /// </summary>
+        private readonly string _tableScopedAppLockStatements;
+
         /// <summary>
         /// Rows that are currently being processed
         /// </summary>
@@ -129,6 +134,7 @@ public SqlTableChangeMonitor(
 
             this._userTableId = userTableId;
             this._telemetryProps = telemetryProps ?? new Dictionary<TelemetryPropertyName, string>();
+            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<string,
         private SqlCommand BuildUpdateTablesPreInvocation(SqlConnection connection, SqlTransaction transaction)
         {
             string updateTablesPreInvocationQuery = $@"
-                {AppLockStatements}
+                {this._tableScopedAppLockStatements}
 
                 DECLARE @min_valid_version bigint;
                 SET @min_valid_version = CHANGE_TRACKING_MIN_VALID_VERSION({this._userTableId});
@@ -834,7 +840,7 @@ private SqlCommand BuildGetChangesCommand(SqlConnection connection, SqlTransacti
             // up regardless since we know it should be processed - no need to check the change version.
             // Once a row is successfully processed the LeaseExpirationTime column is set to NULL.
             string getChangesQuery = $@"
-                {AppLockStatements}
+                {this._tableScopedAppLockStatements}
 
                 DECLARE @last_sync_version bigint;
                 SELECT @last_sync_version = LastSyncVersion
@@ -882,7 +888,7 @@ private async Task<string> 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

src/TriggerBinding/SqlTriggerConstants.cs

@@ -38,46 +38,65 @@ internal static class SqlTriggerConstants
         public const string ConfigKey_SqlTrigger_MaxChangesPerWorker = "Sql_Trigger_MaxChangesPerWorker";
 
         /// <summary>
-        /// 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).
         /// </summary>
-        /// <remarks>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</remarks>
-        public const string AppLockResource = "_az_func_Trigger";
+        public const string GlobalAppLockResource = "_az_func_Trigger";
+        /// <summary>
+        /// 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.
+        /// </summary>
+        public const string TableAppLockResourcePrefix = "_az_func_TT_";
         /// <summary>
         /// 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.
         /// </summary>
         public const int AppLockTimeoutMs = 30000;
 
         /// <summary>
-        /// 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
         /// </summary>
-        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;";
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        /// <param name="userTableId">The SQL Server object ID of the user table</param>
+        /// <returns>T-SQL statements that acquire an exclusive app lock scoped to the given table</returns>
+        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;";
+        }
 
         /// <summary>
         /// There is already an object named '%.*ls' in the database.

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<long> 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<long> 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<long> 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,13 +441,15 @@ INSERT INTO {GlobalStateTableName}
         /// <param name="transaction">The transaction wrapping this command</param>
         /// <param name="leasesTableName">The name of the leases table to create</param>
         /// <param name="primaryKeyColumns">The primary keys of the user table this leases table is for</param>
+        /// <param name="userTableId">The ID of the table being watched</param>
         /// <param name="cancellationToken">Cancellation token to pass to the command</param>
         /// <returns>The time taken in ms to execute the command</returns>
         private async Task<long> CreateLeasesTableAsync(
             SqlConnection connection,
             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<long> 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} (

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