jenkins-bot has submitted this change and it was merged.

Change subject: Don't repopulate cache with potentially lagging DB data
......................................................................


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
---
M includes/NotifUser.php
M includes/mapper/NotificationMapper.php
2 files changed, 10 insertions(+), 8 deletions(-)

Approvals:
  Catrope: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/NotifUser.php b/includes/NotifUser.php
index 3af2c94..4901faf 100644
--- a/includes/NotifUser.php
+++ b/includes/NotifUser.php
@@ -267,10 +267,11 @@
         * 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 @@
                        $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 @@
                $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();
        }
 
diff --git a/includes/mapper/NotificationMapper.php 
b/includes/mapper/NotificationMapper.php
index a3fef50..c43c296 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -96,16 +96,17 @@
         * @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' ),
                        '*',

-- 
To view, visit https://gerrit.wikimedia.org/r/209464
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie4b09eeb04b9827b454cb2d92ee8c674bdd59a19
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to