From 4e276ae407d21b531727964df98ec9088acec196 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 10 Feb 2022 22:12:40 -0800 Subject: [PATCH] CRM_Queue_Queue_* - Respect `$queueSpec['lease_time']` (if given) Before: The lease-time is one of the following: 1. A value requested at runtime 2. The constant 3600 After: The lease-time is either supplied as 1. A value requested at runtime 2. A value configured for the queue 3. The constant 3600 --- CRM/Queue/Queue.php | 12 +++++++----- CRM/Queue/Queue/Memory.php | 20 ++++++++++++-------- CRM/Queue/Queue/Sql.php | 25 +++++++++++++++---------- CRM/Queue/Queue/SqlParallel.php | 19 +++++++++++-------- 4 files changed, 45 insertions(+), 31 deletions(-) diff --git a/CRM/Queue/Queue.php b/CRM/Queue/Queue.php index bce38babe2..ed2d0f4388 100644 --- a/CRM/Queue/Queue.php +++ b/CRM/Queue/Queue.php @@ -20,6 +20,8 @@ */ abstract class CRM_Queue_Queue { + const DEFAULT_LEASE_TIME = 3600; + /** * @var string */ @@ -137,13 +139,13 @@ abstract class CRM_Queue_Queue { /** * Get the next item. * - * @param int $lease_time - * Seconds. - * + * @param int|null $lease_time + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a default. * @return object * with key 'data' that matches the inputted data */ - abstract public function claimItem($lease_time = 3600); + abstract public function claimItem($lease_time = NULL); /** * Get the next item, even if there's an active lease @@ -154,7 +156,7 @@ abstract class CRM_Queue_Queue { * @return object * with key 'data' that matches the inputted data */ - abstract public function stealItem($lease_time = 3600); + abstract public function stealItem($lease_time = NULL); /** * Remove an item from the queue. diff --git a/CRM/Queue/Queue/Memory.php b/CRM/Queue/Queue/Memory.php index 46573a016d..b11630f2f5 100644 --- a/CRM/Queue/Queue/Memory.php +++ b/CRM/Queue/Queue/Memory.php @@ -117,13 +117,15 @@ class CRM_Queue_Queue_Memory extends CRM_Queue_Queue { /** * Get and remove the next item. * - * @param int $leaseTime - * Seconds. - * + * @param int|null $leaseTime + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a queue default (`$queueSpec['lease_time']`) or system default (`DEFAULT_LEASE_TIME`). * @return object * Includes key 'data' that matches the inputted data. */ - public function claimItem($leaseTime = 3600) { + public function claimItem($leaseTime = NULL) { + $leaseTime = $leaseTime ?: $this->getSpec('lease_time') ?: static::DEFAULT_LEASE_TIME; + // foreach hits the items in order -- but we short-circuit after the first foreach ($this->items as $id => $data) { $nowEpoch = CRM_Utils_Time::getTimeRaw(); @@ -149,13 +151,15 @@ class CRM_Queue_Queue_Memory extends CRM_Queue_Queue { /** * Get the next item. * - * @param int $leaseTime - * Seconds. - * + * @param int|null $leaseTime + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a queue default (`$queueSpec['lease_time']`) or system default (`DEFAULT_LEASE_TIME`). * @return object * With key 'data' that matches the inputted data. */ - public function stealItem($leaseTime = 3600) { + public function stealItem($leaseTime = NULL) { + $leaseTime = $leaseTime ?: $this->getSpec('lease_time') ?: static::DEFAULT_LEASE_TIME; + // foreach hits the items in order -- but we short-circuit after the first foreach ($this->items as $id => $data) { $nowEpoch = CRM_Utils_Time::getTimeRaw(); diff --git a/CRM/Queue/Queue/Sql.php b/CRM/Queue/Queue/Sql.php index ff924af533..ae9dc4e3bf 100644 --- a/CRM/Queue/Queue/Sql.php +++ b/CRM/Queue/Queue/Sql.php @@ -37,13 +37,14 @@ class CRM_Queue_Queue_Sql extends CRM_Queue_Queue { /** * Get the next item. * - * @param int $lease_time - * Seconds. - * + * @param int|null $lease_time + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a queue default (`$queueSpec['lease_time']`) or system default (`DEFAULT_LEASE_TIME`). * @return object * With key 'data' that matches the inputted data. */ - public function claimItem($lease_time = 3600) { + public function claimItem($lease_time = NULL) { + $lease_time = $lease_time ?: $this->getSpec('lease_time') ?: static::DEFAULT_LEASE_TIME; $result = NULL; $dao = CRM_Core_DAO::executeQuery('LOCK TABLES civicrm_queue_item WRITE;'); @@ -70,11 +71,13 @@ class CRM_Queue_Queue_Sql extends CRM_Queue_Queue { if ($dao->fetch()) { $nowEpoch = CRM_Utils_Time::getTimeRaw(); $dao->run_count++; - CRM_Core_DAO::executeQuery("UPDATE civicrm_queue_item SET release_time = %1, run_count = %3 WHERE id = %2", [ + $sql = "UPDATE civicrm_queue_item SET release_time = %1, run_count = %3 WHERE id = %2"; + $sqlParams = [ '1' => [date('YmdHis', $nowEpoch + $lease_time), 'String'], '2' => [$dao->id, 'Integer'], '3' => [$dao->run_count, 'Integer'], - ]); + ]; + CRM_Core_DAO::executeQuery($sql, $sqlParams); // (Comment by artfulrobot Sep 2019: Not sure what the below comment means, should be removed/clarified?) // work-around: inconsistent date-formatting causes unintentional breakage # $dao->submit_time = date('YmdHis', strtotime($dao->submit_time)); @@ -92,13 +95,15 @@ class CRM_Queue_Queue_Sql extends CRM_Queue_Queue { /** * Get the next item, even if there's an active lease * - * @param int $lease_time - * Seconds. - * + * @param int|null $lease_time + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a queue default (`$queueSpec['lease_time']`) or system default (`DEFAULT_LEASE_TIME`). * @return object * With key 'data' that matches the inputted data. */ - public function stealItem($lease_time = 3600) { + public function stealItem($lease_time = NULL) { + $lease_time = $lease_time ?: $this->getSpec('lease_time') ?: static::DEFAULT_LEASE_TIME; + $sql = " SELECT id, queue_name, submit_time, release_time, run_count, data FROM civicrm_queue_item diff --git a/CRM/Queue/Queue/SqlParallel.php b/CRM/Queue/Queue/SqlParallel.php index 8ca07e07dd..603436e397 100644 --- a/CRM/Queue/Queue/SqlParallel.php +++ b/CRM/Queue/Queue/SqlParallel.php @@ -37,13 +37,14 @@ class CRM_Queue_Queue_SqlParallel extends CRM_Queue_Queue { /** * Get the next item. * - * @param int $lease_time - * Seconds. - * + * @param int|null $lease_time + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a queue default (`$queueSpec['lease_time']`) or system default (`DEFAULT_LEASE_TIME`). * @return object * With key 'data' that matches the inputted data. */ - public function claimItem($lease_time = 3600) { + public function claimItem($lease_time = NULL) { + $lease_time = $lease_time ?: $this->getSpec('lease_time') ?: static::DEFAULT_LEASE_TIME; $result = NULL; $dao = CRM_Core_DAO::executeQuery('LOCK TABLES civicrm_queue_item WRITE;'); @@ -90,13 +91,15 @@ class CRM_Queue_Queue_SqlParallel extends CRM_Queue_Queue { /** * Get the next item, even if there's an active lease * - * @param int $lease_time - * Seconds. - * + * @param int|null $lease_time + * Hold a lease on the claimed item for $X seconds. + * If NULL, inherit a queue default (`$queueSpec['lease_time']`) or system default (`DEFAULT_LEASE_TIME`). * @return object * With key 'data' that matches the inputted data. */ - public function stealItem($lease_time = 3600) { + public function stealItem($lease_time = NULL) { + $lease_time = $lease_time ?: $this->getSpec('lease_time') ?: static::DEFAULT_LEASE_TIME; + $sql = " SELECT id, queue_name, submit_time, release_time, run_count, data FROM civicrm_queue_item -- 2.25.1