Page MenuHomePhabricator
Create Task
Maniphest T255305

[Investigation] - Consider if the Wikibase cache for CachingEntityRevisionLookup is needed any more
Closed, ResolvedPublic
Actions

Assigned To
Ladsgroup
Authored By
Addshore
Jun 12 2020, 7:18 PM
Tags
Referenced Files
F32083788: image.png
Aug 12 2020, 8:34 PM
F32083353: image.png
Aug 12 2020, 8:30 PM
F32083174: image.png
Aug 12 2020, 8:30 PM
F32083384: image.png
Aug 12 2020, 8:30 PM
F32083477: image.png
Aug 12 2020, 8:30 PM
Subscribers
Addshore
Aklapper
Ladsgroup
Michael
WMDE-leszek

Description

CachingEntityRevisionLookup has existed in Wikibase for a long time.
Its main function is to interface with a shared cache for entity revisions, which is shared between ALL sites of a cluster.

With some of the code written for MCR and RevisionStore refactorings, MediaWiki core now has a BlobStore cache that is shared across the whole cluster.
Since then Wikibase is storing all retrieved entities twice in the cache.

We can now probably remove the layer of caching in Wikibase, but should first evaluate exactly what that will mean in terms of changed db calls etc.
Perhaps CachingEntityRevisionLookup will remain, as the entity revisions are still cached, but the cache is somewhere else.

This would:

  • Reduce Memcached usage in production
  • Reduce cache usage for 3rd party wikibase users
  • Reduce code complexity in Wikibase

Details

SubjectRepoBranchLines +/-
WIP DNM, update EntityRevisionLookupmediawiki/extensions/Wikibasemaster+188 -960
Set caching of CachingEntityRevisionLookup to CACHE_NONE in clientmediawiki/extensions/Wikibasemaster+2 -1
Set caching of CachingEntityRevisionLookup to CACHE_NONE in repomediawiki/extensions/Wikibasemaster+1 -1
Set caching of CachingEntityRevisionLookup to CACHE_NONE in clientmediawiki/extensions/Wikibasewmf/1.36.0-wmf.3+3 -1
Set caching of CachingEntityRevisionLookup to CACHE_NONE in clientmediawiki/extensions/Wikibasewmf/1.36.0-wmf.4+2 -1
Set caching of CachingEntityRevisionLookup to CACHE_NONE in repomediawiki/extensions/Wikibasewmf/1.36.0-wmf.4+1 -1
Customize query in gerrit

Related Objects

Event Timeline

Addshore created this task.Jun 12 2020, 7:18 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 12 2020, 7:18 PM
Addshore added a comment.Jun 12 2020, 9:44 PM
  • CachingEntityRevisionLookup::getEntityRevision has the entity id and rev id to lookup and tried to get it from the cache
  • Failing, it calls fetchEntityRevision
  • This ultimately calls WikiPageEntityRevisionLookup::getEntityRevision
  • This then calls a method on entityMetaDataAccessor, either loadRevisionInformationByRevisionId or loadRevisionInformation

Looking into the meta data accessor I see a note:

* @todo This whole interface may no longer be needed with the introduction of RevisionRecord which can
* represent a revision on any wiki.
* MetaData accessing can probably be killed and instead RevisionRecord just be returned.
  • loadEntity is the called
  • revisionStore->getRevisionById is called, returning out RevisionRecord
  • We then also call loadEntityDataFromWikiPageRevision ? which comes with the note:
		// NOTE: Support for cross-wiki content access in RevisionStore is incomplete when,
		// reading from the pre-MCR database schema, see T201194.
		// For that reason, we have to load and decode the content blob directly,
		// instead of using RevisionRecord::getContent() or SlotRecord::getContent().
		// TODO Once we can rely on the new MCR enabled DB schema, use getContent() directly!

This method then calls blobStore->getBlob which is where the new layer of shared caching lives.

gerritbot added a comment.Jun 12 2020, 10:39 PM

Change 605333 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@master] WIP DNM, update EntityRevisionLookup

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

gerritbot added a project: Patch-For-Review.Jun 12 2020, 10:39 PM
Addshore mentioned this in T255116: Investigate coupling in cache layer (shared cache entries in WMF environment).Jun 17 2020, 9:47 PM
darthmon_wmde added a project: Wikidata-Campsite.Jul 20 2020, 9:27 AM
darthmon_wmde moved this task from Incoming to Prioritized Wikidata Tech Backlog (prioritised from top to bottom) on the Wikidata-Campsite board.
darthmon_wmde added a project: Wikidata-Trailblazing-Exploration.Jul 30 2020, 4:33 PM
darthmon_wmde moved this task from Backlog to Trailblazer Candidates on the Wikidata-Trailblazing-Exploration board.
Addshore renamed this task from Consider if CachingEntityRevisionLookup is needed any more to [Investigation] - Consider if CachingEntityRevisionLookup is needed any more.Aug 4 2020, 12:11 PM
Addshore edited projects, added Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)); removed Wikidata-Campsite.
Maintenance_bot moved this task from incoming to in progress on the Wikidata board.Aug 4 2020, 12:15 PM
Addshore renamed this task from [Investigation] - Consider if CachingEntityRevisionLookup is needed any more to [Investigation] - Consider if the Wikibase cache for CachingEntityRevisionLookup is needed any more.Aug 4 2020, 1:08 PM
Addshore updated the task description. (Show Details)
Michael subscribed.Aug 4 2020, 1:09 PM

