Skip to content

Commit a6c93fd

Browse files
author
Yevhen Miroshnychenko
authored
Merge pull request magento#5639 from magento-thunder/MC-25132
Fixed issues: - MC-25132: Fix deadlocks related to crons / consumers
2 parents 6b6f428 + 44266a3 commit a6c93fd

28 files changed

+799
-234
lines changed
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Cron\Model;
9+
10+
use Magento\Framework\DB\Adapter\AdapterInterface;
11+
use Magento\Framework\DB\Adapter\DeadlockException;
12+
13+
/**
14+
* Retrier for DB actions
15+
*
16+
* If some action throw an exceptions, try
17+
*/
18+
class DeadlockRetrier implements DeadlockRetrierInterface
19+
{
20+
/**
21+
* @inheritdoc
22+
*/
23+
public function execute(callable $callback, AdapterInterface $connection)
24+
{
25+
if ($connection->getTransactionLevel() !== 0) {
26+
return $callback();
27+
}
28+
29+
for ($retries = self::MAX_RETRIES - 1; $retries > 0; $retries--) {
30+
try {
31+
return $callback();
32+
} catch (DeadlockException $e) {
33+
continue;
34+
}
35+
}
36+
37+
return $callback();
38+
}
39+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Cron\Model;
9+
10+
use Magento\Framework\DB\Adapter\AdapterInterface;
11+
12+
/**
13+
* Retrier Interface
14+
*/
15+
interface DeadlockRetrierInterface
16+
{
17+
/**
18+
* Maximum numbers of attempts
19+
*/
20+
public const MAX_RETRIES = 5;
21+
22+
/**
23+
* Runs callback function
24+
*
25+
* If $callback throws an exception DeadlockException,
26+
* this callback will be run maximum self::MAX_RETRIES times or less.
27+
*
28+
* @param callable $callback
29+
* @param AdapterInterface $connection
30+
* @return mixed
31+
*/
32+
public function execute(callable $callback, AdapterInterface $connection);
33+
}

app/code/Magento/Cron/Model/ResourceModel/Schedule.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ public function trySetJobStatusAtomic($scheduleId, $newStatus, $currentStatus)
4949
}
5050

5151
/**
52-
* Sets schedule status only if no existing schedules with the same job code
53-
* have that status. This is used to implement locking for cron jobs.
52+
* Sets schedule status only if no existing schedules with the same job code have that status.
5453
*
54+
* This is used to implement locking for cron jobs.
5555
* If the schedule is currently in $currentStatus and there are no existing
5656
* schedules with the same job code and $newStatus, set the schedule to
5757
* $newStatus and return true. Otherwise, return false.

app/code/Magento/Cron/Model/Schedule.php

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ class Schedule extends \Magento\Framework\Model\AbstractModel
5656
*/
5757
private $dateTimeFactory;
5858

59+
/**
60+
* @var DeadlockRetrierInterface
61+
*/
62+
private $retrier;
63+
5964
/**
6065
* @param \Magento\Framework\Model\Context $context
6166
* @param \Magento\Framework\Registry $registry
@@ -64,6 +69,7 @@ class Schedule extends \Magento\Framework\Model\AbstractModel
6469
* @param array $data
6570
* @param TimezoneInterface|null $timezoneConverter
6671
* @param DateTimeFactory|null $dateTimeFactory
72+
* @param DeadlockRetrierInterface $retrier
6773
*/
6874
public function __construct(
6975
\Magento\Framework\Model\Context $context,
@@ -72,11 +78,13 @@ public function __construct(
7278
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
7379
array $data = [],
7480
TimezoneInterface $timezoneConverter = null,
75-
DateTimeFactory $dateTimeFactory = null
81+
DateTimeFactory $dateTimeFactory = null,
82+
DeadlockRetrierInterface $retrier = null
7683
) {
7784
parent::__construct($context, $registry, $resource, $resourceCollection, $data);
7885
$this->timezoneConverter = $timezoneConverter ?: ObjectManager::getInstance()->get(TimezoneInterface::class);
7986
$this->dateTimeFactory = $dateTimeFactory ?: ObjectManager::getInstance()->get(DateTimeFactory::class);
87+
$this->retrier = $retrier ?: ObjectManager::getInstance()->get(DeadlockRetrierInterface::class);
8088
}
8189

8290
/**
@@ -251,21 +259,42 @@ public function getNumeric($value)
251259
}
252260

253261
/**
254-
* Lock the cron job so no other scheduled instances run simultaneously.
262+
* Sets a job to STATUS_RUNNING only if it is currently in STATUS_PENDING.
255263
*
256-
* Sets a job to STATUS_RUNNING only if it is currently in STATUS_PENDING
257-
* and no other jobs of the same code are currently in STATUS_RUNNING.
258264
* Returns true if status was changed and false otherwise.
259265
*
260266
* @return boolean
261267
*/
262268
public function tryLockJob()
263269
{
264-
if ($this->_getResource()->trySetJobUniqueStatusAtomic(
265-
$this->getId(),
266-
self::STATUS_RUNNING,
267-
self::STATUS_PENDING
268-
)) {
270+
/** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */
271+
$scheduleResource = $this->_getResource();
272+
273+
// Change statuses from running to error for terminated jobs
274+
$this->retrier->execute(
275+
function () use ($scheduleResource) {
276+
return $scheduleResource->getConnection()->update(
277+
$scheduleResource->getTable('cron_schedule'),
278+
['status' => self::STATUS_ERROR],
279+
['job_code = ?' => $this->getJobCode(), 'status = ?' => self::STATUS_RUNNING]
280+
);
281+
},
282+
$scheduleResource->getConnection()
283+
);
284+
285+
// Change status from pending to running for ran jobs
286+
$result = $this->retrier->execute(
287+
function () use ($scheduleResource) {
288+
return $scheduleResource->trySetJobStatusAtomic(
289+
$this->getId(),
290+
self::STATUS_RUNNING,
291+
self::STATUS_PENDING
292+
);
293+
},
294+
$scheduleResource->getConnection()
295+
);
296+
297+
if ($result) {
269298
$this->setStatus(self::STATUS_RUNNING);
270299
return true;
271300
}

0 commit comments

Comments
 (0)