Page MenuHomePhabricator

Only filter for result statuses after collecting metadata
Closed, ResolvedPublic

Description

CheckingResultsBuilder::getResults() first gets all check results from DelegatingConstraintChecker, then filters them for just the statuses that were requested (by default: discarding everything except violation, warning, bad-parameters – see T184937), and then collects the results, including the metadata. This is wrong – we must collect the metadata before filtering, otherwise we’re discarding possible depended entity IDs.

This is the root cause for the 2018-02-26 WBQC incident – for constraint checks with only successful results (where the filtering step resulted in an empty set of depended entity IDs), CachingResultsBuilder would ask for getLatestRevisionIds( [ /* empty array */ ] ). However, this incorrect request only caused large-scale problems due to a combination of other bugs.

Note that this means the latestRevisionIds of all cached results are incorrect (even if they’re not empty, they can still be incomplete), so a full fix to this bug must make sure that we’re not using those incorrect results. The easiest solution is probably to tweak the cache key (e. g. change the v2 component to v2.1).

Event Timeline

Change 414981 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Only filter statuses after collecting metadata

https://gerrit.wikimedia.org/r/414981

High priority because we need this fixed so we can re-enable caching, and adding to sprint because high-priority. (The fix is fairly small except for the test, so I hope it’s not too bad to review…)

Change 414981 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Only filter statuses after collecting metadata

https://gerrit.wikimedia.org/r/414981

Change 415074 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Bump cache key for check results

https://gerrit.wikimedia.org/r/415074

Change 415074 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Bump cache key for check results

https://gerrit.wikimedia.org/r/415074

Change 415285 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.22] Only filter statuses after collecting metadata

https://gerrit.wikimedia.org/r/415285

Change 415289 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.22] Bump cache key for check results

https://gerrit.wikimedia.org/r/415289

Change 415290 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.23] Bump cache key for check results

https://gerrit.wikimedia.org/r/415290

Change 415285 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.22] Only filter statuses after collecting metadata

https://gerrit.wikimedia.org/r/415285

Mentioned in SAL (#wikimedia-operations) [2018-02-28T14:47:26Z] <zfilipin@tin> Synchronized php-1.31.0-wmf.22/extensions/WikibaseQualityConstraints/: SWAT: [[gerrit:415285|Only filter statuses after collecting metadata (T188384)]] (duration: 01m 03s)

Change 415289 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.22] Bump cache key for check results

https://gerrit.wikimedia.org/r/415289

Change 415290 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.23] Bump cache key for check results

https://gerrit.wikimedia.org/r/415290

Mentioned in SAL (#wikimedia-operations) [2018-02-28T15:11:41Z] <zfilipin@tin> Synchronized php-1.31.0-wmf.22/extensions/WikibaseQualityConstraints: SWAT: [[gerrit:415289|Bump cache key for check results (T188384)]] (duration: 01m 02s)

Mentioned in SAL (#wikimedia-operations) [2018-02-28T15:15:55Z] <zfilipin@tin> Synchronized php-1.31.0-wmf.23/extensions/WikibaseQualityConstraints/: SWAT: [[gerrit:415288|Don’t query WikiPageEntityMetaDataAccessor with empty list (T188311)]] [[gerrit:415290|Bump cache key for check results (T188384)]] (duration: 01m 02s)