The investigation should shed light on the following questions:

  • How complicate would removing the Cache for CachingEntityRevisionLookup be?
    • Would this be the wok of a few people over a couple of days or should we have a hike/trailblaze for it?
Ladsgroup subscribed.Aug 11 2020, 11:50 AM

You can store caches as database entries (in objectcache table) by setting $wgMainCacheType to CACHE_DB. I did it and this is I've got so far:

  • We actually ourselves store them twice, once under wikibase_shared/repo:WikiPageEntityRevisionLookup:Q228 and once under repo:WikiPageEntityRevisionLookup:Q228. The content is not exactly the same, I double check what's the difference.
  • The TTL we set for this is 3600 while the TTL for SqlBlobStore is seven days.
  • SqlBlobStore compresses the values while we don't,
  • I can't actually find any cache that stores SqlBlobStore. I might be missing something obvious.

Stay tuned.

Krinkle added a project: Performance-Team (Radar).Aug 11 2020, 1:18 PM
Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.
Ladsgroup claimed this task.Aug 11 2020, 1:27 PM
Ladsgroup moved this task from To Do (prioritised from top to bottom) to Doing on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.
Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptAug 11 2020, 1:27 PM
Maintenance_bot moved this task from Incoming to In progress on the User-Ladsgroup board.Aug 11 2020, 2:15 PM
Ladsgroup added a comment.EditedAug 11 2020, 6:24 PM

I confirm that setting the entity revision lookup cache to none just makes it to fallback to the mediawiki's SqlBlobStore cache (checking from debug logs) and SqlBlobStore cache indeed compresses it (619 is revid of the most recent revision of Q228):

+--------------------------------------------------------+--------------------+
| keyname                                                | char_length(value) |
+--------------------------------------------------------+--------------------+
| WANCache:v:global:revision-row-1.29:repo:684:915       |                392 |
| repo:WBL:WikiPageEntityRevisionLookup:Q228             |               1680 |
| wikibase_shared/repo:WikiPageEntityRevisionLookup:Q228 |               1680 |
+--------------------------------------------------------+--------------------+

The next step would be to make this cache configurable, then set it to cache none in beta, test and production and if no regression shows up (specially in matter of performance regression, mcrouter traffic, nutcracker going nuts, etc.), then we can simply remove it. The only thing I need to make sure right now is that how the fallback from Q228 to rev id 915 works, is it cached too?

Ladsgroup added a comment.Aug 11 2020, 6:43 PM

The page id to revision id (Q228 -> 915) happens through a WikiPageEntityMetaDataAccessor which has an implementation that has basic caching: PrefetchingWikiPageEntityMetaDataAccessor so it's not as severe than I thought.

gerritbot added a comment.Aug 11 2020, 7:06 PM

Change 619547 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Set caching of CachingEntityRevisionLookup to CACHE_NONE

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

Ladsgroup moved this task from Doing to Peer Review on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.Aug 11 2020, 7:09 PM

So my idea for this is to get this cherry-picked (but not merged on master) and deploy it on canaries (10% of traffic) for an hour tomorrow and monitor everything, from logstash to mcrouter, memcached, s8 load, es load and if we don't see a regression, we deploy it to everywhere and leave it until next train and in the next train, we remove everything. How does that sound?

gerritbot added a comment.Aug 12 2020, 12:42 AM

Change 619597 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Set caching of CachingEntityRevisionLookup to CACHE_NONE in repo

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

gerritbot added a comment.Aug 12 2020, 5:31 PM

Change 619615 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.4] Set caching of CachingEntityRevisionLookup to CACHE_NONE in repo

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

gerritbot added a comment.Aug 12 2020, 5:54 PM

Change 619615 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.4] Set caching of CachingEntityRevisionLookup to CACHE_NONE in repo

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.4; 2020-08-11).Aug 12 2020, 6:00 PM
Stashbot added a comment.Aug 12 2020, 6:04 PM

Mentioned in SAL (#wikimedia-operations) [2020-08-12T18:04:18Z] <ladsgroup@deploy1001> Synchronized php-1.36.0-wmf.4/extensions/Wikibase/repo/includes/Store/Sql/SqlStore.php: [[phab:T255305|Set caching of CachingEntityRevisionLookup to CACHE_NONE in repo]] (duration: 01m 06s)

Ladsgroup added a comment.Aug 12 2020, 6:08 PM

In repo:

gerritbot added a comment.Aug 12 2020, 6:15 PM

Change 619616 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.4] Set caching of CachingEntityRevisionLookup to CACHE_NONE in client

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

