Don't repopulate cache with potentially lagging DB data

getNotificationCount & getLastUnreadNotificationTime have an
argument $cached that allows cache to be bypassed & read from
DB. That result is then stored to cache.

In practice, it seems to be used only for cache invalidation.
getLastUnreadNotificationTime didn't allow to specify the DB
to be read from, and EchoNotificationMapper::fetchUnreadByUser
only read from slave.
So when we wanted to invalidate the cache, we would end up
immediately repopulating it with data from a (potentially and
likely) lagging slave.

I've made it accept the DB type, similar to getNotificationCount.

Bug: T98421
Change-Id: Ie4b09eeb04b9827b454cb2d92ee8c674bdd59a19
This commit is contained in:
Matthias Mullie 2015-05-07 13:58:51 +02:00
parent 276b4da02e
commit 6bcece22cc
2 changed files with 10 additions and 8 deletions

View file

@ -267,10 +267,11 @@ class MWEchoNotifUser {
* Returns the timestamp of the last unread notification.
*
* @param boolean $cached Set to false to bypass the cache.
* @param int $dbSource Use master or slave database to pull count
* @param string $section Notification section
* @return bool|MWTimestamp Timestamp of last notification, or false if there is none
*/
public function getLastUnreadNotificationTime( $cached = true, $section = EchoAttributeManager::ALL ) {
public function getLastUnreadNotificationTime( $cached = true, $dbSource = DB_SLAVE, $section = EchoAttributeManager::ALL ) {
global $wgEchoConfig;
if ( $this->mUser->isAnon() ) {
@ -298,13 +299,13 @@ class MWEchoNotifUser {
$eventTypesToLoad = $attributeManager->getUserEnabledEventsbySections( $this->mUser, 'web', array( $section ) );
}
$notifications = $this->notifMapper->fetchUnreadByUser( $this->mUser, 1, $eventTypesToLoad );
$notifications = $this->notifMapper->fetchUnreadByUser( $this->mUser, 1, $eventTypesToLoad, $dbSource );
if ( $notifications ) {
$notification = reset( $notifications );
$timestamp = $notification->getTimestamp();
// store to cache & return
$this->cache->set( $memcKey, $timestamp, 86400 );
$this->cache->set($memcKey, $timestamp, 86400);
return new MWTimestamp( $timestamp );
}
@ -393,9 +394,9 @@ class MWEchoNotifUser {
$this->getNotificationCount( false, $dbSource, EchoAttributeManager::MESSAGE );
// when notification count needs to be updated, last notification may have
// changed too, so we need to invalidate that cache too
$this->getLastUnreadNotificationTime( false, EchoAttributeManager::ALL );
$this->getLastUnreadNotificationTime( false, EchoAttributeManager::ALERT );
$this->getLastUnreadNotificationTime( false, EchoAttributeManager::MESSAGE );
$this->getLastUnreadNotificationTime( false, $dbSource, EchoAttributeManager::ALL );
$this->getLastUnreadNotificationTime( false, $dbSource, EchoAttributeManager::ALERT );
$this->getLastUnreadNotificationTime( false, $dbSource, EchoAttributeManager::MESSAGE );
$this->mUser->invalidateCache();
}

View file

@ -96,16 +96,17 @@ class EchoNotificationMapper extends EchoAbstractMapper {
* @param User $user
* @param int $limit
* @param string[] $eventTypes
* @param int $dbSource Use master or slave database to pull count
* @return EchoNotification[]
*/
public function fetchUnreadByUser( User $user, $limit, array $eventTypes = array() ) {
public function fetchUnreadByUser( User $user, $limit, array $eventTypes = array(), $dbSource = DB_SLAVE ) {
$data = array();
if ( !$eventTypes ) {
return $data;
}
$dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
$dbr = $this->dbFactory->getEchoDb( $dbSource );
$res = $dbr->select(
array( 'echo_notification', 'echo_event' ),
'*',