Page MenuHomePhabricator

Constraint check results are cached independent of language
Closed, ResolvedPublic

Description

@Ladsgroup pointed out that we store localized violation messages in the cache, but use the same cache for requests independent of language, so whichever request to miss the cache writes messages in that request’s language to the cache, and all other requests for the same entity then get cached violation messages in the first request’s language instead of their own.

The quick-and-dirty fix is to add the language to the cache key. Purging gets slightly tricky (we need to delete the cache entry for all possible languages), but otherwise this is fairly simple. However, caching results independently per-language both bloats the cache (caching effectively the same results several times) and slows down constraint checks for lesser-used languages (since they’re less likely to get cached results).

Alternatively, we could change what we cache: instead of caching essentially the full wbcheckconstraints response, cache something closer to a list of CheckResult serializations, after changing CheckResult to hold a Message object instead of a string. This also opens up other opportunities: the wbcheckconstraints response includes entries even for statements with no violations (cf. T178160), which we can’t remove from the API response, but could remove from the value that we cache. (That is: don’t store NullResults, instead recreate them when reading from cache.) And this will also allow us to store results from requests with a wider status parameter in the cache, and use the cache for requests with a narrower parameter, instead of only using the cache if the parameter is exactly violation|warning|bad-parameters.

Related Objects

StatusAssignedTask
ResolvedLucas_Werkmeister_WMDE
OpenNone
ResolvedLydia_Pintscher
ResolvedLydia_Pintscher
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
DeclinedNone
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
ResolvedLucas_Werkmeister_WMDE
OpenNone
ResolvedLucas_Werkmeister_WMDE

Event Timeline

Lucas_Werkmeister_WMDE triaged this task as Normal priority.Jan 25 2018, 12:26 PM
Lucas_Werkmeister_WMDE created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 25 2018, 12:26 PM

I just realized that we also have the same problem in some code that’s already deployed on wikidata.org – the “text matches regex” cache can contain a serialized ConstraintParameterException, where “serialized” means “take the localized message and put it in the cache” :( see I1671255279 / T183992.

Lucas_Werkmeister_WMDE closed this task as Resolved.Mar 27 2018, 10:43 AM

I think we can close this task. It was initially resolved by T185689: Add language to cache key for check results, and now the better solution of T185709: Cache CheckResult serializations per-entity in ObjectCache is also almost ready.