Merge pull request #3514 from annando/worker-locking

Fixed locking behaviour for the worker

boot.php

@@ -40,7 +40,7 @@ define ( 'FRIENDICA_PLATFORM',     'Friendica');
 define ( 'FRIENDICA_CODENAME',     'Asparagus');
 define ( 'FRIENDICA_VERSION',      '3.5.2-rc' );
 define ( 'DFRN_PROTOCOL_VERSION',  '2.23'    );
-define ( 'DB_UPDATE_VERSION',      1225      );
+define ( 'DB_UPDATE_VERSION',      1226      );
 
 /**
  * @brief Constant with a HTML line break.

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
--- Friendica 3.5.2-dev (Asparagus)
+-- Friendica 3.5.2-rc (Asparagus)
--- DB_UPDATE_VERSION 1221
+-- DB_UPDATE_VERSION 1226
 -- ------------------------------------------
 
 
@@ -193,7 +193,7 @@ CREATE TABLE IF NOT EXISTS `contact` (
 --
 CREATE TABLE IF NOT EXISTS `conv` (
 	`id` int(10) unsigned NOT NULL auto_increment,
-	`guid` varchar(64) NOT NULL DEFAULT '',
+	`guid` varchar(255) NOT NULL DEFAULT '',
 	`recips` text,
 	`uid` int(11) NOT NULL DEFAULT 0,
 	`creator` varchar(255) NOT NULL DEFAULT '',
@@ -270,7 +270,7 @@ CREATE TABLE IF NOT EXISTS `fcontact` (
 	`updated` datetime NOT NULL DEFAULT '0001-01-01 00:00:00',
 	 PRIMARY KEY(`id`),
 	 INDEX `addr` (`addr`(32)),
-	 INDEX `url` (`url`)
+	 UNIQUE INDEX `url` (`url`(190))
 ) DEFAULT COLLATE utf8mb4_general_ci;
 
 --
@@ -355,7 +355,7 @@ CREATE TABLE IF NOT EXISTS `gcontact` (
 	`generation` tinyint(3) NOT NULL DEFAULT 0,
 	`server_url` varchar(255) NOT NULL DEFAULT '',
 	 PRIMARY KEY(`id`),
-	 INDEX `nurl` (`nurl`(64)),
+	 UNIQUE INDEX `nurl` (`nurl`(190)),
 	 INDEX `name` (`name`(64)),
 	 INDEX `nick` (`nick`(32)),
 	 INDEX `addr` (`addr`(64)),
@@ -425,7 +425,7 @@ CREATE TABLE IF NOT EXISTS `gserver` (
 	`last_contact` datetime DEFAULT '0001-01-01 00:00:00',
 	`last_failure` datetime DEFAULT '0001-01-01 00:00:00',
 	 PRIMARY KEY(`id`),
-	 INDEX `nurl` (`nurl`(32))
+	 UNIQUE INDEX `nurl` (`nurl`(190))
 ) DEFAULT COLLATE utf8mb4_general_ci;
 
 --
@@ -544,6 +544,7 @@ CREATE TABLE IF NOT EXISTS `item` (
 	 INDEX `uid_parenturi` (`uid`,`parent-uri`(190)),
 	 INDEX `uid_contactid_created` (`uid`,`contact-id`,`created`),
 	 INDEX `authorid_created` (`author-id`,`created`),
+	 INDEX `ownerid` (`owner-id`),
 	 INDEX `uid_uri` (`uid`,`uri`(190)),
 	 INDEX `resource-id` (`resource-id`),
 	 INDEX `contactid_allowcid_allowpid_denycid_denygid` (`contact-id`,`allow_cid`(10),`allow_gid`(10),`deny_cid`(10),`deny_gid`(10)),
@@ -589,7 +590,7 @@ CREATE TABLE IF NOT EXISTS `locks` (
 CREATE TABLE IF NOT EXISTS `mail` (
 	`id` int(10) unsigned NOT NULL auto_increment,
 	`uid` int(10) unsigned NOT NULL DEFAULT 0,
-	`guid` varchar(64) NOT NULL DEFAULT '',
+	`guid` varchar(255) NOT NULL DEFAULT '',
 	`from-name` varchar(255) NOT NULL DEFAULT '',
 	`from-photo` varchar(255) NOT NULL DEFAULT '',
 	`from-url` varchar(255) NOT NULL DEFAULT '',
@@ -608,7 +609,8 @@ CREATE TABLE IF NOT EXISTS `mail` (
 	 INDEX `uid_seen` (`uid`,`seen`),
 	 INDEX `convid` (`convid`),
 	 INDEX `uri` (`uri`(64)),
-	 INDEX `parent-uri` (`parent-uri`(64))
+	 INDEX `parent-uri` (`parent-uri`(64)),
+	 INDEX `contactid` (`contact-id`)
 ) DEFAULT COLLATE utf8mb4_general_ci;
 
 --
@@ -746,6 +748,7 @@ CREATE TABLE IF NOT EXISTS `photo` (
 	`deny_cid` mediumtext,
 	`deny_gid` mediumtext,
 	 PRIMARY KEY(`id`),
+	 INDEX `contactid` (`contact-id`),
 	 INDEX `uid_contactid` (`uid`,`contact-id`),
 	 INDEX `uid_profile` (`uid`,`profile`),
 	 INDEX `uid_album_scale_created` (`uid`,`album`(32),`scale`,`created`),
@@ -1019,6 +1022,9 @@ CREATE TABLE IF NOT EXISTS `thread` (
 	 INDEX `uid_network_created` (`uid`,`network`,`created`),
 	 INDEX `uid_contactid_commented` (`uid`,`contact-id`,`commented`),
 	 INDEX `uid_contactid_created` (`uid`,`contact-id`,`created`),
+	 INDEX `contactid` (`contact-id`),
+	 INDEX `ownerid` (`owner-id`),
+	 INDEX `authorid` (`author-id`),
 	 INDEX `uid_created` (`uid`,`created`),
 	 INDEX `uid_commented` (`uid`,`commented`),
 	 INDEX `uid_wall_created` (`uid`,`wall`,`created`)
@@ -1108,6 +1114,7 @@ CREATE TABLE IF NOT EXISTS `workerqueue` (
 	`created` datetime NOT NULL DEFAULT '0001-01-01 00:00:00',
 	`pid` int(11) NOT NULL DEFAULT 0,
 	`executed` datetime NOT NULL DEFAULT '0001-01-01 00:00:00',
-	 PRIMARY KEY(`id`)
+	 PRIMARY KEY(`id`),
+	 INDEX `priority_created` (`priority`,`created`)
 ) DEFAULT COLLATE utf8mb4_general_ci;
 

include/dbstructure.php

@@ -1742,6 +1742,7 @@ function db_definition() {
 					),
 			"indexes" => array(
 					"PRIMARY" => array("id"),
+					"priority_created" => array("priority", "created"),
 					)
 			);
 

include/poller.php

@@ -79,23 +79,29 @@ function poller_run($argv, $argc){
 
 		// Check free memory
 		if ($a->min_memory_reached()) {
+			logger('Memory limit reached, quitting.', LOGGER_DEBUG);
 			return;
 		}
 
 		// Count active workers and compare them with a maximum value that depends on the load
 		if (poller_too_much_workers()) {
+			logger('Active worker limit reached, quitting.', LOGGER_DEBUG);
 			return;
 		}
 
 		if (!poller_execute($r[0])) {
+			logger('Process execution failed, quitting.', LOGGER_DEBUG);
 			return;
 		}
 
 		// Quit the poller once every hour
-		if (time() > ($starttime + 3600))
+		if (time() > ($starttime + 3600)) {
+			logger('Process lifetime reachted, quitting.', LOGGER_DEBUG);
 			return;
 		}
 	}
+	logger("Couldn't select a workerqueue entry, quitting.", LOGGER_DEBUG);
+}
 
 /**
  * @brief Execute a worker entry
@@ -146,7 +152,6 @@ function poller_execute($queue) {
 	if (function_exists($funcname)) {
 
 		poller_exec_function($queue, $funcname, $argv);
-
 		dba::delete('workerqueue', array('id' => $queue["id"]));
 	} else {
 		logger("Function ".$funcname." does not exist");
@@ -400,6 +405,8 @@ function poller_too_much_workers() {
 
 	$maxqueues = $queues;
 
+	$active = poller_active_workers();
+
 	// Decrease the number of workers at higher load
 	$load = current_load();
 	if ($load) {
@@ -412,8 +419,6 @@ function poller_too_much_workers() {
 		$slope = $maxworkers / pow($maxsysload, $exponent);
 		$queues = ceil($slope * pow(max(0, $maxsysload - $load), $exponent));
 
-		$active = 0;
-
 		// Create a list of queue entries grouped by their priority
 		$listitem = array();
 
@@ -421,17 +426,15 @@ function poller_too_much_workers() {
 		$processes = dba::p("SELECT COUNT(*) AS `running` FROM `process` WHERE NOT EXISTS (SELECT id FROM `workerqueue` WHERE `workerqueue`.`pid` = `process`.`pid`)");
 		if ($process = dba::fetch($processes)) {
 			$listitem[0] = "0:".$process["running"];
-			$active += $process["running"];
 		}
 		dba::close($processes);
 
 		// Now adding all processes with workerqueue entries
 		$entries = dba::p("SELECT COUNT(*) AS `entries`, `priority` FROM `workerqueue` GROUP BY `priority`");
 		while ($entry = dba::fetch($entries)) {
-			$processes = dba::p("SELECT COUNT(*) AS `running` FROM `process` LEFT JOIN `workerqueue` ON `workerqueue`.`pid` = `process`.`pid` WHERE `priority` = ?", $entry["priority"]);
+			$processes = dba::p("SELECT COUNT(*) AS `running` FROM `process` INNER JOIN `workerqueue` ON `workerqueue`.`pid` = `process`.`pid` WHERE `priority` = ?", $entry["priority"]);
 			if ($process = dba::fetch($processes)) {
 				$listitem[$entry["priority"]] = $entry["priority"].":".$process["running"]."/".$entry["entries"];
-				$active += $process["running"];
 			}
 			dba::close($processes);
 		}
@@ -465,8 +468,6 @@ function poller_too_much_workers() {
 			$a = get_app();
 			$a->proc_run($args);
 		}
-	} else {
-		$active = poller_active_workers();
 	}
 
 	return($active >= $queues);
@@ -540,16 +541,16 @@ function poller_passing_slow(&$highest_priority) {
  */
 function poller_worker_process() {
 
-	dba::transaction();
-
 	// Check if we should pass some low priority process
 	$highest_priority = 0;
 
 	if (poller_passing_slow($highest_priority)) {
+		dba::p('LOCK TABLES `workerqueue` WRITE');
+
 		// Are there waiting processes with a higher priority than the currently highest?
 		$r = q("SELECT * FROM `workerqueue`
 				WHERE `executed` <= '%s' AND `priority` < %d
-				ORDER BY `priority`, `created` LIMIT 1 FOR UPDATE",
+				ORDER BY `priority`, `created` LIMIT 1",
 				dbesc(NULL_DATE),
 				intval($highest_priority));
 		if (dbm::is_result($r)) {
@@ -558,18 +559,25 @@ function poller_worker_process() {
 		// Give slower processes some processing time
 		$r = q("SELECT * FROM `workerqueue`
 				WHERE `executed` <= '%s' AND `priority` > %d
-				ORDER BY `priority`, `created` LIMIT 1 FOR UPDATE",
+				ORDER BY `priority`, `created` LIMIT 1",
 				dbesc(NULL_DATE),
 				intval($highest_priority));
 
 		if (dbm::is_result($r)) {
 			return $r;
 		}
+	} else {
+		dba::p('LOCK TABLES `workerqueue` WRITE');
 	}
 
 	// If there is no result (or we shouldn't pass lower processes) we check without priority limit
 	if (!dbm::is_result($r)) {
-		$r = q("SELECT * FROM `workerqueue` WHERE `executed` <= '%s' ORDER BY `priority`, `created` LIMIT 1 FOR UPDATE", dbesc(NULL_DATE));
+		$r = q("SELECT * FROM `workerqueue` WHERE `executed` <= '%s' ORDER BY `priority`, `created` LIMIT 1", dbesc(NULL_DATE));
+	}
+
+	// We only unlock the tables here, when we got no data
+	if (!dbm::is_result($r)) {
+		dba::p('UNLOCK TABLES');
 	}
 
 	return $r;
@@ -578,7 +586,7 @@ function poller_worker_process() {
 /**
  * @brief Assigns a workerqueue entry to the current process
  *
- * All the checks after the update are only needed with MyISAM.
+ * When we are sure that the table locks are working correctly, we can remove the checks from here
  *
  * @param array $queue Workerqueue entry
  *
@@ -587,10 +595,12 @@ function poller_worker_process() {
 function poller_claim_process($queue) {
 	$mypid = getmypid();
 
-	if (!dba::update('workerqueue', array('executed' => datetime_convert(), 'pid' => $mypid),
+	$success = dba::update('workerqueue', array('executed' => datetime_convert(), 'pid' => $mypid),
-			array('id' => $queue["id"], 'pid' => 0))) {
+			array('id' => $queue["id"], 'pid' => 0));
+	dba::p('UNLOCK TABLES');
+
+	if (!$success) {
 		logger("Couldn't update queue entry ".$queue["id"]." - skip this execution", LOGGER_DEBUG);
-		dba::commit();
 		return false;
 	}
 
@@ -598,18 +608,14 @@ function poller_claim_process($queue) {
 	$id = q("SELECT `pid`, `executed` FROM `workerqueue` WHERE `id` = %d", intval($queue["id"]));
 	if (!$id) {
 		logger("Queue item ".$queue["id"]." vanished - skip this execution", LOGGER_DEBUG);
-		dba::commit();
 		return false;
 	} elseif ((strtotime($id[0]["executed"]) <= 0) OR ($id[0]["pid"] == 0)) {
 		logger("Entry for queue item ".$queue["id"]." wasn't stored - skip this execution", LOGGER_DEBUG);
-		dba::commit();
 		return false;
 	} elseif ($id[0]["pid"] != $mypid) {
 		logger("Queue item ".$queue["id"]." is to be executed by process ".$id[0]["pid"]." and not by me (".$mypid.") - skip this execution", LOGGER_DEBUG);
-		dba::commit();
 		return false;
 	}
-	dba::commit();
 	return true;
 }
 

mod/ping.php

@@ -9,6 +9,7 @@ require_once('include/group.php');
 require_once('mod/proxy.php');
 require_once('include/xml.php');
 require_once('include/cache.php');
+require_once('include/enotify.php');
 
 /**
  * @brief Outputs the counts and the lists of various notifications

src/App.php

@@ -900,12 +900,12 @@ class App {
 			return;
 		}
 
-		// If the last worker fork was less than 10 seconds before then don't fork another one.
+		// If the last worker fork was less than 2 seconds before then don't fork another one.
 		// This should prevent the forking of masses of workers.
 		$cachekey = 'app:proc_run:started';
 		$result = Cache::get($cachekey);
 
-		if (!is_null($result) AND ( time() - $result) < 10) {
+		if (!is_null($result) AND ( time() - $result) < 2) {
 			return;
 		}
 

update.php

@@ -1,6 +1,6 @@
 <?php
 
-define('UPDATE_VERSION' , 1225);
+define('UPDATE_VERSION' , 1226);
 
 /**
  *