Page MenuHomePhabricator

'Duplicate get(): echo:seen:alert:time and echo:seen:message:time
Closed, ResolvedPublic

Description

We have a lot of Duplicate get() xxxx fetched N times apparently from Echo. Largely noticeable since 1.28.0-wmf.17, might have happened before. They are of the form:

<wiki>:echo:seen:alert:time:NNNNN
<wiki>:echo:seen:message:time:NNNNN

That indicates some code path fetch the same message twice from memcached when once should be sufficient. The message has been added by Performance-Team to reduce the traffic to memcached.

Event Timeline

hashar created this task.Sep 1 2016, 7:57 PM
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptSep 1 2016, 7:57 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Yes, that was present before.

Tgr added a subscriber: Tgr.Sep 6 2016, 7:40 PM

This is happening at a rate of ~8K/minute so probably every logged-in pageview?

Legoktm added a subscriber: Legoktm.Sep 6 2016, 7:46 PM

There's no in process cache, and we look up the seen time for each user in EchoHooks::onPersonalUrls() and then again in EchoHooks::onOutputPageCheckLastModified(). Adding an in-process cache should fix that.

aaron added a subscriber: aaron.Sep 6 2016, 8:04 PM

Maybe used the CachedBagOStuff class?

aaron moved this task from Inbox to Radar on the Performance-Team board.Sep 6 2016, 8:04 PM

Change 308814 had a related patch set uploaded (by Legoktm):
SeenTime: Wrap cache with CachedBagOStuff

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

Change 308814 merged by jenkins-bot:
SeenTime: Wrap cache with CachedBagOStuff

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

Legoktm closed this task as Resolved.Sep 6 2016, 9:12 PM
Legoktm claimed this task.

Will be deployed in wmf.19. If someone wants to backport this, they also need to backport rMW58038b262164: Turn off "reportDupes" in CachedBagOStuff.

Tgr added a comment.Sep 6 2016, 9:49 PM

As long as you are not passing any parameters to CachedBagOStuff (which b3cdd56 doesn't) the other patch should make no difference.

I think rECHOe1a276fe3dc9: Take seentime into account in the CheckLastModified hook probably made this much worse, and also doesn't use a shared instance so https://gerrit.wikimedia.org/r/308814 probably doesn't help.

demon added a comment.Sep 8 2016, 7:17 PM

I think rECHOe1a276fe3dc9: Take seentime into account in the CheckLastModified hook probably made this much worse, and also doesn't use a shared instance so https://gerrit.wikimedia.org/r/308814 probably doesn't help.

In which case maybe we should follow up making the cache available statically to the class?

I think rECHOe1a276fe3dc9: Take seentime into account in the CheckLastModified hook probably made this much worse, and also doesn't use a shared instance so https://gerrit.wikimedia.org/r/308814 probably doesn't help.

In which case maybe we should follow up making the cache available statically to the class?

Yes, we probably need a singleton instance for the current user or something.

Or, wait, no we don't, we just need the CachedBagOStuff instance to be static.

Change 309378 had a related patch set uploaded (by Chad):
Use static cache for times so it works across instances of SeenTime

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

Change 309378 merged by jenkins-bot:
Use static cache for times so it works across instances of SeenTime

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

Change 309380 had a related patch set uploaded (by Chad):
SeenTime: Wrap cache with CachedBagOStuff

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

Change 309381 had a related patch set uploaded (by Chad):
Use static cache for times so it works across instances of SeenTime

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

Change 309380 merged by jenkins-bot:
SeenTime: Wrap cache with CachedBagOStuff

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

Change 309381 merged by jenkins-bot:
Use static cache for times so it works across instances of SeenTime

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

demon added a comment.Sep 8 2016, 8:14 PM

Already deployed with wmf.18. Fixed the problem.