Extract DeleteCrateFromStorage background job
#9549
Conversation
This background job can later be reused to implement https://rust-lang.github.io/rfcs/3660-crates-io-crate-deletions.html
This ensures that the background job will be retried if a deletion was unsuccessful.
Turbo87
added
C-internal 🔧
| try_join!( | ||
| async { | ||
| info!("{name}: Deleting crate files from S3…"); | ||
| let result = ctx.storage.delete_all_crate_files(name).await; | ||
| result.context("Failed to delete crate files from S3") | ||
| }, | ||
| async { | ||
| info!("{name}: Deleting readme files from S3…"); | ||
| let result = ctx.storage.delete_all_readmes(name).await; | ||
| result.context("Failed to delete readme files from S3") | ||
| }, | ||
| async { | ||
| info!("{name}: Deleting RSS feed from S3…"); | ||
| let feed_id = FeedId::Crate { name }; | ||
| let result = ctx.storage.delete_feed(&feed_id).await; | ||
| result.context("Failed to delete RSS feed from S3") | ||
| } | ||
| )?; |
There was a problem hiding this comment.
is it sufficient for this to be in one job or should it be split into multiple jobs that can be retried individually if they fail? 🤔
There was a problem hiding this comment.
Yeah, what would happen if one of these failed?
There was a problem hiding this comment.
try_join!() cancels the other runnings tasks if one of them fails. it then returns the error, which will make the job fail and will cause it to be retried a little later. AFAIU all of the steps in this job are idempotent though, so retrying isn't a big deal. the job does not perform CDN invalidations yet though.
There was a problem hiding this comment.
Let's say task 1 succeeds first, but task 2 fails, so it cancels task 3. What will the state of task 1 be after all? If it remains done, is it safe to retry?
There was a problem hiding this comment.
I think if every job could be retried even if it's already completed, this wouldn't be a problem.
There was a problem hiding this comment.
If it remains done, is it safe to retry?
yes, as far as I understand this shouldn't be an issue. I guess splitting it up is probably unnecessary over-engineering 😅
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9549 +/- ##
==========================================
- Coverage 89.11% 89.07% -0.04%
==========================================
Files 285 286 +1
Lines 28920 28935 +15
==========================================
+ Hits 25773 25775 +2
- Misses 3147 3160 +13 ☔ View full report in Codecov by Sentry. |
This background job can later be reused to implement https://rust-lang.github.io/rfcs/3660-crates-io-crate-deletions.html
Related: