From 744a25ea96af4079de337ce8e8412cf7ee8f703d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 13 Jan 2023 18:13:59 +0000 Subject: [PATCH 1/5] Don't always start a db txn on Postgres This avoids spurious "there is already a transaction in progress" warnings in PostgreSQL's logs. --- synapse/storage/prepare_database.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 3acdb39da75c..0b8b39c7fde9 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -23,7 +23,7 @@ from synapse.config.homeserver import HomeServerConfig from synapse.storage.database import LoggingDatabaseConnection -from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine +from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION from synapse.storage.types import Cursor @@ -108,9 +108,12 @@ def prepare_database( # so we start one before running anything. This ensures that any upgrades # are either applied completely, or not at all. # - # (psycopg2 automatically starts a transaction as soon as we run any statements - # at all, so this is redundant but harmless there.) - cur.execute("BEGIN TRANSACTION") + # psycopg2 does automatically start transactions, and while this is technically + # harmless to do there as well, doing so results in nested transactions. Those + # will generate warnings in Postgres' logs per query, and we'd rather like to + # avoid doing that. + if isinstance(database_engine, Sqlite3Engine): + cur.execute("BEGIN TRANSACTION") logger.info("%r: Checking existing schema version", databases) version_info = _get_or_create_schema_state(cur, database_engine) From a4aa1506c14c0923e14ac720452ac636e563a0d5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 13 Jan 2023 18:40:23 +0000 Subject: [PATCH 2/5] changelog --- changelog.d/14840.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14840.misc diff --git a/changelog.d/14840.misc b/changelog.d/14840.misc new file mode 100644 index 000000000000..ff6084284a5e --- /dev/null +++ b/changelog.d/14840.misc @@ -0,0 +1 @@ +Prevent "WARNING: there is already a transaction in progress" lines appearing in PostgreSQL's logs on some occasions. \ No newline at end of file From acc4a5c6fac98537bda3c777eec13ffa7a1acfff Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Jan 2023 13:38:01 +0000 Subject: [PATCH 3/5] Postgres: Only begin a txn when autocommit mode is enabled --- synapse/storage/prepare_database.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 0b8b39c7fde9..51afb21058e5 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -108,11 +108,14 @@ def prepare_database( # so we start one before running anything. This ensures that any upgrades # are either applied completely, or not at all. # - # psycopg2 does automatically start transactions, and while this is technically - # harmless to do there as well, doing so results in nested transactions. Those - # will generate warnings in Postgres' logs per query, and we'd rather like to + # psycopg2 does not automatically start transactions when in autocommit mode. + # While it is technically harmless nest transactions in postgres, doing so + # results in warnings in Postgres' logs per query. And we'd rather like to # avoid doing that. - if isinstance(database_engine, Sqlite3Engine): + if isinstance(database_engine, Sqlite3Engine) or ( + isinstance(database_engine, PostgresEngine) + and db_conn.autocommit + ): cur.execute("BEGIN TRANSACTION") logger.info("%r: Checking existing schema version", databases) From ddd7ecb292ccf6f086ec329075d25c4042bc9a38 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 18 Jan 2023 13:42:12 +0000 Subject: [PATCH 4/5] words --- synapse/storage/prepare_database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 51afb21058e5..ba107a196662 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -109,8 +109,8 @@ def prepare_database( # are either applied completely, or not at all. # # psycopg2 does not automatically start transactions when in autocommit mode. - # While it is technically harmless nest transactions in postgres, doing so - # results in warnings in Postgres' logs per query. And we'd rather like to + # While it is technically harmless to nest transactions in postgres, doing so + # results in a warning in Postgres' logs per query. And we'd rather like to # avoid doing that. if isinstance(database_engine, Sqlite3Engine) or ( isinstance(database_engine, PostgresEngine) From 54d595819b6ca7cdf21aa7d4f76de38355c93014 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 9 Feb 2023 19:07:48 +0000 Subject: [PATCH 5/5] lint --- synapse/storage/prepare_database.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index ba107a196662..6c335a931501 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -113,8 +113,7 @@ def prepare_database( # results in a warning in Postgres' logs per query. And we'd rather like to # avoid doing that. if isinstance(database_engine, Sqlite3Engine) or ( - isinstance(database_engine, PostgresEngine) - and db_conn.autocommit + isinstance(database_engine, PostgresEngine) and db_conn.autocommit ): cur.execute("BEGIN TRANSACTION")