Merge pull request #5077 from annando/fix-blocking-ostatus

Fix to OStatus delivery to be not so blocking to other tasks

boot.php

@@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM',     'Friendica');
 define('FRIENDICA_CODENAME',     'The Tazmans Flax-lily');
 define('FRIENDICA_VERSION',      '2018.05-rc');
 define('DFRN_PROTOCOL_VERSION',  '2.23');
-define('DB_UPDATE_VERSION',      1262);
+define('DB_UPDATE_VERSION',      1263);
 define('NEW_UPDATE_ROUTINE_VERSION', 1170);
 
 /**

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
--- Friendica 2018-05-dev (The Tazmans Flax-lily)
+-- Friendica 2018.05-rc (The Tazmans Flax-lily)
--- DB_UPDATE_VERSION 1259
+-- DB_UPDATE_VERSION 1263
 -- ------------------------------------------
 
 
@@ -855,10 +855,12 @@ CREATE TABLE IF NOT EXISTS `push_subscriber` (
 	`callback_url` varchar(255) NOT NULL DEFAULT '' COMMENT '',
 	`topic` varchar(255) NOT NULL DEFAULT '' COMMENT '',
 	`nickname` varchar(255) NOT NULL DEFAULT '' COMMENT '',
-	`push` tinyint unsigned NOT NULL DEFAULT 0 COMMENT '',
+	`push` tinyint unsigned NOT NULL DEFAULT 0 COMMENT 'Retrial counter',
-	`last_update` datetime NOT NULL DEFAULT '0001-01-01 00:00:00' COMMENT '',
+	`last_update` datetime NOT NULL DEFAULT '0001-01-01 00:00:00' COMMENT 'Date of last successful trial',
+	`next_try` datetime NOT NULL DEFAULT '0001-01-01 00:00:00' COMMENT 'Next retrial date',
 	`secret` varchar(255) NOT NULL DEFAULT '' COMMENT '',
-	 PRIMARY KEY(`id`)
+	 PRIMARY KEY(`id`),
+	 INDEX `next_try` (`next_try`)
 ) DEFAULT COLLATE utf8mb4_general_ci;
 
 --
@@ -1088,7 +1090,7 @@ CREATE TABLE IF NOT EXISTS `workerqueue` (
 	 INDEX `pid` (`pid`),
 	 INDEX `parameter` (`parameter`(64)),
 	 INDEX `priority_created` (`priority`,`created`),
-	 INDEX `executed` (`executed`)
+	 INDEX `done_executed` (`done`,`executed`)
 ) DEFAULT COLLATE utf8mb4_general_ci;
 
 

src/Database/DBStructure.php

@@ -1553,12 +1553,14 @@ class DBStructure
 						"callback_url" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""],
 						"topic" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""],
 						"nickname" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""],
-						"push" => ["type" => "tinyint unsigned", "not null" => "1", "default" => "0", "comment" => ""],
+						"push" => ["type" => "tinyint unsigned", "not null" => "1", "default" => "0", "comment" => "Retrial counter"],
-						"last_update" => ["type" => "datetime", "not null" => "1", "default" => NULL_DATE, "comment" => ""],
+						"last_update" => ["type" => "datetime", "not null" => "1", "default" => NULL_DATE, "comment" => "Date of last successful trial"],
+						"next_try" => ["type" => "datetime", "not null" => "1", "default" => NULL_DATE, "comment" => "Next retrial date"],
 						"secret" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""],
 						],
 				"indexes" => [
 						"PRIMARY" => ["id"],
+						"next_try" => ["next_try"],
 						]
 				];
 		$database["queue"] = [

src/Model/PushSubscriber.php

@@ -0,0 +1,37 @@
+<?php
+/**
+ * @file src/Model/PushSubscriber.php
+ */
+namespace Friendica\Model;
+
+use Friendica\Core\Worker;
+use dba;
+
+require_once 'include/dba.php';
+
+class PushSubscriber
+{
+	/**
+	 * @param string $priority Priority for push workers
+	 */
+	public static function publishFeed($default_priority = PRIORITY_HIGH)
+	{
+		// We'll push to each subscriber that has push > 0,
+		// i.e. there has been an update (set in notifier.php).
+		$subscribers = dba::select('push_subscriber', ['id', 'push', 'callback_url'], ["`push` > 0 AND `next_try` < UTC_TIMESTAMP()"]);
+
+		while ($subscriber = dba::fetch($subscribers)) {
+			// We always handle retries with low priority
+			if ($subscriber["push"] > 1) {
+				$priority = PRIORITY_LOW;
+			} else {
+				$priority = $default_priority;
+			}
+
+			logger("Publish feed to " . $subscriber["callback_url"] . " with priority " . $priority, LOGGER_DEBUG);
+			Worker::add($priority, 'PubSubPublish', (int)$subscriber["id"]);
+		}
+
+		dba::close($subscribers);
+	}
+}

