Retry scanner webhook deliveries instead of a fixed grace period - #25377
Retry scanner webhook deliveries instead of a fixed grace period#25377willdurand wants to merge 1 commit into
Conversation
2902e34 to
c8750ab
Compare
|
I initially adjusted the statsd event names but it became too complex, so I reverted that. |
c8750ab to
7cbf573
Compare
|
(rebase) |
|
needs rebase because #25383 stole migration 0089 in scanners |
7cbf573 to
bd2bb21
Compare
|
Heh, of course, sorry. Rebased! |
diox
left a comment
diox
left a comment
There was a problem hiding this comment.
We discussed this and don't like the fact that auto_approve is trigger a retry. The plan is going to try to use celery retry for that.
|
|
||
| # Each attempt doubles the delay since the previous one: 1h, 2h, 4h | ||
| # then 8h by default, i.e. 15h in total before we give up. | ||
| due_date = scanner_result.modified + timedelta( |
There was a problem hiding this comment.
Let's avoid calling variables due_date that close to code that deals with Version and NeedsHumanReview so that we don't confuse that with Version.<due_date>, which is a completely different thing.
| for event in events: | ||
| scanner_result = results_by_event_id.get(event.pk) | ||
|
|
||
| if scanner_result is None: |
There was a problem hiding this comment.
Has this ever happened in production?
| side, so we ask them again with a backoff, up to WEBHOOK_MAX_DELIVERY_ATTEMPTS | ||
| times. | ||
| """ | ||
| version = Version.unfiltered.get(pk=version_pk) |
There was a problem hiding this comment.
Should probably have a no_transforms() on that queryset
2 participants
Fixes mozilla/addons#16428
This PR refactors the "waiting on scanners" logic so that it uses
multiple retry attempts (5 for now) and a backoff strategy. This should
give about 15 hours for scanners blocking auto-approval to return
results. After that, we set NHR as it's done currently.
As soon as this lands, the next
auto_approveCRON task will take careof the versions currently stuck, which (hopefully) should fix most of
them.
This should allow most hiccups to be auto-resolved, while allowing us to
know when things go wrong, still. There is no recovery mechanism in this
patch, beyond the retries. We will probably need something, but that
we'll be dealt with separately.