From c7b5c699ccf510c4eebee29daef5420463a0650a Mon Sep 17 00:00:00 2001
From: ashuai <ashuai@gmail.com>
Date: Fri, 6 Dec 2024 17:17:17 +0800
Subject: [PATCH] Optimize consumeTasks method to address performance issues
 caused by full table scans in SQL queries
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

processed correctly.

## Changes Made

1. **Simplified SQL Query**:
- Removed the SQL condition `->andWhere('command')->ne('')` from the query that selects tasks. This change reduces the complexity of the SQL query and avoids unnecessary database processing.

2. **Added PHP-Level Check**:
- Introduced a PHP-level check to identify tasks with empty `command` fields. If a task has an empty `command`, it skips the actual execution (`consumeTask`) but continues through the regular process to mark the task as `'done'`.

## Benefits

- **Improved Performance**: Simplifying the SQL query can lead to better performance, especially when dealing with large datasets.
- **Maintainability**: Moving the validation logic into the application layer makes the codebase easier to understand and maintain.
- **Consistent Flow**: Ensures that tasks with empty commands are processed consistently without disrupting the existing workflow.
---
 module/cron/control.php | 92 ++++++++++++++++++++++-------------------
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/module/cron/control.php b/module/cron/control.php
index 9276e33cb2..b636b8b481 100755
--- a/module/cron/control.php
+++ b/module/cron/control.php
@@ -350,7 +350,10 @@ public function consumeTasks(int $execId)
             $this->cron->updateTime('consumer', $execId);
 
             /* Consume. */
-            $task = $this->dao->select('*')->from(TABLE_QUEUE)->where('status')->eq('wait')->andWhere('command')->ne('')->orderBy('createdDate')->fetch();
+            $task = $this->dao->select('*')->from(TABLE_QUEUE)->where('status')->eq('wait')
+                // This condition can cause performance issues and should be handled within the application logic instead.
+                // ->andWhere('command')->ne('')
+                ->orderBy('createdDate')->fetch();
             if(!$task) break;
 
             $this->consumeTask($execId, $task);
@@ -370,55 +373,58 @@ public function consumeTask(int $execId, object $task)
     {
         /* Other executor may execute the task at the same time，so we mark execId and wait 500ms to check whether we own it. */
         $this->dao->clearCache();
-        $this->dao->update(TABLE_QUEUE)->set('status')->eq('doing')->set('execId')->eq($execId)->where('id')->eq($task->id)->exec();
-        usleep(500);
-
-        $task = $this->dao->select('*')->from(TABLE_QUEUE)->where('id')->eq($task->id)->fetch();
-        if($task->execId != $execId) return;
-
-        /* Execution command. */
-        $output = '';
-        $return = '';
-
-        unset($_SESSION['company']);
-        unset($this->app->company);
-
-        /* Mark that this request was triggered by the scheduled task, not by the user. */
-        $_SESSION['fromCron'] = true;
-
-        $this->loadModel('common');
-        $this->common->setCompany();
-        $this->common->loadConfigFromDB();
-
-        try
+        if (!empty($task->command)) 
         {
-            if($task->type == 'zentao')
+            $this->dao->update(TABLE_QUEUE)->set('status')->eq('doing')->set('execId')->eq($execId)->where('id')->eq($task->id)->exec();
+            usleep(500);
+    
+            $task = $this->dao->select('*')->from(TABLE_QUEUE)->where('id')->eq($task->id)->fetch();
+            if($task->execId != $execId) return;
+    
+            /* Execution command. */
+            $output = '';
+            $return = '';
+    
+            unset($_SESSION['company']);
+            unset($this->app->company);
+    
+            /* Mark that this request was triggered by the scheduled task, not by the user. */
+            $_SESSION['fromCron'] = true;
+    
+            $this->loadModel('common');
+            $this->common->setCompany();
+            $this->common->loadConfigFromDB();
+    
+            try
             {
-                parse_str($task->command, $params);
-                if(isset($params['moduleName']) and isset($params['methodName']))
+                if($task->type == 'zentao')
                 {
-                    $this->viewType = 'html';
-                    $moduleName     = $params['moduleName'];
-                    $methodName     = $params['methodName'];
-                    $this->app->loadLang($moduleName);
-                    $this->app->loadConfig($moduleName);
-                    unset($params['moduleName'], $params['methodName']);
-                    $output = $this->fetch($moduleName, $methodName, $params);
+                    parse_str($task->command, $params);
+                    if(isset($params['moduleName']) and isset($params['methodName']))
+                    {
+                        $this->viewType = 'html';
+                        $moduleName     = $params['moduleName'];
+                        $methodName     = $params['methodName'];
+                        $this->app->loadLang($moduleName);
+                        $this->app->loadConfig($moduleName);
+                        unset($params['moduleName'], $params['methodName']);
+                        $output = $this->fetch($moduleName, $methodName, $params);
+                    }
+                }
+                elseif($task->type == 'system')
+                {
+                    exec($task->command, $out, $return);
+                    if($out) $output = implode(PHP_EOL, $out);
                 }
             }
-            elseif($task->type == 'system')
+            catch(EndResponseException $endResponseException)
             {
-                exec($task->command, $out, $return);
-                if($out) $output = implode(PHP_EOL, $out);
+                $output = $endResponseException->getContent();
+            }
+            catch(Exception $e)
+            {
+                $output = $e;
             }
-        }
-        catch(EndResponseException $endResponseException)
-        {
-            $output = $endResponseException->getContent();
-        }
-        catch(Exception $e)
-        {
-            $output = $e;
         }
 
         $this->dao->update(TABLE_QUEUE)->set('status')->eq('done')->where('id')->eq($task->id)->exec();