diff options
author | Thomas Steur <tsteur@users.noreply.github.com> | 2019-09-19 03:58:37 +0300 |
---|---|---|
committer | diosmosis <diosmosis@users.noreply.github.com> | 2019-09-19 03:58:37 +0300 |
commit | f91a8090933e384bb503a77a51475157731bbeb6 (patch) | |
tree | c285816ab2d94f2b949d1e205970f389cf5e76c1 | |
parent | 3b60da8343e79a490e7459a4aab580f17a5c6ca4 (diff) |
better handling of setting transaction level (#14899)
-rw-r--r-- | core/DataAccess/LogAggregator.php | 34 | ||||
-rw-r--r-- | core/Db/TransactionLevel.php | 68 | ||||
-rw-r--r-- | tests/PHPUnit/Integration/Db/TransactionLevelTest.php | 60 |
3 files changed, 145 insertions, 17 deletions
diff --git a/core/DataAccess/LogAggregator.php b/core/DataAccess/LogAggregator.php index 9ac71efc87..1f209aa187 100644 --- a/core/DataAccess/LogAggregator.php +++ b/core/DataAccess/LogAggregator.php @@ -277,11 +277,22 @@ class LogAggregator throw $e; } - - $db = new Db\Settings(); - $isInnoDb = strtolower($db->getEngine()) === 'innodb'; - - if (!$isInnoDb) { + $transactionLevel = new Db\TransactionLevel($readerDb); + $canSetTransactionLevel = $transactionLevel->canLikelySetTransactionLevel(); + + if ($canSetTransactionLevel) { + // i know this could be shortened to one if or one line but I want to make sure this line where we + // set uncomitted is easily noticable in the code as it could be missed quite easily otherwise + // we set uncommitted so we don't make the INSERT INTO... SELECT... locking ... we do not want to lock + // eg the visits table + if (!$transactionLevel->setUncommitted()) { + $canSetTransactionLevel = false; + } + } + + if (!$canSetTransactionLevel) { + // transaction level doesn't work... we're instead executing the select individually and then insert the data + // this uses more memory but at least is not locking $all = $readerDb->fetchAll($segmentSelectSql, $segmentSelectBind); if (!empty($all)) { // we're not using batchinsert since this would not support the reader DB. @@ -290,21 +301,10 @@ class LogAggregator return; } - $value = $readerDb->fetchOne('SELECT @@TX_ISOLATION'); - $readerDb->query('SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED'); - $insertIntoStatement = 'INSERT INTO ' . $table . ' (idvisit) ' . $segmentSelectSql; $readerDb->query($insertIntoStatement, $segmentSelectBind); - if ($isInnoDb && !empty($value)) { - $value = strtoupper($value); - $value = str_replace('-', ' ', $value); - if (in_array($value, array('REPEATABLE READ', 'READ COMMITTED', 'SERIALIZABLE'))) { - $readerDb->query('SET SESSION TRANSACTION ISOLATION LEVEL ' . $value); - } elseif ($value !== 'READ UNCOMMITTED') { - $readerDb->query('SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ'); - } - } + $transactionLevel->restorePreviousStatus(); } public function generateQuery($select, $from, $where, $groupBy, $orderBy, $limit = 0, $offset = 0) diff --git a/core/Db/TransactionLevel.php b/core/Db/TransactionLevel.php new file mode 100644 index 0000000000..13e04bb672 --- /dev/null +++ b/core/Db/TransactionLevel.php @@ -0,0 +1,68 @@ +<?php +/** + * Matomo - free/libre analytics platform + * + * @link https://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + * + */ + +namespace Piwik\Db; + +use Piwik\Db; + +class TransactionLevel +{ + private $statusBackup; + + /** + * @var \Piwik\Tracker\Db|\Piwik\Db\AdapterInterface|\Piwik\Db $db + */ + private $db; + + /** + * @param \Piwik\Tracker\Db|\Piwik\Db\AdapterInterface|\Piwik\Db $db + */ + public function __construct($db) + { + $this->db = $db; + } + + public function canLikelySetTransactionLevel() + { + $dbSettings = new Db\Settings(); + + return strtolower($dbSettings->getEngine()) === 'innodb'; + } + + public function setUncommitted() + { + try { + $backup = $this->db->fetchOne('SELECT @@TX_ISOLATION'); + $this->db->query('SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED'); + $this->statusBackup = $backup; + } catch (\Exception $e) { + // catch eg 1665 Cannot execute statement: impossible to write to binary log since BINLOG_FORMAT = STATEMENT and at least one table uses a storage engine limited to row-based logging. InnoDB is limited to row-logging when transaction isolation level is READ COMMITTED or READ UNCOMMITTED + return false; + } + + return true; + } + + public function restorePreviousStatus() + { + if ($this->statusBackup) { + $value = strtoupper($this->statusBackup); + $this->statusBackup = null; + + $value = str_replace('-', ' ', $value); + if (in_array($value, array('REPEATABLE READ', 'READ COMMITTED', 'SERIALIZABLE'))) { + $this->db->query('SET SESSION TRANSACTION ISOLATION LEVEL '.$value); + } elseif ($value !== 'READ UNCOMMITTED') { + $this->db->query('SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ'); + } + } + + } + +} diff --git a/tests/PHPUnit/Integration/Db/TransactionLevelTest.php b/tests/PHPUnit/Integration/Db/TransactionLevelTest.php new file mode 100644 index 0000000000..3438108879 --- /dev/null +++ b/tests/PHPUnit/Integration/Db/TransactionLevelTest.php @@ -0,0 +1,60 @@ +<?php +/** + * Matomo - free/libre analytics platform + * + * @link https://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Tests\Integration\Db; + +use Piwik\Db; +use Piwik\Db\TransactionLevel; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; + +/** + * @group Funnels + * @group TransactionLevelTest + * @group TransactionLevel + * @group Plugins + */ +class TransactionLevelTest extends IntegrationTestCase +{ + /** + * @var TransactionLevel + */ + private $level; + + /** + * @var \Piwik\Tracker\Db|\Piwik\Db\AdapterInterface|\Piwik\Db $db + */ + private $db; + + public function setUp() + { + parent::setUp(); + $this->db = Db::get(); + $this->level = new TransactionLevel($this->db); + } + + public function test_canLikelySetTransactionLevel() + { + $this->assertTrue($this->level->canLikelySetTransactionLevel()); + } + + public function test_setUncommitted_restorePreviousStatus() + { + $value = $this->db->fetchOne('SELECT @@TX_ISOLATION'); + $this->assertSame('REPEATABLE-READ', $value); + + $this->level->setUncommitted(); + $value = $this->db->fetchOne('SELECT @@TX_ISOLATION'); + + $this->assertSame('READ-UNCOMMITTED', $value); + $this->level->restorePreviousStatus(); + + $value = $this->db->fetchOne('SELECT @@TX_ISOLATION'); + $this->assertSame('REPEATABLE-READ', $value); + } + +} |