diff --git a/config/global.php b/config/global.php index 4013d9d9c52..5499b0920ad 100644 --- a/config/global.php +++ b/config/global.php @@ -216,6 +216,6 @@ 'archiving.performance.logger' => null, \Piwik\CronArchive\Performance\Logger::class => DI\object()->constructorParameter('logger', DI\get('archiving.performance.logger')), - + \Piwik\Concurrency\LockBackend::class => \DI\get(\Piwik\Concurrency\LockBackend\MySqlLockBackend::class), ); diff --git a/core/Concurrency/Lock.php b/core/Concurrency/Lock.php index 5136fccbf35..f63bec640a3 100644 --- a/core/Concurrency/Lock.php +++ b/core/Concurrency/Lock.php @@ -12,6 +12,8 @@ class Lock { + const MAX_KEY_LEN = 70; + /** * @var LockBackend */ @@ -46,6 +48,30 @@ public function getAllAcquiredLockKeys() return $this->backend->getKeysMatchingPattern($this->lockKeyStart . '*'); } + public function execute($id, $callback) + { + if (Common::mb_strlen($id) > self::MAX_KEY_LEN) { + // Lock key might be too long for DB column, so we hash it but leave the start of the original as well + // to make it more readable + $md5Len = 32; + $id = Common::mb_substr($id, 0, self::MAX_KEY_LEN - $md5Len - 1) . md5($id); + } + + $i = 0; + while (!$this->acquireLock($id)) { + $i++; + usleep( 100 * 1000 ); // 100ms + if ($i > 50) { // give up after 5seconds (50 * 100ms) + throw new \Exception('Could not get the lock for ID: ' . $id); + } + }; + try { + return $callback(); + } finally { + $this->unlock(); + } + } + public function acquireLock($id, $ttlInSeconds = 60) { $this->lockKey = $this->lockKeyStart . $id; diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index 1a6e6519b0d..a82931b64d4 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -10,6 +10,7 @@ use Exception; use Piwik\Common; +use Piwik\Concurrency\Lock; use Piwik\Date; use Piwik\Db\SchemaInterface; use Piwik\Db; @@ -312,7 +313,7 @@ public function getTablesCreateSql() ) ENGINE=$engine DEFAULT CHARSET=utf8 ", 'locks' => "CREATE TABLE `{$prefixTables}locks` ( - `key` VARCHAR(70) NOT NULL, + `key` VARCHAR(".Lock::MAX_KEY_LEN.") NOT NULL, `value` VARCHAR(255) NULL DEFAULT NULL, `expiry_time` BIGINT UNSIGNED DEFAULT 9999999999, PRIMARY KEY (`key`) diff --git a/core/Settings/Storage/Backend/BaseSettingsTable.php b/core/Settings/Storage/Backend/BaseSettingsTable.php new file mode 100644 index 00000000000..3b22fefc915 --- /dev/null +++ b/core/Settings/Storage/Backend/BaseSettingsTable.php @@ -0,0 +1,41 @@ +lock = StaticContainer::getContainer()->make( + Lock::class, + array ('lockKeyStart' => 'PluginSettingsTable') + ); + } + + protected function initDbIfNeeded() + { + if (!isset($this->db)) { + // we do not want to create a db connection on backend creation + $this->db = Db::get(); + } + } + +} \ No newline at end of file diff --git a/core/Settings/Storage/Backend/MeasurableSettingsTable.php b/core/Settings/Storage/Backend/MeasurableSettingsTable.php index e635f331bbb..77c56150fc1 100644 --- a/core/Settings/Storage/Backend/MeasurableSettingsTable.php +++ b/core/Settings/Storage/Backend/MeasurableSettingsTable.php @@ -10,6 +10,8 @@ namespace Piwik\Settings\Storage\Backend; use Piwik\Common; +use Piwik\Concurrency\Lock; +use Piwik\Container\StaticContainer; use Piwik\Db; use Exception; use Piwik\Version; @@ -19,7 +21,7 @@ * * If a value that needs to be stored is an array, will insert a new row for each value of this array. */ -class MeasurableSettingsTable implements BackendInterface +class MeasurableSettingsTable extends BaseSettingsTable { /** * @var int @@ -31,13 +33,10 @@ class MeasurableSettingsTable implements BackendInterface */ private $pluginName; - /** - * @var Db\AdapterInterface - */ - private $db; - public function __construct($idSite, $pluginName) { + parent::__construct(); + if (empty($pluginName)) { throw new Exception('No plugin name given for MeasurableSettingsTable backend'); } @@ -50,39 +49,19 @@ public function __construct($idSite, $pluginName) $this->pluginName = $pluginName; } - private function initDbIfNeeded() - { - if (!isset($this->db)) { - // we need to avoid db creation on instance creation, especially important in tracker mode - // the db might be never actually used when values are eg fetched from a cache - $this->db = Db::get(); - } - } - - /** - * @inheritdoc - */ - public function getStorageId() - { - return 'MeasurableSettings_' . $this->idSite . '_' . $this->pluginName; - } - /** * Saves (persists) the current setting values in the database. + * @param array $values Key/value pairs of setting values to be written */ public function save($values) { $this->initDbIfNeeded(); - $table = $this->getTableName(); - - $this->delete(); - + $valuesKeep = array(); foreach ($values as $name => $value) { if (!isset($value)) { continue; } - if (is_array($value) || is_object($value)) { $jsonEncoded = 1; $value = json_encode($value); @@ -94,12 +73,28 @@ public function save($values) $value = (int) $value; } } + $valuesKeep[] = array($this->idSite, $this->pluginName, $name, $value, $jsonEncoded); + } - $sql = "INSERT INTO $table (`idsite`, `plugin_name`, `setting_name`, `setting_value`, `json_encoded`) VALUES (?, ?, ?, ?, ?)"; - $bind = array($this->idSite, $this->pluginName, $name, $value, $jsonEncoded); + $columns = array('idsite', 'plugin_name', 'setting_name', 'setting_value', 'json_encoded'); - $this->db->query($sql, $bind); - } + $table = $this->getTableName(); + $lockKey = $this->getStorageId(); + $this->lock->execute($lockKey, function() use ($valuesKeep, $table, $columns) { + $this->delete(); + // No values = nothing to save + if (!empty($valuesKeep)) { + Db\BatchInsert::tableInsertBatchSql($table, $columns, $valuesKeep, true); + } + }); + } + + /** + * @inheritdoc + */ + public function getStorageId() + { + return 'MeasurableSettings_' . $this->idSite . '_' . $this->pluginName; } private function jsonEncodedMissingError(Exception $e) @@ -149,7 +144,7 @@ public function load() return $flat; } - private function getTableName() + protected function getTableName() { return Common::prefixTable('site_setting'); } diff --git a/core/Settings/Storage/Backend/PluginSettingsTable.php b/core/Settings/Storage/Backend/PluginSettingsTable.php index d12b15cf2f4..fddd60fc36d 100644 --- a/core/Settings/Storage/Backend/PluginSettingsTable.php +++ b/core/Settings/Storage/Backend/PluginSettingsTable.php @@ -10,6 +10,8 @@ namespace Piwik\Settings\Storage\Backend; use Piwik\Common; +use Piwik\Concurrency\Lock; +use Piwik\Container\StaticContainer; use Piwik\Db; use Exception; use Piwik\Version; @@ -19,7 +21,7 @@ * * If a value that needs to be stored is an array, will insert a new row for each value of this array. */ -class PluginSettingsTable implements BackendInterface +class PluginSettingsTable extends BaseSettingsTable { /** * @var string @@ -31,13 +33,10 @@ class PluginSettingsTable implements BackendInterface */ private $userLogin; - /** - * @var Db\AdapterInterface - */ - private $db; - public function __construct($pluginName, $userLogin) { + parent::__construct(); + if (empty($pluginName)) { throw new Exception('No plugin name given for PluginSettingsTable backend'); } @@ -50,14 +49,6 @@ public function __construct($pluginName, $userLogin) $this->userLogin = $userLogin; } - private function initDbIfNeeded() - { - if (!isset($this->db)) { - // we do not want to create a db connection on backend creation - $this->db = Db::get(); - } - } - /** * @inheritdoc */ @@ -68,20 +59,18 @@ public function getStorageId() /** * Saves (persists) the current setting values in the database. + * @param array $values Key/value pairs of setting values to be written */ public function save($values) { $this->initDbIfNeeded(); - $table = $this->getTableName(); - - $this->delete(); + $valuesKeep = array(); foreach ($values as $name => $value) { if (!isset($value)) { continue; } - if (is_array($value) || is_object($value)) { $jsonEncoded = 1; $value = json_encode($value); @@ -94,11 +83,20 @@ public function save($values) } } - $sql = "INSERT INTO $table (`plugin_name`, `user_login`, `setting_name`, `setting_value`, `json_encoded`) VALUES (?, ?, ?, ?, ?)"; - $bind = array($this->pluginName, $this->userLogin, $name, $value, $jsonEncoded); - - $this->db->query($sql, $bind); + $valuesKeep[] = array($this->pluginName, $this->userLogin, $name, $value, $jsonEncoded); } + + $columns = array('plugin_name', 'user_login', 'setting_name', 'setting_value', 'json_encoded'); + + $table = $this->getTableName(); + $lockKey = $this->getStorageId(); + $this->lock->execute($lockKey, function() use ($valuesKeep, $table, $columns) { + $this->delete(); + // No values = nothing to save + if (!empty($valuesKeep)) { + Db\BatchInsert::tableInsertBatchSql($table, $columns, $valuesKeep); + } + }); } private function jsonEncodedMissingError(Exception $e) @@ -145,7 +143,7 @@ public function load() return $flat; } - private function getTableName() + protected function getTableName() { return Common::prefixTable('plugin_setting'); } diff --git a/plugins/CorePluginsAdmin/Model/TagManagerTeaser.php b/plugins/CorePluginsAdmin/Model/TagManagerTeaser.php index 54fdb1e9b19..9eb4a5ff8d7 100644 --- a/plugins/CorePluginsAdmin/Model/TagManagerTeaser.php +++ b/plugins/CorePluginsAdmin/Model/TagManagerTeaser.php @@ -68,7 +68,7 @@ public function reset() Option::delete(self::DISABLE_GLOBALLY_KEY); // no need to keep any old login entries - $this->getTable()->save(array()); + $this->getTable()->delete(); } public function isEnabledGlobally()