From 6baeb659a7bc6b8f5834486884aa893627811adf Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 9 Sep 2024 17:00:54 -0700 Subject: [PATCH] fix: set default mysql isolation level to 'READ COMMITTED' (#30174) Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- .gitattributes | 1 + superset/config.py | 14 ++++++----- superset/initialization/__init__.py | 25 +++++++++++++++++++ .../integration_tests/superset_test_config.py | 3 --- .../superset_test_config_thumbnails.py | 3 --- 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/.gitattributes b/.gitattributes index 79f44a6b2..5f47e6acc 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ docker/**/*.sh text eol=lf +*.svg binary diff --git a/superset/config.py b/superset/config.py index 670c51893..b4ce65949 100644 --- a/superset/config.py +++ b/superset/config.py @@ -196,12 +196,14 @@ SQLALCHEMY_DATABASE_URI = ( # SQLALCHEMY_DATABASE_URI = 'mysql://myapp@localhost/myapp' # SQLALCHEMY_DATABASE_URI = 'postgresql://root:password@localhost/myapp' -# The default MySQL isolation level is REPEATABLE READ whereas the default PostgreSQL -# isolation level is READ COMMITTED. All backends should use READ COMMITTED (or similar) -# to help ensure consistent behavior. -SQLALCHEMY_ENGINE_OPTIONS = { - "isolation_level": "SERIALIZABLE", # SQLite does not support READ COMMITTED. -} +# This config is exposed through flask-sqlalchemy, and can be used to set your metadata +# database connection settings. You can use this to set arbitrary connection settings +# that may be specific to the database engine you are using. +# Note that you can use this to set the isolation level of your database, as in +# `SQLALCHEMY_ENGINE_OPTIONS = {"isolation_level": "READ COMMITTED"}` +# Also note that we recommend READ COMMITTED for regular operation. +# Find out more here https://flask-sqlalchemy.palletsprojects.com/en/3.1.x/config/ +SQLALCHEMY_ENGINE_OPTIONS = {} # In order to hook up a custom password store for all SQLALCHEMY connections # implement a function that takes a single argument of type 'sqla.engine.url', diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 2601cda9d..10686b872 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -32,6 +32,7 @@ from flask_session import Session from werkzeug.middleware.proxy_fix import ProxyFix from superset.constants import CHANGE_ME_SECRET_KEY +from superset.databases.utils import make_url_safe from superset.extensions import ( _event_logger, APP_DIR, @@ -482,12 +483,36 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods self.configure_wtf() self.configure_middlewares() self.configure_cache() + self.set_db_default_isolation() with self.superset_app.app_context(): self.init_app_in_ctx() self.post_init() + def set_db_default_isolation(self) -> None: + # This block sets the default isolation level for mysql to READ COMMITTED if not + # specified in the config. You can set your isolation in the config by using + # SQLALCHEMY_ENGINE_OPTIONS + eng_options = self.config["SQLALCHEMY_ENGINE_OPTIONS"] or {} + isolation_level = eng_options.get("isolation_level") + set_isolation_level_to = None + + if not isolation_level: + backend = make_url_safe( + self.config["SQLALCHEMY_DATABASE_URI"] + ).get_backend_name() + if backend in ("mysql", "postgresql"): + set_isolation_level_to = "READ COMMITTED" + + if set_isolation_level_to: + logger.info( + "Setting database isolation level to %s", + set_isolation_level_to, + ) + with self.superset_app.app_context(): + db.engine.execution_options(isolation_level=set_isolation_level_to) + def configure_auth_provider(self) -> None: machine_auth_provider_factory.init_app(self.superset_app) diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py index 19f8c3029..e1b3ce016 100644 --- a/tests/integration_tests/superset_test_config.py +++ b/tests/integration_tests/superset_test_config.py @@ -59,9 +59,6 @@ if make_url(SQLALCHEMY_DATABASE_URI).get_backend_name() == "sqlite": "removed in a future version of Superset." ) -if make_url(SQLALCHEMY_DATABASE_URI).get_backend_name() in ("postgresql", "mysql"): - SQLALCHEMY_ENGINE_OPTIONS["isolation_level"] = "READ COMMITTED" # noqa: F405 - # Speeding up the tests.integration_tests. PRESTO_POLL_INTERVAL = 0.1 HIVE_POLL_INTERVAL = 0.1 diff --git a/tests/integration_tests/superset_test_config_thumbnails.py b/tests/integration_tests/superset_test_config_thumbnails.py index 0850d8d79..a8e78d187 100644 --- a/tests/integration_tests/superset_test_config_thumbnails.py +++ b/tests/integration_tests/superset_test_config_thumbnails.py @@ -41,9 +41,6 @@ if make_url(SQLALCHEMY_DATABASE_URI).get_backend_name() == "sqlite": in a future version of Superset." ) -if make_url(SQLALCHEMY_DATABASE_URI).get_backend_name() in ("postgresql", "mysql"): - SQLALCHEMY_ENGINE_OPTIONS["isolation_level"] = "READ COMMITTED" # noqa: F405 - SQL_SELECT_AS_CTA = True SQL_MAX_ROW = 666