Bsitu has uploaded a new change for review.
https://gerrit.wikimedia.org/r/159413
Change subject: Add job to keep user echo notifications in reasonable volume
......................................................................
Add job to keep user echo notifications in reasonable volume
This is not tested and needs some test cases
Change-Id: I4d4fa4c987a1732e5e29536a7669e28c34d4ab18
---
M Echo.php
M controller/NotificationController.php
M includes/mapper/NotificationMapper.php
M includes/mapper/TargetPageMapper.php
A jobs/NotificationDeleteJob.php
5 files changed, 174 insertions(+), 4 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/13/159413/1
diff --git a/Echo.php b/Echo.php
index 5db62d9..6cb646c 100644
--- a/Echo.php
+++ b/Echo.php
@@ -239,10 +239,10 @@
// The max number showed in bundled message, eg, <user> and 99+ others <action>
$wgEchoMaxNotificationCount = 99;
-// The max number allowed to be updated on a web request, when we mark all
notification
-// as read, it's a bad idea to update on a web request if the number is
incredibly
-// huge, to prevent this, we just fetch 2000 thousand records and mark them as
read.
-// This would cover most of the use cases.
+// The max number of notifications allowed for a user to do a live update,
+// this is also the number of max notifications allowed for a user to have
+// @FIXME - the name is not intuitive, probably change it when the deleteJob
patch
+// is deployed to both deployment branches
$wgEchoMaxUpdateCount = 2000;
// The time interval between each bundle email in seconds
diff --git a/controller/NotificationController.php
b/controller/NotificationController.php
index 3233d5c..8fbdd25 100644
--- a/controller/NotificationController.php
+++ b/controller/NotificationController.php
@@ -74,7 +74,9 @@
$type = $event->getType();
$notifyTypes = self::getEventNotifyTypes( $type );
+ $userIds = array();
foreach ( self::getUsersToNotifyForEvent( $event ) as $user ) {
+ $userIds[$user->getId()] = $user->getId();
$userNotifyTypes = $notifyTypes;
wfRunHooks( 'EchoGetNotificationTypes', array( $user,
$event, &$userNotifyTypes ) );
@@ -83,6 +85,30 @@
self::doNotification( $event, $user, $type );
}
}
+ if ( $userIds ) {
+ self::enqueueDeleteJob( $userIds, $event );
+ }
+ }
+
+ /**
+ * Schedule a job to check and delete older notifications
+ *
+ * @param int $userIds
+ * @param EchoEvent $event
+ */
+ public static function enqueueDeleteJob( array $userIds, EchoEvent
$event ) {
+ // Do nothing if there is no user
+ if ( !$userIds ) {
+ return;
+ }
+
+ $job = new EchoNotificationDeleteJob(
+ $event->getTitle() ?: Title::newMainPage(),
+ array(
+ 'userIds' => $userIds
+ )
+ );
+ JobQueueGroup::singleton()->push( $job );
}
/**
diff --git a/includes/mapper/NotificationMapper.php
b/includes/mapper/NotificationMapper.php
index 52aca10..4d20bf2 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -206,4 +206,57 @@
}
}
+ /**
+ * Fetch a notification by user in the specified offset. The caller
should
+ * know that pass a big number for offset is not going to work
+ * @param User $user
+ * @param int $offset
+ * @return EchoNotification|bool
+ */
+ public function FetchByUserOffset( User $user, $offset ) {
+ $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
+ $row = $dbr->selectRow(
+ array( 'echo_notification', 'echo_event' ),
+ array( '*' ),
+ array(
+ 'notification_user' => $user->getId(),
+ 'notification_bundle_base' => 1
+ ),
+ __METHOD__,
+ array(
+ 'ORDER BY' => 'notification_timestamp DESC,
notification_event DESC',
+ 'OFFSET' => $offset,
+ 'LIMIT' => 1
+ )
+ array(
+ 'echo_event' => array( 'LEFT JOIN',
'notification_event=event_id' ),
+ )
+ );
+
+ if ( $row ) {
+ return EchoNotification::newFromRow( $row );
+ } else {
+ return false;
+ }
+ }
+
+ /**
+ * Batch delete notifications by user and eventId offset
+ * @param User $user
+ * @param int $eventId
+ * @return boolean
+ */
+ public function deleteByUserEventOffset( User $user, $eventId ) {
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+ $res = $dbw->delete(
+ 'echo_notification',
+ array(
+ 'notification_user' => $user->getId(),
+ 'notification_event < ' . (int)$eventId
+ ),
+ __METHOD__
+ );
+ return $res;
+ }
+
}
diff --git a/includes/mapper/TargetPageMapper.php
b/includes/mapper/TargetPageMapper.php
index 9083e44..e0fbaad 100644
--- a/includes/mapper/TargetPageMapper.php
+++ b/includes/mapper/TargetPageMapper.php
@@ -141,6 +141,27 @@
}
/**
+ * Delete multiple EchoTargetPage records by user & event_id offset
+ *
+ * @param User $user
+ * @param int $eventId
+ * @return boolean
+ */
+ public function deleteByUserEventOffset( User $user, $eventId ) {
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+
+ $res = $dbw->delete(
+ 'echo_target_page',
+ array(
+ 'etp_user' => $user->getId(),
+ 'etp_event < ' => (int)$eventId
+ ),
+ __METHOD__
+ );
+ return $res;
+ }
+
+ /**
* Delete multiple EchoTargetPage records by user
*
* @param User $user
diff --git a/jobs/NotificationDeleteJob.php b/jobs/NotificationDeleteJob.php
new file mode 100644
index 0000000..fb1e10e
--- /dev/null
+++ b/jobs/NotificationDeleteJob.php
@@ -0,0 +1,70 @@
+<?php
+
+/**
+ * This job is created when sending notifications to the target users. The
purpose
+ * of this job is to delete older notifications when the number of
notifications a
+ * user has is more than $wgEchoMaxUpdateCount, it does not make sense to have
tons
+ * of notifications in the history while users wouldn't bother to click 'load
more'
+ * like 100 times to see them. What we gain from this is we could run expensive
+ * queries otherwise that would requires adding index and data denormalization.
+ */
+class EchoNotificationDeleteJob extends Job {
+
+ /**
+ * UserIds to be processed
+ * @var int[]
+ */
+ protected $userIds = array();
+
+ /**
+ * @var MWEchoDbFactory
+ */
+ protected $dbFactory;
+
+ /**
+ * @param Title
+ * @param array
+ */
+ function __construct( $title, $params ) {
+ parent::__construct( __CLASS__, $title, $params );
+ $this->userIds = $params['userIds'];
+ $dbFactory = MWEchoDbFactory::newFromDefault();
+ }
+
+ /**
+ * Run the job of finding & deleting older notifications
+ */
+ function run() {
+ global $wgEchoMaxUpdateCount;
+
+ $updateCount = 0;
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+ $notifMapper = new EchoNotificationMapper();
+ $targetMapper = new EchoTargetPageMapper();
+
+ foreach ( $this->users as $userId ) {
+ $user = User::newFromId( $userId );
+ $notif = $notifMapper->FetchByUserOffset( $user,
$wgEchoMaxUpdateCount );
+ if ( $notif ) {
+ $dbw->startAtomic( __METHOD__ );
+ $res = $notifMapper->deleteByUserEventOffset(
$user, $notif->getEvent()->getId() );
+ if ( $res ) {
+ $res =
$targetMapper->deleteByUserEventOffset( $user, $notif->getEvent()->getId() );
+ }
+ $dbw->endAtomic( __METHOD__ );
+ if ( $res ) {
+ $updateCount++;
+ $notifUser =
MWEchoNotifUser::newFromUser( $user );
+ $notifUser->resetNotificationCount(
DB_MASTER );
+ }
+ // Wait for slave if we are doing a lot of
updates
+ if ( $updateCount > 10 ) {
+ $this->dbFactory->waitForSlaves();
+ $updateCount = 0;
+ }
+ }
+ }
+ return true;
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/159413
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4d4fa4c987a1732e5e29536a7669e28c34d4ab18
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Bsitu <bs...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
