-
Notifications
You must be signed in to change notification settings - Fork 64
Add additional trigger debug logging #1103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -394,7 +394,6 @@ public enum TelemetryMeasureName | |||
GetColumnDefinitionsDurationMs, | |||
GetPrimaryKeysDurationMs, | |||
GetUnprocessedChangesDurationMs, | |||
GetLockedRowCountDurationMs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The duration wasn't being calculated correctly so I just removed to keep things simpler. We haven't really used these durations for anything useful so far anyways - we can always add it back in if needed.
{AppLockStatements} | ||
|
||
DECLARE @last_sync_version bigint; | ||
SELECT @last_sync_version = LastSyncVersion | ||
FROM {GlobalStateTableName} | ||
WHERE UserFunctionID = '{this._userFunctionId}' AND UserTableID = {this._userTableId}; | ||
|
||
SELECT COUNT(*) | ||
DECLARE @lease_locked_count int; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored this to just create the message in the query itself. I could have returned a table with the numeric values, but having to read the data table and do all that extra stuff didn't seem worth it when we can just do it directly in the T-SQL.
{ | ||
var commandSw = Stopwatch.StartNew(); | ||
leaseLockedRowsCount = (int)await getLeaseLockedRowCountCommand.ExecuteScalarAsyncWithLogging(this._logger, CancellationToken.None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note I fixed an issue here with not passing in the cancellation token. We should be doing that for all queries we execute so that if the function execution is cancelled we'll properly cancel the queries we're running right away.
* Add additional debug logging * Add max attempts to lease locked check * log when match condition is null --------- Co-authored-by: MaddyDev <[email protected]>
This will help us and users diagnose issues with the trigger function - especially for being able to track when the change version we're looking at changes so we can see what change version the function is looking at for a given point in time.
New messages:
[PreInvocation] Updated LastSyncVersion from 0 to 121
[PostInvocation] Updated LastSyncVersion from 153 to 155 MaxAttemptRowsToBeDeleted=1
Renewed leases for 1 rows
Clearing internal state for 1 rows
Updated messages:
Executed GetChangesCommand in GetTableChangesAsync. 0 available changed rows. (0 found with lease locks and 1 ignored because they've reached the max attempt limit)