Skip to content
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

Retry background jobs #1353

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Retry background jobs #1353

merged 1 commit into from
Jul 22, 2024

Conversation

kartiki975
Copy link
Contributor

@kartiki975 kartiki975 commented Jul 19, 2024

TLDR

This closes an internal Shopify ticket that suspects that CreateReleaseStatusesJob was failing in creating release statuses due to a potential Gtihub API error response. In actuality, bugsnag errors are related to ConcurrentJobError. Some other open errors are linked to a similar issue in CreateOnGithubJob, primarily involving temporary spikes and some 504 response errors.

In this PR, I decided to drop duplicate background jobs for CreateReleaseStatusesJob, and retry on server errors from Github if that is indeed the case.

Further thought process

These were some recommendations I made initially:
Option 1: Implement a retry mechanism for both CreateOnGithubJob and CreateReleaseStatusesJob upon receiving a 504 response.

Option 2: Schedule a periodic job to address ReleaseStatuses and CommitDeploymentStatuses with a nil github_id, ensuring that race conditions do not occur with other concurrently initiated jobs.

Option 1 seems like a good start pointing b/c it targets server errors on Github's end (including the 504 response, Octokit::GatewayTimeout) and is lower effort than Option 2; I initially attempted to target 504 only along with the existing errors but that isn't possible b/c such subclass doesn't exist. If this solution fails, Option 2 or another alternate solution can be tried in a follow up PR.

Request for reviewers

Are you against this code change? Would you recommend another solution?

@kartiki975 kartiki975 force-pushed the retry-background-jobs branch from 1421040 to 2f60a35 Compare July 19, 2024 17:14
@kartiki975 kartiki975 requested a review from a team July 19, 2024 17:17
@kartiki975 kartiki975 marked this pull request as ready for review July 19, 2024 17:18
@kartiki975 kartiki975 self-assigned this Jul 19, 2024
@kartiki975 kartiki975 changed the title Retry background jobs [WIP --- DO NOT REVIEW] Retry background jobs Jul 19, 2024
@kartiki975 kartiki975 changed the title [WIP --- DO NOT REVIEW] Retry background jobs Retry background jobs Jul 19, 2024
Update create_release_statuses_job.rb
@kartiki975 kartiki975 force-pushed the retry-background-jobs branch from 80edc0c to c4b5f7b Compare July 22, 2024 14:51
@kartiki975 kartiki975 merged commit 8448ab9 into main Jul 22, 2024
17 checks passed
@kartiki975 kartiki975 deleted the retry-background-jobs branch July 22, 2024 14:54
@kartiki975 kartiki975 restored the retry-background-jobs branch July 22, 2024 14:54
@kartiki975 kartiki975 deleted the retry-background-jobs branch July 22, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants