potiuk commented on a change in pull request #10933:
URL: https://github.com/apache/airflow/pull/10933#discussion_r487997298
##########
File path: airflow/utils/orm_event_handlers.py
##########
@@ -32,6 +32,9 @@ def setup_event_handlers(engine):
"""
Setups event handlers.
"""
+
+ idle_session_timeout = conf.getfloat("core",
"sql_alchemy_idle_transaction_timeout", fallback=10)
Review comment:
I guess we should have add it to the defaultconfig/template?
##########
File path: airflow/utils/orm_event_handlers.py
##########
@@ -52,6 +55,40 @@ def set_mysql_timezone(dbapi_connection, connection_record):
cursor.execute("SET time_zone = '+00:00'")
cursor.close()
+ is_mariadb = engine.dialect.server_version_info and "MariaDB" in
engine.dialect.server_version_info
+ if idle_session_timeout > 0:
+ if is_mariadb:
+ #
https://mariadb.com/kb/en/mariadb-1032-release-notes/#variables
+ if engine.dialect.server_version_info > (10, 3):
+ setting = "idle_write_transaction_timeout"
+ else:
+ setting = "idle_readwrite_transaction_timeout"
+
+ @event.listens_for(engine, "connect")
+ def set_mysql_transaction_idle_timeout(dbapi_connection,
connection_record):
+ cursor = dbapi_connection.cursor()
+ cursor.execute("SET @@SESSION.%s = %s", (setting,
int(idle_session_timeout),))
+ cursor.close()
+ else:
+ # MySQL doesn't have the equivalent of postgres'
idle_in_transaction_session_timeout -- the
+ # best we can do is set the time we wait just set the timeout
for any idle connection
+ @event.listens_for(engine, "connect")
+ def set_mysql_transaction_idle_timeout(dbapi_connection,
connection_record):
Review comment:
Do you think (from observation) it might have an undesireable effect for
MySQL ? From what I understand, the side effect of this change is that when
workers will be idle, they will close and re-open connections continuously and
frequently. From what I know this is much more problematic for Postgres
(because closing/reopening the connection spawns a new process and is a costly
event.
For Postgres this is mitigated by PBBouncer (and with Postgres' transaction
idle timeout, this will not happen, really (it's only some long, hanging
transaction that will be impacted).
For MySQL also I understand that such wait timeout will close any connection
that has no activity rather quickly (including those that have open
transaction)?
Is HA introducing some long hanging transactions in the long term that could
impact this? Are you aware of some long transactions already present in Airflow?
Since the default value for MySQL is 28.880 (8 hours), I believe this one
might have a tremendous impact on the behavior of MySQL in some cases.
What is the effect on HA if we do not set this timeout to a long value?
It's not yet visible as we see only this PR for now but I would love to
understand what kind of problems we can expect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
