From 17ab15a3d02c335f2d9d333ac3773c037e796cf5 Mon Sep 17 00:00:00 2001 From: Brion Vibber 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') 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 From 5f4c6ec626d3d641f0712b276deb32b218b7a330 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 3 Jun 2010 16:58:45 -0700 Subject: Skip enqueueing to outgoing bridges on incoming remote messages. Twitter, Facebook, RSSCloud, and OStatus checks were enqueued on these when they'd never do anything but churn the queue servers. Notice::isLocal() can replace a number of manual checks for $notice->is_local being LOCAL_PUBLIC or LOCAL_NONPUBLIC. --- classes/Notice.php | 12 ++++++++++++ lib/util.php | 5 ++--- plugins/Facebook/FacebookPlugin.php | 2 +- plugins/OStatus/OStatusPlugin.php | 6 ++++-- plugins/RSSCloud/RSSCloudPlugin.php | 18 +++--------------- plugins/TwitterBridge/TwitterBridgePlugin.php | 2 +- 6 files changed, 23 insertions(+), 22 deletions(-) (limited to 'classes') diff --git a/classes/Notice.php b/classes/Notice.php index 3d7d21533..cda632885 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -1861,4 +1861,16 @@ class Notice extends Memcached_DataObject return $ns; } + /** + * Determine whether the notice was locally created + * + * @return boolean locality + */ + + public function isLocal() + { + return ($this->is_local == Notice::LOCAL_PUBLIC || + $this->is_local == Notice::LOCAL_NONPUBLIC); + } + } diff --git a/lib/util.php b/lib/util.php index 59d5132ec..049001aba 100644 --- a/lib/util.php +++ b/lib/util.php @@ -1235,9 +1235,8 @@ function common_enqueue_notice($notice) $transports[] = 'jabber'; } - // @fixme move these checks into QueueManager and/or individual handlers - if ($notice->is_local == Notice::LOCAL_PUBLIC || - $notice->is_local == Notice::LOCAL_NONPUBLIC) { + // We can skip these for gatewayed notices. + if ($notice->isLocal()) { $transports = array_merge($transports, $localTransports); if ($xmpp) { $transports[] = 'public'; diff --git a/plugins/Facebook/FacebookPlugin.php b/plugins/Facebook/FacebookPlugin.php index 5dba73a5d..19989a952 100644 --- a/plugins/Facebook/FacebookPlugin.php +++ b/plugins/Facebook/FacebookPlugin.php @@ -585,7 +585,7 @@ class FacebookPlugin extends Plugin function onStartEnqueueNotice($notice, &$transports) { - if (self::hasKeys()) { + if (self::hasKeys() && $notice->isLocal()) { array_push($transports, 'facebook'); } return true; diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 5b153216e..5a657c83d 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -102,8 +102,10 @@ class OStatusPlugin extends Plugin */ function onStartEnqueueNotice($notice, &$transports) { - // put our transport first, in case there's any conflict (like OMB) - array_unshift($transports, 'ostatus'); + if ($notice->isLocal()) { + // put our transport first, in case there's any conflict (like OMB) + array_unshift($transports, 'ostatus'); + } return true; } diff --git a/plugins/RSSCloud/RSSCloudPlugin.php b/plugins/RSSCloud/RSSCloudPlugin.php index 661c32141..c1951cdbf 100644 --- a/plugins/RSSCloud/RSSCloudPlugin.php +++ b/plugins/RSSCloud/RSSCloudPlugin.php @@ -192,24 +192,12 @@ class RSSCloudPlugin extends Plugin function onStartEnqueueNotice($notice, &$transports) { - array_push($transports, 'rsscloud'); + if ($notice->isLocal()) { + array_push($transports, 'rsscloud'); + } return true; } - /** - * Determine whether the notice was locally created - * - * @param Notice $notice the notice in question - * - * @return boolean locality - */ - - function _isLocal($notice) - { - return ($notice->is_local == Notice::LOCAL_PUBLIC || - $notice->is_local == Notice::LOCAL_NONPUBLIC); - } - /** * Create the rsscloud_subscription table if it's not * already in the DB diff --git a/plugins/TwitterBridge/TwitterBridgePlugin.php b/plugins/TwitterBridge/TwitterBridgePlugin.php index 1a0a69682..65b3a6b38 100644 --- a/plugins/TwitterBridge/TwitterBridgePlugin.php +++ b/plugins/TwitterBridge/TwitterBridgePlugin.php @@ -221,7 +221,7 @@ class TwitterBridgePlugin extends Plugin */ function onStartEnqueueNotice($notice, &$transports) { - if (self::hasKeys()) { + if (self::hasKeys() && $notice->isLocal()) { // Avoid a possible loop if ($notice->source != 'twitter') { array_push($transports, 'twitter'); -- cgit v1.2.3-54-g00ecf From 8b9436e8ae1ebcc7ef10752bb9666939200e26aa Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 3 Jun 2010 17:49:20 -0700 Subject: Option to divert PuSH items directly to the target site's queue when local --- classes/Status_network.php | 29 ++++++++++++++++++++--------- lib/stompqueuemanager.php | 9 +++++---- plugins/OStatus/classes/HubSub.php | 31 +++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 13 deletions(-) (limited to 'classes') diff --git a/classes/Status_network.php b/classes/Status_network.php index a452c32ce..4a1f2c374 100644 --- a/classes/Status_network.php +++ b/classes/Status_network.php @@ -149,21 +149,15 @@ class Status_network extends Safe_DataObject $this->decache(); # while we still have the values! return parent::delete(); } - + /** * @param string $servername hostname - * @param string $pathname URL base path * @param string $wildcard hostname suffix to match wildcard config + * @return mixed Status_network or null */ - static function setupSite($servername, $pathname, $wildcard) + static function getFromHostname($servername, $wildcard) { - global $config; - $sn = null; - - // XXX I18N, probably not crucial for hostnames - // XXX This probably needs a tune up - if (0 == strncasecmp(strrev($wildcard), strrev($servername), strlen($wildcard))) { // special case for exact match if (0 == strcasecmp($servername, $wildcard)) { @@ -182,6 +176,23 @@ class Status_network extends Safe_DataObject } } } + return $sn; + } + + /** + * @param string $servername hostname + * @param string $pathname URL base path + * @param string $wildcard hostname suffix to match wildcard config + */ + static function setupSite($servername, $pathname, $wildcard) + { + global $config; + + $sn = null; + + // XXX I18N, probably not crucial for hostnames + // XXX This probably needs a tune up + $sn = self::getFromHostname($servername, $wildcard); if (!empty($sn)) { diff --git a/lib/stompqueuemanager.php b/lib/stompqueuemanager.php index de4ba7f01..91faa8c36 100644 --- a/lib/stompqueuemanager.php +++ b/lib/stompqueuemanager.php @@ -115,11 +115,12 @@ class StompQueueManager extends QueueManager * * @param mixed $object * @param string $queue + * @param string $siteNickname optional override to drop into another site's queue * * @return boolean true on success * @throws StompException on connection or send error */ - public function enqueue($object, $queue) + public function enqueue($object, $queue, $siteNickname=null) { $this->_connect(); if (common_config('queue', 'stomp_enqueue_on')) { @@ -134,7 +135,7 @@ class StompQueueManager extends QueueManager } else { $idx = $this->defaultIdx; } - return $this->_doEnqueue($object, $queue, $idx); + return $this->_doEnqueue($object, $queue, $idx, $siteNickname); } /** @@ -144,10 +145,10 @@ class StompQueueManager extends QueueManager * @return boolean true on success * @throws StompException on connection or send error */ - protected function _doEnqueue($object, $queue, $idx) + protected function _doEnqueue($object, $queue, $idx, $siteNickname=null) { $rep = $this->logrep($object); - $envelope = array('site' => common_config('site', 'nickname'), + $envelope = array('site' => $siteNickname ? $siteNickname : common_config('site', 'nickname'), 'handler' => $queue, 'payload' => $this->encode($object)); $msg = serialize($envelope); diff --git a/plugins/OStatus/classes/HubSub.php b/plugins/OStatus/classes/HubSub.php index cdace3c1f..9748b4a56 100644 --- a/plugins/OStatus/classes/HubSub.php +++ b/plugins/OStatus/classes/HubSub.php @@ -260,6 +260,37 @@ class HubSub extends Memcached_DataObject $retries = intval(common_config('ostatus', 'hub_retries')); } + if (common_config('ostatus', 'local_push_bypass')) { + // If target is a local site, bypass the web server and drop the + // item directly into the target's input queue. + $url = parse_url($this->callback); + $wildcard = common_config('ostatus', 'local_wildcard'); + $site = Status_network::getFromHostname($url['host'], $wildcard); + + if ($site) { + if ($this->secret) { + $hmac = 'sha1=' . hash_hmac('sha1', $atom, $this->secret); + } else { + $hmac = ''; + } + + // Hack: at the moment we stick the subscription ID in the callback + // URL so we don't have to look inside the Atom to route the subscription. + // For now this means we need to extract that from the target URL + // so we can include it in the data. + $parts = explode('/', $url['path']); + $subId = intval(array_pop($parts)); + + $data = array('feedsub_id' => $subId, + 'post' => $atom, + 'hmac' => $hmac); + common_log(LOG_DEBUG, "Cross-site PuSH bypass enqueueing straight to $site->nickname feed $subId"); + $qm = QueueManager::get(); + $qm->enqueue($data, 'pushin', $site->nickname); + return; + } + } + // We dare not clone() as when the clone is discarded it'll // destroy the result data for the parent query. // @fixme use clone() again when it's safe to copy an -- cgit v1.2.3-54-g00ecf