Page MenuHomePhabricator

Update Echo's caching strategy for multi-dc compatibility
Closed, ResolvedPublic

Description

As suggested in T164505#3237739 and following comments, Echo should use a purge-on-write, cache-from-slave WANCache approach to caching, rather than its current recache-on-write strategy. This is probably necessary to make Echo work in a multi-DC environment, and will also address T164505: Read queries detected on the x1 master by Echo extension more fully.

Details

Related Gerrit Patches:

Event Timeline

Catrope created this task.May 9 2017, 6:22 PM
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptMay 9 2017, 6:22 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Catrope added a subscriber: aaron.May 9 2017, 6:23 PM

@aaron Do I understand correctly that Echo's current strategy, which is to overwrite (as opposed to purge) entries in WANObjectCache, needs to be fixed for multi-DC?

aaron added a comment.May 9 2017, 8:14 PM

Sounds correct to me.

Catrope claimed this task.May 23 2018, 1:51 PM

Change 436204 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/Echo@master] CachedList: Use getWithSetCallback()

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

Change 436205 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/Echo@master] NotifUser: Redo caching strategy for multi-DC compatibility

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

Change 436204 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] CachedList: Use getWithSetCallback()

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

These patches fix most of the issue, but there's still one left: flagCacheWithNewTalkNotification() and flagCacheWithNoTalkNotification() still call WANObjectCache::set(). It's a lot harder to figure out what that cache key actually means (and what the set callback for this key would look like), so I'm going to come back to this a little later.

Change 436697 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/Echo@master] NotifUser: Remove basically-unused talk notification cache

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

Change 436205 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] NotifUser: Redo caching strategy for multi-DC compatibility

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

Change 441377 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/extensions/Echo@master] [WIP] Make "echo-new-talk-notification" key use getWithSetCallback()

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

Change 441377 abandoned by Aaron Schulz:
[WIP] Make "echo-new-talk-notification" key use getWithSetCallback()

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

Restricted Application added a project: Growth-Team. · View Herald TranscriptJul 13 2018, 2:19 PM

Change 436697 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] NotifUser: Remove basically-unused talk notification cache

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

aaron added a comment.Aug 10 2018, 8:02 PM

Can this task be closed?

Catrope closed this task as Resolved.Aug 13 2018, 7:01 PM

Can this task be closed?

Yes, the last patch for it was just merged, I think we're done now.

Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.