Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Add retries to queries run by ImportAzureCdnStatistics #763

Merged
merged 8 commits into from
Jun 4, 2019

Conversation

scottbommarito
Copy link

@scottbommarito scottbommarito commented Jun 3, 2019

I saw a lot of deadlettered stats blobs due to retriable SQL exceptions while I was on-call last week.

Specifically, I saw a lot of

Transaction (Process ID 192) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.

I looked at the code and saw that we had no retry logic around it at all. I looked at our other SQL calls in NuGet.Jobs and noticed that we typically use DapperExtensions to retry, but due to the nature of the SQL queries we are running here (bulk SQL copy and a structured parameter) it was not possible to use Dapper as well so I did some refactoring to allow us to share the retry logic between Dapper and non-Dapper queries.


transaction.Rollback();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transaction will be automatically rolled back when an exception is thrown at await bulkCopy.WriteToServerAsync(downloadFactsDataTable);...no need for this call.

@scottbommarito scottbommarito changed the title Add retries to StoreLogFileAggregatesAsync query Add retries to queries run by ImportAzureCdnStatistics Jun 3, 2019
Copy link
Contributor

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but please get sign off from someone that's familiar with this code base.

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but please get sign-off from someone more expert in SQL error codes and retriability.

@scottbommarito
Copy link
Author

Thanks for the reviews everyone! I will deploy this to DEV and make sure everything is okay and then merge.

@scottbommarito scottbommarito merged commit df119ae into dev Jun 4, 2019
@scottbommarito scottbommarito deleted the sb-importsqlretries branch June 4, 2019 21:06
joelverhagen added a commit that referenced this pull request Jul 31, 2020
Activate assembly modules and add more base configuration types to DI
Consider 4XX as successfully for telemetry purposes
Add KnownOperation for the V3 search endpoint
Progress on NuGet/Engineering#3020
joelverhagen added a commit that referenced this pull request Oct 26, 2020
Activate assembly modules and add more base configuration types to DI
Consider 4XX as successfully for telemetry purposes
Add KnownOperation for the V3 search endpoint
Progress on NuGet/Engineering#3020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants