potiuk commented on PR #56044: URL: https://github.com/apache/airflow/pull/56044#issuecomment-3424800646
I am kind of hesitant to make such a change which goes against the recommendations of sqlalchemy docs. Looking at the docs (and following what @tirkarthi wrote in https://github.com/apache/airflow/issues/56879#issuecomment-3422562615 -> maybe a better solution here will be to follow the second option from https://docs.sqlalchemy.org/en/20/core/pooling.html#using-connection-pools-with-multiprocessing-or-os-fork ? > 3. Call [Engine.dispose()](https://docs.sqlalchemy.org/en/20/core/connections.html#sqlalchemy.engine.Engine.dispose) directly before the child process is created. This will also cause the child process to start with a new connection pool, while ensuring the parent connections are not transferred to the child process: > > ```python > engine = create_engine("mysql://user:pass@host/dbname") > > def run_in_process(): > with engine.connect() as conn: > conn.execute(text("...")) > > # before process starts, ensure engine.dispose() is called > engine.dispose() > p = Process(target=run_in_process) > p.start() This will dispose (with default close=True) all the pooled connections before the fork and this will pretty much automatically make sure that there is no garbage-colleciton induced event sending QUIT (because this will happen synchronously in the parent's dispose command). That means that the parent process will have to re-create the connections, yes but that's an acceptable overhead. And we could do it only for `mysql` dialect and **still** use `dispose(close=False)` in the forks (no harm by closing not opened connections). This would keep it more aligned with the recommendations from sqlalchemy docs, while targetting the apparent misbehaviour of mysql driver (which BTW - we should likely create an issue for in mysql driver if not done already - because it seems that the buggy behaviour is in the driver itself). It would also avoid the custom behaviours - both "dispose(close=True)` in parent and "dispose(close=False)` in the forks are recommended and "standard" behaviours that sqlalchemy expects and recommends. -- 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]