src/Worker/Notifier.php

@@ -11,6 +11,7 @@ use Friendica\Database\DBM;
 use Friendica\Model\Contact;
 use Friendica\Model\Group;
 use Friendica\Model\User;
+use Friendica\Model\PushSubscriber;
 use Friendica\Network\Probe;
 use Friendica\Protocol\Diaspora;
 use Friendica\Protocol\OStatus;
@@ -501,13 +502,12 @@ class Notifier {
 			// Set push flag for PuSH subscribers to this topic,
 			// they will be notified in queue.php
 			$condition = ['push' => false, 'nickname' => $owner['nickname']];
-			dba::update('push_subscriber', ['push' => true], $condition);
+			dba::update('push_subscriber', ['push' => true, 'next_try' => NULL_DATE], $condition);
 
 			logger('Activating internal PuSH for item '.$item_id, LOGGER_DEBUG);
 
 			// Handling the pubsubhubbub requests
-			Worker::add(['priority' => PRIORITY_HIGH, 'created' => $a->queue['created'], 'dont_fork' => true],
+			PushSubscriber::publishFeed($a->queue['priority']);
-					'PubSubPublish');
 		}
 
 		logger('notifier: calling hooks for ' . $cmd . ' ' . $item_id, LOGGER_DEBUG);

src/Worker/PubSubPublish.php

@@ -12,6 +12,7 @@ use Friendica\Core\Worker;
 use Friendica\Database\DBM;
 use Friendica\Protocol\OStatus;
 use Friendica\Util\Network;
+use Friendica\Util\DateTimeFormat;
 use dba;
 
 require_once 'include/items.php';
