Skip to content

Commit

Permalink
fix: set default mysql isolation level to 'READ COMMITTED'
Browse files Browse the repository at this point in the history
#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
  • Loading branch information
mistercrunch committed Sep 6, 2024
1 parent 043b5d4 commit 1a3cb62
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
docker/**/*.sh text eol=lf
*.svg binary
7 changes: 0 additions & 7 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions superset/initialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 1a3cb62

Please sign in to comment.