bug(task): ChainedTask no longer works

[guikingone]

bug(task): ChainedTask no longer works

[Guikingone]

Hi @jvancoillie 👋🏻

Small update on this issue, the fix is ready on the linked PR, as I'm always searching for performances and features, I've started to implement a lazy-loaded task list via the same PR, it must be ready to merge pretty soon, sorry for the delay 🙂

[Guikingone]

Hum, ok, got it, this may requires a new configuration to solve this, I haven't thought about the isRunning attribute.

[jvancoillie]

Hello @Guikingone,

I just tried with the same configuration the ChainedType task plays both subtasks well but now it is the last task Task 6 that is no longer executed.

bash-5.1# bin/console scheduler:consume

 // Quit the worker with CONTROL-C.

 ! [NOTE] The tasks output can be displayed if the -vv option is used

 [OK] Task "foo" succeed. (Duration: < 1 sec, Memory used: 23.1 MiB)

 [OK] Task "bar" succeed. (Duration: < 1 sec, Memory used: 23.3 MiB)

 [OK] Task "chained" succeed. (Duration: < 1 sec, Memory used: 23.3 MiB)

 [OK] Task "task5" succeed. (Duration: < 1 sec, Memory used: 23.4 MiB)

+---------+----------------------+------------+---------------------------+---------------------------+-------------------------+-----------------------------+---------+------+
| Name    | Description          | Expression | Last execution date       | Next execution date       | Last execution duration | Last execution memory usage | State   | Tags |
+---------+----------------------+------------+---------------------------+---------------------------+-------------------------+-----------------------------+---------+------+
| chained | chained with 2 tasks | * * * * *  | 2021-05-24T12:48:27+00:00 | 2021-05-24T12:53:00+00:00 | < 1 sec                 | 23.3 MiB                    | enabled |      |
| task5   | command task 5       | * * * * *  | 2021-05-24T12:48:27+00:00 | 2021-05-24T12:53:00+00:00 | < 1 sec                 | 23.4 MiB                    | enabled |      |
| task6   | command task 6       | * * * * *  | Not executed              | 2021-05-24T12:53:00+00:00 | Not tracked             | Not tracked                 | enabled |      |
+---------+----------------------+------------+---------------------------+---------------------------+-------------------------+-----------------------------+---------+------+

[jvancoillie]

Nice it works ! great job.

[Guikingone]

Hi @jvancoillie 👋🏻

The PR has been merged, the fix will be released via 0.5.0

[jvancoillie]

Hello @Guikingone 👋

I think I identified the problem.

When launching the chainedTask the worker switch to running then the task foo is evaluated, the worker is running handleTask is skipped, go trough the finally bloc who disable running. Then when evaluating the task bar the worker is not more running and bar is executed.

That's why the footask is not executed.

[Guikingone]

Hi @jvancoillie 👋🏻

Strange, regarding this sentence:

the worker is running handleTask is skipped, go trough the finally bloc who disable running

If the worker is skipping the handleTask, this might be related to an error 🤔

Does the logs add any additional informations?

[jvancoillie]

Hello @Guikingone 👋

I think it's still a bad written expression because of English. (Pourquoi j'ai pas mieux suivi les cours d'anglais 😢)

Unless I'm mistaken, I don't see why the block finally will raise an error? there is no thrown exeption in this block?.

SchedulerBundle/src/Worker/Worker.php

Lines 146 to 167 in 1f6d4b5

	try {
	$this->middlewareStack->runPreExecutionMiddleware($task);
	if (!$this->isRunning) {
	$this->isRunning = true;
	$this->dispatch(new WorkerRunningEvent($this));
	$this->handleTask($runner, $task);
	}
	$this->middlewareStack->runPostExecutionMiddleware($task);
	} catch (Throwable $throwable) {
	$failedTask = new FailedTask($task, $throwable->getMessage());
	$this->failedTasks->add($failedTask);
	$this->dispatch(new TaskFailedEvent($failedTask));
	} finally {
	$lockedTask->release();
	$this->isRunning = false;
	$this->lastExecutedTask = $task;
	$this->dispatch(new WorkerRunningEvent($this, true));
	++$tasksCount;
	}

