potiuk commented on PR #39484: URL: https://github.com/apache/airflow/pull/39484#issuecomment-2101122299
And just to add a little bit of context - why I am surprised to see it .... It's not that I am stubborn. I just looked deeper: The current context manager behaviour has been introduced in 2020: https://github.com/apache/airflow/pull/7542 And previously the implementation looked like that: ``` # Recreate the process pool each sync in case processes in the pool die self._sync_pool = Pool(processes=num_processes) ``` So at least we know **why** we are recreating the pool in the first place with every sync - to handle the case where the processes die. And indeed the Process Pool does not have "self-healing" option, so if the processes will die, they will not be recreated and eventually Celery executor will stop sending tasks. This is why I consider those changes as "risky" - because there might be some good reasons why we are doing it in the first place. And the current proposal does not handle the case to be honest. So if we change the behaviour here, we need to handle this case as well and be sure that what we are doing is not causing more problems that might hit us back. And we need to know if we **really** need to do it. I would be really surprises that no-one noticed the slow-down you mentioned :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
