diff --git a/mailpoet/lib/API/JSON/v1/SendingQueue.php b/mailpoet/lib/API/JSON/v1/SendingQueue.php index 6455ca6828..0db8c4ca8d 100644 --- a/mailpoet/lib/API/JSON/v1/SendingQueue.php +++ b/mailpoet/lib/API/JSON/v1/SendingQueue.php @@ -6,8 +6,8 @@ use MailPoet\API\JSON\Endpoint as APIEndpoint; use MailPoet\API\JSON\Error as APIError; use MailPoet\API\JSON\Response; use MailPoet\Config\AccessControl; +use MailPoet\Cron\ActionScheduler\Actions\DaemonTrigger; use MailPoet\Cron\CronTrigger; -use MailPoet\Cron\DaemonActionSchedulerRunner; use MailPoet\Cron\Triggers\WordPress; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; @@ -57,8 +57,8 @@ class SendingQueue extends APIEndpoint { /** @var SettingsController */ private $settings; - /** @var DaemonActionSchedulerRunner */ - private $actionSchedulerRunner; + /** @var DaemonTrigger */ + private $actionSchedulerDaemonTriggerAction; public function __construct( SubscribersFeature $subscribersFeature, @@ -69,7 +69,7 @@ class SendingQueue extends APIEndpoint { MailerFactory $mailerFactory, Scheduler $scheduler, SettingsController $settings, - DaemonActionSchedulerRunner $actionSchedulerRunner, + DaemonTrigger $actionSchedulerDaemonTriggerAction, NewsletterValidator $newsletterValidator ) { $this->subscribersFeature = $subscribersFeature; @@ -80,7 +80,7 @@ class SendingQueue extends APIEndpoint { $this->mailerFactory = $mailerFactory; $this->scheduler = $scheduler; $this->settings = $settings; - $this->actionSchedulerRunner = $actionSchedulerRunner; + $this->actionSchedulerDaemonTriggerAction = $actionSchedulerDaemonTriggerAction; $this->newsletterValidator = $newsletterValidator; } @@ -257,7 +257,7 @@ class SendingQueue extends APIEndpoint { $newsletter->getStatus() === NewsletterEntity::STATUS_SENDING && $this->settings->get('cron_trigger.method') === CronTrigger::METHOD_ACTION_SCHEDULER ) { - $this->actionSchedulerRunner->trigger(); + $this->actionSchedulerDaemonTriggerAction->process(); } } } diff --git a/mailpoet/lib/AdminPages/Pages/Help.php b/mailpoet/lib/AdminPages/Pages/Help.php index f10d13a379..2ef58e00bb 100644 --- a/mailpoet/lib/AdminPages/Pages/Help.php +++ b/mailpoet/lib/AdminPages/Pages/Help.php @@ -3,8 +3,9 @@ namespace MailPoet\AdminPages\Pages; use MailPoet\AdminPages\PageRenderer; +use MailPoet\Cron\ActionScheduler\Actions\DaemonRun; +use MailPoet\Cron\ActionScheduler\Actions\DaemonTrigger; use MailPoet\Cron\CronHelper; -use MailPoet\Cron\DaemonActionSchedulerRunner; use MailPoet\Helpscout\Beacon; use MailPoet\Mailer\MailerLog; use MailPoet\Router\Endpoints\CronDaemon; @@ -87,9 +88,9 @@ class Help { $actionSchedulerData = []; $actionSchedulerData['version'] = \ActionScheduler_Versions::instance()->latest_version(); $actionSchedulerData['storage'] = str_replace('ActionScheduler_', '', get_class(\ActionScheduler_Store::instance())); - $actionSchedulerData['latestTrigger'] = $this->getLatestActionSchedulerActionDate(DaemonActionSchedulerRunner::DAEMON_TRIGGER_SCHEDULER_ACTION); - $actionSchedulerData['latestCompletedTrigger'] = $this->getLatestActionSchedulerActionDate(DaemonActionSchedulerRunner::DAEMON_TRIGGER_SCHEDULER_ACTION, 'complete'); - $actionSchedulerData['latestCompletedRun'] = $this->getLatestActionSchedulerActionDate(DaemonActionSchedulerRunner::DAEMON_RUN_SCHEDULER_ACTION, 'complete'); + $actionSchedulerData['latestTrigger'] = $this->getLatestActionSchedulerActionDate(DaemonTrigger::NAME); + $actionSchedulerData['latestCompletedTrigger'] = $this->getLatestActionSchedulerActionDate(DaemonTrigger::NAME, 'complete'); + $actionSchedulerData['latestCompletedRun'] = $this->getLatestActionSchedulerActionDate(DaemonRun::NAME, 'complete'); return $actionSchedulerData; } diff --git a/mailpoet/lib/Cron/ActionScheduler/Actions/DaemonRun.php b/mailpoet/lib/Cron/ActionScheduler/Actions/DaemonRun.php new file mode 100644 index 0000000000..ade6a80471 --- /dev/null +++ b/mailpoet/lib/Cron/ActionScheduler/Actions/DaemonRun.php @@ -0,0 +1,100 @@ +wp = $wp; + $this->daemon = $daemon; + $this->wordpressTrigger = $wordpressTrigger; + $this->cronHelper = $cronHelper; + $this->remoteExecutorHandler = $remoteExecutorHandler; + $this->actionScheduler = $actionScheduler; + } + + public function init(): void { + $this->wp->addAction(self::NAME, [$this, 'process']); + $this->wp->addFilter('action_scheduler_maximum_execution_time_likely_to_be_exceeded', [$this, 'storeRemainingExecutionLimit'], 10, 5); + } + + /** + * Run daemon that processes scheduled tasks for limited time + */ + public function process(): void { + $this->wp->addAction('action_scheduler_after_process_queue', [$this, 'afterProcess']); + $this->wp->addAction('mailpoet_cron_get_execution_limit', [$this, 'getDaemonExecutionLimit']); + $this->daemon->run($this->cronHelper->createDaemon($this->cronHelper->createToken())); + } + + /** + * Callback for adjusting the execution for the cron daemon (MailPoet\Cron\Daemon) + */ + public function getDaemonExecutionLimit(): int { + return $this->remainingExecutionLimit; + } + + /** + * After Action Scheduler finishes queue always check there is more work to do and in case there is we trigger additional runner. + */ + public function afterProcess(): void { + if ($this->wordpressTrigger->checkExecutionRequirements()) { + // The automatic rescheduling schedules the next recurring action to run after 1 second. + // So we need to wait before we trigger new remote executor to avoid skipping the action + sleep(2); + $this->remoteExecutorHandler->triggerExecutor(); + } else { + $this->actionScheduler->unscheduleAction(self::NAME); + } + } + + /** + * This method is hooked into action_scheduler_maximum_execution_time_likely_to_be_exceeded + * It checks how much action scheduler execution time is left for the daemon to run + */ + public function storeRemainingExecutionLimit($likelyExceeded, $runner, $processedActions, $executionTime, $maxExecutionTime): bool { + $newLimit = floor(($maxExecutionTime - $executionTime) - self::EXECUTION_LIMIT_MARGIN); + $this->remainingExecutionLimit = intval(max($newLimit, 0)); + return (bool)$likelyExceeded; + } +} diff --git a/mailpoet/lib/Cron/ActionScheduler/Actions/DaemonTrigger.php b/mailpoet/lib/Cron/ActionScheduler/Actions/DaemonTrigger.php new file mode 100644 index 0000000000..ec0dd4f6ec --- /dev/null +++ b/mailpoet/lib/Cron/ActionScheduler/Actions/DaemonTrigger.php @@ -0,0 +1,61 @@ +wp = $wp; + $this->wordpressTrigger = $wordpressTrigger; + $this->remoteExecutorHandler = $remoteExecutorHandler; + $this->actionScheduler = $actionScheduler; + } + + public function init() { + $this->wp->addAction(self::NAME, [$this, 'process']); + if (!$this->actionScheduler->hasScheduledAction(self::NAME)) { + $this->actionScheduler->scheduleRecurringAction($this->wp->currentTime('timestamp'), 20, self::NAME); + } + } + + /** + * In regular intervals checks if there are scheduled tasks to execute. + * In case there are tasks it spawns a recurring action. + */ + public function process(): void { + $hasJobsToDo = $this->wordpressTrigger->checkExecutionRequirements(); + if (!$hasJobsToDo) { + $this->actionScheduler->unscheduleAction(DaemonRun::NAME); + return; + } + if ($this->actionScheduler->hasScheduledAction(DaemonRun::NAME)) { + return; + } + // Start recurring action with minimal interval to ensure continuous execution of the daemon + $this->actionScheduler->scheduleRecurringAction($this->wp->currentTime('timestamp') - 1, 1, DaemonRun::NAME); + $this->remoteExecutorHandler->triggerExecutor(); + } +} diff --git a/mailpoet/lib/Cron/DaemonActionSchedulerRunner.php b/mailpoet/lib/Cron/DaemonActionSchedulerRunner.php index b1e485b373..5f9bd5528c 100644 --- a/mailpoet/lib/Cron/DaemonActionSchedulerRunner.php +++ b/mailpoet/lib/Cron/DaemonActionSchedulerRunner.php @@ -2,127 +2,44 @@ namespace MailPoet\Cron; +use MailPoet\Cron\ActionScheduler\Actions\DaemonRun; +use MailPoet\Cron\ActionScheduler\Actions\DaemonTrigger; use MailPoet\Cron\ActionScheduler\ActionScheduler; use MailPoet\Cron\ActionScheduler\RemoteExecutorHandler; -use MailPoet\Cron\Triggers\WordPress; -use MailPoet\WP\Functions as WPFunctions; class DaemonActionSchedulerRunner { - const DAEMON_RUN_SCHEDULER_ACTION = 'mailpoet/cron/daemon-run'; - const DAEMON_TRIGGER_SCHEDULER_ACTION = 'mailpoet/cron/daemon-trigger'; - - const EXECUTION_LIMIT_MARGIN = 10; // 10 seconds - - /** @var Daemon */ - private $daemon; - - /** @var WordPress */ - private $wordpressTrigger; - - /** @var CronHelper */ - private $cronHelper; - - /** @var WPFunctions */ - private $wp; - /** @var ActionScheduler */ private $actionScheduler; /** @var RemoteExecutorHandler */ private $remoteExecutorHandler; - /** - * The inital value is set based on default cron execution limit battle tested in MailPoet custom cron runner (an older version of the background processing). - * The default limit in PHP is 30s so it leaves 10 execution margin. - * @var int - */ - private $remainingExecutionLimit = 20; + /** @var DaemonTrigger */ + private $daemonTriggerAction; + + /** @var DaemonRun */ + private $daemonRunAction; public function __construct( - Daemon $daemon, - CronHelper $cronHelper, - WordPress $wordpressTrigger, - WPFunctions $wp, ActionScheduler $actionScheduler, - RemoteExecutorHandler $remoteExecutorHandler + RemoteExecutorHandler $remoteExecutorHandler, + DaemonTrigger $daemonTriggerAction, + DaemonRun $daemonRunAction ) { - $this->cronHelper = $cronHelper; - $this->daemon = $daemon; - $this->wordpressTrigger = $wordpressTrigger; - $this->wp = $wp; $this->actionScheduler = $actionScheduler; $this->remoteExecutorHandler = $remoteExecutorHandler; + $this->daemonTriggerAction = $daemonTriggerAction; + $this->daemonRunAction = $daemonRunAction; } public function init(): void { - $this->wp->addAction(self::DAEMON_RUN_SCHEDULER_ACTION, [$this, 'run']); - $this->wp->addAction(self::DAEMON_TRIGGER_SCHEDULER_ACTION, [$this, 'trigger']); + $this->daemonRunAction->init(); + $this->daemonTriggerAction->init(); $this->remoteExecutorHandler->init(); - $this->wp->addFilter('action_scheduler_maximum_execution_time_likely_to_be_exceeded', [$this, 'storeRemainingExecutionLimit'], 10, 5); - if (!$this->actionScheduler->hasScheduledAction(self::DAEMON_TRIGGER_SCHEDULER_ACTION)) { - $this->actionScheduler->scheduleRecurringAction($this->wp->currentTime('timestamp'), 20, self::DAEMON_TRIGGER_SCHEDULER_ACTION); - } } public function deactivate(): void { - $this->actionScheduler->unscheduleAction(self::DAEMON_TRIGGER_SCHEDULER_ACTION); - $this->actionScheduler->unscheduleAction(self::DAEMON_RUN_SCHEDULER_ACTION); - } - - /** - * In regular intervals checks if there are scheduled tasks to execute. - * In case there are tasks it spawns a recurring action. - */ - public function trigger(): void { - $hasJobsToDo = $this->wordpressTrigger->checkExecutionRequirements(); - if (!$hasJobsToDo) { - $this->actionScheduler->unscheduleAction(self::DAEMON_RUN_SCHEDULER_ACTION); - return; - } - if ($this->actionScheduler->hasScheduledAction(self::DAEMON_RUN_SCHEDULER_ACTION)) { - return; - } - // Start recurring action with minimal interval to ensure continuous execution of the daemon - $this->actionScheduler->scheduleRecurringAction($this->wp->currentTime('timestamp') - 1, 1, self::DAEMON_RUN_SCHEDULER_ACTION); - $this->remoteExecutorHandler->triggerExecutor(); - } - - /** - * Run daemon that processes scheduled tasks for limited time (default 20 seconds) - */ - public function run(): void { - $this->wp->addAction('action_scheduler_after_process_queue', [$this, 'afterProcess']); - $this->wp->addAction('mailpoet_cron_get_execution_limit', [$this, 'getDaemonExecutionLimit']); - $this->daemon->run($this->cronHelper->createDaemon($this->cronHelper->createToken())); - } - - /** - * Callback for a hook for adjusting the execution for the cron daemon - */ - public function getDaemonExecutionLimit(): int { - return $this->remainingExecutionLimit; - } - - /** - * After Action Scheduler finishes queue always check there is more work to do and in case there is trigger additional runner. - */ - public function afterProcess(): void { - if ($this->wordpressTrigger->checkExecutionRequirements()) { - sleep(2); // Add short sleep to ensure next action ready to be processed since minimal schedule interval is 1 second - $this->remoteExecutorHandler->triggerExecutor(); - } else { - $this->actionScheduler->unscheduleAction(self::DAEMON_RUN_SCHEDULER_ACTION); - } - } - - /** - * This method is hooked into action_scheduler_maximum_execution_time_likely_to_be_exceeded - * and used to listen on how many execution time is needed. - * The execution limit is then used for the daemon run - */ - public function storeRemainingExecutionLimit($likelyExceeded, $runner, $processedActions, $executionTime, $maxExecutionTime): bool { - $newLimit = floor(($maxExecutionTime - $executionTime) - self::EXECUTION_LIMIT_MARGIN); - $this->remainingExecutionLimit = intval(max($newLimit, 0)); - return (bool)$likelyExceeded; + $this->actionScheduler->unscheduleAction(DaemonTrigger::NAME); + $this->actionScheduler->unscheduleAction(DaemonRun::NAME); } } diff --git a/mailpoet/lib/DI/ContainerConfigurator.php b/mailpoet/lib/DI/ContainerConfigurator.php index 5d5811397c..8283af8667 100644 --- a/mailpoet/lib/DI/ContainerConfigurator.php +++ b/mailpoet/lib/DI/ContainerConfigurator.php @@ -199,6 +199,8 @@ class ContainerConfigurator implements IContainerConfigurator { $container->autowire(\MailPoet\Cron\DaemonActionSchedulerRunner::class)->setPublic(true); $container->autowire(\MailPoet\Cron\ActionScheduler\ActionScheduler::class)->setPublic(true); $container->autowire(\MailPoet\Cron\ActionScheduler\RemoteExecutorHandler::class)->setPublic(true); + $container->autowire(\MailPoet\Cron\ActionScheduler\Actions\DaemonRun::class)->setPublic(true); + $container->autowire(\MailPoet\Cron\ActionScheduler\Actions\DaemonTrigger::class)->setPublic(true); $container->autowire(\MailPoet\Cron\Workers\SendingQueue\SendingErrorHandler::class)->setPublic(true); $container->autowire(\MailPoet\Cron\Workers\SendingQueue\SendingThrottlingHandler::class)->setPublic(true); $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\Scheduler::class); diff --git a/mailpoet/tests/integration/Cron/DaemonActionSchedulerRunnerTest.php b/mailpoet/tests/integration/Cron/DaemonActionSchedulerRunnerTest.php index ce5d14b34a..def72dac80 100644 --- a/mailpoet/tests/integration/Cron/DaemonActionSchedulerRunnerTest.php +++ b/mailpoet/tests/integration/Cron/DaemonActionSchedulerRunnerTest.php @@ -2,6 +2,8 @@ namespace MailPoet\Cron; +use MailPoet\Cron\ActionScheduler\Actions\DaemonRun; +use MailPoet\Cron\ActionScheduler\Actions\DaemonTrigger; use MailPoet\Cron\ActionScheduler\ActionScheduler; use MailPoet\Cron\Workers\UnsubscribeTokens; use MailPoet\Entities\ScheduledTaskEntity; @@ -33,7 +35,7 @@ class DaemonActionSchedulerRunnerTest extends \MailPoetTest { expect($actions)->count(1); $action = reset($actions); $this->assertInstanceOf(\ActionScheduler_Action::class, $action); - expect($action->get_hook())->equals(DaemonActionSchedulerRunner::DAEMON_TRIGGER_SCHEDULER_ACTION); + expect($action->get_hook())->equals(DaemonTrigger::NAME); } public function testItDeactivateAllTasks(): void { @@ -48,17 +50,19 @@ class DaemonActionSchedulerRunnerTest extends \MailPoetTest { public function testTriggerDoesNotTriggerAnythingIfThereAreNoJobs(): void { $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(0); - $this->actionSchedulerRunner->trigger(); + $triggerAction = $this->diContainer->get(DaemonTrigger::class); + $triggerAction->process(); $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(0); } public function testTriggerUnschedulesRunJobIfThereIsNoMoreWork(): void { $actionScheduler = $this->diContainer->get(ActionScheduler::class); - $actionScheduler->scheduleRecurringAction(time() + 60, 1, DaemonActionSchedulerRunner::DAEMON_RUN_SCHEDULER_ACTION); + $actionScheduler->scheduleRecurringAction(time() + 60, 1, DaemonRun::NAME); $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(1); - $this->actionSchedulerRunner->trigger(); + $triggerAction = $this->diContainer->get(DaemonTrigger::class); + $triggerAction->process(); $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(0); } @@ -68,12 +72,13 @@ class DaemonActionSchedulerRunnerTest extends \MailPoetTest { $this->createDueScheduledTask(); $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(0); - $this->actionSchedulerRunner->trigger(); + $triggerAction = $this->diContainer->get(DaemonTrigger::class); + $triggerAction->process(); $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(2); $action = reset($actions); $this->assertInstanceOf(\ActionScheduler_Action::class, $action); - expect($action->get_hook())->equals(DaemonActionSchedulerRunner::DAEMON_RUN_SCHEDULER_ACTION); + expect($action->get_hook())->equals(DaemonRun::NAME); $this->cleanup(); } @@ -85,14 +90,15 @@ class DaemonActionSchedulerRunnerTest extends \MailPoetTest { 'name' => 'John', 'address' => 'john@example.com', ]); + $runAction = $this->diContainer->get(DaemonRun::class); // Activate filter for watching execution limit. // This normally happens in DaemonActionSchedulerRunner::init but it can't be called in tests since it cause some background requests and made test flaky $wp = $this->diContainer->get(Functions::class); - $wp->addFilter('action_scheduler_maximum_execution_time_likely_to_be_exceeded', [$this->actionSchedulerRunner, 'storeRemainingExecutionLimit'], 10, 5); - expect($this->actionSchedulerRunner->getDaemonExecutionLimit())->equals(20); // Verify initial execution limit + $wp->addFilter('action_scheduler_maximum_execution_time_likely_to_be_exceeded', [$runAction, 'storeRemainingExecutionLimit'], 10, 5); + expect($runAction->getDaemonExecutionLimit())->equals(20); // Verify initial execution limit $actionScheduler = $this->diContainer->get(ActionScheduler::class); - $actionScheduler->scheduleRecurringAction(time() - 1, 100, DaemonActionSchedulerRunner::DAEMON_RUN_SCHEDULER_ACTION); + $actionScheduler->scheduleRecurringAction(time() - 1, 100, DaemonRun::NAME); $actions = $this->getMailPoetScheduledActions(); expect($actions)->count(1); $doneActions = $this->getMailPoetCompleteActions(); @@ -110,8 +116,8 @@ class DaemonActionSchedulerRunnerTest extends \MailPoetTest { expect($actions)->count(1); // Verify execution limit after run. floor(30 - some time taken by previous action) - 10s (safety execution timout margin) - expect($this->actionSchedulerRunner->getDaemonExecutionLimit())->greaterThan(0); - expect($this->actionSchedulerRunner->getDaemonExecutionLimit())->lessThan(20); + expect($runAction->getDaemonExecutionLimit())->greaterThan(0); + expect($runAction->getDaemonExecutionLimit())->lessThan(20); } private function getMailPoetScheduledActions(): array {