fix(api): make PollForMediaWikiJobsJob skip soft-deleted wikis

[dati18]

Bug: T433575

[tarrow]

Does this help us much? Are we just turning an error into a warning? Maybe this is still useful though.

I guess the question is what causes this to happen? Such a long runtime for this job that race conditions are common?

[dati18]

Good question.

It does turn one failure mode from error to warning, but importantly it prevents a single stale domain from sabotaging the whole polling run.

To answer "Why this happens":

1. The job reads a list of domains.
2. Later, for each domain, resolver does another lookup.
3. If a wiki is soft-deleted (or otherwise missing) between those moments, resolver throws UnknownWikiDomainException.

The root cause I think is "double-read on mutable data" under multiple updates. The longer the runtime, the longer the gap between 1 and 3, the higher the probability. So basically it's the gap between the time of "reading the data" vs "using the data"

[tarrow]

nice! Sounds like we are getting closer to the root cause.

So could we refactor the job so we do this look up earlier? Or even in a single query?

I guess there might still be a case where the wiki really has now gone away

[dati18]

Good idea, so you want to weed out bad domains instead of just "skipping" it?

[tarrow]

I think you are correct that there is a double read. Actually there is a triple read:

1. once to get the list of Wikis
2. once to get the backendHost
3. once when then MW backendHost determines the configuration of the Wiki

I am suggesting we merge the first two cases into one and get the correct backend hosts at the same time as we get the list of Wikis. There is still a chance that by the time we get to polling the Wikis that the 3rd read fails because it has been deleted but this is still a better solution.

Are you sure that marking the job as failed is sabotaging the whole run? I assumed that the job would still iterate over all Wikis but it would retry (sooner than planned) because it was marked as failed.

[dati18]

I dug a bit deeper, and I agree on 2 points:

• "triple read" is actually correct
• merging reads 1 and 2 is a good idea (and better design) than simply catching exception

The only point I'm not sure that I can agree with is that markedAsFailed() in PollForMediaWikiJobsJob.php doesn't abort the current foreach loop and trigger a retry automatically by itself. There is no exception thrown, so the job handler completes, the jobs queue removes the job from the queue, and it ends. So if I understand it correctly, markedAsFailed here is basically a status flag, not a loop controller

So I refactored the job to resolve each wiki's backend URL once up front, so it doesn't "re-query" the resolver and the database per wiki during the poll loop

PollForMediaWikiJobsJob.php:

• The job now loads the active wikis once, using Wiki::with('wikiDb')->get()
• Instead of resolving the backend URL inside each polling step, it resolves it once per wiki while iterating

MediaWikiHostResolver.php:

• Added new helpers:
  • getBackendUrlForWiki(), so the job can pass in a loaded wiki model directly
  • getBackendHostForWiki(), to compute the backend host from the wiki object

-> The job now does one read of wiki data and one host-resolving per wiki, rather than re-querying resolver state again and again
-> That reduces the chance of hitting stale state between the initial wiki list and the later backend host resolution