-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[ISSUE-10650][BUGFIX] Prevent running again already running cron job #11465
Changes from 1 commit
41e0af4
12bd537
110c59c
f1b6f1f
896c8fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,11 @@ class ProcessCronQueueObserver implements ObserverInterface | |
*/ | ||
protected $_pendingSchedules; | ||
|
||
/** | ||
* @var \Magento\Cron\Model\ResourceModel\Schedule\Collection | ||
*/ | ||
protected $_runningSchedules; | ||
|
||
/** | ||
* @var \Magento\Cron\Model\ConfigInterface | ||
*/ | ||
|
@@ -214,7 +219,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) | |
/** @var \Magento\Cron\Model\Schedule $schedule */ | ||
foreach ($pendingJobs as $schedule) { | ||
$jobConfig = isset($jobsRoot[$schedule->getJobCode()]) ? $jobsRoot[$schedule->getJobCode()] : null; | ||
if (!$jobConfig) { | ||
if (!$jobConfig || $this->isJobRunning($schedule->getJobCode())) { | ||
continue; | ||
} | ||
|
||
|
@@ -250,6 +255,25 @@ public function execute(\Magento\Framework\Event\Observer $observer) | |
} | ||
} | ||
|
||
/** | ||
* @param $jobCode | ||
* | ||
* @return bool | ||
*/ | ||
protected function isJobRunning($jobCode) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be great to use private instead of protected |
||
{ | ||
$runningJobs = $this->_getRunningSchedules(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be great to replace this part to SQL query, that will be much effective than getting collection and iterate it. |
||
|
||
/** @var \Magento\Cron\Model\Schedule $schedule */ | ||
foreach ($runningJobs as $schedule) { | ||
if ($schedule->getData('job_code') == $jobCode) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* Execute job by calling specific class::method | ||
* | ||
|
@@ -317,6 +341,22 @@ protected function _getPendingSchedules() | |
return $this->_pendingSchedules; | ||
} | ||
|
||
/** | ||
* Return job collection from data base with status 'running' | ||
* | ||
* @return \Magento\Cron\Model\ResourceModel\Schedule\Collection | ||
*/ | ||
protected function _getRunningSchedules() | ||
{ | ||
if (!$this->_runningSchedules) { | ||
$this->_runningSchedules = $this->_scheduleFactory->create()->getCollection()->addFieldToFilter( | ||
'status', | ||
Schedule::STATUS_RUNNING | ||
)->load(); | ||
} | ||
return $this->_runningSchedules; | ||
} | ||
|
||
/** | ||
* Generate cron schedule | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is new we can make this private and remove the
_
from the name. All new properties should be private or public.