From 664bfbfbcf7b1a5118016a402cbd9e0bb3cf4d43 Mon Sep 17 00:00:00 2001 From: Andrii Kasian Date: Wed, 7 Oct 2020 11:39:16 -0500 Subject: [PATCH 1/3] Improvement of connection retry mechanism --- .../Framework/DB/Adapter/Pdo/Mysql.php | 181 ++++++++---------- .../DB/Test/Unit/Adapter/Pdo/MysqlTest.php | 132 ++++++++----- 2 files changed, 171 insertions(+), 142 deletions(-) diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index c5e17a97c9f01..55c1268fc54d1 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -66,6 +66,7 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface /** * MEMORY engine type for MySQL tables + * @deprecated Use InnoDB/ExtraDB engine instead */ public const ENGINE_MEMORY = 'MEMORY'; @@ -86,7 +87,7 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface * * @var int */ - protected $_transactionLevel = 0; + protected $_transactionLevel = 0; /** * Whether transaction was rolled back or not @@ -100,28 +101,28 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface * * @var bool */ - protected $_connectionFlagsSet = false; + protected $_connectionFlagsSet = false; /** * Tables DDL cache * * @var array */ - protected $_ddlCache = []; + protected $_ddlCache = []; /** * SQL bind params. Used temporarily by regexp callback. * * @var array */ - protected $_bindParams = []; + protected $_bindParams = []; /** * Autoincrement for bind value. Used by regexp callback. * * @var int */ - protected $_bindIncrement = 0; + protected $_bindIncrement = 0; /** * Cache frontend adapter instance @@ -467,6 +468,9 @@ private function createConnection() /** * Run RAW Query * + * @deprecated + * Use \Magento\Framework\DB\Adapter\AdapterInterface::query instead + * * @param string $sql * @return \Zend_Db_Statement_Interface * @throws \PDOException @@ -490,6 +494,9 @@ public function rawQuery($sql) /** * Run RAW query and Fetch First row * + * @deprecated + * Use \Magento\Framework\DB\Adapter\AdapterInterface::fetchRow instead + * * @param string $sql * @param string|int $field * @return mixed|null @@ -581,11 +588,12 @@ protected function _query($sql, $bind = []) // Check to reconnect if ($pdoException && $triesCount < self::MAX_CONNECTION_RETRIES && in_array($pdoException->errorInfo[1], $connectionErrors) + && $this->getTransactionLevel() == 0 // Connect retry doesn't make sense in transaction ) { $retry = true; $triesCount++; $this->closeConnection(); - + usleep($triesCount * 100000); $this->_connect(); } @@ -736,6 +744,7 @@ protected function _unQuote($string) * Normalizes mixed positional-named bind to positional bind, and replaces named placeholders in query to * '?' placeholders. * + * @deprecated It's not used now * @param string $sql * @param array $bind * @return $this @@ -1461,42 +1470,6 @@ public function getIndexList($tableName, $schemaName = null) return $ddl; } - /** - * Remove duplicate entry for create key - * - * @param string $table - * @param array $fields - * @param string[] $ids - * @return $this - */ - protected function _removeDuplicateEntry($table, $fields, $ids) - { - $where = []; - $i = 0; - foreach ($fields as $field) { - $where[] = $this->quoteInto($field . '=?', $ids[$i++]); - } - - if (!$where) { - return $this; - } - $whereCond = implode(' AND ', $where); - $sql = sprintf('SELECT COUNT(*) as `cnt` FROM `%s` WHERE %s', $table, $whereCond); - - $cnt = $this->rawFetchRow($sql, 'cnt'); - if ($cnt > 1) { - $sql = sprintf( - 'DELETE FROM `%s` WHERE %s LIMIT %d', - $table, - $whereCond, - $cnt - 1 - ); - $this->rawQuery($sql); - } - - return $this; - } - /** * Creates and returns a new \Magento\Framework\DB\Select object for this adapter. * @@ -1871,47 +1844,51 @@ public function modifyColumnByDdl($tableName, $columnName, $definition, $flushDa * Retrieve column data type by data from describe table * * @param array $column - * @return string|null - * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @return string + * @throws \Zend_Db_Adapter_Exception */ protected function _getColumnTypeByDdl($column) { // phpstan:ignore - switch ($column['DATA_TYPE']) { - case 'bool': - return Table::TYPE_BOOLEAN; - case 'tinytext': - case 'char': - case 'varchar': - case 'text': - case 'mediumtext': - case 'longtext': - return Table::TYPE_TEXT; - case 'blob': - case 'mediumblob': - case 'longblob': - return Table::TYPE_BLOB; - case 'tinyint': - case 'smallint': - return Table::TYPE_SMALLINT; - case 'mediumint': - case 'int': - return Table::TYPE_INTEGER; - case 'bigint': - return Table::TYPE_BIGINT; - case 'datetime': - return Table::TYPE_DATETIME; - case 'timestamp': - return Table::TYPE_TIMESTAMP; - case 'date': - return Table::TYPE_DATE; - case 'float': - return Table::TYPE_FLOAT; - case 'decimal': - case 'numeric': - return Table::TYPE_DECIMAL; + static $mapping = [ + 'bool' => Table::TYPE_BOOLEAN, + + 'tinytext' => Table::TYPE_TEXT, + 'char' => Table::TYPE_TEXT, + 'varchar' => Table::TYPE_TEXT, + 'text' => Table::TYPE_TEXT, + 'mediumtext' => Table::TYPE_TEXT, + 'longtext' => Table::TYPE_TEXT, + + 'blob' => Table::TYPE_BLOB, + 'mediumblob' => Table::TYPE_BLOB, + 'longblob' => Table::TYPE_BLOB, + + 'tinyint' => Table::TYPE_SMALLINT, + 'smallint' => Table::TYPE_SMALLINT, + + 'mediumint' => Table::TYPE_INTEGER, + 'int' => Table::TYPE_INTEGER, + + 'bigint' => Table::TYPE_BIGINT, + + 'datetime' => Table::TYPE_DATETIME, + + 'timestamp' => Table::TYPE_TIMESTAMP, + + 'date' => Table::TYPE_DATE, + + 'float' => Table::TYPE_FLOAT, + + 'decimal' => Table::TYPE_DECIMAL, + 'numeric' => Table::TYPE_DECIMAL, + ]; + + if (!isset($mapping[$column['DATA_TYPE']])) { + throw new \Zend_Db_Adapter_Exception('Unknown data type: ' . $column['DATA_TYPE']); } - return null; + + return $mapping[$column['DATA_TYPE']]; } /** @@ -2798,24 +2775,7 @@ public function addIndex( $query .= sprintf(' ADD %s (%s)', $condition, $fieldSql); - $cycle = true; - while ($cycle === true) { - try { - $result = $this->rawQuery($query); - $cycle = false; - } catch (\Exception $e) { - if (in_array(strtolower($indexType), ['primary', 'unique'])) { - $match = []; - // phpstan:ignore - if (preg_match('#SQLSTATE\[23000\]: [^:]+: 1062[^\']+\'([\d.-]+)\'#', $e->getMessage(), $match)) { - $ids = explode('-', $match[1]); - $this->_removeDuplicateEntry($tableName, $fields, $ids); - continue; - } - } - throw $e; - } - } + $result = $this->rawQuery($query); $this->resetDdlCache($tableName, $schemaName); @@ -4120,4 +4080,33 @@ public function closeConnection() } parent::closeConnection(); } + + /** + * @inheritDoc + */ + public function quote($value, $type = null) + { + $connectionErrors = [ + 2006, // SQLSTATE[HY000]: General error: 2006 MySQL server has gone away + 2013, // SQLSTATE[HY000]: General error: 2013 Lost connection to MySQL server during query + ]; + $triesCount = 0; + while (true) { + try { + return parent::quote($value, $type); + } catch (\PDOException $e) { + // Check to reconnect + if (in_array($e->errorInfo[1], $connectionErrors) + && $this->getTransactionLevel() == 0 + && $triesCount < self::MAX_CONNECTION_RETRIES + ) { + $triesCount++; + $this->closeConnection(); + usleep($triesCount * 100000); + } else { + throw $e; + } + } + } + } } diff --git a/lib/internal/Magento/Framework/DB/Test/Unit/Adapter/Pdo/MysqlTest.php b/lib/internal/Magento/Framework/DB/Test/Unit/Adapter/Pdo/MysqlTest.php index cf29393820f8b..8886706658145 100644 --- a/lib/internal/Magento/Framework/DB/Test/Unit/Adapter/Pdo/MysqlTest.php +++ b/lib/internal/Magento/Framework/DB/Test/Unit/Adapter/Pdo/MysqlTest.php @@ -502,6 +502,90 @@ public function testConfigValidationByPortWithException() ); } + /** + * @return array + */ + public static function quoteProvider(): array + { + return [ + [2006, 10], + [2006, 1], + [2013, 1], + [2013, 5], + ]; + } + + /** + * @test + * @dataProvider quoteProvider + * + * @param $error + * @param $max + */ + public function quoteRetry($error, $max) + { + $exception = new \PDOException(); + $exception->errorInfo[1] = $error; + $adapter = $this->getMysqlPdoAdapterMock(['_connect']); + for ($i = 0; $i < $max; $i++) { + $adapter->expects($this->at($i))->method('_connect')->willThrowException($exception); + } + $this->assertEquals(0, $adapter->quote(0)); + } + + /** + * @test + */ + public function quoteNoRetryMoreThen10Times() + { + $this->expectException(\PDOException::class); + $exception = new \PDOException(); + $adapter = $this->getMysqlPdoAdapterMock(['_connect']); + $exception->errorInfo[1] = 2006; + for ($i = 0; $i < 11; $i++) { + $adapter->expects($this->at($i))->method('_connect')->willThrowException($exception); + } + $this->assertEquals(0, $adapter->quote(0)); + } + + /** + * @test + */ + public function quoteNoRetryOnOtherErrors() + { + $this->expectException(\PDOException::class); + $exception = new \PDOException(); + $exception->errorInfo[1] = 234234234; + $adapter = $this->getMysqlPdoAdapterMock(['_connect']); + $adapter->expects($this->at(0))->method('_connect')->willThrowException($exception); + $this->assertEquals(0, $adapter->quote(0)); + } + + /** + * @test + */ + public function quoteNoRetryInTransaction() + { + $this->expectException(\PDOException::class); + $exception = new \PDOException(); + $exception->errorInfo[1] = 2006; + + $adapter = $this->getMysqlPdoAdapterMock(['_connect']); + $resourceProperty = new \ReflectionProperty( + get_class($adapter), + '_transactionLevel' + ); + $resourceProperty->setAccessible(true); + $resourceProperty->setValue($adapter, 1); + + $adapter->expects($this->at(0))->method('_connect')->willThrowException($exception); + try { + $this->assertEquals(0, $adapter->quote(0)); + } finally { + $resourceProperty->setValue($adapter, 0); + } + } + /** * @param string $indexName * @param string $indexType @@ -517,8 +601,7 @@ public function testAddIndexWithDuplicationsInDB( string $indexType, array $keyLists, string $query, - string $exceptionMessage, - array $ids + string $exceptionMessage ) { $tableName = 'core_table'; $fields = ['sku', 'field2']; @@ -541,7 +624,6 @@ public function testAddIndexWithDuplicationsInDB( 'quoteIdentifier', '_getTableName', 'rawQuery', - '_removeDuplicateEntry', 'resetDdlCache', ]); $this->addConnectionMock($adapter); @@ -587,11 +669,7 @@ public function testAddIndexWithDuplicationsInDB( ) ) ->willThrowException($exception); - $adapter - ->expects($this->exactly((int)in_array(strtolower($indexType), ['primary', 'unique']))) - ->method('_removeDuplicateEntry') - ->with($tableName, $fields, $ids) - ->willThrowException($exception); + $adapter ->expects($this->never()) ->method('resetDdlCache'); @@ -619,44 +697,6 @@ public function addIndexWithDuplicationsInDBDataProvider(): array 'exceptionMessage' => 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry \'1-1-1\' ' . 'for key \'SOME_UNIQUE_INDEX\', query was: ' . 'ALTER TABLE `%s` ADD UNIQUE `SOME_UNIQUE_INDEX` (%s)', - 'ids' => [1, 1, 1], - ], - 'Existing unique index' => [ - 'indexName' => 'SOME_UNIQUE_INDEX', - 'indexType' => AdapterInterface::INDEX_TYPE_UNIQUE, - 'keyLists' => [ - 'PRIMARY' => [ - 'INDEX_TYPE' => [ - AdapterInterface::INDEX_TYPE_PRIMARY - ] - ], - 'SOME_UNIQUE_INDEX' => [ - 'INDEX_TYPE' => [ - AdapterInterface::INDEX_TYPE_UNIQUE - ] - ], - ], - 'query' => 'ALTER TABLE `%s` DROP INDEX `SOME_UNIQUE_INDEX`, ADD UNIQUE `SOME_UNIQUE_INDEX` (%s)', - 'exceptionMessage' => 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry \'1-2-5\' ' - . 'for key \'SOME_UNIQUE_INDEX\', query was: ' - . 'ALTER TABLE `%s` DROP INDEX `SOME_UNIQUE_INDEX`, ADD UNIQUE `SOME_UNIQUE_INDEX` (%s)', - 'ids' => [1, 2, 5], - ], - 'New primary index' => [ - 'indexName' => 'PRIMARY', - 'indexType' => AdapterInterface::INDEX_TYPE_PRIMARY, - 'keyLists' => [ - 'SOME_UNIQUE_INDEX' => [ - 'INDEX_TYPE' => [ - AdapterInterface::INDEX_TYPE_UNIQUE - ] - ], - ], - 'query' => 'ALTER TABLE `%s` ADD PRIMARY KEY (%s)', - 'exceptionMessage' => 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry \'1-3-4\' ' - . 'for key \'PRIMARY\', query was: ' - . 'ALTER TABLE `%s` ADD PRIMARY KEY (%s)', - 'ids' => [1, 3, 4], ], ]; } From dd8f5a050639b08a4ddc2472a4094f2f7247fc24 Mon Sep 17 00:00:00 2001 From: Andrii Kasian Date: Tue, 16 Jan 2024 12:56:17 -0600 Subject: [PATCH 2/3] Update lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php --- lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index fde00d5687ae3..2ff750ad16633 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -4228,7 +4228,7 @@ public function quote($value, $type = null) } } } - + } /** * Disable show internals with var_dump * From 9ec43cb72a5c1a60bd08693a205f83f18611b4e9 Mon Sep 17 00:00:00 2001 From: Andrii Kasian Date: Tue, 16 Jan 2024 12:57:10 -0600 Subject: [PATCH 3/3] Update lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php --- lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index 2ff750ad16633..29b71f5c03429 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -4229,6 +4229,7 @@ public function quote($value, $type = null) } } } + /** * Disable show internals with var_dump *