From 21eb9a4b2e1bcae51e389df84bba6100b825eb69 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 3 Jun 2017 07:25:01 +0000 Subject: [PATCH] Fixed locking behaviour for the worker --- boot.php | 2 +- database.sql | 25 +++++++++++++-------- include/dbstructure.php | 1 + include/poller.php | 50 +++++++++++++++++++++++------------------ mod/ping.php | 1 + src/App.php | 4 ++-- update.php | 2 +- 7 files changed, 50 insertions(+), 35 deletions(-) diff --git a/boot.php b/boot.php index 48381f5435..9474d2d6c5 100644 --- a/boot.php +++ b/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. diff --git a/database.sql b/database.sql index 73547b3058..2b40552eb7 100644 --- a/database.sql +++ b/database.sql @@ -1,6 +1,6 @@ -- ------------------------------------------ --- Friendica 3.5.2-dev (Asparagus) --- DB_UPDATE_VERSION 1221 +-- Friendica 3.5.2-rc (Asparagus) +-- 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; diff --git a/include/dbstructure.php b/include/dbstructure.php index 413395905d..441e7be7f7 100644 --- a/include/dbstructure.php +++ b/include/dbstructure.php @@ -1742,6 +1742,7 @@ function db_definition() { ), "indexes" => array( "PRIMARY" => array("id"), + "priority_created" => array("priority", "created"), ) ); diff --git a/include/poller.php b/include/poller.php index 0c5bebbda5..22fb65a4f8 100644 --- a/include/poller.php +++ b/include/poller.php @@ -79,22 +79,28 @@ 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); } /** @@ -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), - array('id' => $queue["id"], 'pid' => 0))) { + $success = dba::update('workerqueue', array('executed' => datetime_convert(), 'pid' => $mypid), + 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; } diff --git a/mod/ping.php b/mod/ping.php index 17180c74ee..8ebf4add38 100644 --- a/mod/ping.php +++ b/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 diff --git a/src/App.php b/src/App.php index aaaf6b2451..d671c5f1ab 100644 --- a/src/App.php +++ b/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; } diff --git a/update.php b/update.php index 9c509be4b7..695ed39919 100644 --- a/update.php +++ b/update.php @@ -1,6 +1,6 @@