Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set default mysql isolation level to 'READ COMMITTED' #30174

Merged
merged 7 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated bycatch, I assume..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, Ill search for say a model name and the erd.svg muddies my git grep

14 changes: 8 additions & 6 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,14 @@ 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.
}
# 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',
Expand Down
25 changes: 25 additions & 0 deletions superset/initialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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,
Expand Down Expand Up @@ -482,12 +483,36 @@ 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")
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)

Expand Down
3 changes: 0 additions & 3 deletions tests/integration_tests/superset_test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@
"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
Expand Down
3 changes: 0 additions & 3 deletions tests/integration_tests/superset_test_config_thumbnails.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@
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

Expand Down
Loading