Only take app lock out at beginning of transaction (#983)

[thecrockster]

Move sp_getapplock from being prepended to every SQL query to being called once at the beginning of each transaction via a new AcquireAppLockAsync helper method in SqlTriggerUtils.

Since the app lock is transaction-scoped, subsequent acquisitions within the same transaction were no-ops that only added noise to query logs. This reduces redundant sp_getapplock calls from ~12 per cycle to 5 (one per transaction).

Changes:

• Add AcquireAppLockAsync static method to SqlTriggerUtils
• Remove {AppLockStatements} from all 13 individual query strings
• Call AcquireAppLockAsync once after BeginTransaction in all 5 transaction sites across SqlTableChangeMonitor, SqlTriggerListener, and SqlTriggerMetricsProvider

Closes #983

Description

Please provide a detailed description. Be as descriptive as possible - include information about what is being changed,
why it's being changed, and any links to relevant issues. If this is closing an existing issue use one of the issue linking keywords to link the issue to this PR and have it automatically close when completed.

In addition, go through the checklist below and check each item as you validate it is either handled or not applicable to this change.

Code Changes

• Unit tests are added, if possible
• Integration tests are added if the change is modifying existing behavior of one or more of the bindings
• New or changed code follows the C# style guidelines defined in .editorconfig
• All changes MUST be backwards compatible and changes to the shared az_func.GlobalState table must be compatible with all prior versions of the extension
• Use the ILogger instance to log relevant information, especially information useful for debugging or troubleshooting
• Use async and await for all long-running operations
• Ensure proper usage and propagation of CancellationToken
• T-SQL is safe from SQL Injection attacks through the use of SqlParameters and proper escaping/sanitization of input

Dependencies

• If updating dependencies, run dotnet restore --force-evaluate to update the lock files and ensure that there are NO major versions updates in either src/packages.lock.json or Worker.Extensions.Sql/src/packages.lock.json. If there are, contact the dev team for instructions.

Documentation

• Add samples if the change is modifying or adding functionality
• Update relevant documentation in the docs

[chlafreniere]

Thanks for this and the other PRs, @thecrockster! We'll review them next week.

[Copilot]

Pull request overview

Refactors SQL trigger applock acquisition so sp_getapplock is executed once per transaction (instead of being prepended to each individual query), reducing redundant calls and log noise while preserving the existing transaction-level synchronization strategy.

Changes:

• Added SqlTriggerUtils.AcquireAppLockAsync to execute AppLockStatements once at transaction start.
• Removed {AppLockStatements} from the SQL text of multiple queries across trigger components.
• Inserted AcquireAppLockAsync calls immediately after BeginTransaction(...) at the transaction entry points in listener/monitor/metrics code.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/TriggerBinding/SqlTriggerUtils.cs	Adds helper to acquire applock via a dedicated command.
src/TriggerBinding/SqlTriggerMetricsProvider.cs	Acquires applock once per metrics transaction; removes inline applock SQL from query text.
src/TriggerBinding/SqlTriggerListener.cs	Acquires applock once for the startup schema/state initialization transaction; removes inline applock SQL from DDL/DML statements.
src/TriggerBinding/SqlTableChangeMonitor.cs	Acquires applock once per transactional phase (get/renew/release) and removes inline applock SQL from internal queries.

Comments suppressed due to low confidence (1)

src/TriggerBinding/SqlTriggerListener.cs:153

• This transaction block now acquires the app lock once up-front, but it still relies on implicit rollback via SqlTransaction disposal if anything throws before Commit(). To align with the other transaction sites (and the #983 concern about ensuring rollbacks on error), consider wrapping the transaction body in a try/catch that explicitly rolls back and logs/telemeters rollback failures.

                    using (SqlTransaction transaction = connection.BeginTransaction(System.Data.IsolationLevel.RepeatableRead))
                    {
                        await AcquireAppLockAsync(connection, transaction, this._logger, 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);
                        transaction.Commit();
                    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.