Prevent running again already running cron group

app/code/Magento/Cron/Observer/ProcessCronQueueObserver.php

@@ -56,6 +56,16 @@ class ProcessCronQueueObserver implements ObserverInterface
      */
     const SECONDS_IN_MINUTE = 60;
 
+    /**
+     * How long to wait for cron group to become unlocked
+     */
+    const LOCK_TIMEOUT = 5;
+
+    /**
+     * Static lock prefix for cron group locking
+     */
+    const LOCK_PREFIX = 'CRON_GROUP_';
+
     /**
      * @var \Magento\Cron\Model\ResourceModel\Schedule\Collection
      */
@@ -116,6 +126,11 @@ class ProcessCronQueueObserver implements ObserverInterface
      */
     private $state;
 
+    /**
+     * @var \Magento\Framework\Lock\LockManagerInterface
+     */
+    private $lockManager;
+
     /**
      * @var array
      */
@@ -138,6 +153,7 @@ class ProcessCronQueueObserver implements ObserverInterface
      * @param \Magento\Framework\Process\PhpExecutableFinderFactory $phpExecutableFinderFactory
      * @param \Psr\Log\LoggerInterface $logger
      * @param \Magento\Framework\App\State $state
+     * @param \Magento\Framework\Lock\LockManagerInterface $lockManager
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -151,7 +167,8 @@ public function __construct(
         \Magento\Framework\Stdlib\DateTime\DateTime $dateTime,
         \Magento\Framework\Process\PhpExecutableFinderFactory $phpExecutableFinderFactory,
         \Psr\Log\LoggerInterface $logger,
-        \Magento\Framework\App\State $state
+        \Magento\Framework\App\State $state,
+        \Magento\Framework\Lock\LockManagerInterface $lockManager
     ) {
         $this->_objectManager = $objectManager;
         $this->_scheduleFactory = $scheduleFactory;
@@ -164,6 +181,7 @@ public function __construct(
         $this->phpExecutableFinder = $phpExecutableFinderFactory->create();
         $this->logger = $logger;
         $this->state = $state;
+        $this->lockManager = $lockManager;
     }
 
     /**
@@ -186,8 +204,6 @@ 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
@@ -211,6 +227,21 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                 continue;
             }
 
+            // Note: we acquire the lock here instead of above, as it should be taken by standalone (child) process,
+            // not by the parent process.
+            if (!$this->lockManager->acquireLock(self::LOCK_PREFIX . $groupId, self::LOCK_TIMEOUT)) {
+                $this->logger->warning(
+                    sprintf(
+                        "Could not acquire lock for cron group: %s, skipping run",
+                        $groupId
+                    )
+                );
+                continue;
+            }
+
+            $this->_cleanup($groupId);
+            $this->_generate($groupId);
+
             /** @var \Magento\Cron\Model\Schedule $schedule */
             foreach ($pendingJobs as $schedule) {
                 $jobConfig = isset($jobsRoot[$schedule->getJobCode()]) ? $jobsRoot[$schedule->getJobCode()] : null;
@@ -247,6 +278,8 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                 }
                 $schedule->save();
             }
+
+            $this->lockManager->releaseLock(self::LOCK_PREFIX . $groupId);
         }
     }
 

app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php

@@ -84,6 +84,11 @@ class ProcessCronQueueObserverTest extends \PHPUnit\Framework\TestCase
      */
     protected $appStateMock;
 
