Dynflow / dynflow

DYNamic workFLOW orchestration engine
http://dynflow.github.io
MIT License
121 stars 44 forks source link

Remove suspend folllowing process_timeout #419

Closed ludovicus3 closed 4 months ago

ludovicus3 commented 1 year ago

The suspend after timeout can cause a race condition with poll_external_task or resume_external_action if process_timeout doesn't call fail or raise an exception.

adamruzicka commented 1 year ago

Thank you for sending this in. Am I getting it right the flow is mostly 1) Polling action is started 2) It polls a couple of times 3) A timeout comes in, but the action overrides process_timeout so that it doesn't fail 4) The action suspends 5) Next poll comes in and the polling goes on

Did I get that right? What would be the use case for implementing process_timeout without having the action fail or suspend itself? Making it succeed on timeout?

ludovicus3 commented 1 year ago

So in process_timeout, I write to external_task. Then suspend gets called. Eventually poll_external_task gets called and overwrites external_task. I have to add an extra line to my poll_external_task to check if external_task == and return. The timeout event never seems to flow down to the done?

adamruzicka commented 1 year ago

Right, that is a scenario we never really considered, thank you for the explanation. Please do let me think on it for a bit

ludovicus3 commented 1 year ago

Sure, perhaps the question I considered myself when investigating my issue will help you.

Does the suspend after process_timeout ever get reached if fail is called or an exception raised in process_timeout?

I believe the answer is no, but if I'm wrong, then this commit may break things.

adamruzicka commented 1 year ago

Does the suspend after process_timeout ever get reached if fail is called or an exception raised in process_timeout?

You are correct, it should not, however I'm aware of code where people appear to override process_timeout in a way that it sometimes does not raise/fail and then they rely on the suspend being called. Let me check with them and I'll get back to you