Page MenuHomePhabricator

Echo DB writes on page views
Closed, ResolvedPublic

Description

Seeing lots of these in the logs:

Expectation (writes <= 0) by MediaWiki::main not met:
query-m: UPDATE echo_notification SET notification_read_timestamp = 'X' AND notification_read_timestamp IS NULL
TransactionProfiler.php line 311 calls wfBacktrace()
TransactionProfiler.php line 200 calls TransactionProfiler->reportExpectationViolated()
Database.php line 1006 calls TransactionProfiler->recordQueryCompletion()
Database.php line 2001 calls DatabaseBase->query()
UserNotificationGateway.php line 56 calls DatabaseBase->update()
NotifUser.php line 338 calls EchoUserNotificationGateway->markRead()
Hooks.php line 657 calls MWEchoNotifUser->markRead()
Hooks.php line 204 calls EchoHooks::onPersonalUrls()
SkinTemplate.php line 688 calls Hooks::run()
SkinTemplate.php line 455 calls SkinTemplate->buildPersonalUrls()
SkinTemplate.php line 240 calls SkinTemplate->prepareQuickTemplate()
OutputPage.php line 2305 calls SkinTemplate->outputPage()
MediaWiki.php line 677 calls OutputPage->output()
MediaWiki.php line 474 calls MediaWiki->main()
index.php line 41 calls MediaWiki->run()
index.php line 3 calls include()

Related Objects

Event Timeline

aaron created this task.Nov 3 2015, 11:09 AM
aaron raised the priority of this task from to Needs Triage.
aaron updated the task description. (Show Details)
aaron added projects: Notifications, Performance.
aaron added a subscriber: aaron.
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptNov 3 2015, 11:09 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I think these are inherently from read actions (e.g. visiting a topic marks that topic notification read), so probably need to use some workaround, e.g. EnqueueJob.

Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptNov 3 2015, 6:33 PM
aaron added a comment.Nov 3 2015, 6:39 PM

Any reason not to just use DeferredUpdates::addCallableUpdate()? I see there is a comment saying it should be deferred and was wondering if there was some problem with doing so.

Any reason not to just use DeferredUpdates::addCallableUpdate()? I see there is a comment saying it should be deferred and was wondering if there was some problem with doing so.

I think we use that somewhere else already, and it works fine except that it causes the number of unread notifs to be slightly out of date? @Legoktm would know better than me.

aaron added a comment.Nov 3 2015, 8:31 PM

I assume the change would affect the code below:

$notifUser = MWEchoNotifUser::newFromUser( $user );
$msgCount = $notifUser->getMessageCount();
$alertCount = $notifUser->getAlertCount();

The $notifUser->markRead( $eventIds ) updates the master and resets the new cache values. This code then sees them in cache. So viewing a page might still show notifications for that page as unread until the *next* page view. Not sure how well this works with multiple datacenters even now. It mostly would since the user would stay on the closest DC, so the cluster-local set() would normally be seen. OTOH, then a user edits and sticks to the master DC for a while, they'd start using the other DC memcached clusters, so the counts would be wrong.

Change 250761 had a related patch set uploaded (by Aaron Schulz):
Defer onPersonalUrls() DB writes to post-send

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

Change 250761 merged by jenkins-bot:
Defer onPersonalUrls() DB writes to post-send

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

Legoktm closed this task as Resolved.Mar 8 2016, 6:05 PM
Legoktm claimed this task.

Change 276240 had a related patch set uploaded (by Legoktm):
Defer onPersonalUrls() DB writes to post-send (try #2)

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

Legoktm reopened this task as Open.Mar 9 2016, 7:10 PM

Patch was reverted due to T129299: BadMethodCall in PersonalUrls

Change 276240 merged by jenkins-bot:
Defer onPersonalUrls() DB writes to post-send (try #2)

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

Legoktm closed this task as Resolved.Mar 29 2016, 9:52 PM