Page MenuHomePhabricator

Indicate that constraint check results are potentially stale
Closed, ResolvedPublic

Description

Constraint check results can sometimes be stale – this is already the case, due to the query service’s cache (causing T170672: Constraints gadget continues to show a resolved unique value violation for a while after a merge), but will become even worse once we implement proper constraint result caching (T179839). We should indicate this in the API response, and the gadget should alert the user to it (gently).

Patch-For-Review:

Event Timeline

Change 389774 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add staleness indication to CheckResult and API

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

Change 389775 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ancillary messages to ConstraintReportPanel

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

Change 389776 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ancillary message for stale results to gadget

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

Jonas updated the task description. (Show Details)Nov 9 2017, 1:13 PM

Change 390281 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Propagate whether query result was cached

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

Change 390395 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add maximum age to CachedResult

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

Okay, after discussion with @thiemowmde, the new implementation plan is:

  • Add a CachingMetadata value object that holds the max-age and possibly additional information in the future. It also has a static constructor to merge several CachingMetadata objects into a new one.
  • Add a CachedBoolean class and a CachedArray class which wrap a value and some CachingMetadata. Make the helpers return those.
  • A CheckResult can optionally hold a single CachingMetadata object, with a simple getter/setter interface.

And here’s how I would split that up across commits:

  1. add CachingMetadata + test (and we might as well start with a max-age right away, no need to keep that in a separate change)
  2. add CachedArray, subclass CachedQueryResults, use in runQuery()
  3. add CachedBoolean, use in SparqlHelper+TypeCheckerHelper
  4. add CachingMetadata to CheckResult and API response
    • question: should the CachingMetadata know how to serialize itself to an array for the API response, or does this logic live in the API?
  5. add ancillary message to gadget (not sure if this commit even needs any changes apart from a rebase… but I suppose yes, if we want to include the unit “seconds” in the max-age field name)

Change 391277 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingMetadata value object

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

Change 391278 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedArray, CachedQueryResults containers

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

Change 391279 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedBool container and use in helpers

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

Change 391280 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedEntityIds container and use in SparqlHelper

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

Change 391281 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingMetadata to CheckResult and API output

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

Change 389774 abandoned by Lucas Werkmeister (WMDE):
Add CachedResult class and indicate in API response

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

Change 390281 abandoned by Lucas Werkmeister (WMDE):
Propagate whether query result was cached

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

Change 390395 abandoned by Lucas Werkmeister (WMDE):
Add maximum age to CachedResult

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

Change 391277 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingMetadata value object

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

Change 391278 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedArray, CachedQueryResults containers

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

Change 391602 had a related patch set uploaded (by Addshore; owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.8] Add CachingMetadata value object

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

Change 391603 had a related patch set uploaded (by Addshore; owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.8] Add CachedArray, CachedQueryResults containers

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

Change 391602 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.8] Add CachingMetadata value object

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

Change 391603 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@wmf/1.31.0-wmf.8] Add CachedArray, CachedQueryResults containers

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

Change 391279 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedBool container and use in helpers

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

Change 389775 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ancillary messages to ConstraintReportPanel

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

Change 391280 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedEntityIds container and use in SparqlHelper

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

Change 391281 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingMetadata to CheckResult and API output

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

Change 391799 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Minor fixes to cache code

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

Change 389776 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ancillary message for cached results to gadget

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

Change 391799 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Minor fixes to cache code

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

Lucas_Werkmeister_WMDE closed this task as Resolved.Nov 16 2017, 12:40 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)
Lucas_Werkmeister_WMDE moved this task from Review to Done on the Wikidata-Sprint-2017-11-07 board.

Done, as far as I can tell.