From 79adaa58e826f604480578533c62c9d5b9aff88a Mon Sep 17 00:00:00 2001 From: Sam Wheating Date: Thu, 6 May 2021 22:10:17 -0400 Subject: [PATCH 1/3] Enforce READ COMMITTED isolation when using mysql --- airflow/settings.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/airflow/settings.py b/airflow/settings.py index 0db4a0ac1646c3..8af36e97adfa04 100644 --- a/airflow/settings.py +++ b/airflow/settings.py @@ -287,6 +287,15 @@ def prepare_engine_args(disable_connection_pool=False): engine_args['pool_recycle'] = pool_recycle engine_args['pool_pre_ping'] = pool_pre_ping engine_args['max_overflow'] = max_overflow + + # The default isolation level for MySQL (REPEATABLE READ) can introduce inconsistencies when + # running multiple schedulers, as repeated queries on the same session may read from stale snapshots. + # 'READ COMMITTED' is the default value for PostgreSQL. + # More information here: + # https://dev.mysql.com/doc/refman/8.0/en/innodb-transaction-isolation-levels.html" + if SQL_ALCHEMY_CONN.startswith('mysql'): + engine_args['isolation_level'] = 'READ COMMITTED' + return engine_args From 1aba8000a09a8539ed592f590149c80bc25e7b31 Mon Sep 17 00:00:00 2001 From: Sam Wheating Date: Fri, 7 May 2021 13:40:18 -0400 Subject: [PATCH 2/3] Fixing up tests, removing indentation --- airflow/settings.py | 14 +++++++------- tests/core/test_sqlalchemy_config.py | 2 ++ tests/www/test_app.py | 2 ++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/airflow/settings.py b/airflow/settings.py index 8af36e97adfa04..580aa97c6b991f 100644 --- a/airflow/settings.py +++ b/airflow/settings.py @@ -288,13 +288,13 @@ def prepare_engine_args(disable_connection_pool=False): engine_args['pool_pre_ping'] = pool_pre_ping engine_args['max_overflow'] = max_overflow - # The default isolation level for MySQL (REPEATABLE READ) can introduce inconsistencies when - # running multiple schedulers, as repeated queries on the same session may read from stale snapshots. - # 'READ COMMITTED' is the default value for PostgreSQL. - # More information here: - # https://dev.mysql.com/doc/refman/8.0/en/innodb-transaction-isolation-levels.html" - if SQL_ALCHEMY_CONN.startswith('mysql'): - engine_args['isolation_level'] = 'READ COMMITTED' + # The default isolation level for MySQL (REPEATABLE READ) can introduce inconsistencies when + # running multiple schedulers, as repeated queries on the same session may read from stale snapshots. + # 'READ COMMITTED' is the default value for PostgreSQL. + # More information here: + # https://dev.mysql.com/doc/refman/8.0/en/innodb-transaction-isolation-levels.html" + if SQL_ALCHEMY_CONN.startswith('mysql'): + engine_args['isolation_level'] = 'READ COMMITTED' return engine_args diff --git a/tests/core/test_sqlalchemy_config.py b/tests/core/test_sqlalchemy_config.py index c4c909bd203f14..a38748e4d1c090 100644 --- a/tests/core/test_sqlalchemy_config.py +++ b/tests/core/test_sqlalchemy_config.py @@ -57,6 +57,7 @@ def test_configure_orm_with_default_values( pool_pre_ping=True, pool_recycle=1800, pool_size=5, + isolation_level='READ COMMITTED', ) @patch('airflow.settings.setup_event_handlers') @@ -80,6 +81,7 @@ def test_sql_alchemy_connect_args( connect_args=SQL_ALCHEMY_CONNECT_ARGS, poolclass=NullPool, encoding='utf-8', + isolation_level='READ COMMITTED', ) @patch('airflow.settings.setup_event_handlers') diff --git a/tests/www/test_app.py b/tests/www/test_app.py index 47b8f812faf831..2e2579898da7e4 100644 --- a/tests/www/test_app.py +++ b/tests/www/test_app.py @@ -223,6 +223,8 @@ def debug_view(): def test_should_set_sqlalchemy_engine_options(self): app = application.cached_app(testing=True) engine_params = {'pool_size': 3, 'pool_recycle': 120, 'pool_pre_ping': True, 'max_overflow': 5} + if app.config['SQLALCHEMY_DATABASE_URI'].startswith('mysql'): + engine_params['isolation_level'] = 'READ COMMITTED' assert app.config['SQLALCHEMY_ENGINE_OPTIONS'] == engine_params @conf_vars( From ef6e45d73d7463605004d945352c7b5b70cdf9b4 Mon Sep 17 00:00:00 2001 From: Sam Wheating Date: Fri, 7 May 2021 17:01:10 -0400 Subject: [PATCH 3/3] Fixing test --- tests/core/test_sqlalchemy_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/core/test_sqlalchemy_config.py b/tests/core/test_sqlalchemy_config.py index a38748e4d1c090..de621413c0dd10 100644 --- a/tests/core/test_sqlalchemy_config.py +++ b/tests/core/test_sqlalchemy_config.py @@ -76,12 +76,15 @@ def test_sql_alchemy_connect_args( } with conf_vars(config): settings.configure_orm() + engine_args = {} + if settings.SQL_ALCHEMY_CONN.startswith('mysql'): + engine_args['isolation_level'] = 'READ COMMITTED' mock_create_engine.assert_called_once_with( settings.SQL_ALCHEMY_CONN, connect_args=SQL_ALCHEMY_CONNECT_ARGS, poolclass=NullPool, encoding='utf-8', - isolation_level='READ COMMITTED', + **engine_args, ) @patch('airflow.settings.setup_event_handlers')