Flowpack
FEATURE: Persist unpersisted changes in CLI after messageFinished signal
Once a messageFinished signal is emitted, the Persistence Manager persist all unpersisted changes. This is to have the jobmanager act as a normal CLI command, known from the a CommandController command
Resolves #44
Yeah, I guess you are not wrong in that regard. The jobqueue should be reliable about the processing of messages, and a message is processed after it finishes doing what it does, including persisting data.
If you depend on persistAll() after processing, you effectively opt into "at most once" processing, while you probably do want "at least once" in pretty much all cases.
We should document that better and maybe create an easy way to conciously opt into this? i.e. toggle that automatic "persistAll" via a setting?
Edit: See my comment in https://github.com/Flowpack/jobqueue-common/issues/44#issuecomment-579878540 - I do think we should bring the auto-persistence in line between isolated and non-isolated execution, but probably messageFinished isn't the right place.
I'm in favor of running everything isolated instead. That will call the persistence manager as normal cli and not produce any unwanted issues from calling the persistenceManager
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?
Anyone have some thoughts about my suggestion in the issue? I still think that would be the better place to hook the persist call into, because otherwise you have "at most once" behaviour (work is done, message marked finished, but the persist call might still fail). It's also the same semantic as with the executeIsolated case - since the persist call happens in the subprocess, but the message is "finished" only afterwards.
I'd maybe call that signal JobExecuted and put it at the end of the executeJobForMessage() method.
@albe I don't know which is better :-)
My sole argument for running isolated, is that the SQL for updating the message queue (it's using the connection directly) is that, if the persistence fails, so does the updating of the jobqueue. So we will not end up with "completed" jobs, where persistence failed.
I don't mind what proposal goes on - I just want users of the package not to have different experiences depending on the configuration setting :-)
I don't mind what proposal goes on - I just want users of the package not to have different experiences depending on the configuration setting
Absolutely, that's exactly why I suggest the JobExecuted signal and put the persistAll() there
Another old one that need a decision, it seems… do you people still remember your thoughts? 😎
