kestra-io / kestra

:zap: Workflow Automation Platform. Orchestrate & Schedule code in any language, run anywhere, 500+ plugins. Alternative to Zapier, Rundeck, Camunda, Airflow...
https://kestra.io
Apache License 2.0
9.7k stars 802 forks source link

Ability to close Workers & schedulers properly #1668

Open brian-mulier-p opened 1 year ago

brian-mulier-p commented 1 year ago

Expected Behavior

We should be able to try-with-resource around a worker and a scheduler in tests for eg. (but also ensure proper shutdown of the app).

Actual Behaviour

Closing a worker will close all its related queues. Also, closing 1 queue leads to every queue unable to execute any task anymore (since queue-closing will close the executor pool which is currently static and with all queues).

Steps To Reproduce

No response

Environment Information

Example flow

No response

brian-mulier-p commented 1 year ago

As discussed, the proper way to handle it would probably be to hold the Runnable to stop queue consumptions in a common util

loicmathieu commented 1 year ago

Another way would be to let Micronaut close queues when closing the application context using bean lifecycle (which handle beans dependencies so should be safe).

brian-mulier-p commented 1 year ago

As discussed again with Loic, the question is, do we want to be able to try-with properly around a scheduler & a worker or forcing a Micronaut context reconstruct is enough ? As said the Micronaut context based solution would add some more test time as we would need to force the deletion of the context after each test which is using an embed Scheduler / worker. However it leads to better time-to-delivery and probably side-effect free.

loicmathieu commented 1 year ago

We can do both by providing adding @PreDestroy on the close method. The important point is that we need to remove all direct calls to close, except on test.

brian-mulier-p commented 1 year ago

Yes but then we still have to remove the queues closing calls if we want to be able to close the worker & scheduler in tests

loicmathieu commented 5 months ago

@brian-mulier-p can we condidere that this has been done with the new way we close our server component with the changes done in 0.16 by @fhussonnois ?

brian-mulier-p commented 5 months ago

Idk we have to test if the try with worker / scheduler works properly