From a88c9b4dd6a80dd1c75cd2f8a0b8bbaa4aa647b5 Mon Sep 17 00:00:00 2001 From: Markus Fasselt Date: Sun, 14 Oct 2018 18:57:17 +0200 Subject: [PATCH] Add symfony/lock support --- composer.json | 1 + src/Event.php | 101 ++++++++++++++++++-- src/EventRunner.php | 2 + tests/Functional/PreventOverlappingTest.php | 35 +++++-- 4 files changed, 125 insertions(+), 14 deletions(-) diff --git a/composer.json b/composer.json index addeeae2..69b44bf8 100644 --- a/composer.json +++ b/composer.json @@ -36,6 +36,7 @@ "symfony/console": "^2.7 || ^3.3 || ^4.0", "symfony/dependency-injection": "^2.7 || ^3.3 || ^4.0", "symfony/filesystem": "^2.7 || ^3.3 || ^4.0", + "symfony/lock": "^3.4 || ^4.1", "symfony/process": "^2.7.11 || ^3.3 || ^4.0", "symfony/yaml": "^2.7 || ^3.3 || ^4.0" }, diff --git a/src/Event.php b/src/Event.php index c9d7f320..e241fdc2 100644 --- a/src/Event.php +++ b/src/Event.php @@ -11,6 +11,9 @@ use Crunz\Pinger\PingableInterface; use Crunz\Pinger\PingableTrait; use SuperClosure\Serializer; +use Symfony\Component\Lock\Factory; +use Symfony\Component\Lock\Lock; +use Symfony\Component\Lock\StoreInterface; use Symfony\Component\Process\Process; /** @@ -64,6 +67,7 @@ class Event implements PingableInterface * @var Logger */ public $logger; + /** * The event's unique identifier. * @@ -154,6 +158,21 @@ class Event implements PingableInterface 'week' => 5, ]; + /** + * The symfony lock factory that is used to acquire locks. If the value is null, but preventOverlapping = true + * crunz falls back to filesystem locks. + * + * @var Factory|null + */ + private $lockFactory; + + /** + * Contains the timestamp of the last lock refresh. + * + * @var int + */ + private $lastLockRefresh = 0; + /** * Create a new event instance. * @@ -712,13 +731,19 @@ public function user($user) /** * Do not allow the event to overlap each other. * - * @param string|int $safe_duration + * By default, the lock is acquired through file system locks. Alternatively, you can pass a symfony lock store + * that will be responsible for the locking. + * + * @param StoreInterface $store * * @return $this */ - public function preventOverlapping() + public function preventOverlapping(StoreInterface $store = null) { $this->preventOverlapping = true; + if (null !== $store) { + $this->lockFactory = new Factory($store); + } // Skip the event if it's locked (processing) $this->skip(function () { @@ -727,10 +752,7 @@ public function preventOverlapping() // Delete the lock file when the event is completed $this->after(function () { - $lockfile = $this->lockFile(); - if (file_exists($lockfile)) { - unlink($lockfile); - } + $this->releaseLock(); }); return $this; @@ -1018,6 +1040,13 @@ public function afterCallbacks() */ public function isLocked() { + if (null !== $this->lockFactory) { + $lock = $this->createLockObject(); + + return $lock->isAcquired(); + } + + // If no symfony lock object is given, fall back to file system locks. $pid = $this->lastPid(); $hasPid = (null !== $pid); @@ -1048,9 +1077,69 @@ public function lastPid() */ public function lockFile() { + if (null !== $this->lockFactory) { + throw new \BadMethodCallException('We are not using file based locking, but instead symfony/lock'); + } + return rtrim(sys_get_temp_dir(), '/') . '/crunz-' . md5($this->buildCommand()); } + /** + * If this event is prevented from overlapping, this method should be called regularly to refresh the lock. + */ + public function refreshLock() + { + if (!$this->preventOverlapping) { + return; + } + + if (null === $this->lockFactory) { + // In case of file based locking there is nothing to do. + } + + // Refresh lock every 10s + $lockRefreshNeeded = $this->lastLockRefresh < (time() - 10); + if ($lockRefreshNeeded) { + $this->createLockObject()->refresh(); + } + } + + /** + * Get the symfony lock object for the task. + * + * @return Lock + */ + protected function createLockObject() + { + if (null === $this->lockFactory) { + throw new \BadMethodCallException( + 'No lock factory. Please call preventOverlapping() with lock storage first.' + ); + } + + $ttl = 30; + + return $this->lockFactory->createLock('crunz-' . md5($this->buildCommand()), $ttl); + } + + /** + * Release the lock after the command completed. + */ + protected function releaseLock() + { + if (null !== $this->lockFactory) { + $lock = $this->createLockObject(); + $lock->release(); + + return; + } + + $lockfile = $this->lockFile(); + if (file_exists($lockfile)) { + unlink($lockfile); + } + } + /** * Get the default output depending on the OS. * diff --git a/src/EventRunner.php b/src/EventRunner.php index 9dd057b2..c983a934 100644 --- a/src/EventRunner.php +++ b/src/EventRunner.php @@ -150,6 +150,8 @@ protected function manageStartedEvents() /** @var Event $event */ foreach ($events as $eventKey => $event) { + $event->refreshLock(); + $proc = $event->getProcess(); if ($proc->isRunning()) { continue; diff --git a/tests/Functional/PreventOverlappingTest.php b/tests/Functional/PreventOverlappingTest.php index ad1b6e0c..d3feb878 100644 --- a/tests/Functional/PreventOverlappingTest.php +++ b/tests/Functional/PreventOverlappingTest.php @@ -1,7 +1,5 @@ assertEquals([$event1], $schedule1->dueEvents(new \DateTimeZone(date_default_timezone_get()))); $this->assertEquals([$event2], $schedule2->dueEvents(new \DateTimeZone(date_default_timezone_get()))); - // Start schedule, so that event1 will be locked + // Start schedule1, so that event1 will be locked $eventRunner->handle(new NullOutput(), [$schedule1]); // Event is locked and therefore not due (even over the boundaries of multiple independent events and schedules) @@ -65,18 +71,32 @@ public function testPreventOverlapping() // Wait until the process finished while ($event1->isLocked()) { + // Verify the events are still locked + $this->assertEquals([], $schedule1->dueEvents(new \DateTimeZone(date_default_timezone_get()))); + $this->assertEquals([], $schedule2->dueEvents(new \DateTimeZone(date_default_timezone_get()))); + $this->assertTrue($event1->isLocked()); + $this->assertTrue($event2->isLocked()); + $eventRunner->manageStartedEvents(); - usleep(10000); + usleep(50000); } // Assert both locks were removed $this->assertFalse($event1->isLocked()); $this->assertFalse($event2->isLocked()); } -} -class MyEventRunner extends EventRunner { + public function lockDataProvider() + { + return [ + [null], // Default file locking + [new FlockStore()], + ]; + } +} +class MyEventRunner extends EventRunner +{ /** * Manage the running processes. * @@ -101,7 +121,6 @@ public function manageStartedEvents() $schedule->dismissEvent($eventKey); } - if (!\count($schedule->events())) { $this->invoke($schedule->afterCallbacks()); unset($this->schedules[$scheduleKey]);