From 61394aa8ac3585c9b7f1e15b6c581af01854987d Mon Sep 17 00:00:00 2001
From: Brion Vibber <brion@pobox.com>
Date: Fri, 2 Apr 2010 15:39:43 -0700
Subject: Don't save duplicate messages into a user's packed inbox. We've
 already got the packed box loaded at insert time, so we can simply unpack it
 and check before doing the update query. Should help with dupes that come in
 when inbox distrib jobs die and get restarted, etc.

---
 classes/Inbox.php | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

(limited to 'classes/Inbox.php')

diff --git a/classes/Inbox.php b/classes/Inbox.php
index 014ba3d82..2533210b7 100644
--- a/classes/Inbox.php
+++ b/classes/Inbox.php
@@ -96,12 +96,23 @@ class Inbox extends Memcached_DataObject
         $inbox = new Inbox();
 
         $inbox->user_id = $user_id;
-        $inbox->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids));
+        $inbox->pack($ids);
         $inbox->fake = true;
 
         return $inbox;
     }
 
+    /**
+     * Append the given notice to the given user's inbox.
+     * Caching updates are managed for the inbox itself.
+     *
+     * If the notice is already in this inbox, the second
+     * add will be silently dropped.
+     *
+     * @param int @user_id
+     * @param int $notice_id
+     * @return boolean success
+     */
     static function insertNotice($user_id, $notice_id)
     {
         $inbox = DB_DataObject::staticGet('inbox', 'user_id', $user_id);
@@ -114,6 +125,13 @@ class Inbox extends Memcached_DataObject
             return false;
         }
 
+        $ids = $inbox->unpack();
+        if (in_array(intval($notice_id), $ids)) {
+            // Already in there, we probably re-ran some inbox adds
+            // due to an error. Skip the dupe silently.
+            return true;
+        }
+
         $result = $inbox->query(sprintf('UPDATE inbox '.
                                         'set notice_ids = concat(cast(0x%08x as binary(4)), '.
                                         'substr(notice_ids, 1, %d)) '.
@@ -150,7 +168,7 @@ class Inbox extends Memcached_DataObject
             }
         }
 
-        $ids = unpack('N*', $inbox->notice_ids);
+        $ids = $inbox->unpack();
 
         if (!empty($since_id)) {
             $newids = array();
@@ -229,4 +247,21 @@ class Inbox extends Memcached_DataObject
         }
         return new ArrayWrapper($items);
     }
+
+    /**
+     * Saves a list of integer notice_ids into a packed blob in this object.
+     * @param array $ids list of integer notice_ids
+     */
+    protected function pack(array $ids)
+    {
+        $this->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids));
+    }
+
+    /**
+     * @return array of integer notice_ids
+     */
+    protected function unpack()
+    {
+        return unpack('N*', $this->notice_ids);
+    }
 }
-- 
cgit v1.2.3-54-g00ecf


From 17ab15a3d02c335f2d9d333ac3773c037e796cf5 Mon Sep 17 00:00:00 2001
From: Brion Vibber <brion@pobox.com>
Date: Tue, 1 Jun 2010 13:53:44 -0700
Subject: Fix memory leak in Inbox::addToInbox() (usage of raw
 DB_DataObject::staticGet, which leaks memory into a process-global cache). On
 my test setup, this fixes inbox delivery to 10,000 local recipients from
 background queuedaemon running with a 32mb memory limit, completes the job
 within a minute from start.

---
 classes/Inbox.php | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

(limited to 'classes/Inbox.php')

diff --git a/classes/Inbox.php b/classes/Inbox.php
index 2533210b7..430419ba5 100644
--- a/classes/Inbox.php
+++ b/classes/Inbox.php
@@ -115,9 +115,12 @@ class Inbox extends Memcached_DataObject
      */
     static function insertNotice($user_id, $notice_id)
     {
-        $inbox = DB_DataObject::staticGet('inbox', 'user_id', $user_id);
-
-        if (empty($inbox)) {
+		// Going straight to the DB rather than trusting our caching
+		// during an update. Note: not using DB_DataObject::staticGet,
+		// which is unsafe to use directly (in-process caching causes
+		// memory leaks, which accumulate in queue processes).
+        $inbox = new Inbox();
+        if (!$inbox->get('user_id', $user_id)) {
             $inbox = Inbox::initialize($user_id);
         }
 
-- 
cgit v1.2.3-54-g00ecf