From cdc50dc38924f396dff8d50ef019c24f2a7b7bfd Mon Sep 17 00:00:00 2001 From: Matthieu Aubry <mattab@users.noreply.github.com> Date: Mon, 26 Sep 2016 13:03:59 +1300 Subject: [PATCH] On very high traffic Piwik servers, prevent integer overflow by making auto_increment fields BIGINT UNSIGNED (#10548) * Change Latitude and Longitude data types to DECIMAL instead of FLOAT * Add system test that forces an integer overflow, currently failing as expected * Make all overflow-able fields BIGINT UNSIGNED * Make overflow-able fields BIGINT UNSIGNED * These fields were not covered by the new SimulateAutoIncrementIntegerOverflow test * fix comment * Make it clear that id* fields are UNSIGNED when installing Piwik * BIGINT(10) for consistency * Update core fields to BIGINT * UI tests: New fields to be updated to bigint --- core/Db/Schema/Mysql.php | 34 ++++---- core/Updates/3.0.0-b1.php | 44 +++++++++- plugins/Actions/Columns/EntryPageTitle.php | 2 +- plugins/Actions/Columns/EntryPageUrl.php | 2 +- plugins/Actions/Columns/ExitPageTitle.php | 2 +- plugins/Actions/Columns/ExitPageUrl.php | 2 +- plugins/Actions/Columns/PageTitle.php | 2 +- plugins/Actions/Columns/PageUrl.php | 2 +- .../Contents/Columns/ContentInteraction.php | 2 +- plugins/Contents/Columns/ContentName.php | 2 +- plugins/Contents/Columns/ContentPiece.php | 2 +- plugins/Contents/Columns/ContentTarget.php | 2 +- plugins/Events/Columns/EventAction.php | 2 +- plugins/Events/Columns/EventCategory.php | 2 +- .../PHPUnit/Fixtures/OneVisitorTwoVisits.php | 22 +++++ ...mulateAutoIncrementIntegerOverflowTest.php | 78 ++++++++++++++++++ .../CoreUpdaterDb_main.png | Bin 131 -> 131 bytes 17 files changed, 172 insertions(+), 30 deletions(-) create mode 100644 tests/PHPUnit/System/SimulateAutoIncrementIntegerOverflowTest.php diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index fa0c096413..0511c65e23 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -128,7 +128,7 @@ class Mysql implements SchemaInterface ", 'log_action' => "CREATE TABLE {$prefixTables}log_action ( - idaction INTEGER(10) UNSIGNED NOT NULL AUTO_INCREMENT, + idaction BIGINT(10) UNSIGNED NOT NULL AUTO_INCREMENT, name TEXT, hash INTEGER(10) UNSIGNED NOT NULL, type TINYINT UNSIGNED NULL, @@ -139,7 +139,7 @@ class Mysql implements SchemaInterface ", 'log_visit' => "CREATE TABLE {$prefixTables}log_visit ( - idvisit INTEGER(10) UNSIGNED NOT NULL AUTO_INCREMENT, + idvisit BIGINT(10) UNSIGNED NOT NULL AUTO_INCREMENT, idsite INTEGER(10) UNSIGNED NOT NULL, idvisitor BINARY(8) NOT NULL, visit_last_action_time DATETIME NOT NULL, @@ -156,15 +156,15 @@ class Mysql implements SchemaInterface idsite int(10) UNSIGNED NOT NULL, idvisitor BINARY(8) NOT NULL, server_time DATETIME NOT NULL, - idvisit INTEGER(10) UNSIGNED NOT NULL, + idvisit BIGINT(10) UNSIGNED NOT NULL, idorder varchar(100) NOT NULL, - idaction_sku INTEGER(10) UNSIGNED NOT NULL, - idaction_name INTEGER(10) UNSIGNED NOT NULL, - idaction_category INTEGER(10) UNSIGNED NOT NULL, - idaction_category2 INTEGER(10) UNSIGNED NOT NULL, - idaction_category3 INTEGER(10) UNSIGNED NOT NULL, - idaction_category4 INTEGER(10) UNSIGNED NOT NULL, - idaction_category5 INTEGER(10) UNSIGNED NOT NULL, + idaction_sku BIGINT(10) UNSIGNED NOT NULL, + idaction_name BIGINT(10) UNSIGNED NOT NULL, + idaction_category BIGINT(10) UNSIGNED NOT NULL, + idaction_category2 BIGINT(10) UNSIGNED NOT NULL, + idaction_category3 BIGINT(10) UNSIGNED NOT NULL, + idaction_category4 BIGINT(10) UNSIGNED NOT NULL, + idaction_category5 BIGINT(10) UNSIGNED NOT NULL, price FLOAT NOT NULL, quantity INTEGER(10) UNSIGNED NOT NULL, deleted TINYINT(1) UNSIGNED NOT NULL, @@ -174,12 +174,12 @@ class Mysql implements SchemaInterface ", 'log_conversion' => "CREATE TABLE `{$prefixTables}log_conversion` ( - idvisit int(10) unsigned NOT NULL, + idvisit BIGINT(10) unsigned NOT NULL, idsite int(10) unsigned NOT NULL, idvisitor BINARY(8) NOT NULL, server_time datetime NOT NULL, - idaction_url int(11) default NULL, - idlink_va int(11) default NULL, + idaction_url BIGINT(10) UNSIGNED default NULL, + idlink_va BIGINT(10) UNSIGNED default NULL, idgoal int(10) NOT NULL, buster int unsigned NOT NULL, idorder varchar(100) default NULL, @@ -192,12 +192,12 @@ class Mysql implements SchemaInterface ", 'log_link_visit_action' => "CREATE TABLE {$prefixTables}log_link_visit_action ( - idlink_va INTEGER(11) UNSIGNED NOT NULL AUTO_INCREMENT, + idlink_va BIGINT(10) UNSIGNED NOT NULL AUTO_INCREMENT, idsite int(10) UNSIGNED NOT NULL, idvisitor BINARY(8) NOT NULL, - idvisit INTEGER(10) UNSIGNED NOT NULL, - idaction_url_ref INTEGER(10) UNSIGNED NULL DEFAULT 0, - idaction_name_ref INTEGER(10) UNSIGNED NULL, + idvisit BIGINT(10) UNSIGNED NOT NULL, + idaction_url_ref BIGINT(10) UNSIGNED NULL DEFAULT 0, + idaction_name_ref BIGINT(10) UNSIGNED NULL, custom_float FLOAT NULL DEFAULT NULL, PRIMARY KEY(idlink_va), INDEX index_idvisit(idvisit) diff --git a/core/Updates/3.0.0-b1.php b/core/Updates/3.0.0-b1.php index 8f62683f2a..9e88c80b63 100644 --- a/core/Updates/3.0.0-b1.php +++ b/core/Updates/3.0.0-b1.php @@ -53,7 +53,7 @@ class Updates_3_0_0_b1 extends Updates $migrations = $this->getDashboardMigrations($allDashboards, $allGoals); $migrations = $this->getPluginSettingsMigrations($migrations); $migrations = $this->getSiteSettingsMigrations($migrations); - $migrations[] = $this->migration->db->changeColumnType('log_link_visit_action', 'idaction_name_ref', 'INTEGER(10) UNSIGNED NULL'); + $migrations = $this->getBigIntPreventOverflowMigrations($migrations); return $migrations; } @@ -86,6 +86,48 @@ class Updates_3_0_0_b1 extends Updates }); } + /** + * @param Migration[] $queries + * @return Migration[] + */ + private function getBigIntPreventOverflowMigrations($queries) + { + $queries[] = $this->migration->db->changeColumnTypes('log_action', array( + 'idaction' => 'BIGINT(10) UNSIGNED NOT NULL AUTO_INCREMENT' + )); + + $queries[] = $this->migration->db->changeColumnTypes('log_visit', array( + 'idvisit' => 'BIGINT(10) UNSIGNED NOT NULL AUTO_INCREMENT' + )); + + $queries[] = $this->migration->db->changeColumnTypes('log_conversion_item', array( + 'idvisit' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_sku' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_name' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_category' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_category2' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_category3' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_category4' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_category5' => 'BIGINT(10) UNSIGNED NOT NULL', + )); + + $queries[] = $this->migration->db->changeColumnTypes('log_conversion', array( + 'idvisit' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_url' => 'BIGINT(10) UNSIGNED default NULL', + 'idlink_va' => 'BIGINT(10) UNSIGNED default NULL', + )); + + $queries[] = $this->migration->db->changeColumnTypes('log_link_visit_action', array( + 'idlink_va' => 'BIGINT(10) UNSIGNED NOT NULL AUTO_INCREMENT', + 'idvisit' => 'BIGINT(10) UNSIGNED NOT NULL', + 'idaction_url_ref' => 'BIGINT(10) UNSIGNED NULL DEFAULT 0', + // Note; this column is also made NULLable #9231 + 'idaction_name_ref' => 'BIGINT(10) UNSIGNED NULL', + )); + + return $queries; + } + /** * @param Migration[] $queries * @return Migration[] diff --git a/plugins/Actions/Columns/EntryPageTitle.php b/plugins/Actions/Columns/EntryPageTitle.php index 72e80f8783..c23568cc0a 100644 --- a/plugins/Actions/Columns/EntryPageTitle.php +++ b/plugins/Actions/Columns/EntryPageTitle.php @@ -18,7 +18,7 @@ use Piwik\Tracker\Visitor; class EntryPageTitle extends VisitDimension { protected $columnName = 'visit_entry_idaction_name'; - protected $columnType = 'INTEGER(11) UNSIGNED NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED NULL'; protected function configureSegments() { diff --git a/plugins/Actions/Columns/EntryPageUrl.php b/plugins/Actions/Columns/EntryPageUrl.php index 8adf0ab2f6..5ff80f43f5 100644 --- a/plugins/Actions/Columns/EntryPageUrl.php +++ b/plugins/Actions/Columns/EntryPageUrl.php @@ -18,7 +18,7 @@ use Piwik\Tracker\Visitor; class EntryPageUrl extends VisitDimension { protected $columnName = 'visit_entry_idaction_url'; - protected $columnType = 'INTEGER(11) UNSIGNED NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED NULL'; protected function configureSegments() { diff --git a/plugins/Actions/Columns/ExitPageTitle.php b/plugins/Actions/Columns/ExitPageTitle.php index 766897bd32..4d238b51bb 100644 --- a/plugins/Actions/Columns/ExitPageTitle.php +++ b/plugins/Actions/Columns/ExitPageTitle.php @@ -18,7 +18,7 @@ use Piwik\Tracker\Visitor; class ExitPageTitle extends VisitDimension { protected $columnName = 'visit_exit_idaction_name'; - protected $columnType = 'INTEGER(11) UNSIGNED NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED NULL'; protected function configureSegments() { diff --git a/plugins/Actions/Columns/ExitPageUrl.php b/plugins/Actions/Columns/ExitPageUrl.php index 1ef487f06b..36c8a31458 100644 --- a/plugins/Actions/Columns/ExitPageUrl.php +++ b/plugins/Actions/Columns/ExitPageUrl.php @@ -18,7 +18,7 @@ use Piwik\Tracker\Visitor; class ExitPageUrl extends VisitDimension { protected $columnName = 'visit_exit_idaction_url'; - protected $columnType = 'INTEGER(11) UNSIGNED NULL DEFAULT 0'; + protected $columnType = 'BIGINT(10) UNSIGNED NULL DEFAULT 0'; protected function configureSegments() { diff --git a/plugins/Actions/Columns/PageTitle.php b/plugins/Actions/Columns/PageTitle.php index 6517bede92..7590a456c9 100644 --- a/plugins/Actions/Columns/PageTitle.php +++ b/plugins/Actions/Columns/PageTitle.php @@ -15,7 +15,7 @@ use Piwik\Plugins\Actions\Segment; class PageTitle extends ActionDimension { protected $columnName = 'idaction_name'; - protected $columnType = 'INTEGER(10) UNSIGNED'; + protected $columnType = 'BIGINT(10) UNSIGNED'; protected function configureSegments() { diff --git a/plugins/Actions/Columns/PageUrl.php b/plugins/Actions/Columns/PageUrl.php index 1f9a7035e5..b33d0cfae8 100644 --- a/plugins/Actions/Columns/PageUrl.php +++ b/plugins/Actions/Columns/PageUrl.php @@ -15,7 +15,7 @@ use Piwik\Plugins\Actions\Segment; class PageUrl extends ActionDimension { protected $columnName = 'idaction_url'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/plugins/Contents/Columns/ContentInteraction.php b/plugins/Contents/Columns/ContentInteraction.php index 1c3632d3cc..003581a7f1 100644 --- a/plugins/Contents/Columns/ContentInteraction.php +++ b/plugins/Contents/Columns/ContentInteraction.php @@ -17,7 +17,7 @@ use Piwik\Tracker\Request; class ContentInteraction extends ActionDimension { protected $columnName = 'idaction_content_interaction'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/plugins/Contents/Columns/ContentName.php b/plugins/Contents/Columns/ContentName.php index 5ed5485791..1ad252e05b 100644 --- a/plugins/Contents/Columns/ContentName.php +++ b/plugins/Contents/Columns/ContentName.php @@ -17,7 +17,7 @@ use Piwik\Tracker\Request; class ContentName extends ActionDimension { protected $columnName = 'idaction_content_name'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/plugins/Contents/Columns/ContentPiece.php b/plugins/Contents/Columns/ContentPiece.php index 6affe9a3d0..bc75f63cfb 100644 --- a/plugins/Contents/Columns/ContentPiece.php +++ b/plugins/Contents/Columns/ContentPiece.php @@ -17,7 +17,7 @@ use Piwik\Tracker\Request; class ContentPiece extends ActionDimension { protected $columnName = 'idaction_content_piece'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/plugins/Contents/Columns/ContentTarget.php b/plugins/Contents/Columns/ContentTarget.php index 26c7a0d7a2..72d9ae7952 100644 --- a/plugins/Contents/Columns/ContentTarget.php +++ b/plugins/Contents/Columns/ContentTarget.php @@ -17,7 +17,7 @@ use Piwik\Tracker\Request; class ContentTarget extends ActionDimension { protected $columnName = 'idaction_content_target'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/plugins/Events/Columns/EventAction.php b/plugins/Events/Columns/EventAction.php index 79b3b5867b..9db6d1de0b 100644 --- a/plugins/Events/Columns/EventAction.php +++ b/plugins/Events/Columns/EventAction.php @@ -18,7 +18,7 @@ use Piwik\Tracker\Request; class EventAction extends ActionDimension { protected $columnName = 'idaction_event_action'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/plugins/Events/Columns/EventCategory.php b/plugins/Events/Columns/EventCategory.php index 7c6b5fd02a..2a0477d221 100644 --- a/plugins/Events/Columns/EventCategory.php +++ b/plugins/Events/Columns/EventCategory.php @@ -18,7 +18,7 @@ use Piwik\Tracker\Request; class EventCategory extends ActionDimension { protected $columnName = 'idaction_event_category'; - protected $columnType = 'INTEGER(10) UNSIGNED DEFAULT NULL'; + protected $columnType = 'BIGINT(10) UNSIGNED DEFAULT NULL'; protected function configureSegments() { diff --git a/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php b/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php index 822f6eb9ff..451cc98b11 100644 --- a/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php +++ b/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php @@ -7,7 +7,9 @@ */ namespace Piwik\Tests\Fixtures; +use Piwik\Common; use Piwik\Date; +use Piwik\Db; use Piwik\Plugins\Goals\API as APIGoals; use Piwik\Plugins\SitesManager\API as APISitesManager; use Piwik\Tests\Framework\Fixture; @@ -25,10 +27,13 @@ class OneVisitorTwoVisits extends Fixture public $useThirdPartyCookies = false; public $useSiteSearch = false; public $excludeMozilla = false; + public $simulateIntegerOverflow = false; + public $maxUnsignedIntegerValue = '4294967295'; public function setUp() { $this->setUpWebsitesAndGoals(); + $this->simulateIntegerOverflow(); $this->trackVisits(); } @@ -37,6 +42,23 @@ class OneVisitorTwoVisits extends Fixture // empty } + private function simulateIntegerOverflow() + { + if(!$this->simulateIntegerOverflow) { + return; + } + + $overflow = $this->maxUnsignedIntegerValue; + + $table = Common::prefixTable('log_visit'); + Db::query("INSERT INTO $table (idvisit) VALUES ($overflow)"); + $table = Common::prefixTable('log_action'); + Db::query("INSERT INTO $table (idaction) VALUES ($overflow)"); + $table = Common::prefixTable('log_link_visit_action'); + Db::query("INSERT INTO $table (idlink_va) VALUES ($overflow)"); + + } + private function setUpWebsitesAndGoals() { if (!self::siteCreated($idSite = 1)) { diff --git a/tests/PHPUnit/System/SimulateAutoIncrementIntegerOverflowTest.php b/tests/PHPUnit/System/SimulateAutoIncrementIntegerOverflowTest.php new file mode 100644 index 0000000000..026c7719a1 --- /dev/null +++ b/tests/PHPUnit/System/SimulateAutoIncrementIntegerOverflowTest.php @@ -0,0 +1,78 @@ +<?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\Tests\System; + +use Piwik\Common; +use Piwik\Db; +use Piwik\Tests\Framework\TestCase\SystemTestCase; +use Piwik\Tests\Fixtures\OneVisitorTwoVisits; + +/** + * Simulates the case where there are more than 4 billion visits and pages, and check that Piwik + * can handle this use case without hitting MySQL INT UNSIGNED overflow. + * + * This system test will compare against the OneVisitorTwoVisitsWithCookieSupportTest system test + * (OneVisitorTwoVisits_withCookieSupport* expected api responses) and expect the same API responses. + * + * @group SimulateAutoIncrementIntegerOverflowTest + * @group Core + */ +class SimulateAutoIncrementIntegerOverflowTest extends SystemTestCase +{ + /** OneVisitorTwoVisits */ + public static $fixture = null; // initialized below class + + /** + * @dataProvider getApiForTesting + */ + public function testApi($api, $params) + { + $this->runApiTests($api, $params); + $this->checkAutoIncrementIdsAreMoreThanFourBillion(); + } + + private function checkAutoIncrementIdsAreMoreThanFourBillion() + { + $fieldsThatShouldNotOverflow = array( + 'log_visit' => 'idvisit', + 'log_action' => 'idaction', + 'log_link_visit_action' => 'idlink_va' + ); + $this->assertGreaterThan(4294967294, self::$fixture->maxUnsignedIntegerValue); + + foreach($fieldsThatShouldNotOverflow as $table => $autoIncrementField) { + $table = Common::prefixTable($table); + $value = Db::fetchOne("SELECT MAX($autoIncrementField) FROM $table "); + $this->assertGreaterThan(self::$fixture->maxUnsignedIntegerValue, $value, 'in ' . $table); + } + } + + public function getApiForTesting() + { + $apiToCall = array( + 'VisitTime', 'VisitsSummary', 'VisitorInterest', 'VisitFrequency', 'DevicesDetection', + 'UserCountry', + 'Provider', 'Goals', 'CustomVariables', 'CoreAdminHome', 'DevicePlugins', + 'Actions', 'Referrers', + ); + + return array( + array($apiToCall, array( + 'idSite' => self::$fixture->idSite, + 'date' => self::$fixture->dateTime, + 'compareAgainst' => 'OneVisitorTwoVisits_withCookieSupport', + )) + ); + } + +} + +SimulateAutoIncrementIntegerOverflowTest::$fixture = new OneVisitorTwoVisits(); +SimulateAutoIncrementIntegerOverflowTest::$fixture->useThirdPartyCookies = true; +SimulateAutoIncrementIntegerOverflowTest::$fixture->useSiteSearch = true; +SimulateAutoIncrementIntegerOverflowTest::$fixture->simulateIntegerOverflow = true; \ No newline at end of file diff --git a/tests/UI/expected-screenshots/CoreUpdaterDb_main.png b/tests/UI/expected-screenshots/CoreUpdaterDb_main.png index 95f59ef227406dd68c0275a3d1d583819fc7d5a2..868a987f49191fce57bc48cfaf43f403e8a86acd 100644 GIT binary patch delta 84 zcmV~$u@QhU2mrvd%@mG+1Og1<5MbcW+Rio$II{n1TiZ8ICFkU%K#USJ9^8dh6Bvrh drW*N#i~@-m1~?<bv0HMaU+uZy+)=0>+J53o7h?bb delta 84 zcmV~$u?@f=3<N;YGDSu}7~U{MhVa`&N}Vn(3uNTn?X=T}6OAnDb&4;b6H&C9xJjsj dti`7UCz`peF({J->k2kA1M_pg>!|1|$UpVf7q0*S -- GitLab