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,19 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                 continue;
             }
 
+            if (!$this->lockManager->setLock(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 +276,8 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                 }
                 $schedule->save();
             }
+
+            $this->lockManager->releaseLock($groupId);
         }
     }
 

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
+     */
+    protected $model;
+
+    /**
+     * @var \Magento\Framework\ObjectManagerInterface
+     */
+    protected $objectManager;
+
+    public 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->setLock($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,72 @@
+<?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
+     */
+    public function setLock(string $name, int $timeout = -1): bool
+    {
+        $this->checkLength($name);
+
+        return (bool)$this->resource->getConnection()->query("SELECT GET_LOCK(?, ?);", array((string)$name, (int)$timeout))
+            ->fetchColumn();
+    }
+
+    /**
+     * Releases a lock for name
+     *
+     * @param string $name lock name
+     * @return bool
+     */
+    public function releaseLock(string $name): bool
+    {
+        $this->checkLength($name);
+
+        return (bool)$this->resource->getConnection()->query("SELECT RELEASE_LOCK(?);", array((string)$name))->fetchColumn();
+    }
+
+    /**
+     * Tests of lock is set for name
+     *
+     * @param string $name lock name
+     * @return bool
+     */
+    public function isLocked(string $name): bool
+    {
+        $this->checkLength($name);
+
+        return (bool)$this->resource->getConnection()->query("SELECT IS_USED_LOCK(?);", array((string)$name))->fetchColumn();
+    }
+
+    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 setLock(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/Test/Unit/Backend/DatabaseTest.php

@@ -0,0 +1,80 @@
+<?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 testSetLock()
+    {
+        $this->statement->expects($this->once())
+            ->method('fetchColumn')
+            ->willReturn(true);
+
+        $this->assertTrue($this->database->setLock('testLock'));
+    }
+
+    /**
+     * @expectedException \Magento\Framework\Exception\InputException
+     */
+    public function testSetLockWithTooLongName()
+    {
+        $this->database->setLock('BbXbyf9rIY5xuAVdviQJmh76FyoeeVHTDpcjmcImNtgpO4Hnz4xk76ZGEyYALvrQu');
+    }
+}