mirror of
https://github.com/phpbb/phpbb.git
synced 2025-06-07 20:08:53 +00:00
[feature/system-cron] Abstract the database locking mechanism out of cron.
Added a number of tests for the locking mechanism which can now lock arbitrary config variables. PHPBB3-9596
This commit is contained in:
Nils Adermann
Oleg Pudeyev
parent
6e5e4721d8
commit
3a3a8bb96d
5 changed files with 243 additions and 109 deletions
|
|
@ -33,9 +33,9 @@ function output_image()
|
|||
// flush();
|
||||
}
|
||||
|
||||
function do_cron($run_tasks)
|
||||
function do_cron($cron_lock, $run_tasks)
|
||||
{
|
||||
global $cron_lock, $config;
|
||||
global $config;
|
||||
|
||||
foreach ($run_tasks as $task)
|
||||
{
|
||||
|
|
@ -73,7 +73,7 @@ else
|
|||
output_image();
|
||||
}
|
||||
|
||||
$cron_lock = new phpbb_cron_lock();
|
||||
$cron_lock = new phpbb_lock_db('cron_lock', $config, $db);
|
||||
if ($cron_lock->lock())
|
||||
{
|
||||
if ($config['use_system_cron'])
|
||||
|
|
@ -103,11 +103,11 @@ if ($cron_lock->lock())
|
|||
}
|
||||
if ($use_shutdown_function)
|
||||
{
|
||||
register_shutdown_function('do_cron', $run_tasks);
|
||||
register_shutdown_function('do_cron', $cron_lock, $run_tasks);
|
||||
}
|
||||
else
|
||||
{
|
||||
do_cron($run_tasks);
|
||||
do_cron($cron_lock, $run_tasks);
|
||||
}
|
||||
}
|
||||
else
|
||||
|
|
|
|||
|
|
@ -1,104 +0,0 @@
|
|||
<?php
|
||||
/**
|
||||
*
|
||||
* @package phpBB3
|
||||
* @copyright (c) 2010 phpBB Group
|
||||
* @license http://opensource.org/licenses/gpl-license.php GNU Public License
|
||||
*
|
||||
*/
|
||||
|
||||
/**
|
||||
* @ignore
|
||||
*/
|
||||
if (!defined('IN_PHPBB'))
|
||||
{
|
||||
exit;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cron lock class
|
||||
* @package phpBB3
|
||||
*/
|
||||
class phpbb_cron_lock
|
||||
{
|
||||
/**
|
||||
* Unique identifier for this lock.
|
||||
*
|
||||
* @var string
|
||||
*/
|
||||
private $cron_id;
|
||||
|
||||
/**
|
||||
* Tries to acquire the cron lock by updating
|
||||
* the 'cron_lock' configuration variable in the database.
|
||||
*
|
||||
* As a lock may only be held by one process at a time, lock
|
||||
* acquisition may fail if another process is holding the lock
|
||||
* or if another process obtained the lock but never released it.
|
||||
* Locks are forcibly released after a timeout of 1 hour.
|
||||
*
|
||||
* @return bool true if lock was acquired
|
||||
* false otherwise
|
||||
*/
|
||||
public function lock()
|
||||
{
|
||||
global $config, $db;
|
||||
|
||||
if (!isset($config['cron_lock']))
|
||||
{
|
||||
set_config('cron_lock', '0', true);
|
||||
}
|
||||
|
||||
// make sure cron doesn't run multiple times in parallel
|
||||
if ($config['cron_lock'])
|
||||
{
|
||||
// if the other process is running more than an hour already we have to assume it
|
||||
// aborted without cleaning the lock
|
||||
$time = explode(' ', $config['cron_lock']);
|
||||
$time = $time[0];
|
||||
|
||||
if ($time + 3600 >= time())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
$this->cron_id = time() . ' ' . unique_id();
|
||||
|
||||
$sql = 'UPDATE ' . CONFIG_TABLE . "
|
||||
SET config_value = '" . $db->sql_escape($this->cron_id) . "'
|
||||
WHERE config_name = 'cron_lock'
|
||||
AND config_value = '" . $db->sql_escape($config['cron_lock']) . "'";
|
||||
$db->sql_query($sql);
|
||||
|
||||
// another cron process altered the table between script start and UPDATE query so exit
|
||||
if ($db->sql_affectedrows() != 1)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Releases the cron lock.
|
||||
*
|
||||
* The lock must have been previously obtained, that is, lock() call
|
||||
* was issued and returned true.
|
||||
*
|
||||
* Note: Attempting to release a cron lock that is already released,
|
||||
* that is, calling unlock() multiple times, is harmless.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function unlock()
|
||||
{
|
||||
global $db;
|
||||
|
||||
$sql = 'UPDATE ' . CONFIG_TABLE . "
|
||||
SET config_value = '0'
|
||||
WHERE config_name = 'cron_lock'
|
||||
AND config_value = '" . $db->sql_escape($this->cron_id) . "'";
|
||||
$db->sql_query($sql);
|
||||
}
|
||||
}
|
||||
158
phpBB/includes/lock/db.php
Normal file
158
phpBB/includes/lock/db.php
Normal file
|
|
@ -0,0 +1,158 @@
|
|||
<?php
|
||||
/**
|
||||
*
|
||||
* @package phpBB3
|
||||
* @copyright (c) 2010 phpBB Group
|
||||
* @license http://opensource.org/licenses/gpl-license.php GNU Public License
|
||||
*
|
||||
*/
|
||||
|
||||
/**
|
||||
* @ignore
|
||||
*/
|
||||
if (!defined('IN_PHPBB'))
|
||||
{
|
||||
exit;
|
||||
}
|
||||
|
||||
/**
|
||||
* Database locking class
|
||||
* @package phpBB3
|
||||
*/
|
||||
class phpbb_lock_db
|
||||
{
|
||||
/**
|
||||
* Name of the config variable this lock uses
|
||||
* @var string
|
||||
*/
|
||||
private $config_name;
|
||||
|
||||
/**
|
||||
* Unique identifier for this lock.
|
||||
*
|
||||
* @var string
|
||||
*/
|
||||
private $unique_id;
|
||||
|
||||
/**
|
||||
* Stores the state of this lock
|
||||
* @var bool
|
||||
*/
|
||||
private $locked;
|
||||
|
||||
/**
|
||||
* The phpBB configuration
|
||||
* @var array
|
||||
*/
|
||||
private $config;
|
||||
|
||||
/**
|
||||
* A database connection
|
||||
* @var dbal
|
||||
*/
|
||||
private $db;
|
||||
|
||||
/**
|
||||
* Creates a named released instance of the lock.
|
||||
*
|
||||
* You have to call lock to actually create the lock.
|
||||
*
|
||||
* @param string $config_name A config variable to be used for locking
|
||||
* @param array $config The phpBB configuration
|
||||
* @param dbal $db A database connection
|
||||
*/
|
||||
public function __construct($config_name, $config, dbal $db)
|
||||
{
|
||||
$this->config_name = $config_name;
|
||||
$this->config = $config;
|
||||
$this->db = $db;
|
||||
}
|
||||
|
||||
/**
|
||||
* Tries to acquire the cron lock by updating
|
||||
* the 'cron_lock' configuration variable in the database.
|
||||
*
|
||||
* As a lock may only be held by one process at a time, lock
|
||||
* acquisition may fail if another process is holding the lock
|
||||
* or if another process obtained the lock but never released it.
|
||||
* Locks are forcibly released after a timeout of 1 hour.
|
||||
*
|
||||
* @return bool true if lock was acquired
|
||||
* false otherwise
|
||||
*/
|
||||
public function lock()
|
||||
{
|
||||
if ($this->locked)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!isset($this->config[$this->config_name]))
|
||||
{
|
||||
set_config($this->config_name, '0', true);
|
||||
global $config;
|
||||
$this->config = $config;
|
||||
}
|
||||
$lock_value = $this->config[$this->config_name];
|
||||
|
||||
// make sure cron doesn't run multiple times in parallel
|
||||
if ($lock_value)
|
||||
{
|
||||
// if the other process is running more than an hour already we have to assume it
|
||||
// aborted without cleaning the lock
|
||||
$time = explode(' ', $lock_value);
|
||||
$time = $time[0];
|
||||
|
||||
if ($time + 3600 >= time())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
$this->unique_id = time() . ' ' . unique_id();
|
||||
|
||||
$sql = 'UPDATE ' . CONFIG_TABLE . "
|
||||
SET config_value = '" . $this->db->sql_escape($this->unique_id) . "'
|
||||
WHERE config_name = '" . $this->db->sql_escape($this->config_name) . "'
|
||||
AND config_value = '" . $this->db->sql_escape($lock_value) . "'";
|
||||
$this->db->sql_query($sql);
|
||||
|
||||
if ($this->db->sql_affectedrows())
|
||||
{
|
||||
$this->locked = true;
|
||||
}
|
||||
else
|
||||
{
|
||||
// another cron process altered the table between script start and
|
||||
// UPDATE query so return false
|
||||
$this->locked = false;
|
||||
}
|
||||
|
||||
return $this->locked;
|
||||
}
|
||||
|
||||
/**
|
||||
* Releases the cron lock.
|
||||
*
|
||||
* The lock must have been previously obtained, that is, lock() call
|
||||
* was issued and returned true.
|
||||
*
|
||||
* Note: Attempting to release a cron lock that is already released,
|
||||
* that is, calling unlock() multiple times, is harmless.
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
public function unlock()
|
||||
{
|
||||
if ($this->locked)
|
||||
{
|
||||
$sql = 'UPDATE ' . CONFIG_TABLE . "
|
||||
SET config_value = '0'
|
||||
WHERE config_name = '" . $this->db->sql_escape($this->config_name) . "'
|
||||
AND config_value = '" . $this->db->sql_escape($this->unique_id) . "'";
|
||||
$this->db->sql_query($sql);
|
||||
|
||||
$this->locked = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
67
tests/lock/db_test.php
Normal file
67
tests/lock/db_test.php
Normal file
|
|
@ -0,0 +1,67 @@
|
|||
<?php
|
||||
/**
|
||||
*
|
||||
* @package testing
|
||||
* @copyright (c) 2010 phpBB Group
|
||||
* @license http://opensource.org/licenses/gpl-license.php GNU Public License
|
||||
*
|
||||
*/
|
||||
|
||||
require_once __DIR__ . '/../../phpBB/includes/functions.php';
|
||||
|
||||
class phpbb_lock_db_test extends phpbb_database_test_case
|
||||
{
|
||||
private $db;
|
||||
private $config;
|
||||
private $lock;
|
||||
|
||||
public function getDataSet()
|
||||
{
|
||||
return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/config.xml');
|
||||
}
|
||||
|
||||
public function setUp()
|
||||
{
|
||||
global $db, $config;
|
||||
|
||||
$db = $this->db = $this->new_dbal();
|
||||
$config = $this->config = array('rand_seed' => '', 'rand_seed_last_update' => '0');
|
||||
$this->lock = new phpbb_lock_db('test_lock', $this->config, $this->db);
|
||||
}
|
||||
|
||||
public function test_new_lock()
|
||||
{
|
||||
global $config;
|
||||
|
||||
$this->assertTrue($this->lock->lock());
|
||||
$this->assertTrue(isset($config['test_lock']), 'Lock was created');
|
||||
|
||||
$lock2 = new phpbb_lock_db('test_lock', $config, $this->db);
|
||||
$this->assertFalse($lock2->lock());
|
||||
|
||||
$this->lock->unlock();
|
||||
$this->assertEquals('0', $config['test_lock'], 'Lock was released');
|
||||
}
|
||||
|
||||
public function test_expire_lock()
|
||||
{
|
||||
$lock = new phpbb_lock_db('foo_lock', $this->config, $this->db);
|
||||
$this->assertTrue($lock->lock());
|
||||
}
|
||||
|
||||
public function test_double_lock()
|
||||
{
|
||||
global $config;
|
||||
|
||||
$this->assertTrue($this->lock->lock());
|
||||
$this->assertTrue(isset($config['test_lock']), 'Lock was created');
|
||||
|
||||
$value = $config['test_lock'];
|
||||
|
||||
$this->assertTrue($this->lock->lock());
|
||||
$this->assertEquals($value, $config['test_lock'], 'Second lock was ignored');
|
||||
|
||||
$this->lock->unlock();
|
||||
$this->assertEquals('0', $config['test_lock'], 'Lock was released');
|
||||
}
|
||||
}
|
||||
13
tests/lock/fixtures/config.xml
Normal file
13
tests/lock/fixtures/config.xml
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
<?xml version="1.0" encoding="UTF-8" ?>
|
||||
<dataset>
|
||||
<table name="phpbb_config">
|
||||
<column>config_name</column>
|
||||
<column>config_value</column>
|
||||
<column>is_dynamic</column>
|
||||
<row>
|
||||
<value>foo_lock</value>
|
||||
<value>1 abcd</value>
|
||||
<value>1</value>
|
||||
</row>
|
||||
</table>
|
||||
</dataset>
|
||||
Loading…
Add table
Reference in a new issue