Make App Lock timeout configurable (#982) (#1213)

* Make App Lock timeout configurable (#982)

Add configurable AppLockTimeoutMs setting for the SQL trigger binding's
application lock timeout. Previously hardcoded to 30 seconds, the timeout
can now be configured via:
- Sql_Trigger_AppLockTimeoutMs app setting
- AppLockTimeoutMs property in host.json SqlOptions

The default remains 30000ms (30 seconds) with a minimum of 1000ms (1 second).

Changes:
- Convert static AppLockStatements field to GetAppLockStatements(int) method
- Add AppLockTimeoutMs property to SqlOptions with validation
- Read config from app settings with SqlOptions fallback in SqlTableChangeMonitor,
  SqlTriggerListener, and SqlScalerProvider
- Pass appLockTimeoutMs through ScaleMonitor/TargetScaler/MetricsProvider chain
- Add telemetry for HasConfiguredAppLockTimeout and AppLockTimeoutMs
- Add unit tests for new config and GetAppLockStatements method
- Update documentation in BindingsOverview.md and TriggerBinding.md

Closes #982

* Address review feedback: rename MinimumAppLockTimeoutMs and add validation

- Rename DefaultMinimumAppLockTimeoutMs to MinimumAppLockTimeoutMs (public const)
  to clarify it represents the minimum accepted value, not a default
- Add min-value validation in SqlScalerProvider for app-settings value
- Tighten validation in SqlTableChangeMonitor and SqlTriggerListener to
  check against MinimumAppLockTimeoutMs instead of just > 0

Co-authored-by: Copilot <[email protected]>

* Address PR #1213 review comments

- SqlScalerProvider: Use nullable int for AppLockTimeoutMs config to
  distinguish 'not configured' from 'explicitly set to 0'
- SqlTriggerListener: Add HasConfiguredAppLockTimeout property and
  AppLockTimeoutMs measure to StartListener telemetry event
- SqlTriggerConstants: Merge duplicate XML summary doc comments on
  GetAppLockStatements method
- SqlOptionsTests: Add test for negative AppLockTimeoutMs values

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: george.crocker <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Charles Gagnon <[email protected]>

Author: 4 people

docs/BindingsOverview.md

@@ -20,6 +20,7 @@
       - [Sql\_Trigger\_MaxBatchSize](#sql_trigger_maxbatchsize)
       - [Sql\_Trigger\_PollingIntervalMs](#sql_trigger_pollingintervalms)
       - [Sql\_Trigger\_MaxChangesPerWorker](#sql_trigger_maxchangesperworker)
+      - [Sql\_Trigger\_AppLockTimeoutMs](#sql_trigger_applocktimeoutms)
       - [WEBSITE\_SITE\_NAME](#website_site_name)
     - [Scaling for Trigger Bindings](#scaling-for-trigger-bindings)
     - [Retry support for Trigger Bindings](#retry-support-for-trigger-bindings)
@@ -153,6 +154,10 @@ The delay in milliseconds between processing each batch of changes.
 
 The upper limit on the number of pending changes in the user table that are allowed per application-worker. If the count of changes exceeds this limit, it may result in a scale out. The setting only applies for Azure Function Apps with runtime driven scaling enabled. See the [Scaling](#scaling-for-trigger-bindings) section for more information.
 
+#### Sql_Trigger_AppLockTimeoutMs
+
+The timeout in milliseconds for acquiring the application lock used to prevent deadlocks when processing changes. The default value is 30000 (30 seconds). The minimum allowed value is 1000 (1 second).
+
 #### WEBSITE_SITE_NAME
 
 If this setting exists, it will be used to generate a unique identifier for the function that is used for tracking function state. If not specified, this unique identifier will be generated from the [IHostIdProvider.GetHostIdAsync](https://github.com/Azure/azure-webjobs-sdk/blob/dev/src/Microsoft.Azure.WebJobs.Host/Executors/IHostIdProvider.cs#L14).

docs/TriggerBinding.md

@@ -119,6 +119,6 @@ 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 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. The timeout for acquiring this lock defaults to 30 seconds and can be configured using the `Sql_Trigger_AppLockTimeoutMs` [application setting](./BindingsOverview.md#sql_trigger_applocktimeoutms) or the `AppLockTimeoutMs` host.json option.
 
 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.

src/Common/SqlOptions.cs

@@ -20,6 +20,8 @@ public class SqlOptions : IOptionsFormatter
         public const int DefaultPollingIntervalMs = 1000;
         private const int DefaultMinimumPollingIntervalMs = 100;
         public const int DefaultMaxChangesPerWorker = 1000;
+        public const int DefaultAppLockTimeoutMs = 30000;
+        public const int MinimumAppLockTimeoutMs = 1000;
         /// <summary>
         /// Maximum number of changes to process in each iteration of the loop
         /// </summary>
@@ -30,6 +32,7 @@ public class SqlOptions : IOptionsFormatter
         private int _pollingIntervalMs = DefaultPollingIntervalMs;
         private readonly int _minPollingInterval = DefaultMinimumPollingIntervalMs;
         private int _maxChangesPerWorker = DefaultMaxChangesPerWorker;
+        private int _appLockTimeoutMs = DefaultAppLockTimeoutMs;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="SqlOptions"/> class.
@@ -97,6 +100,27 @@ public int MaxChangesPerWorker
             }
         }
 
+        /// <summary>
+        /// Gets or sets the timeout in milliseconds for acquiring the application lock.
+        /// The default is 30000 (30 seconds).
+        /// </summary>
+        public int AppLockTimeoutMs
+        {
+            get => this._appLockTimeoutMs;
+
+            set
+            {
+                if (value < MinimumAppLockTimeoutMs)
+                {
+                    string message = string.Format(System.Globalization.CultureInfo.CurrentCulture,
+                        "AppLockTimeoutMs must not be less than {0}Ms.", MinimumAppLockTimeoutMs);
+                    throw new ArgumentException(message, nameof(value));
+                }
+
+                this._appLockTimeoutMs = value;
+            }
+        }
+
         /// <inheritdoc/>
         [EditorBrowsable(EditorBrowsableState.Never)]
         string IOptionsFormatter.Format()
@@ -105,7 +129,8 @@ string IOptionsFormatter.Format()
             {
                 { nameof(this.MaxBatchSize), this.MaxBatchSize },
                 { nameof(this.PollingIntervalMs), this.PollingIntervalMs },
-                { nameof(this.MaxChangesPerWorker), this.MaxChangesPerWorker }
+                { nameof(this.MaxChangesPerWorker), this.MaxChangesPerWorker },
+                { nameof(this.AppLockTimeoutMs), this.AppLockTimeoutMs }
             };
 
             return options.ToString(Formatting.Indented);
@@ -117,7 +142,8 @@ internal SqlOptions Clone()
             {
                 _maxBatchSize = this._maxBatchSize,
                 _pollingIntervalMs = this._pollingIntervalMs,
-                _maxChangesPerWorker = this._maxChangesPerWorker
+                _maxChangesPerWorker = this._maxChangesPerWorker,
+                _appLockTimeoutMs = this._appLockTimeoutMs
             };
             return copy;
         }

src/Telemetry/Telemetry.cs

@@ -365,6 +365,7 @@ public enum TelemetryPropertyName
         HasConfiguredMaxBatchSize,
         HasConfiguredMaxChangesPerWorker,
         HasConfiguredPollingInterval,
+        HasConfiguredAppLockTimeout,
         LeasesTableName,
         QueryType,
         ScaleRecommendation,
@@ -384,6 +385,7 @@ public enum TelemetryPropertyName
     public enum TelemetryMeasureName
     {
         AcquireLeasesDurationMs,
+        AppLockTimeoutMs,
         BatchCount,
         BatchSize,
         CommandDurationMs,

src/TriggerBinding/SqlScalerProvider.cs

@@ -38,11 +38,18 @@ public SqlScalerProvider(IServiceProvider serviceProvider, TriggerMetadata trigg
             int configAppSettingsMaxChangesPerWorker = config.GetValue<int>(SqlTriggerConstants.ConfigKey_SqlTrigger_MaxChangesPerWorker);
             // Override the maxChangesPerWorker value from config if the value is set in the trigger appsettings
             int maxChangesPerWorker = configAppSettingsMaxChangesPerWorker != 0 ? configAppSettingsMaxChangesPerWorker : configOptionsMaxChangesPerWorker != 0 ? configOptionsMaxChangesPerWorker : SqlOptions.DefaultMaxChangesPerWorker;
+            int configOptionsAppLockTimeoutMs = options.Value.AppLockTimeoutMs;
+            int? configAppSettingsAppLockTimeoutMs = config.GetValue<int?>(SqlTriggerConstants.ConfigKey_SqlTrigger_AppLockTimeoutMs);
+            int appLockTimeoutMs = configAppSettingsAppLockTimeoutMs ?? (configOptionsAppLockTimeoutMs != 0 ? configOptionsAppLockTimeoutMs : SqlOptions.DefaultAppLockTimeoutMs);
+            if (appLockTimeoutMs < SqlOptions.MinimumAppLockTimeoutMs)
+            {
+                throw new InvalidOperationException($"Invalid value for configuration setting '{SqlTriggerConstants.ConfigKey_SqlTrigger_AppLockTimeoutMs}'. Value must not be less than {SqlOptions.MinimumAppLockTimeoutMs}ms.");
+            }
             string userDefinedLeasesTableName = sqlMetadata.LeasesTableName;
             string userFunctionId = sqlMetadata.UserFunctionId;
 
-            this._scaleMonitor = new SqlTriggerScaleMonitor(userFunctionId, userTable, userDefinedLeasesTableName, connectionString, maxChangesPerWorker, logger);
-            this._targetScaler = new SqlTriggerTargetScaler(userFunctionId, userTable, userDefinedLeasesTableName, connectionString, maxChangesPerWorker, logger);
+            this._scaleMonitor = new SqlTriggerScaleMonitor(userFunctionId, userTable, userDefinedLeasesTableName, connectionString, maxChangesPerWorker, appLockTimeoutMs, logger);
+            this._targetScaler = new SqlTriggerTargetScaler(userFunctionId, userTable, userDefinedLeasesTableName, connectionString, maxChangesPerWorker, appLockTimeoutMs, logger);
         }
 
         public IScaleMonitor GetMonitor()

src/TriggerBinding/SqlTableChangeMonitor.cs

@@ -65,6 +65,7 @@ internal sealed class SqlTableChangeMonitor<T> : IDisposable
         /// Delay in ms between processing each batch of changes
         /// </summary>
         private readonly int _pollingIntervalInMs;
+        private readonly string _appLockStatements;
         private readonly CancellationTokenSource _cancellationTokenSourceCheckForChanges = new CancellationTokenSource();
         private readonly CancellationTokenSource _cancellationTokenSourceRenewLeases = new CancellationTokenSource();
         private CancellationTokenSource _cancellationTokenSourceExecutor = new CancellationTokenSource();
@@ -144,15 +145,24 @@ public SqlTableChangeMonitor(
             {
                 throw new InvalidOperationException($"Invalid value for configuration setting '{ConfigKey_SqlTrigger_PollingInterval}'. Ensure that the value is a positive integer.");
             }
+            int? configuredAppLockTimeout = configuration.GetValue<int?>(ConfigKey_SqlTrigger_AppLockTimeoutMs);
+            int appLockTimeoutMs = configuredAppLockTimeout ?? this._sqlOptions.AppLockTimeoutMs;
+            if (appLockTimeoutMs < SqlOptions.MinimumAppLockTimeoutMs)
+            {
+                throw new InvalidOperationException($"Invalid value for configuration setting '{ConfigKey_SqlTrigger_AppLockTimeoutMs}'. Value must not be less than {SqlOptions.MinimumAppLockTimeoutMs}ms.");
+            }
+            this._appLockStatements = GetAppLockStatements(appLockTimeoutMs);
             TelemetryInstance.TrackEvent(
                 TelemetryEventName.TriggerMonitorStart,
                 new Dictionary<TelemetryPropertyName, string>(telemetryProps) {
                         { TelemetryPropertyName.HasConfiguredMaxBatchSize, (configuredMaxBatchSize != null).ToString() },
                         { TelemetryPropertyName.HasConfiguredPollingInterval, (configuredPollingInterval != null).ToString() },
+                        { TelemetryPropertyName.HasConfiguredAppLockTimeout, (configuredAppLockTimeout != null).ToString() },
                 },
                 new Dictionary<TelemetryMeasureName, double>() {
                     { TelemetryMeasureName.MaxBatchSize, this._maxBatchSize },
-                    { TelemetryMeasureName.PollingIntervalMs, this._pollingIntervalInMs }
+                    { TelemetryMeasureName.PollingIntervalMs, this._pollingIntervalInMs },
+                    { TelemetryMeasureName.AppLockTimeoutMs, appLockTimeoutMs }
                 }
             );
 
@@ -789,7 +799,7 @@ private static SqlChangeOperation GetChangeOperation(IReadOnlyDictionary<string,
         private SqlCommand BuildUpdateTablesPreInvocation(SqlConnection connection, SqlTransaction transaction)
         {
             string updateTablesPreInvocationQuery = $@"
-                {AppLockStatements}
+                {this._appLockStatements}
 
                 DECLARE @min_valid_version bigint;
                 SET @min_valid_version = CHANGE_TRACKING_MIN_VALID_VERSION({this._userTableId});
@@ -834,7 +844,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._appLockStatements}
 
                 DECLARE @last_sync_version bigint;
                 SELECT @last_sync_version = LastSyncVersion
@@ -882,7 +892,7 @@ private async Task<string> GetLeaseLockedOrMaxAttemptRowCountMessage(SqlConnecti
             //  * NULL LeaseExpirationTime OR LeaseExpirationTime <= Current Time
             //  * No attempts remaining (Attempt count = Max attempts)
             string getLeaseLockedOrMaxAttemptRowCountQuery = $@"
-                {AppLockStatements}
+                {this._appLockStatements}
 
                 DECLARE @last_sync_version bigint;
                 SELECT @last_sync_version = LastSyncVersion
@@ -948,7 +958,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._appLockStatements}
 
                     WITH {acquireLeasesCte} AS ( SELECT * FROM OPENJSON(@rowData) WITH ({string.Join(",", cteColumnDefinitions)}) )
                     MERGE INTO {this._bracketedLeasesTableName}
@@ -989,7 +999,7 @@ private SqlCommand BuildRenewLeasesCommand(SqlConnection connection, SqlTransact
                 return null;
             }
             string renewLeasesQuery = $@"
-                {AppLockStatements}
+                {this._appLockStatements}
 
                 UPDATE {this._bracketedLeasesTableName}
                 SET {LeasesTableLeaseExpirationTimeColumnName} = DATEADD(second, {LeaseIntervalInSeconds}, SYSDATETIME())
@@ -1021,7 +1031,7 @@ private SqlCommand BuildReleaseLeasesCommand(SqlConnection connection, SqlTransa
             const string rowDataParameter = "@rowData";
 
             string releaseLeasesQuery =
-$@"{AppLockStatements}
+$@"{this._appLockStatements}
 
 WITH {releaseLeasesCte} AS ( SELECT * FROM OPENJSON(@rowData) WITH ({string.Join(",", cteColumnDefinitions)}) )
 UPDATE {this._bracketedLeasesTableName}
@@ -1053,7 +1063,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._appLockStatements}
 
                 DECLARE @current_last_sync_version bigint;
                 SELECT @current_last_sync_version = LastSyncVersion

src/TriggerBinding/SqlTriggerConstants.cs

@@ -36,6 +36,7 @@ internal static class SqlTriggerConstants
         public const string ConfigKey_SqlTrigger_MaxBatchSize = "Sql_Trigger_MaxBatchSize";
         public const string ConfigKey_SqlTrigger_PollingInterval = "Sql_Trigger_PollingIntervalMs";
         public const string ConfigKey_SqlTrigger_MaxChangesPerWorker = "Sql_Trigger_MaxChangesPerWorker";
+        public const string ConfigKey_SqlTrigger_AppLockTimeoutMs = "Sql_Trigger_AppLockTimeoutMs";
 
         /// <summary>
         /// The resource name to use for getting the application lock. We use the same resource name for all instances
@@ -45,13 +46,15 @@ internal static class SqlTriggerConstants
         /// working on different tables aren't blocking each other</remarks>
         public const string AppLockResource = "_az_func_Trigger";
         /// <summary>
-        /// Timeout for acquiring the application lock - 30sec chosen as a reasonable value to ensure we aren't
+        /// Default 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;
+        public const int DefaultAppLockTimeoutMs = 30000;
 
         /// <summary>
-        /// T-SQL statements for getting an application lock. This is used to prevent deadlocks - primarily when multiple instances
+        /// Generates T-SQL statements for getting an application lock with the specified timeout.
+        ///
+        /// 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
@@ -70,14 +73,19 @@ internal static class SqlTriggerConstants
         /// 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;
+        /// <param name="appLockTimeoutMs">Timeout in milliseconds for acquiring the application lock</param>
+        /// <returns>T-SQL statements for acquiring the application lock</returns>
+        public static string GetAppLockStatements(int appLockTimeoutMs)
+        {
+            return $@"DECLARE @result int;
                 EXEC @result = sp_getapplock @Resource = '{AppLockResource}',
                             @LockMode = 'Exclusive',
-                            @LockTimeout = {AppLockTimeoutMs}
+                            @LockTimeout = {appLockTimeoutMs}
                 IF @result < 0
                 BEGIN
                     RAISERROR('Unable to acquire exclusive lock on {AppLockResource}. Result = %d', 16, 1, @result)
                 END;";
+        }
 
         /// <summary>
         /// There is already an object named '%.*ls' in the database.