Queue jobs executed in CLI does not persist repository changes unless executed isolated

[sorenmalling]

Behavior

I'm having a job with a execute() method like this

$transfer = new Transfer(...);
$this->transferRepository->add($transfer);
return true;

The new transfer object is not persisted to the database table when the message is finished

Expected behavior

Similar to other CLI command, I expected the new $transfer object to be persisted

Proposed change

Have a signal/slot that call the persistAll on the persistenceManager once a messageFinished signal is sent

[bwaidelich]

@sorenmalling Thanks for this contribution but I'm not sure if it should be in the responsibility of the JobQueue package to persist unrelated changes.
Why don't you do

$transfer = new Transfer(...);
$this->transferRepository->add($transfer);
$this->persistenceManager->persistAll();

yourself?
That's the only way to detect (and handle) errors in the job itself – otherwise the job might be marked done even though the objects weren't actually persisted

[bwaidelich]

..But I guess we should maybe improve the README in that regard, seems to be a common pitfall

[sorenmalling]

@bwaidelich

persist unrelated changes

What is unrelated in this situation? The persistAll is called, only after a messageFinished signal from the jobmanager, meaning only changes there is being persisted? The call is similar to how Flow it self call persistAll after end action.

The issue in behavior is, that do I enable executeIsolated the job is benig processed as a separate CLI command, and then "automatically" call the persistAll (from Neos.Flow's Package.php). This is to bring the same functionality to jobs not being run as isolated.

[sorenmalling]

/me updated the description accordingly

[bwaidelich]

What is unrelated in this situation?

Unrelated to the JobQueue implementation.
The messageFinished signal can occur at any time during a request and it would suddenly persist changes before the framework shutdown – potentially changes from 3rd party packages even.

I don't think that that alone is a major issue – no code should rely on the fact that persistAll is called at the end of the request.

But especially in job queues you usually only want to mark the job done if it has been processed successfully – and you can only tell with immediate persistence. Otherwise you risk loss of information.

(somewhat controversial) final speech: In general I would not suggest this pattern of deferred persistence at all and I think it has been the main source of confusion and bugs with Flow. But obviously I don't want to block this feature if others think it's helpful. So I'll remove my -1 and shall shout "I told you so" in a few years 😄

[sorenmalling]

But isn't this (running the jobqueue) done in it's own request with it's own instance of the persistence manager, that will not persist other stuff? Like when i do a request to any other of my controller->action combinations?

Alternative suggestion could be to run everyting isolated - that will remove a different experience depending on a option setting (and introduce the persistence as known from other CLI job)

[bwaidelich]

But isn't this (running the jobqueue) done in it's own request

Not necessarily. JobManager:: waitAndExecute() is public API and can be called at any time (and it is by the FakeQueue for example).

But what about the whole deferred persistence / lost information topic – don't you agree to that?

[sorenmalling]

Purely looking at the code, I can agree. But looking at how the jobmanager is being executed as CLI, I see a behavior that does not match normal CLI scripts. That is why I raise the concern.

I found a solution in running isolated (see my reply above) and suggest that everything is being run as isolated to have the same expectation regarding persistence.

[albe]

The issue in behavior is, that do I enable executeIsolated the job is benig processed as a separate CLI command, and then "automatically" call the persistAll (from Neos.Flow's Package.php). This is to bring the same functionality to jobs not being run as isolated.

Okay, now I get the root issue. Basically the persistence behaviour is totally different between "isolated" (does auto-persistence) and "non-isolated" (no auto-persistence) execution. That is something that should indeed be tackled. But maybe the messageFinished is not the optimal place? A good place would be between the executeJobForMessage and the queue->finish(...) call - or to make it behave same for "isolated" and "non-isolated", add a signal at the end of executeJobForMessage and do a persist call there? @bwaidelich

PS: Also note, that the automatic persistence after executeJobForMessage would mean that job:work would always persist after every single message. Probably should be possible to "batch" in that work mode.

[sorenmalling]

@albe You're right - my original issue and pr tried to describe and solve a issue on one go :)

I can not, from reading the code, see any issue in running all jobs isoated. And we ensure what was originally the discovered issue:

• ensure persistence
• ensure that each job exits succesfully (all exceptions will be tackled by exceptionhandler and not marked as done in the queue)
• ensure always same behavior (since we remove the isolated context)

Win win :-)