+    /**
+     * @var \Magento\Framework\Lock\LockManagerInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $lockManagerMock;
+
     /**
      * @var \Magento\Cron\Model\ResourceModel\Schedule|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -135,6 +140,12 @@ protected function setUp()
             ->disableOriginalConstructor()
             ->getMock();
 
+        $this->lockManagerMock = $this->getMockBuilder(\Magento\Framework\Lock\LockManagerInterface::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->lockManagerMock->method('acquireLock')->willReturn(true);
+        $this->lockManagerMock->method('releaseLock')->willReturn(true);
+
         $this->observer = $this->createMock(\Magento\Framework\Event\Observer::class);
 
         $this->dateTimeMock = $this->getMockBuilder(\Magento\Framework\Stdlib\DateTime\DateTime::class)
@@ -170,7 +181,8 @@ protected function setUp()
             $this->dateTimeMock,
             $phpExecutableFinderFactory,
             $this->loggerMock,
-            $this->appStateMock
+            $this->appStateMock,
+            $this->lockManagerMock
         );
     }
 
@@ -813,22 +825,22 @@ public function testMissedJobsCleanedInTime()
         $this->_config->expects($this->exactly(2))->method('getJobs')->will($this->returnValue($jobConfig));
 
         $this->_scopeConfig->expects($this->at(0))->method('getValue')
+            ->with($this->equalTo('system/cron/test_group/use_separate_process'))
+            ->will($this->returnValue(0));
+        $this->_scopeConfig->expects($this->at(1))->method('getValue')
             ->with($this->equalTo('system/cron/test_group/history_cleanup_every'))
             ->will($this->returnValue(10));
-        $this->_scopeConfig->expects($this->at(1))->method('getValue')
+        $this->_scopeConfig->expects($this->at(2))->method('getValue')
             ->with($this->equalTo('system/cron/test_group/schedule_lifetime'))
             ->will($this->returnValue(2*24*60));
-        $this->_scopeConfig->expects($this->at(2))->method('getValue')
-            ->with($this->equalTo('system/cron/test_group/history_success_lifetime'))
-            ->will($this->returnValue(0));
         $this->_scopeConfig->expects($this->at(3))->method('getValue')
-            ->with($this->equalTo('system/cron/test_group/history_failure_lifetime'))
+            ->with($this->equalTo('system/cron/test_group/history_success_lifetime'))
             ->will($this->returnValue(0));
         $this->_scopeConfig->expects($this->at(4))->method('getValue')
-            ->with($this->equalTo('system/cron/test_group/schedule_generate_every'))
+            ->with($this->equalTo('system/cron/test_group/history_failure_lifetime'))
             ->will($this->returnValue(0));
         $this->_scopeConfig->expects($this->at(5))->method('getValue')
-            ->with($this->equalTo('system/cron/test_group/use_separate_process'))
+            ->with($this->equalTo('system/cron/test_group/schedule_generate_every'))
             ->will($this->returnValue(0));
 
         $this->_collection->expects($this->any())->method('addFieldToFilter')->will($this->returnSelf());

app/etc/di.xml

@@ -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"/>

dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/DatabaseTest.php

@@ -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 testLockAndRelease()
+    {
+        $name = 'test_lock';
+
+        $this->assertFalse($this->model->isLocked($name));
+
+        $this->assertTrue($this->model->acquireLock($name));
+        $this->assertTrue($this->model->isLocked($name));
+
+        $this->assertTrue($this->model->releaseLock($name));
+        $this->assertFalse($this->model->isLocked($name));
+    }
+
+    public function testReleaseLockWithoutExistingLock()
+    {
+        $name = 'test_lock';
+
+        $this->assertFalse($this->model->isLocked($name));
+        $this->assertFalse($this->model->releaseLock($name));
+    }
+}

lib/internal/Magento/Framework/Lock/Backend/Database.php

@@ -0,0 +1,83 @@
+<?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\ResourceConnection;
+use Magento\Framework\Exception\InputException;
+use Magento\Framework\Phrase;
+
+class Database implements \Magento\Framework\Lock\LockManagerInterface
+{
+    /** @var ResourceConnection */
+    private $resource;
+
+    public function __construct(
+        ResourceConnection $resource
+    ) {
+        $this->resource = $resource;
+    }
+
+    /**
+     * 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
+     */
+    public function acquireLock(string $name, int $timeout = -1): bool
+    {
+        $this->checkLength($name);
+
+        return (bool)$this->resource->getConnection()->query("SELECT GET_LOCK(?, ?);", [(string)$name, (int)$timeout])
+            ->fetchColumn();
+    }
+
+    /**
+     * Releases a lock for name
+     *
+     * @param string $name lock name
+     * @return bool
+     * @throws InputException
+     */
+    public function releaseLock(string $name): bool
+    {
+        $this->checkLength($name);
+
+        return (bool)$this->resource->getConnection()->query("SELECT RELEASE_LOCK(?);", [(string)$name])->fetchColumn();
+    }
+
+    /**
+     * Tests of lock is set for name
+     *
+     * @param string $name lock name
+     * @return bool
+     * @throws InputException
+     */
+    public function isLocked(string $name): bool
+    {
+        $this->checkLength($name);
+
+        return (bool)$this->resource->getConnection()->query("SELECT IS_USED_LOCK(?);", [(string)$name])->fetchColumn();
+    }
+
+    /**
+     * Checks for max length of lock name
+     *
+     * Limited to 64 characters in MySQL.
+     *
+     * @param string $name
+     * @throws InputException
+     */
+    private function checkLength(string $name)
+    {
+        if (strlen($name) > 64) {
+            throw new InputException(new Phrase('Lock name too long'));
+        }
+    }
+}

lib/internal/Magento/Framework/Lock/LockManagerInterface.php

@@ -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 acquireLock(string $name, int $timeout = -1): bool;
+
+    /**
+     * Releases a lock
+     *
+     * @param string $name lock name
+     * @return bool
+     * @api
+     */
+    public function releaseLock(string $name): bool;
+
+    /**
+     * Tests if lock is set
+     *
+     * @param string $name lock name
+     * @return bool
+     * @api
+     */
+    public function isLocked(string $name): bool;
+}

lib/internal/Magento/Framework/Lock/README.md

@@ -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:
+* *acquireLock* - Acquires a named lock
+* *releaseLock* - Releases a named lock
+* *isLocked* - Tests if a named lock exists