[PM-38324] Optimize DeleteSendsJob

[harr1424]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-38324

📔 Objective

During resolution of bitwarden/clients#16965 DbOps suggested a few improvements to the DeleteSendsJob.

• Chunked fetch — add to Send_ReadByDeletionDateBefore and loop in the job so one run can't  pull an unbounded backlog into memory + a tight delete loop.

• Set-based Send_DeleteMany — delete the batch in one statement and bump each affected user once (collapsing  N bumps  ~distinct-user count), instead of N separate Send_DeleteById calls.

• Optionally throttle between batches so a backlog drain spreads out rather than saturating the Hyperscale log.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the chunked DeleteSendsJob loop and its three stop conditions (empty batch, zero-progress/cumulative-skip guard, short batch), the new DeleteManySendsAsync batch command and its blob-failure skip semantics, and both ORM DeleteManyAsync implementations for delete/revision-bump atomicity and post-commit storage recompute. Verified the Send_DeleteMany and Send_ReadByDeletionDateBefore procedures against the dated migration and the SSDT sources, confirmed the @BatchSize default keeps the read procedure backward compatible during a rolling deploy, and confirmed IX_Send_DeletionDate supports the new TOP (@BatchSize) ... ORDER BY [DeletionDate] plan. Checked that every GetManyByDeletionDateAsync caller and both ISendRepository / INonAnonymousSendCommand implementations were updated, and read the new Admin, Core, and integration tests.

Findings from earlier review rounds were re-verified at ea0f7f8c7 and are resolved; no new findings this round.

[codecov]

Codecov Report

❌ Patch coverage is 93.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.40%. Comparing base (25f1f9c) to head (ea0f7f8).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...ucture.Dapper/Tools/Repositories/SendRepository.cs	82.60%	4 Missing ⚠️
...tityFramework/Tools/Repositories/SendRepository.cs	86.66%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8282      +/-   ##
==========================================
+ Coverage   63.66%   69.40%   +5.73%     
==========================================
  Files        2430     2467      +37     
  Lines      104962   105801     +839     
  Branches     9506     9560      +54     
==========================================
+ Hits        66822    73429    +6607     
+ Misses      35844    29936    -5908     
- Partials     2296     2436     +140     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mkincaid-bw]

❓If skippedIds reaches the batch size, does that mean that new expired Sends will not get deleted until someone intervenes to handle the stuck Sends? If that happens, do we have something monitoring this?

[harr1424]

If skippedIds reaches the batch size, does that mean that new expired Sends will not get deleted until someone intervenes to handle the stuck Sends?

Yes but keep in mind currently a single bad Send can cause this situation, and this happened earlier this year in our US Prod environment. This PR will improve resilience up until the batch size is met, this PR proposes 2,000 Sends per batch.

If that happens, do we have something monitoring this?

The job will log at a warning level. Can you propose a strategy more likely to raise an alarm? I am not certain how or whom is monitoring our logs at given levels. Would an error level log be more appropriate?