The ChainedTask when it is executed sets the running property to true

SchedulerBundle/src/Worker/Worker.php

Line 150 in 1f6d4b5

$this->isRunning = true;

Then in the execution of the subtasks, the first is skipped because the property is set to true by the parent task (ChainedTask) .

SchedulerBundle/src/Worker/Worker.php

Line 149 in 1f6d4b5

if (!$this->isRunning) {

Then the finaly block switches back to false

SchedulerBundle/src/Worker/Worker.php

Line 162 in 1f6d4b5

$this->isRunning = false;

and the next sub-task can be executed

Voilà.

[Guikingone]

Hi @jvancoillie 👋🏻

Could you test on your side using dev-issue/125? It works locally but as you have a "specific" environment and configuration, it could be good to validate against it 🙂

[jvancoillie]

the error is in the count of tasks executedTasksCount, which considers that all the tasks have been executed and exits the loop while I still have tasks.
This counter is reset to zero each time the method execute is called, but overwrites the value of the parent worker as well.

I also have ROLLBACK in the logs, I think it must come from the subtasks. i will take a closer look and give you feedbacks

[jvancoillie]

That's right, the subtasks are updated in the same way as a simple task or in the case of a ChainedTask only the parent task needs to be updated.

"The given task cannot be updated as the identifier or the body is invalid"

[Guikingone]

Alright, I've pushed a new version on dev-issue/125, as I break the API on ChainedTask and the Worker, could you test it locally after clearing the cache?

Thanks again for the feedback and sorry for the issue 🙂

[jvancoillie]

No problem, I'll test it right away.
I was studying the problem and I tested by adding a local variable on the task counter that we decrease each time. it seemed to work.

it looked like this

 while (!$this->getOptions()['shouldStop']) {
            $tasks = $this->getTasks($tasks);
            $dueTasksCount = count($tasks);

            foreach ($tasks as $task) {
                if (end($tasks) === $task && !$this->checkTaskState($task)) {
                    break 2;
                }

                if (!$this->checkTaskState($task)) {
                    continue;
                }

                $lockedTask = $this->lockFactory->createLock($task->getName());
                if (end($tasks) === $task && !$lockedTask->acquire()) {
                    break 2;
                }

                if (!$lockedTask->acquire()) {
                    continue;
                }

                $this->dispatch(new WorkerRunningEvent($this));

                foreach ($this->runners as $runner) {
                    if (!$runner->support($task)) {
                        continue;
                    }

                    if (null !== $task->getExecutionDelay() && 0 !== $this->getSleepDuration()) {
                        usleep($task->getExecutionDelay());
                    }

                    try {
                        $this->middlewareStack->runPreExecutionMiddleware($task);

                        if (!$this->getOptions()['isRunning']) {
                            $this->options['isRunning'] = true;
                            $this->dispatch(new WorkerRunningEvent($this));
                            $this->handleTask($runner, $task);
                        }

                        $this->middlewareStack->runPostExecutionMiddleware($task);
                    } catch (Throwable $throwable) {
                        $failedTask = new FailedTask($task, $throwable->getMessage());
                        $this->failedTasks->add($failedTask);
                        $this->dispatch(new TaskFailedEvent($failedTask));
                    } finally {
                        $lockedTask->release();
                        $this->options['isRunning'] = false;
                        $this->options['lastExecutedTask'] = $task;
                        $this->dispatch(new WorkerRunningEvent($this, true));

                        ++$this->options['executedTasksCount'];
                        --$dueTasksCount;
                    }

                    if ($this->getOptions()['shouldStop']) {
                        break 3;
                    }
                }

                if ($this->getOptions()['shouldStop'] || (0 === $dueTasksCount && !$this->getOptions()['sleepUntilNextMinute'])) {
                    break 2;
                }
            }

            if ($this->getOptions()['sleepUntilNextMinute']) {
                sleep($this->getSleepDuration());

                $this->execute($options);
            }
        }

I look at what you did right away.

[jvancoillie]

Oups i got an error :(

 [ERROR] An error occurred when executing the tasks

         Return value of SchedulerBundle\Worker\AbstractWorker::getRunners() must be of the type array, object returned

[Guikingone]

Should be fixed, sorry 😅