Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow setting transaction limit for db connections #10440

Merged
merged 5 commits into from
Aug 2, 2021
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 changelog.d/10440.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow setting transaction limit for database connections.
4 changes: 4 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,9 @@ caches:
# 'name' gives the database engine to use: either 'sqlite3' (for SQLite) or
# 'psycopg2' (for PostgreSQL).
#
# 'txn_limit' gives the maximum number of transactions to run per connection
# before reconnecting. Defaults to 0, which means no limit.
#
# 'args' gives options which are passed through to the database engine,
# except for options starting 'cp_', which are used to configure the Twisted
# connection pool. For a reference to valid arguments, see:
Expand All @@ -740,6 +743,7 @@ caches:
#
#database:
# name: psycopg2
# txn_limit: 10000
# args:
# user: synapse_user
# password: secretpassword
Expand Down
4 changes: 4 additions & 0 deletions synapse/config/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
# 'name' gives the database engine to use: either 'sqlite3' (for SQLite) or
# 'psycopg2' (for PostgreSQL).
#
# 'txn_limit' gives the maximum number of transactions to run per connection
# before reconnecting. Defaults to 0, which means no limit.
#
# 'args' gives options which are passed through to the database engine,
# except for options starting 'cp_', which are used to configure the Twisted
# connection pool. For a reference to valid arguments, see:
Expand All @@ -53,6 +56,7 @@
#
#database:
# name: psycopg2
# txn_limit: 10000
# args:
# user: synapse_user
# password: secretpassword
Expand Down
21 changes: 21 additions & 0 deletions synapse/storage/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# limitations under the License.
import logging
import time
from collections import defaultdict
from sys import intern
from time import monotonic as monotonic_time
from typing import (
Expand Down Expand Up @@ -397,6 +398,7 @@ def __init__(
):
self.hs = hs
self._clock = hs.get_clock()
self._txn_limit = database_config.config.get("txn_limit", 0)
self._database_config = database_config
self._db_pool = make_pool(hs.get_reactor(), database_config, engine)

Expand All @@ -406,6 +408,9 @@ def __init__(
self._current_txn_total_time = 0.0
self._previous_loop_ts = 0.0

# Transaction counter: key is the twisted thread id, value is the current count
self._txn_counters: Dict[int, int] = defaultdict(int)

# TODO(paul): These can eventually be removed once the metrics code
# is running in mainline, and we have some nice monitoring frontends
# to watch it
Expand Down Expand Up @@ -750,10 +755,26 @@ def inner_func(conn, *args, **kwargs):
sql_scheduling_timer.observe(sched_duration_sec)
context.add_database_scheduled(sched_duration_sec)

if self._txn_limit > 0:
tid = self._db_pool.threadID()
self._txn_counters[tid] += 1

if self._txn_counters[tid] > self._txn_limit:
logger.debug(
"Reconnecting database connection over transaction limit"
)
conn.reconnect()
opentracing.log_kv(
{"message": "reconnected due to txn limit"}
)
self._txn_counters[tid] = 1

if self.engine.is_connection_closed(conn):
logger.debug("Reconnecting closed database connection")
conn.reconnect()
opentracing.log_kv({"message": "reconnected"})
if self._txn_limit > 0:
self._txn_counters[tid] = 1

try:
if db_autocommit:
Expand Down
36 changes: 36 additions & 0 deletions tests/storage/test_txn_limit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Copyright 2014-2021 The Matrix.org Foundation C.I.C.
Copy link
Member

Choose a reason for hiding this comment

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

we don't ask you to assign copyright, so this is yours:

Suggested change
# Copyright 2014-2021 The Matrix.org Foundation C.I.C.
# Copyright 2021 Toni Spets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fine 😄

#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from tests import unittest


class SQLTransactionLimitTestCase(unittest.HomeserverTestCase):
"""Test SQL transaction limit doesn't break transactions."""

def make_homeserver(self, reactor, clock):
return self.setup_test_homeserver(db_txn_limit=1000)

def test_config(self):
db_config = self.hs.config.get_single_database()
self.assertEqual(db_config.config["txn_limit"], 1000)

def test_select(self):
def do_select(txn):
txn.execute("SELECT 1")

db_pool = self.hs.get_datastores().databases[0]

# force txn limit to roll over at least once
for i in range(0, 1001):
self.get_success_or_raise(db_pool.runInteraction("test_select", do_select))
3 changes: 3 additions & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ def setup_test_homeserver(
"args": {"database": ":memory:", "cp_min": 1, "cp_max": 1},
}

if "db_txn_limit" in kwargs:
database_config["txn_limit"] = kwargs["db_txn_limit"]

database = DatabaseConnectionConfig("master", database_config)
config.database.databases = [database]

Expand Down