@@ -19,78 +20,75 @@ require_once 'include/items.php';
 class PubSubPublish {
 	public static function execute($pubsubpublish_id = 0)
 	{
-		global $a;
-
 		if ($pubsubpublish_id == 0) {
-			// We'll push to each subscriber that has push > 0,
+			return;
-			// i.e. there has been an update (set in notifier.php).
-			$r = q("SELECT `id`, `callback_url` FROM `push_subscriber` WHERE `push` > 0 ORDER BY `last_update` DESC");
-
-			foreach ($r as $rr) {
-				logger("Publish feed to ".$rr["callback_url"], LOGGER_DEBUG);
-				Worker::add(['priority' => PRIORITY_HIGH, 'created' => $a->queue['created'], 'dont_fork' => true],
-						'PubSubPublish', (int)$rr["id"]);
-			}
 		}
 
 		self::publish($pubsubpublish_id);
-
-		return;
 	}
 
 	private static function publish($id) {
 		global $a;
 
-		$r = q("SELECT * FROM `push_subscriber` WHERE `id` = %d", intval($id));
+		$subscriber = dba::selectFirst('push_subscriber', [], ['id' => $id]);
-		if (!DBM::is_result($r)) {
+		if (!DBM::is_result($subscriber)) {
 			return;
 		}
 
-		$rr = $r[0];
-
 		/// @todo Check server status with PortableContact::checkServer()
 		// Before this can be done we need a way to safely detect the server url.
 
-		logger("Generate feed of user ".$rr['nickname']." to ".$rr['callback_url']." - last updated ".$rr['last_update'], LOGGER_DEBUG);
+		logger("Generate feed of user " . $subscriber['nickname']. " to " . $subscriber['callback_url']. " - last updated " . $subscriber['last_update'], LOGGER_DEBUG);
 
-		$last_update = $rr['last_update'];
+		$last_update = $subscriber['last_update'];
-		$params = OStatus::feed($rr['nickname'], $last_update);
+		$params = OStatus::feed($subscriber['nickname'], $last_update);
 
 		if (!$params) {
 			return;
 		}
 
-		$hmac_sig = hash_hmac("sha1", $params, $rr['secret']);
+		$hmac_sig = hash_hmac("sha1", $params, $subscriber['secret']);
 
 		$headers = ["Content-type: application/atom+xml",
 				sprintf("Link: <%s>;rel=hub,<%s>;rel=self",
-					System::baseUrl().'/pubsubhubbub/'.$rr['nickname'],
+					System::baseUrl() . '/pubsubhubbub/' . $subscriber['nickname'],
-					$rr['topic']),
+					$subscriber['topic']),
-				"X-Hub-Signature: sha1=".$hmac_sig];
+				"X-Hub-Signature: sha1=" . $hmac_sig];
 
-		logger('POST '.print_r($headers, true)."\n".$params, LOGGER_DATA);
+		logger('POST ' . print_r($headers, true) . "\n" . $params, LOGGER_DATA);
 
-		Network::post($rr['callback_url'], $params, $headers);
+		Network::post($subscriber['callback_url'], $params, $headers);
 		$ret = $a->get_curl_code();
 
+		$condition = ['id' => $subscriber['id']];
+
 		if ($ret >= 200 && $ret <= 299) {
-			logger('successfully pushed to '.$rr['callback_url']);
+			logger('Successfully pushed to ' . $subscriber['callback_url']);
 
 			// set last_update to the "created" date of the last item, and reset push=0
-			$fields = ['push' => 0, 'last_update' => $last_update];
+			$fields = ['push' => 0, 'next_try' => NULL_DATE, 'last_update' => $last_update];
-			dba::update('push_subscriber', $fields, ['id' => $rr['id']]);
+			dba::update('push_subscriber', $fields, $condition);
 
 		} else {
-			logger('error when pushing to '.$rr['callback_url'].' HTTP: '.$ret);
+			logger('Delivery error when pushing to ' . $subscriber['callback_url'] . ' HTTP: ' . $ret);
 
 			// we use the push variable also as a counter, if we failed we
 			// increment this until some upper limit where we give up
-			$new_push = intval($rr['push']) + 1;
+			$retrial = $subscriber['push'];
 
-			if ($new_push > 30) // OK, let's give up
+			if ($retrial > 14) {
-				$new_push = 0;
+				dba::update('push_subscriber', ['push' => 0, 'next_try' => NULL_DATE], $condition);
+				logger('Delivery error: Giving up for ' . $subscriber['callback_url'], LOGGER_DEBUG);
+			} else {
+				// Calculate the delay until the next trial
+				$delay = (($retrial + 3) ** 4) + (rand(1, 30) * ($retrial + 1));
+				$next = DateTimeFormat::utc('now + ' . $delay . ' seconds');
 
-			dba::update('push_subscriber', ['push' => $new_push], ['id' => $rr['id']]);
+				$retrial = $retrial + 1;
+
+				dba::update('push_subscriber', ['push' => $retrial, 'next_try' => $next], $condition);
+				logger('Delivery error: Next try (' . $retrial . ') for ' . $subscriber['callback_url'] . ' at ' . $next, LOGGER_DEBUG);
+			}
 		}
 	}
 }

src/Worker/Queue.php

@@ -10,6 +10,7 @@ use Friendica\Core\Config;
 use Friendica\Core\Worker;
 use Friendica\Database\DBM;
 use Friendica\Model\Queue as QueueModel;
+use Friendica\Model\PushSubscriber;
 use Friendica\Protocol\DFRN;
 use Friendica\Protocol\Diaspora;
 use Friendica\Protocol\PortableContact;
@@ -34,7 +35,7 @@ class Queue
 			logger('filling queue jobs - start');
 
 			// Handling the pubsubhubbub requests
-			Worker::add(['priority' => PRIORITY_HIGH, 'dont_fork' => true], 'PubSubPublish');
+			PushSubscriber::publishFeed();
 
 			$r = dba::inArray(dba::p("SELECT `id` FROM `queue` WHERE `next` < UTC_TIMESTAMP() ORDER BY `batch`, `cid`"));