| brennen | |
| Nov 13 2024, 10:31 PM |
| F57698697: 2024-11-13-15:30:49.png | |
| Nov 13 2024, 10:31 PM |
[{reqId}] {exception_url} Wikimedia\Assert\PreconditionException: Precondition failed: page must exist522 of these in logs, all for 1.44.0-wmf.2 (T375661). Big initial spike, intermittent since then:
This is strange. This covered the time when T378983: Add Link recommendation are not being processed by CirrusSearch (November 2024) was acute. But from
The error-message itself sounds like we maybe need to add special handling for a page being move or a page being deleted or something like that?
I checked a couple of the errors, and all were related to newly created pages. I think this was caused by us switching the wgGETempLinkRecommendationSwitchTagClearHook flag to true (SAL). In that deployment, we started to call WeightedTagsUpdater::resetWeightedTags as part of onPageSaveComplete. The problem is that onPageSaveComplete runs right after the page was created. Because of that, it wouldn't be surprising if certain parts of MediaWiki did not yet realise the page actually exists.
Given the purpose of the hook is to invalidate the Add-Link weighted tag if is present in the search index, we can just stop running the hook if the page is freshly created (the only way how the weighted tag could get to the search index is by GrowthExperiments calling WeightedTagsUpdater::updateWeightedTags, and the only way how that could happen is if the page exists already, which it didn't, as it is just getting created). That way, we can avoid figuring out how to get the valid ID (preventing ourselves from getting into a similar rabbit hole like T380500).
The good question is how would we detect whether the page is getting created. I can't see any param that would inform us of that. Maybe this is something DomainEvent could help us with (T380505)? Unlike onPageSaveComplete's signature, PageUpdatedEvent allows subscribers to determine whether they're working with a very first edit on a page (among other things, it provides getOldRevision (which can be null if there is no previous revision).
As of now, the errors aren't getting logged, but that is because EventBusWeightedTagsUpdater (the implementation we use since T378983) does not have a precondition of "the updated page exists". Regardless of whether the errors are getting logged or not, I think it makes sense to not run WeightedTagsUpdater::resetWeightedTags if there is nothing to reset (=for fresh pages), and I think that should be a simple enough change to make as part of bugfixing. Moving to sprint (but other Growth engineers should feel free to unsprintize it if they disagree or if it turns to be a lot of additional effort).
Change #1117544 had a related patch set uploaded (by Michael Große; author: Michael Große):
[mediawiki/extensions/GrowthExperiments@master] fix(AddLink): Don't try to clear recommendations for new page
Change #1117544 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] fix(AddLink): Don't try to clear recommendations for new page
QA Note: Not really something to test here beyond asserting that this error no longer occurs in logstash after this change arrives on Group 2 on Thursday, February 2oth. This error is logged in a deferred update after a new page was created, no user-visible impact.