Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent running again already running cron group #12497

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c959393
Add LockManager and Backend to Framework functionalities
paveq Nov 30, 2017
702c93a
Implement Locker in CronQueueObserver
paveq Nov 30, 2017
fcda71d
Do cleanup and generate of schedules under cron group lock
paveq Nov 30, 2017
016387c
Add scalar type hints, slight change of wording
paveq Nov 30, 2017
23cb078
Code style fix
paveq Nov 30, 2017
b69945b
Add static prefix for cron locks
paveq Dec 1, 2017
071df9f
Add doc block
paveq Dec 1, 2017
7a88a9a
Add unit and integration tests for locking
paveq Dec 1, 2017
2972e21
Fix failing tests
paveq Dec 1, 2017
4ad9c5d
Amend docblocks
paveq Dec 1, 2017
1e546c8
Add README for Lock library, fix issue with lock release, add a comment
paveq Dec 1, 2017
47759c8
rename setLock to acquireLock for consistency
paveq Dec 1, 2017
dd0b861
Add lockManagerMock for ProcessCronQueueObserverTest
paveq Dec 1, 2017
d1b56b2
Code style fix
paveq Dec 1, 2017
c2e5520
Fix ProcessCronQueueObserverTest
paveq Dec 1, 2017
7992e1e
Add DB name as lock prefix
paveq Dec 5, 2017
2a7099e
Fix failing unit test
paveq Dec 5, 2017
79310b9
Fix code style violations
paveq Dec 5, 2017
229476f
Add checks for pre-MySQL 5.7 locking, print out lock name if too long
paveq Feb 21, 2018
d12581e
Fix failing test case
paveq Feb 21, 2018
bf8409b
Fix failing static test
paveq Feb 21, 2018
209b2bd
Simplify method verbs in LockInterface
paveq Feb 22, 2018
c8be788
#12497 Prevent running again already running cron group
kandy Mar 24, 2018
83b17fe
Merge remote-tracking branch 'mainline/2.2-develop' into cron_group_l…
kandy Mar 25, 2018
79d60cb
#12497 Prevent running again already running cron group
kandy Mar 25, 2018
50dcc3d
magento/magento2#12497
Mar 28, 2018
175229e
magento/magento2#12497
Mar 28, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
427 changes: 279 additions & 148 deletions app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php

Large diffs are not rendered by default.

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions app/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
<preference for="Magento\Framework\Locale\ListsInterface" type="Magento\Framework\Locale\TranslatedLists" />
<preference for="Magento\Framework\Locale\AvailableLocalesInterface" type="Magento\Framework\Locale\Deployed\Codes" />
<preference for="Magento\Framework\Locale\OptionInterface" type="Magento\Framework\Locale\Deployed\Options" />
<preference for="Magento\Framework\Lock\LockManagerInterface" type="Magento\Framework\Lock\Backend\Database" />
<preference for="Magento\Framework\Api\AttributeTypeResolverInterface" type="Magento\Framework\Reflection\AttributeTypeResolver" />
<preference for="Magento\Framework\Api\Search\SearchResultInterface" type="Magento\Framework\Api\Search\SearchResult" />
<preference for="Magento\Framework\Api\Search\SearchCriteriaInterface" type="Magento\Framework\Api\Search\SearchCriteria"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

/**
* \Magento\Framework\Lock\Backend\Database test case
*/
namespace Magento\Framework\Lock\Backend;

class DatabaseTest extends \PHPUnit\Framework\TestCase
{
/**
* @var \Magento\Framework\Lock\Backend\Database
*/
private $model;

/**
* @var \Magento\Framework\ObjectManagerInterface
*/
private $objectManager;

protected function setUp()
{
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
$this->model = $this->objectManager->create(\Magento\Framework\Lock\Backend\Database::class);
}

public function testLockAndUnlock()
{
$name = 'test_lock';

$this->assertFalse($this->model->isLocked($name));

$this->assertTrue($this->model->lock($name));
$this->assertTrue($this->model->isLocked($name));

$this->assertTrue($this->model->unlock($name));
$this->assertFalse($this->model->isLocked($name));
}

public function testUnlockWithoutExistingLock()
{
$name = 'test_lock';

$this->assertFalse($this->model->isLocked($name));
$this->assertFalse($this->model->unlock($name));
}
}
158 changes: 158 additions & 0 deletions lib/internal/Magento/Framework/Lock/Backend/Database.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);
namespace Magento\Framework\Lock\Backend;

use Magento\Framework\App\DeploymentConfig;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\Config\ConfigOptionsListConstants;
use Magento\Framework\Exception\AlreadyExistsException;
use Magento\Framework\Exception\InputException;
use Magento\Framework\Phrase;

class Database implements \Magento\Framework\Lock\LockManagerInterface
{
/** @var ResourceConnection */
private $resource;

/** @var DeploymentConfig */
private $deploymentConfig;

/** @var string Lock prefix */
private $prefix;

/** @var string|false Holds current lock name if set, otherwise false */
private $currentLock = false;

public function __construct(
ResourceConnection $resource,
DeploymentConfig $deploymentConfig,
string $prefix = null
) {
$this->resource = $resource;
$this->deploymentConfig = $deploymentConfig;
$this->prefix = $prefix;
}

/**
* Sets a lock for name
*
* @param string $name lock name
* @param int $timeout How long to wait lock acquisition in seconds, negative value means infinite timeout
* @return bool
* @throws InputException
* @throws AlreadyExistsException
*/
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(?, ?);",
[(string)$name, (int)$timeout]
)->fetchColumn();

if ($result === true) {
$this->currentLock = $name;
}

return $result;
}

/**
* Releases a lock for name
*
* @param string $name lock name
* @return bool
* @throws InputException
*/
public function unlock(string $name): bool
{
$name = $this->addPrefix($name);

$result = (bool)$this->resource->getConnection()->query(
"SELECT RELEASE_LOCK(?);",
[(string)$name]
)->fetchColumn();

if ($result === true) {
$this->currentLock = false;
}

return $result;
}

/**
* Tests of lock is set for name
*
* @param string $name lock name
* @return bool
* @throws InputException
*/
public function isLocked(string $name): bool
{
$name = $this->addPrefix($name);

return (bool)$this->resource->getConnection()->query(
"SELECT IS_USED_LOCK(?);",
[(string)$name]
)->fetchColumn();
}

/**
* Adds prefix and checks for max length of lock name
*
* Limited to 64 characters in MySQL.
*
* @param string $name
* @return string $name
* @throws InputException
*/
private function addPrefix(string $name): string
{
$name = $this->getPrefix() . '|' . $name;

if (strlen($name) > 64) {
throw new InputException(new Phrase('Lock name too long: %1...', [substr($name, 0, 64)]));
}

return $name;
}

/**
* Get installation specific lock prefix to avoid lock conflicts
*
* @return string lock prefix
*/
private function getPrefix(): string
{
if ($this->prefix === null) {
$this->prefix = (string)$this->deploymentConfig->get(
ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTION_DEFAULT
. '/'
. ConfigOptionsListConstants::KEY_NAME,
''
);
}

return $this->prefix;
}
}
44 changes: 44 additions & 0 deletions lib/internal/Magento/Framework/Lock/LockManagerInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);
namespace Magento\Framework\Lock;

/**
* Interface of a lock manager
*
* @api
*/
interface LockManagerInterface
{
/**
* Sets a lock
*
* @param string $name lock name
* @param int $timeout How long to wait lock acquisition in seconds, negative value means infinite timeout
* @return bool
* @api
*/
public function lock(string $name, int $timeout = -1): bool;

/**
* Releases a lock
*
* @param string $name lock name
* @return bool
* @api
*/
public function unlock(string $name): bool;

/**
* Tests if lock is set
*
* @param string $name lock name
* @return bool
* @api
*/
public function isLocked(string $name): bool;
}
8 changes: 8 additions & 0 deletions lib/internal/Magento/Framework/Lock/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Lock library

Lock library provides mechanism to acquire Magento system-wide lock. Default implementation is based on MySQL locks, where any locks are automatically released on connection close.

The library provides interface *LockManagerInterface* which provides following methods:
* *lock* - Acquires a named lock
* *unlock* - Releases a named lock
* *isLocked* - Tests if a named lock exists
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Framework\Lock\Test\Unit\Backend;

use Magento\Framework\Lock\Backend\Database;

class DatabaseTest extends \PHPUnit\Framework\TestCase
{
/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\ResourceConnection
*/
private $resource;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Adapter\AdapterInterface
*/
private $connection;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Zend_Db_Statement_Interface
*/
private $statement;

/**
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
*/
private $objectManager;

/** @var Database $database */
private $database;

protected function setUp()
{
$this->connection = $this->getMockBuilder(\Magento\Framework\DB\Adapter\AdapterInterface::class)
->disableOriginalConstructor()
->getMockForAbstractClass();
$this->resource = $this->getMockBuilder(\Magento\Framework\App\ResourceConnection::class)
->disableOriginalConstructor()
->getMock();
$this->statement = $this->getMockBuilder(\Zend_Db_Statement_Interface::class)
->disableOriginalConstructor()
->getMockForAbstractClass();

$this->resource->expects($this->any())
->method('getConnection')
->willReturn($this->connection);

$this->connection->expects($this->any())
->method('query')
->willReturn($this->statement);

$this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);

/** @var Database $database */
$this->database = $this->objectManager->getObject(
Database::class,
['resource' => $this->resource]
);
}

public function testLock()
{
$this->statement->expects($this->once())
->method('fetchColumn')
->willReturn(true);

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

/**
* @expectedException \Magento\Framework\Exception\InputException
*/
public function testlockWithTooLongName()
{
$this->database->lock('BbXbyf9rIY5xuAVdviQJmh76FyoeeVHTDpcjmcImNtgpO4Hnz4xk76ZGEyYALvrQu');
}

/**
* @expectedException \Magento\Framework\Exception\AlreadyExistsException
*/
public function testlockWithAlreadyAcquiredLockInSameSession()
{
$this->statement->expects($this->any())
->method('fetchColumn')
->willReturn(true);

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