NotifUser: Make resetNotificationCount() default to DB_MASTER

We almost always call it with DB_MASTER, and I'm pretty sure that the
one call that uses DB_REPLICA introduces a race condition. I didn't want
to change that quite yet, though, so I left it in for now.

Change-Id: Ia5a59fdda357b799e327b8ed224f3ccb09509a8a
This commit is contained in:
Roan Kattouw 2018-05-25 19:45:49 +02:00 committed by Catrope
parent 544129492d
commit 32b82fa1fe
5 changed files with 18 additions and 10 deletions

View file

@ -1417,9 +1417,9 @@ class EchoHooks {
public static function onMergeAccountFromTo( User &$oldUser, User &$newUser ) {
DeferredUpdates::addCallableUpdate( function () use ( $oldUser, $newUser ) {
MWEchoNotifUser::newFromUser( $oldUser )->resetNotificationCount( DB_MASTER );
MWEchoNotifUser::newFromUser( $oldUser )->resetNotificationCount();
if ( !$newUser->isAnon() ) {
MWEchoNotifUser::newFromUser( $newUser )->resetNotificationCount( DB_MASTER );
MWEchoNotifUser::newFromUser( $newUser )->resetNotificationCount();
}
} );

View file

@ -362,7 +362,7 @@ class MWEchoNotifUser {
$updated = $this->userNotifGateway->markRead( $eventIds );
if ( $updated ) {
// Update notification count in cache
$this->resetNotificationCount( DB_MASTER );
$this->resetNotificationCount();
// After this 'mark read', is there any unread edit-user-talk
// remaining? If not, we should clear the newtalk flag.
@ -395,7 +395,7 @@ class MWEchoNotifUser {
$updated = $this->userNotifGateway->markUnRead( $eventIds );
if ( $updated ) {
// Update notification count in cache
$this->resetNotificationCount( DB_MASTER );
$this->resetNotificationCount();
// After this 'mark unread', is there any unread edit-user-talk?
// If so, we should add the edit-user-talk flag
@ -468,12 +468,18 @@ class MWEchoNotifUser {
}
/**
* Invalidate cache and update echo_unread_wikis if x-wiki notifications is enabled
* NOTE: Consider calling this function from a deferred update since it may access the db
* Invalidate cache and update echo_unread_wikis if x-wiki notifications is enabled.
*
* @param int $dbSource Use master or replica database to pull count
* This updates the user's touched timestamp, as well as the value returned by getGlobalUpdateTime().
*
* NOTE: Consider calling this function from a deferred update, since it will read from and write to
* the master DB if cross-wiki notifications are enabled.
*
* @param int $dbSource Use master or replica database to pull count.
* Only used if $wgEchoCrossWikiNotifications is enabled.
* Do not set this to DB_REPLICA unless you know what you're doing.
*/
public function resetNotificationCount( $dbSource = DB_REPLICA ) {
public function resetNotificationCount( $dbSource = DB_MASTER ) {
global $wgEchoCrossWikiNotifications;
if ( $wgEchoCrossWikiNotifications ) {
// Schedule an update to the echo_unread_wikis table

View file

@ -34,6 +34,8 @@ class EchoModerationController {
// users whose notifications have been moderated.
foreach ( $affectedUserIds as $userId ) {
$user = User::newFromId( $userId );
// TODO this looks like it's still susceptible to a race condition, if the notification count
// was reset from the master just before
MWEchoNotifUser::newFromUser( $user )->resetNotificationCount( DB_REPLICA );
}
} );

View file

@ -59,7 +59,7 @@ class EchoNotificationDeleteJob extends Job {
);
if ( $res ) {
$notifUser = MWEchoNotifUser::newFromUser( $user );
$notifUser->resetNotificationCount( DB_MASTER );
$notifUser->resetNotificationCount();
}
}

View file

@ -136,7 +136,7 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable {
// Add listener to refresh notification count upon insert
$notifMapper->attachListener( 'insert', 'refresh-notif-count',
function () use ( $notifUser, $section ) {
$notifUser->resetNotificationCount( DB_MASTER );
$notifUser->resetNotificationCount();
}
);