Skip to content

Commit

Permalink
Merge pull request #5651 from magento-performance/MC-31920
Browse files Browse the repository at this point in the history
[Performance] Let DB lock manager be the primary locker
  • Loading branch information
duhon authored May 8, 2020
2 parents 40a7876 + 24e0ae2 commit 0423eae
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 30 deletions.
2 changes: 1 addition & 1 deletion app/code/Magento/Config/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@

<virtualType name="systemConfigQueryLocker" type="Magento\Framework\Cache\LockGuardedCacheLoader">
<arguments>
<argument name="locker" xsi:type="object">Magento\Framework\Lock\Backend\Cache</argument>
<argument name="locker" xsi:type="object">Magento\Framework\Lock\Backend\Database</argument>
</arguments>
</virtualType>

Expand Down
2 changes: 1 addition & 1 deletion app/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1786,7 +1786,7 @@
</type>
<type name="Magento\Framework\Cache\LockGuardedCacheLoader">
<arguments>
<argument name="locker" xsi:type="object">Magento\Framework\Lock\Backend\Cache</argument>
<argument name="locker" xsi:type="object">Magento\Framework\Lock\Backend\Database</argument>
</arguments>
</type>
<preference for="Magento\Framework\HTTP\AsyncClientInterface" type="Magento\Framework\HTTP\AsyncClient\GuzzleAsyncClient" />
Expand Down
2 changes: 1 addition & 1 deletion dev/tests/integration/etc/di/preferences/ce.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
\Magento\Framework\App\Config\ScopeConfigInterface::class => \Magento\TestFramework\App\Config::class,
\Magento\Framework\App\ResourceConnection\ConfigInterface::class =>
\Magento\Framework\App\ResourceConnection\Config::class,
\Magento\Framework\Lock\Backend\Cache::class =>
\Magento\Framework\Lock\Backend\Database::class =>
\Magento\TestFramework\Lock\Backend\DummyLocker::class,
\Magento\Framework\Session\SessionStartChecker::class => \Magento\TestFramework\Session\SessionStartChecker::class,
\Magento\Framework\HTTP\AsyncClientInterface::class => \Magento\TestFramework\HTTP\AsyncClientInterfaceMock::class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

namespace Magento\Framework\Lock\Backend;

use Magento\Framework\App\ResourceConnection;
use Magento\Framework\App\DeploymentConfig;

/**
* \Magento\Framework\Lock\Backend\Database test case
*/
Expand All @@ -25,7 +28,10 @@ class DatabaseTest extends \PHPUnit\Framework\TestCase
protected function setUp(): void
{
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
$this->model = $this->objectManager->create(\Magento\Framework\Lock\Backend\Database::class);
$resourceConnection = $this->objectManager->create(ResourceConnection::class);
$deploymentConfig = $this->objectManager->create(DeploymentConfig::class);
// create object with new otherwise dummy locker is created because of di.xml preference for integration tests
$this->model = new Database($resourceConnection, $deploymentConfig);
}

public function testLockAndUnlock()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
*/
namespace Magento\MessageQueue\Model\Cron;

use Magento\Framework\App\DeploymentConfig;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\Lock\Backend\Database;
use Magento\Framework\MessageQueue\Consumer\ConfigInterface as ConsumerConfigInterface;
use Magento\Framework\Lock\LockManagerInterface;
use Magento\Framework\App\DeploymentConfig\FileReader;
Expand Down Expand Up @@ -83,7 +86,10 @@ protected function setUp(): void
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
$this->shellMock = $this->getMockBuilder(ShellInterface::class)
->getMockForAbstractClass();
$this->lockManager = $this->objectManager->get(LockManagerInterface::class);
$resourceConnection = $this->objectManager->create(ResourceConnection::class);
$deploymentConfig = $this->objectManager->create(DeploymentConfig::class);
// create object with new otherwise dummy locker is created because of di.xml preference for integration tests
$this->lockManager = new Database($resourceConnection, $deploymentConfig);
$this->consumerConfig = $this->objectManager->get(ConsumerConfigInterface::class);
$this->reader = $this->objectManager->get(FileReader::class);
$this->filesystem = $this->objectManager->get(Filesystem::class);
Expand Down
21 changes: 4 additions & 17 deletions lib/internal/Magento/Framework/Lock/Backend/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
use Magento\Framework\App\DeploymentConfig;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\Config\ConfigOptionsListConstants;
use Magento\Framework\Exception\AlreadyExistsException;
use Magento\Framework\Phrase;
use Magento\Framework\DB\ExpressionConverter;
use Magento\Framework\Exception\FileSystemException;
use Magento\Framework\Exception\RuntimeException;

/**
* Implementation of the lock manager on the basis of MySQL.
Expand Down Expand Up @@ -68,7 +68,8 @@ public function __construct(
* @param string $name lock name
* @param int $timeout How long to wait lock acquisition in seconds, negative value means infinite timeout
* @return bool
* @throws AlreadyExistsException
* @throws FileSystemException
* @throws RuntimeException
* @throws \Zend_Db_Statement_Exception
*/
public function lock(string $name, int $timeout = -1): bool
Expand All @@ -78,20 +79,6 @@ public function lock(string $name, int $timeout = -1): bool
}
$name = $this->addPrefix($name);

/**
* Before MySQL 5.7.5, only a single simultaneous lock per connection can be acquired.
* This limitation can be removed once MySQL minimum requirement has been raised,
* currently we support MySQL 5.6 way only.
*/
if ($this->currentLock) {
throw new AlreadyExistsException(
new Phrase(
'Current connection is already holding lock for %1, only single lock allowed',
[$this->currentLock]
)
);
}

$result = (bool)$this->resource->getConnection()->query(
"SELECT GET_LOCK(?, ?);",
[$name, $timeout < 0 ? self::MAX_LOCK_TIME : $timeout]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Magento\Framework\App\DeploymentConfig;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\Exception\AlreadyExistsException;
use Magento\Framework\Lock\Backend\Database;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use PHPUnit\Framework\MockObject\MockObject;
Expand Down Expand Up @@ -90,7 +89,6 @@ protected function setUp(): void
}

/**
* @throws AlreadyExistsException
* @throws \Zend_Db_Statement_Exception
*/
public function testLock()
Expand All @@ -107,7 +105,6 @@ public function testLock()
}

/**
* @throws AlreadyExistsException
* @throws \Zend_Db_Statement_Exception
*/
public function testlockWithTooLongName()
Expand All @@ -124,12 +121,10 @@ public function testlockWithTooLongName()
}

/**
* @throws AlreadyExistsException
* @throws \Zend_Db_Statement_Exception
*/
public function testlockWithAlreadyAcquiredLockInSameSession()
{
$this->expectException('Magento\Framework\Exception\AlreadyExistsException');
$this->deploymentConfig
->method('isDbAvailable')
->with()
Expand All @@ -138,12 +133,11 @@ public function testlockWithAlreadyAcquiredLockInSameSession()
->method('fetchColumn')
->willReturn(true);

$this->database->lock('testLock');
$this->database->lock('differentLock');
$this->assertTrue($this->database->lock('testLock'));
$this->assertTrue($this->database->lock('differentLock'));
}

/**
* @throws AlreadyExistsException
* @throws \Zend_Db_Statement_Exception
*/
public function testLockWithUnavailableDeploymentConfig()
Expand Down

0 comments on commit 0423eae

Please sign in to comment.