From 1a3cb6216e092bb22c2904e7aa338cdef766ccb9 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 5 Sep 2024 18:27:03 -0700 Subject: [PATCH] fix: set default mysql isolation level to 'READ COMMITTED' https://github.com/apache/superset/pull/28628 had the virtuous intent to align mysql and postgres to a consistent isolation_level (READ COMMITTED), which seems fitted for Superset. Though instead of doing this, and because sqlite doesn't support that specific one, it set the default to SERIALIZABLE which seems to exist by many engines. Here I'm realizing we need dynamic defaults for isolation_level based on which engine is in used, which can't be done in config.py as we don't know yet what engine will be set by the admin at that time. So I thought the superset/initialization package might be the right place for this. Open to other solutions, but I think this one works. * using DB_CONNECTION_MUTATOR, but that one applies only to analytics workloads, not the metadata db --- .gitattributes | 1 + superset/config.py | 7 ------- superset/initialization/__init__.py | 12 ++++++++++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.gitattributes b/.gitattributes index 79f44a6b26377..5f47e6acc8eec 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 670c518931078..4e6e84af4c9c4 100644 --- a/superset/config.py +++ b/superset/config.py @@ -196,13 +196,6 @@ def _try_json_readsha(filepath: str, length: int) -> str | None: # 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. -} - # 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', # returns a password and set SQLALCHEMY_CUSTOM_PASSWORD_STORE. diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 2601cda9d7da7..4ed4d82eb1eee 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -482,12 +482,24 @@ def init_app(self) -> None: 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") + if not isolation_level and self.config["SQLALCHEMY_DATABASE_URI"].startswith( + "mysql" + ): + db.engine.execution_options(isolation_level="READ COMMITTED") + def configure_auth_provider(self) -> None: machine_auth_provider_factory.init_app(self.superset_app)