Repository navigation
Conversation
The current description gives an overview of what this PR does, which is useful but ultimately can likely be figured out by looking at the diff. What would be really useful to add to the description is why we are we making this change. What was the original context/issue? Why does this change achieve its goal? Etc. P.S. there are also some typos in the description ;) |
418d0dc to
e9185d6
Compare
74bebe7 to
3ac45dc
Compare
There was a problem hiding this comment.
This looks correct. However, I have some suggested improvements.
Also, the PR description still talks about the "schedule without overlapping" change that was removed from this PR.
I see you're on annual leave now, so I'll take over this PR.
I think that another possible reason we are getting this "Integrity constraint violation" log spam is because the 'redis' queue has a retry_after of only 100 seconds but UpdateWikiDaiyMetricJob::$timeout is set to 1hr - meaning that the job can be retried by another queue worker before the job is expected to have finished or has been terminated. The API chart sets Horizon tries to 3 by default so jobs are retied by default. I think the ShouldBeUnique interface is only evaluated at dispatch time, not when a job is retried. I think Job Middleware is run so we could use the WithoutOverlapping middleware (not to be confused with Schedule::withoutOverlapping() which is named similarly but functions completely differently).
3c02c42 to
e1146f4
Compare
e1146f4 to
0877388
Compare
outdooracorn
left a comment
There was a problem hiding this comment.
Good to go from my side but I touched it last so will let someone else also approve.
Prevent
WikiMetricsfrom collecting metrics if a record for today already exists in the database, as that would result in a primary key integrity constraint violation.This should help reduce logs like this:
Bug: T423554