wjddn279 commented on PR #56044: URL: https://github.com/apache/airflow/pull/56044#issuecomment-3425430378
@potiuk Thank you for your response. I agree with your approach that, specifically for MySQL, disconnecting all pooled connections before forking can ensure connection safety in forked processes. This is a solution that hadn't occurred to me. However, I still have the following concerns: Disposing of the engine means temporarily disconnecting all connections in the pool, which could potentially trigger race conditions that are difficult to predict. Nevertheless, I believe that operations with significant side effects, such as connection closing, should be performed at precisely controlled and intentional points (such as explicit connection closure upon program termination or explicit blocking of DB connections in workers) rather than being executed non-deterministically. While this is recommended in the official documentation, I'm not sure if it was designed with highly multi-process environments like Airflow in mind. The official documentation states the following: <img width="890" height="62" alt="image" src="https://github.com/user-attachments/assets/baf081a1-000d-40e8-9094-e309866df4f0"; /> These four approaches are described as guard logic that should be implemented when using an Engine object as-is in child processes. The approach I've implemented in this PR completely creates a new Engine object in the child process, and by creating a new object, I understood that the situation has changed such that we no longer need to follow the documentation's recommendations. Additionally, by creating a new Session object, we can eliminate the risk factors mentioned below. <img width="871" height="124" alt="image" src="https://github.com/user-attachments/assets/c8b55357-fbbe-42ac-853e-c57345aa9ef1"; /> As you mentioned, the approach I've adopted doesn't appear to be explicitly mentioned in the official documentation, and I believe your suggested solution would also clearly resolve the issue. It's certainly possible to re-modify the code by adopting the approach you and @tirkarthi suggested. However, I would like to hear more of your thoughts on my reasoning for the solution. -- 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]