gerritbot added a comment.Aug 12 2020, 6:15 PM

Change 619617 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.3] Set caching of CachingEntityRevisionLookup to CACHE_NONE in client

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

Ladsgroup added a comment.Aug 12 2020, 6:17 PM

One thing that was mentioned is that we should use getKnownCurrentRevision in revision lookup that has internal caching.

gerritbot added a comment.Aug 12 2020, 6:38 PM

Change 619616 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.4] Set caching of CachingEntityRevisionLookup to CACHE_NONE in client

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

Stashbot added a comment.Aug 12 2020, 6:50 PM

Mentioned in SAL (#wikimedia-operations) [2020-08-12T18:50:11Z] <ladsgroup@deploy1001> Synchronized php-1.36.0-wmf.4/extensions/Wikibase/client/includes/Store/Sql/DirectSqlStore.php: [[phab:T255305|Set caching of CachingEntityRevisionLookup to CACHE_NONE in client]] (duration: 02m 13s)

gerritbot added a comment.Aug 12 2020, 7:16 PM

Change 619617 abandoned by Ladsgroup:
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.3] Set caching of CachingEntityRevisionLookup to CACHE_NONE in client

Reason:
wmf.4 is being deployed everywhere instead

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

Ladsgroup added a comment.EditedAug 12 2020, 8:30 PM

Now it's deployed across the fleet. Here's my analysis:

  • I was worried that constant decompressing of values coming from memcached would elevate the CPU usage but it didn't
  • I was worried that since wikibase has the caches keyed by entity id instead of revision id (which sqlblob store key them based on revision id) would increase the read on s8, this actually happened:

image.png (257×1 px, 53 KB)

(You can guess when it got deployed everywhere)
but it's not big enough to cause any major disruption (note that the graph doesn't start at zero).

  • Looking at mc* hosts and the overall network graphs of mcrouter/memcached, I see a rough 10% decrease in network but the noise makes it hard to show you:

image.png (273×918 px, 45 KB)

  • When I deployed it, the hits of SqlBlobStore didn't change much (it's pretty high anyway, 1M/minute) but misses doubled which is expected, the cache is warm. It's still putting the long tail into memcached:

image.png (547×1 px, 66 KB)

  • Naturally External Storage had a little pressure but recovered.

image.png (770×1 px, 145 KB)

  • After TTL of Wikibase memcached entries expiring, evictions are going up:

image.png (275×925 px, 35 KB)

No noticeable increase in user-facing error or fatals or timeouts has been observed.

I leave Xghui of two client cases:

Both are the second time requests to warm up their caches. The increase in DB is obvious but the easiest solution is to use RevisionLookup::getKnownCurrentRevision() instead of PrefetchingWikiPageEntityMetaDataAccessor which has a much better cache.

Addshore mentioned this in T260275: EntityLookupException: the execution time limit of 60 seconds was exceeded.Aug 12 2020, 9:50 PM
gerritbot added a comment.Aug 12 2020, 10:00 PM

Change 619597 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Set caching of CachingEntityRevisionLookup to CACHE_NONE in repo

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

gerritbot added a comment.Aug 12 2020, 10:00 PM

Change 619547 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Set caching of CachingEntityRevisionLookup to CACHE_NONE in client

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.5; 2020-08-18); removed MW-1.36-notes (1.36.0-wmf.4; 2020-08-11).Aug 12 2020, 10:00 PM
gerritbot added a comment.Aug 13 2020, 9:21 AM

Change 605333 abandoned by Addshore:
[mediawiki/extensions/Wikibase@master] WIP DNM, update EntityRevisionLookup

Reason:
The cache was removed in https://phabricator.wikimedia.org/T255305, this draft might still be useful for actually using RevisionStore directly, but that will likely now be done as a separate ticket.

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

Michael moved this task from Peer Review to Test (Verification) on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.Aug 14 2020, 4:00 PM
Michael removed a project: Patch-For-Review.

As far as I can tell, all associated patches have been either merged or abandoned.

Ladsgroup mentioned this in T260600: Drop CachingEntityRevisionLookup.Aug 17 2020, 7:51 PM
Ladsgroup mentioned this in T260606: Use RevisionLookup::getKnownCurrentRevision instead of PrefetchingWikiPageEntityMetaDataAccessor.Aug 17 2020, 8:05 PM
WMDE-leszek closed this task as Resolved.Sep 25 2020, 12:14 PM
WMDE-leszek moved this task from Test (Verification) to Done on the Wikidata-Campsite (Wikidata-Campsite-Iteration-∞ (On Hold)) board.
WMDE-leszek subscribed.

I note the conclusion of the investigation has been the cache is not needed. A follow up task T260600 has been filed.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL