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.

Sign up for GitHub

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

Instantly apply configuration changes in the cron schedule #9957

Merged
merged 15 commits into from
Jun 21, 2017
Merged

Instantly apply configuration changes in the cron schedule #9957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a3ca437
Clean up the cron observer before adding edits
Jun 14, 2017
8ffa191
Change the order or run, schedule, cleanup to cleanup, schedule, run
Jun 14, 2017
5053fa0
Make cron observer methods public.
Jun 14, 2017
8f24e0f
clean up pending jobs that have been disabled in the config
Jun 14, 2017
48f8ca4
The variable name $ts is more clear if it is named $timestamp
Jun 14, 2017
1cf8ba5
Remove scheduled jobs that do not match their cron expression
Jun 14, 2017
0518a6c
Clean up underscores before making other changes
Jun 15, 2017
cd6d480
Make sure that all current unit tests are functioning again
Jun 15, 2017
a92720d
Make some functions private, because they are not useful outside
Jun 15, 2017
6d40bab
Add a unit test that checks if mismatching schedules will be deleted …
Jun 15, 2017
04f9a0b
fix static test fails
Jun 15, 2017
d205a5e
Everything that is not extended can be private
Jun 15, 2017
1427a38
rollback backword incompatible refactoring
Jun 18, 2017
130b576
rollback backword incompatible refactoring
Jun 18, 2017
7327a83
rollback backword incompatible refactoring
Jun 18, 2017
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
131 changes: 108 additions & 23 deletions app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ class ProcessCronQueueObserver implements ObserverInterface
*/
private $state;

/**
* @var array
*/
private $invalid = [];

/**
* @var array
*/
private $jobs;

/**
* @param \Magento\Framework\ObjectManagerInterface $objectManager
* @param \Magento\Cron\Model\ScheduleFactory $scheduleFactory
Expand Down Expand Up @@ -176,17 +186,21 @@ public function execute(\Magento\Framework\Event\Observer $observer)
$phpPath = $this->phpExecutableFinder->find() ?: 'php';

foreach ($jobGroupsRoot as $groupId => $jobsRoot) {
$this->_cleanup($groupId);
$this->_generate($groupId);
if ($this->_request->getParam('group') !== null
&& $this->_request->getParam('group') !== '\'' . ($groupId) . '\''
&& $this->_request->getParam('group') !== $groupId) {
&& $this->_request->getParam('group') !== $groupId
) {
continue;
}
if (($this->_request->getParam(self::STANDALONE_PROCESS_STARTED) !== '1') && (
$this->_scopeConfig->getValue(
'system/cron/' . $groupId . '/use_separate_process',
\Magento\Store\Model\ScopeInterface::SCOPE_STORE
) == 1
)) {
)
) {
$this->_shell->execute(
$phpPath . ' %s cron:run --group=' . $groupId . ' --' . Cli::INPUT_KEY_BOOTSTRAP . '='
. self::STANDALONE_PROCESS_STARTED . '=1',
Expand All @@ -197,6 +211,7 @@ public function execute(\Magento\Framework\Event\Observer $observer)
continue;
}

/** @var \Magento\Cron\Model\Schedule $schedule */
foreach ($pendingJobs as $schedule) {
$jobConfig = isset($jobsRoot[$schedule->getJobCode()]) ? $jobsRoot[$schedule->getJobCode()] : null;
if (!$jobConfig) {
Expand Down Expand Up @@ -232,9 +247,6 @@ public function execute(\Magento\Framework\Event\Observer $observer)
}
$schedule->save();
}

$this->_generate($groupId);
$this->_cleanup($groupId);
}
}

Expand Down Expand Up @@ -336,8 +348,10 @@ protected function _generate($groupId)
/**
* generate global crontab jobs
*/
$jobs = $this->_config->getJobs();
$jobs = $this->getJobs();
$this->invalid = [];
$this->_generateJobs($jobs[$groupId], $exists, $groupId);
$this->cleanupScheduleMismatches();

/**
* save time schedules generation was ran with no expiration
Expand All @@ -363,17 +377,7 @@ protected function _generate($groupId)
protected function _generateJobs($jobs, $exists, $groupId)
{
foreach ($jobs as $jobCode => $jobConfig) {
$cronExpression = null;
if (isset($jobConfig['config_path'])) {
$cronExpression = $this->getConfigSchedule($jobConfig) ?: null;
}

if (!$cronExpression) {
if (isset($jobConfig['schedule'])) {
$cronExpression = $jobConfig['schedule'];
}
}

$cronExpression = $this->getCronExpression($jobConfig);
if (!$cronExpression) {
continue;
}
Expand All @@ -385,13 +389,15 @@ protected function _generateJobs($jobs, $exists, $groupId)
}

/**
* Clean existed jobs
* Clean expired jobs
*
* @param string $groupId
* @return $this
*/
protected function _cleanup($groupId)
{
$this->cleanupDisabledJobs($groupId);

// check if history cleanup is needed
$lastCleanup = (int)$this->_cache->load(self::CACHE_KEY_LAST_HISTORY_CLEANUP_AT . $groupId);
$historyCleanUp = (int)$this->_scopeConfig->getValue(
Expand Down Expand Up @@ -478,13 +484,20 @@ protected function saveSchedule($jobCode, $cronExpression, $timeInterval, $exist
$currentTime = $this->timezone->scopeTimeStamp();
$timeAhead = $currentTime + $timeInterval;
for ($time = $currentTime; $time < $timeAhead; $time += self::SECONDS_IN_MINUTE) {
$ts = strftime('%Y-%m-%d %H:%M:00', $time);
if (!empty($exists[$jobCode . '/' . $ts])) {
// already scheduled
$scheduledAt = strftime('%Y-%m-%d %H:%M:00', $time);
$alreadyScheduled = !empty($exists[$jobCode . '/' . $scheduledAt]);
$schedule = $this->generateSchedule($jobCode, $cronExpression, $time);
$valid = $schedule->trySchedule();
if (!$valid) {
if ($alreadyScheduled) {
if (!isset($this->invalid[$jobCode])) {
$this->invalid[$jobCode] = [];
}
$this->invalid[$jobCode][] = $scheduledAt;
}
continue;
}
$schedule = $this->generateSchedule($jobCode, $cronExpression, $time);
if ($schedule->trySchedule()) {
if (!$alreadyScheduled) {
// time matches cron expression
$schedule->save();
}
Expand Down Expand Up @@ -523,4 +536,76 @@ protected function getScheduleTimeInterval($groupId)

return $scheduleAheadFor;
}

/**
* Clean up scheduled jobs that are disabled in the configuration
* This can happen when you turn off a cron job in the config and flush the cache
*
* @param string $groupId
* @return void
*/
public function cleanupDisabledJobs($groupId)
{
$jobs = $this->getJobs();
foreach ($jobs[$groupId] as $jobCode => $jobConfig) {
if (!$this->getCronExpression($jobConfig)) {
/** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */
$scheduleResource = $this->_scheduleFactory->create()->getResource();
$scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [
'status=?' => Schedule::STATUS_PENDING,
'job_code=?' => $jobCode,
]);
}
}
}

/**
* @param array $jobConfig
* @return null|string
*/
private function getCronExpression($jobConfig)
{
$cronExpression = null;
if (isset($jobConfig['config_path'])) {
$cronExpression = $this->getConfigSchedule($jobConfig) ?: null;
}

if (!$cronExpression) {
if (isset($jobConfig['schedule'])) {
$cronExpression = $jobConfig['schedule'];
}
}
return $cronExpression;
}

/**
* Clean up scheduled jobs that do not match their cron expression anymore
* This can happen when you change the cron expression and flush the cache
*
* @return $this
*/
private function cleanupScheduleMismatches()
{
foreach ($this->invalid as $jobCode => $scheduledAtList) {
/** @var \Magento\Cron\Model\ResourceModel\Schedule $scheduleResource */
$scheduleResource = $this->_scheduleFactory->create()->getResource();
$scheduleResource->getConnection()->delete($scheduleResource->getMainTable(), [
'status=?' => Schedule::STATUS_PENDING,
'job_code=?' => $jobCode,
'scheduled_at in (?)' => $scheduledAtList,
]);
}
return $this;
}

/**
* @return array
*/
private function getJobs()
{
if ($this->jobs === null) {
$this->jobs = $this->_config->getJobs();
}
return $this->jobs;
}
}
Loading