Merge pull request #6583 from piwik/6417_removeLocks

Do not lock when creating a new archive id

12 files changed, 408 insertions, 95 deletions

diff --git a/core/DataAccess/ArchiveWriter.php b/core/DataAccess/ArchiveWriter.php
index 1ea6b0d08c..7e0086d795 100644
--- a/core/DataAccess/ArchiveWriter.php
+++ b/core/DataAccess/ArchiveWriter.php
@@ -11,7 +11,6 @@ namespace Piwik\DataAccess;
 use Exception;
 use Piwik\ArchiveProcessor\Rules;
 use Piwik\ArchiveProcessor;
-use Piwik\Common;
 use Piwik\Db;
 use Piwik\Db\BatchInsert;
 use Piwik\Period;
@@ -139,28 +138,10 @@ class ArchiveWriter
 
     protected function allocateNewArchiveId()
     {
-        $this->idArchive = $this->insertNewArchiveId();
-        return $this->idArchive;
-    }
-
-    /**
-     * Locks the archive table to generate a new archive ID.
-     *
-     * We lock to make sure that
-     * if several archiving processes are running at the same time (for different websites and/or periods)
-     * then they will each use a unique archive ID.
-     *
-     * @return int
-     */
-    protected function insertNewArchiveId()
-    {
         $numericTable = $this->getTableNumeric();
-        $idSite = $this->idSite;
-        $date = date("Y-m-d H:i:s");
 
-        $id = $this->getModel()->insertNewArchiveId($numericTable, $idSite, $date);
-
-        return $id;
+        $this->idArchive = $this->getModel()->allocateNewArchiveId($numericTable);
+        return $this->idArchive;
     }
 
     private function getModel()
diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php
index 5cc25635af..dd020af083 100644
--- a/core/DataAccess/Model.php
+++ b/core/DataAccess/Model.php
@@ -12,6 +12,7 @@ use Exception;
 use Piwik\Common;
 use Piwik\Db;
 use Piwik\DbHelper;
+use Piwik\Sequence;
 
 /**
  * Cleans up outdated archives
@@ -20,7 +21,6 @@ use Piwik\DbHelper;
  */
 class Model
 {
-    const PREFIX_SQL_LOCK = "locked_";
 
     /**
      * Returns the archives IDs that have already been invalidated and have been since re-processed.
@@ -181,53 +181,37 @@ class Model
                 throw $e;
             }
         }
+
+        try {
+            if (ArchiveTableCreator::NUMERIC_TABLE === ArchiveTableCreator::getTypeFromTableName($tableName)) {
+                $sequence = new Sequence($tableName);
+                $sequence->create();
+            }
+        } catch (Exception $e) {
+
+        }
     }
 
-    /**
-     * Locks the archive table to generate a new archive ID.
-     *
-     * We lock to make sure that
-     * if several archiving processes are running at the same time (for different websites and/or periods)
-     * then they will each use a unique archive ID.
-     *
-     * @return int
-     */
-    public function insertNewArchiveId($numericTable, $idSite, $date)
+    public function allocateNewArchiveId($numericTable)
     {
-        $this->acquireArchiveTableLock($numericTable);
-
-        $locked = self::PREFIX_SQL_LOCK . Common::generateUniqId();
-
-        $insertSql = "INSERT INTO $numericTable "
-            . " SELECT IFNULL( MAX(idarchive), 0 ) + 1,
-                                '" . $locked . "',
-                                " . (int)$idSite . ",
-                                '" . $date . "',
-                                '" . $date . "',
-                                0,
-                                '" . $date . "',
-                                0 "
-            . " FROM $numericTable as tb1";
-        Db::get()->exec($insertSql);
-
-        $this->releaseArchiveTableLock($numericTable);
-
-        $selectIdSql = "SELECT idarchive FROM $numericTable WHERE name = ? LIMIT 1";
-        $id = Db::get()->fetchOne($selectIdSql, $locked);
-        return $id;
+        $sequence  = new Sequence($numericTable);
+        $idarchive = $sequence->getNextId();
+
+        return $idarchive;
     }
 
     public function deletePreviousArchiveStatus($numericTable, $archiveId, $doneFlag)
     {
+        $dbLockName = "deletePreviousArchiveStatus.$numericTable.$archiveId";
+
         // without advisory lock here, the DELETE would acquire Exclusive Lock
-        $this->acquireArchiveTableLock($numericTable);
+        $this->acquireArchiveTableLock($dbLockName);
 
-        Db::query("DELETE FROM $numericTable WHERE idarchive = ? AND (name = '" . $doneFlag
-                . "' OR name LIKE '" . self::PREFIX_SQL_LOCK . "%')",
+        Db::query("DELETE FROM $numericTable WHERE idarchive = ? AND (name = '" . $doneFlag . "')",
             array($archiveId)
         );
 
-        $this->releaseArchiveTableLock($numericTable);
+        $this->releaseArchiveTableLock($dbLockName);
     }
 
     public function insertRecord($tableName, $fields, $record, $name, $value)
@@ -257,24 +241,16 @@ class Model
         return "((name IN ($allDoneFlags)) AND (value IN (" . implode(',', $possibleValues) . ")))";
     }
 
-    protected function acquireArchiveTableLock($numericTable)
+    protected function acquireArchiveTableLock($dbLockName)
     {
-        $dbLockName = $this->getArchiveLockName($numericTable);
-
         if (Db::getDbLock($dbLockName, $maxRetries = 30) === false) {
-            throw new Exception("allocateNewArchiveId: Cannot get named lock $dbLockName.");
+            throw new Exception("Cannot get named lock $dbLockName.");
         }
     }
 
-    protected function releaseArchiveTableLock($numericTable)
+    protected function releaseArchiveTableLock($dbLockName)
     {
-        $dbLockName = $this->getArchiveLockName($numericTable);
         Db::releaseDbLock($dbLockName);
     }
 
-    protected function getArchiveLockName($numericTable)
-    {
-        return "allocateNewArchiveId.$numericTable";
-    }
-
 }
diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php
index eb8c1f71f1..8f0eea8a26 100644
--- a/core/Db/Schema/Mysql.php
+++ b/core/Db/Schema/Mysql.php
@@ -265,6 +265,13 @@ class Mysql implements SchemaInterface
                                         INDEX index_period_archived(period, ts_archived)
                                       ) ENGINE=$engine DEFAULT CHARSET=utf8
             ",
+
+            'sequence'        => "CREATE TABLE {$prefixTables}sequence (
+                                      `name` VARCHAR(120) NOT NULL,
+                                      `value` BIGINT(20) UNSIGNED NOT NULL ,
+                                      PRIMARY KEY(`name`)
+                                  ) ENGINE=$engine DEFAULT CHARSET=utf8
+            ",
         );
 
         return $tables;
@@ -468,30 +475,26 @@ class Mysql implements SchemaInterface
 
     private function getTablePrefix()
     {
-        $dbInfos      = Db::getDatabaseConfig();
-        $prefixTables = $dbInfos['tables_prefix'];
-
-        return $prefixTables;
+        return $this->getDbSettings()->getTablePrefix();
     }
 
     private function getTableEngine()
     {
-        $dbInfos = Db::getDatabaseConfig();
-        $engine  = $dbInfos['type'];
-
-        return $engine;
+        return $this->getDbSettings()->getEngine();
     }
 
     private function getDb(){
         return Db::get();
     }
 
-    private function getDbName()
+    private function getDbSettings()
     {
-        $dbInfos = Db::getDatabaseConfig();
-        $dbName  = $dbInfos['dbname'];
+        return new Db\Settings();
+    }
 
-        return $dbName;
+    private function getDbName()
+    {
+        return $this->getDbSettings()->getDbName();
     }
 
     private function getAllExistingTables($prefixTables = false)
diff --git a/core/Db/Settings.php b/core/Db/Settings.php
new file mode 100644
index 0000000000..afb06abc2f
--- /dev/null
+++ b/core/Db/Settings.php
@@ -0,0 +1,43 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Db;
+use Piwik\Db;
+
+/**
+ * Schema abstraction
+ *
+ * Note: no relation to the ZF proposals for Zend_Db_Schema_Manager
+ *
+ * @method static \Piwik\Db\Schema getInstance()
+ */
+class Settings
+{
+    public function getEngine()
+    {
+        return $this->getDbSetting('type');
+    }
+
+    public function getTablePrefix()
+    {
+        return $this->getDbSetting('tables_prefix');
+    }
+
+    public function getDbName()
+    {
+        return $this->getDbSetting('dbname');
+    }
+
+    private function getDbSetting($key)
+    {
+        $dbInfos = Db::getDatabaseConfig();
+        $engine  = $dbInfos[$key];
+
+        return $engine;
+    }
+}
diff --git a/core/Sequence.php b/core/Sequence.php
new file mode 100644
index 0000000000..655332dced
--- /dev/null
+++ b/core/Sequence.php
@@ -0,0 +1,108 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik;
+
+use Exception;
+
+/**
+ * Used for generating auto increment ids.
+ *
+ * Example:
+ *
+ * $sequence = new Sequence('my_sequence_name');
+ * $id = $sequence->getNextId();
+ * $db->insert('anytable', array('id' => $id, '...' => '...'));
+ */
+class Sequence
+{
+    private $name;
+
+    /**
+     * The name of the table or sequence you want to get an id for.
+     *
+     * @param string $name eg 'archive_numeric_2014_11'
+     */
+    public function __construct($name)
+    {
+        $this->name = $name;
+    }
+
+    private function getTableName()
+    {
+        return Common::prefixTable('sequence');
+    }
+
+    /**
+     * Creates / initializes a new sequence.
+     *
+     * @param int $initialValue
+     * @return int The actually used value to initialize the table.
+     *
+     * @throws \Exception in case a sequence having this name already exists.
+     */
+    public function create($initialValue = 0)
+    {
+        $initialValue = (int) $initialValue;
+
+        $table = $this->getTableName();
+        $db    = $this->getDb();
+
+        $db->insert($table, array('name' => $this->name, 'value' => $initialValue));
+
+        return $initialValue;
+    }
+
+    /**
+     * Get / allocate / reserve a new id for the current sequence. Important: Getting the next id will fail in case
+     * no such sequence exists. Make sure to create one if needed, see {@link create()}.
+     *
+     * @return int
+     * @throws Exception
+     */
+    public function getNextId()
+    {
+        $table = $this->getTableName();
+        $sql   = 'UPDATE ' . $table . ' SET value = LAST_INSERT_ID(value + 1) WHERE name = ?';
+
+        $db       = $this->getDb();
+        $result   = $db->query($sql, array($this->name));
+        $rowCount = $result->rowCount();
+
+        if (1 !== $rowCount) {
+            throw new Exception("Sequence '" . $this->name . "' not found.");
+        }
+
+        $createdId = $db->lastInsertId();
+
+        return (int) $createdId;
+    }
+
+    /**
+     * Returns the current max id.
+     * @return int
+     * @internal
+     */
+    public function getCurrentId()
+    {
+        $table = $this->getTableName();
+        $sql   = 'SELECT value FROM ' . $table . ' WHERE name = ?';
+
+        $db = $this->getDb();
+        $id = $db->fetchOne($sql, array($this->name));
+
+        if (!empty($id) || '0' === $id || 0 === $id) {
+            return (int) $id;
+        }
+    }
+
+    private function getDb()
+    {
+        return Db::get();
+    }
+}
diff --git a/core/Updates/2.9.0-b1.php b/core/Updates/2.9.0-b1.php
index f36f52a6bd..5720bf076e 100644
--- a/core/Updates/2.9.0-b1.php
+++ b/core/Updates/2.9.0-b1.php
@@ -20,7 +20,6 @@ class Updates_2_9_0_b1 extends Updates
     static function getSql()
     {
         $sql = array();
-
         $sql = self::updateBrowserEngine($sql);
 
         return $sql;
@@ -45,10 +44,10 @@ class Updates_2_9_0_b1 extends Updates
 
         $browserEngineMatch = array(
             'Trident' => array('IE'),
-            'Gecko'   => array('NS', 'PX', 'FF', 'FB', 'CA', 'GA', 'KM', 'MO', 'SM', 'CO', 'FE', 'KP', 'KZ', 'TB'),
-            'KHTML'   => array('KO'),
-            'WebKit'  => array('SF', 'CH', 'OW', 'AR', 'EP', 'FL', 'WO', 'AB', 'IR', 'CS', 'FD', 'HA', 'MI', 'GE', 'DF', 'BB', 'BP', 'TI', 'CF', 'RK', 'B2', 'NF'),
-            'Presto'  => array('OP'),
+            'Gecko' => array('NS', 'PX', 'FF', 'FB', 'CA', 'GA', 'KM', 'MO', 'SM', 'CO', 'FE', 'KP', 'KZ', 'TB'),
+            'KHTML' => array('KO'),
+            'WebKit' => array('SF', 'CH', 'OW', 'AR', 'EP', 'FL', 'WO', 'AB', 'IR', 'CS', 'FD', 'HA', 'MI', 'GE', 'DF', 'BB', 'BP', 'TI', 'CF', 'RK', 'B2', 'NF'),
+            'Presto' => array('OP'),
         );
 
         // Update visits, fill in now missing engine
diff --git a/core/Updates/2.9.0-b7.php b/core/Updates/2.9.0-b7.php
new file mode 100644
index 0000000000..1efcf8360c
--- /dev/null
+++ b/core/Updates/2.9.0-b7.php
@@ -0,0 +1,90 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Updates;
+
+use Piwik\Common;
+use Piwik\DataAccess\ArchiveTableCreator;
+use Piwik\Db;
+use Piwik\Updater;
+use Piwik\Updates;
+
+class Updates_2_9_0_b7 extends Updates
+{
+    static function getSql()
+    {
+        $sql = array();
+        $sql = self::addCreateSequenceTableQuery($sql);
+        $sql = self::addArchivingIdMigrationQueries($sql);
+
+        return $sql;
+    }
+
+    static function update()
+    {
+        Updater::updateDatabase(__FILE__, self::getSql());
+    }
+
+    private static function addArchivingIdMigrationQueries($sql)
+    {
+        $tables = ArchiveTableCreator::getTablesArchivesInstalled();
+
+        foreach ($tables as $table) {
+            $type = ArchiveTableCreator::getTypeFromTableName($table);
+
+            if ($type === ArchiveTableCreator::NUMERIC_TABLE) {
+                $maxId = Db::fetchOne('SELECT MAX(idarchive) FROM ' . $table);
+
+                if (!empty($maxId)) {
+                    $maxId = (int) $maxId + 500;
+                } else {
+                    $maxId = 1;
+                }
+
+                $query = self::getQueryToCreateSequence($table, $maxId);
+                $sql[$query] = false;
+            }
+        }
+
+        return $sql;
+    }
+
+    private static function getQueryToCreateSequence($name, $initialValue)
+    {
+        $table = self::getSequenceTableName();
+        $query = sprintf("INSERT INTO %s (name, value) VALUES ('%s', %d)", $table, $name, $initialValue);
+
+        return $query;
+    }
+
+    /**
+     * @return string
+     */
+    private static function addCreateSequenceTableQuery($sql)
+    {
+        $dbSettings = new Db\Settings();
+        $engine = $dbSettings->getEngine();
+        $table  = self::getSequenceTableName();
+
+        $query = "CREATE TABLE `$table` (
+                `name` VARCHAR(120) NOT NULL,
+                `value` BIGINT(20) UNSIGNED NOT NULL,
+                PRIMARY KEY(`name`)
+        ) ENGINE=$engine DEFAULT CHARSET=utf8";
+
+        $sql[$query] = 1050;
+
+        return $sql;
+    }
+
+    private static function getSequenceTableName()
+    {
+        return Common::prefixTable('sequence');
+    }
+
+}
diff --git a/core/Version.php b/core/Version.php
index 789d7d29d3..ba95441792 100644
--- a/core/Version.php
+++ b/core/Version.php
@@ -12,7 +12,6 @@ namespace Piwik;
 /**
  * Piwik version information.
  *
- *
  * @api
  */
 final class Version
@@ -21,5 +20,5 @@ final class Version
      * The current Piwik version.
      * @var string
      */
-    const VERSION = '2.9.0-b6';
+    const VERSION = '2.9.0-b7';
 }
diff --git a/plugins/TestRunner/Commands/TestsSetupFixture.php b/plugins/TestRunner/Commands/TestsSetupFixture.php
index 8c053b1711..cda1a5c585 100644
--- a/plugins/TestRunner/Commands/TestsSetupFixture.php
+++ b/plugins/TestRunner/Commands/TestsSetupFixture.php
@@ -237,10 +237,12 @@ class TestsSetupFixture extends ConsoleCommand
         require_once PIWIK_INCLUDE_PATH . '/tests/PHPUnit/IntegrationTestCase.php';
 
         $fixturesToLoad = array(
+            '/tests/PHPUnit/Fixtures/*.php',
             '/tests/PHPUnit/UI/Fixtures/*.php',
             '/plugins/*/tests/Fixtures/*.php',
             '/plugins/*/Test/Fixtures/*.php',
         );
+
         foreach($fixturesToLoad as $fixturePath) {
             foreach (glob(PIWIK_INCLUDE_PATH . $fixturePath) as $file) {
                 require_once $file;
diff --git a/tests/PHPUnit/Integration/DataAccess/ModelTest.php b/tests/PHPUnit/Integration/DataAccess/ModelTest.php
index 62ccaeaade..270253b82a 100644
--- a/tests/PHPUnit/Integration/DataAccess/ModelTest.php
+++ b/tests/PHPUnit/Integration/DataAccess/ModelTest.php
@@ -31,17 +31,17 @@ class Core_DataAccess_ModelTest extends IntegrationTestCase
 
     public function test_insertNewArchiveId()
     {
-        $this->assertCreatedArchiveId(1);
-        $this->assertCreatedArchiveId(2);
-        $this->assertCreatedArchiveId(3);
-        $this->assertCreatedArchiveId(4);
-        $this->assertCreatedArchiveId(5, 2);
-        $this->assertCreatedArchiveId(6, 2);
+        $this->assertAllocatedArchiveId(1);
+        $this->assertAllocatedArchiveId(2);
+        $this->assertAllocatedArchiveId(3);
+        $this->assertAllocatedArchiveId(4);
+        $this->assertAllocatedArchiveId(5);
+        $this->assertAllocatedArchiveId(6);
     }
 
-    private function assertCreatedArchiveId($expectedId, $siteId = 1)
+    private function assertAllocatedArchiveId($expectedId)
     {
-        $id = $this->model->insertNewArchiveId($this->tableName, $siteId, '2014-01-01 00:01:02');
+        $id = $this->model->allocateNewArchiveId($this->tableName);
 
         $this->assertEquals($expectedId, $id);
     }
diff --git a/tests/PHPUnit/Integration/SequenceTest.php b/tests/PHPUnit/Integration/SequenceTest.php
new file mode 100644
index 0000000000..f42617cf06
--- /dev/null
+++ b/tests/PHPUnit/Integration/SequenceTest.php
@@ -0,0 +1,112 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+use Piwik\Db;
+use Piwik\Sequence;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * Class Core_SequenceTest
+ *
+ * @group Core
+ * @group Sequence
+ */
+class Core_SequenceTest extends IntegrationTestCase
+{
+    /**
+     * @var Sequence
+     */
+    private $sequence;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->sequence = new Sequence('mySequence0815');
+        $this->sequence->create();
+    }
+
+    public function test_create_shouldAddNewSequenceWithInitalId1()
+    {
+        $sequence = $this->getEmptySequence();
+
+        $id = $sequence->create();
+        $this->assertSame(0, $id);
+
+        // verify
+        $id = $sequence->getCurrentId();
+        $this->assertSame(0, $id);
+    }
+
+    public function test_create_WithCustomInitialValue()
+    {
+        $sequence = $this->getEmptySequence();
+
+        $id = $sequence->create(11);
+        $this->assertSame(11, $id);
+
+        // verify
+        $id = $sequence->getCurrentId();
+        $this->assertSame(11, $id);
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage Duplicate entry
+     */
+    public function test_create_shouldFailIfSequenceAlreadyExists()
+    {
+        $this->sequence->create();
+    }
+
+    public function test_getNextId_shouldGenerateNextId()
+    {
+        $this->assertNextIdGenerated(1);
+        $this->assertNextIdGenerated(2);
+        $this->assertNextIdGenerated(3);
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage Sequence 'notCreatedSequence' not found
+     */
+    public function test_getNextId_shouldFailIfThereIsNoSequenceHavingThisName()
+    {
+        $sequence = $this->getEmptySequence();
+        $sequence->getNextId();
+    }
+
+    private function assertNextIdGenerated($expectedId)
+    {
+        $id = $this->sequence->getNextId();
+        $this->assertSame($expectedId, $id);
+
+        // verify
+        $id = $this->sequence->getCurrentId();
+        $this->assertSame($expectedId, $id);
+    }
+
+    public function test_getCurrentId_shouldReturnTheCurrentIdAsInt()
+    {
+        $id = $this->sequence->getCurrentId();
+        $this->assertSame(0, $id);
+    }
+
+    public function test_getCurrentId_shouldReturnNullIfSequenceDoesNotExist()
+    {
+        $sequence = $this->getEmptySequence();
+        $id = $sequence->getCurrentId();
+        $this->assertNull($id);
+    }
+
+    private function getEmptySequence()
+    {
+        return new Sequence('notCreatedSequence');
+    }
+
+
+}
\ No newline at end of file
diff --git a/tests/resources/OmniFixture-dump.sql.gz b/tests/resources/OmniFixture-dump.sql.gz
index f1f56e6d9b..a7151a5d6b 100644
--- a/tests/resources/OmniFixture-dump.sql.gz
+++ b/tests/resources/OmniFixture-dump.sql.gz