diff --git a/CHANGES.md b/CHANGES.md
index 29711c60ce..5d4e80499e 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,3 +1,20 @@
+Synapse 1.21.0rc2 (2020-10-02)
+==============================
+
+Features
+--------
+
+- Convert additional templates from inline HTML to Jinja2 templates. ([\#8444](https://github.com/matrix-org/synapse/issues/8444))
+
+Bugfixes
+--------
+
+- Fix a regression in v1.21.0rc1 which broke thumbnails of remote media. ([\#8438](https://github.com/matrix-org/synapse/issues/8438))
+- Do not expose the experimental `uk.half-shot.msc2778.login.application_service` flow in the login API, which caused a compatibility problem with Element iOS. ([\#8440](https://github.com/matrix-org/synapse/issues/8440))
+- Fix malformed log line in new federation "catch up" logic. ([\#8442](https://github.com/matrix-org/synapse/issues/8442))
+- Fix DB query on startup for negative streams which caused long start up times. Introduced in [\#8374](https://github.com/matrix-org/synapse/issues/8374). ([\#8447](https://github.com/matrix-org/synapse/issues/8447))
+
+
Synapse 1.21.0rc1 (2020-10-01)
==============================
diff --git a/changelog.d/8438.bugfix b/changelog.d/8438.bugfix
deleted file mode 100644
index 3edc394149..0000000000
--- a/changelog.d/8438.bugfix
+++ /dev/null
@@ -1 +0,0 @@
-Fix a regression in v1.21.0rc1 which broke thumbnails of remote media.
diff --git a/changelog.d/8440.bugfix b/changelog.d/8440.bugfix
deleted file mode 100644
index 84d5f541d1..0000000000
--- a/changelog.d/8440.bugfix
+++ /dev/null
@@ -1 +0,0 @@
-Do not expose the experimental `uk.half-shot.msc2778.login.application_service` flow in the login API.
diff --git a/changelog.d/8442.bugfix b/changelog.d/8442.bugfix
deleted file mode 100644
index 6f779a1de5..0000000000
--- a/changelog.d/8442.bugfix
+++ /dev/null
@@ -1 +0,0 @@
-Fix malformed log line in new federation "catch up" logic.
diff --git a/changelog.d/8444.bugfix b/changelog.d/8444.bugfix
deleted file mode 100644
index 30c4328d4b..0000000000
--- a/changelog.d/8444.bugfix
+++ /dev/null
@@ -1 +0,0 @@
-Convert additional templates from inline HTML to Jinja2 templates.
diff --git a/changelog.d/8447.bugfix b/changelog.d/8447.bugfix
deleted file mode 100644
index 88edaf322e..0000000000
--- a/changelog.d/8447.bugfix
+++ /dev/null
@@ -1 +0,0 @@
-Fix DB query on startup for negative streams which caused long start up times. Introduced in #8374.
diff --git a/changelog.d/8456.misc b/changelog.d/8456.misc
new file mode 100644
index 0000000000..ccd260069b
--- /dev/null
+++ b/changelog.d/8456.misc
@@ -0,0 +1 @@
+Reduce number of serialization errors of `MultiWriterIdGenerator._update_table`.
diff --git a/changelog.d/8475.misc b/changelog.d/8475.misc
new file mode 100644
index 0000000000..69bcb04097
--- /dev/null
+++ b/changelog.d/8475.misc
@@ -0,0 +1 @@
+Add Groovy Gorilla to the list of distributions we build `.deb`s for.
diff --git a/scripts-dev/build_debian_packages b/scripts-dev/build_debian_packages
index d055cf3287..d0685c8b35 100755
--- a/scripts-dev/build_debian_packages
+++ b/scripts-dev/build_debian_packages
@@ -25,6 +25,7 @@ DISTS = (
"ubuntu:xenial",
"ubuntu:bionic",
"ubuntu:focal",
+ "ubuntu:groovy",
)
DESC = '''\
diff --git a/synapse/__init__.py b/synapse/__init__.py
index 4706974508..500558bbdf 100644
--- a/synapse/__init__.py
+++ b/synapse/__init__.py
@@ -48,7 +48,7 @@ try:
except ImportError:
pass
-__version__ = "1.21.0rc1"
+__version__ = "1.21.0rc2"
if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)):
# We import here so that we don't have to install a bunch of deps when
diff --git a/synapse/storage/database.py b/synapse/storage/database.py
index 79ec8f119d..6116191b16 100644
--- a/synapse/storage/database.py
+++ b/synapse/storage/database.py
@@ -403,6 +403,24 @@ class DatabasePool:
*args: Any,
**kwargs: Any
) -> R:
+ """Start a new database transaction with the given connection.
+
+ Note: The given func may be called multiple times under certain
+ failure modes. This is normally fine when in a standard transaction,
+ but care must be taken if the connection is in `autocommit` mode that
+ the function will correctly handle being aborted and retried half way
+ through its execution.
+
+ Args:
+ conn
+ desc
+ after_callbacks
+ exception_callbacks
+ func
+ *args
+ **kwargs
+ """
+
start = monotonic_time()
txn_id = self._TXN_ID
@@ -508,7 +526,12 @@ class DatabasePool:
sql_txn_timer.labels(desc).observe(duration)
async def runInteraction(
- self, desc: str, func: "Callable[..., R]", *args: Any, **kwargs: Any
+ self,
+ desc: str,
+ func: "Callable[..., R]",
+ *args: Any,
+ db_autocommit: bool = False,
+ **kwargs: Any
) -> R:
"""Starts a transaction on the database and runs a given function
@@ -518,6 +541,18 @@ class DatabasePool:
database transaction (twisted.enterprise.adbapi.Transaction) as
its first argument, followed by `args` and `kwargs`.
+ db_autocommit: Whether to run the function in "autocommit" mode,
+ i.e. outside of a transaction. This is useful for transactions
+ that are only a single query.
+
+ Currently, this is only implemented for Postgres. SQLite will still
+ run the function inside a transaction.
+
+ WARNING: This means that if func fails half way through then
+ the changes will *not* be rolled back. `func` may also get
+ called multiple times if the transaction is retried, so must
+ correctly handle that case.
+
args: positional args to pass to `func`
kwargs: named args to pass to `func`
@@ -538,6 +573,7 @@ class DatabasePool:
exception_callbacks,
func,
*args,
+ db_autocommit=db_autocommit,
**kwargs
)
@@ -551,7 +587,11 @@ class DatabasePool:
return cast(R, result)
async def runWithConnection(
- self, func: "Callable[..., R]", *args: Any, **kwargs: Any
+ self,
+ func: "Callable[..., R]",
+ *args: Any,
+ db_autocommit: bool = False,
+ **kwargs: Any
) -> R:
"""Wraps the .runWithConnection() method on the underlying db_pool.
@@ -560,6 +600,9 @@ class DatabasePool:
database connection (twisted.enterprise.adbapi.Connection) as
its first argument, followed by `args` and `kwargs`.
args: positional args to pass to `func`
+ db_autocommit: Whether to run the function in "autocommit" mode,
+ i.e. outside of a transaction. This is useful for transaction
+ that are only a single query. Currently only affects postgres.
kwargs: named args to pass to `func`
Returns:
@@ -575,6 +618,13 @@ class DatabasePool:
start_time = monotonic_time()
def inner_func(conn, *args, **kwargs):
+ # We shouldn't be in a transaction. If we are then something
+ # somewhere hasn't committed after doing work. (This is likely only
+ # possible during startup, as `run*` will ensure changes are
+ # committed/rolled back before putting the connection back in the
+ # pool).
+ assert not self.engine.in_transaction(conn)
+
with LoggingContext("runWithConnection", parent_context) as context:
sched_duration_sec = monotonic_time() - start_time
sql_scheduling_timer.observe(sched_duration_sec)
@@ -584,7 +634,14 @@ class DatabasePool:
logger.debug("Reconnecting closed database connection")
conn.reconnect()
- return func(conn, *args, **kwargs)
+ try:
+ if db_autocommit:
+ self.engine.attempt_to_set_autocommit(conn, True)
+
+ return func(conn, *args, **kwargs)
+ finally:
+ if db_autocommit:
+ self.engine.attempt_to_set_autocommit(conn, False)
return await make_deferred_yieldable(
self._db_pool.runWithConnection(inner_func, *args, **kwargs)
diff --git a/synapse/storage/engines/_base.py b/synapse/storage/engines/_base.py
index 908cbc79e3..d6d632dc10 100644
--- a/synapse/storage/engines/_base.py
+++ b/synapse/storage/engines/_base.py
@@ -97,3 +97,20 @@ class BaseDatabaseEngine(Generic[ConnectionType], metaclass=abc.ABCMeta):
"""Gets a string giving the server version. For example: '3.22.0'
"""
...
+
+ @abc.abstractmethod
+ def in_transaction(self, conn: Connection) -> bool:
+ """Whether the connection is currently in a transaction.
+ """
+ ...
+
+ @abc.abstractmethod
+ def attempt_to_set_autocommit(self, conn: Connection, autocommit: bool):
+ """Attempt to set the connections autocommit mode.
+
+ When True queries are run outside of transactions.
+
+ Note: This has no effect on SQLite3, so callers still need to
+ commit/rollback the connections.
+ """
+ ...
diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py
index ff39281f85..7719ac32f7 100644
--- a/synapse/storage/engines/postgres.py
+++ b/synapse/storage/engines/postgres.py
@@ -15,7 +15,8 @@
import logging
-from ._base import BaseDatabaseEngine, IncorrectDatabaseSetup
+from synapse.storage.engines._base import BaseDatabaseEngine, IncorrectDatabaseSetup
+from synapse.storage.types import Connection
logger = logging.getLogger(__name__)
@@ -119,6 +120,7 @@ class PostgresEngine(BaseDatabaseEngine):
cursor.execute("SET synchronous_commit TO OFF")
cursor.close()
+ db_conn.commit()
@property
def can_native_upsert(self):
@@ -171,3 +173,9 @@ class PostgresEngine(BaseDatabaseEngine):
return "%i.%i" % (numver / 10000, numver % 10000)
else:
return "%i.%i.%i" % (numver / 10000, (numver % 10000) / 100, numver % 100)
+
+ def in_transaction(self, conn: Connection) -> bool:
+ return conn.status != self.module.extensions.STATUS_READY # type: ignore
+
+ def attempt_to_set_autocommit(self, conn: Connection, autocommit: bool):
+ return conn.set_session(autocommit=autocommit) # type: ignore
diff --git a/synapse/storage/engines/sqlite.py b/synapse/storage/engines/sqlite.py
index 8a0f8c89d1..5db0f0b520 100644
--- a/synapse/storage/engines/sqlite.py
+++ b/synapse/storage/engines/sqlite.py
@@ -17,6 +17,7 @@ import threading
import typing
from synapse.storage.engines import BaseDatabaseEngine
+from synapse.storage.types import Connection
if typing.TYPE_CHECKING:
import sqlite3 # noqa: F401
@@ -86,6 +87,7 @@ class Sqlite3Engine(BaseDatabaseEngine["sqlite3.Connection"]):
db_conn.create_function("rank", 1, _rank)
db_conn.execute("PRAGMA foreign_keys = ON;")
+ db_conn.commit()
def is_deadlock(self, error):
return False
@@ -105,6 +107,14 @@ class Sqlite3Engine(BaseDatabaseEngine["sqlite3.Connection"]):
"""
return "%i.%i.%i" % self.module.sqlite_version_info
+ def in_transaction(self, conn: Connection) -> bool:
+ return conn.in_transaction # type: ignore
+
+ def attempt_to_set_autocommit(self, conn: Connection, autocommit: bool):
+ # Twisted doesn't let us set attributes on the connections, so we can't
+ # set the connection to autocommit mode.
+ pass
+
# Following functions taken from: https://github.com/coleifer/peewee
diff --git a/synapse/storage/util/id_generators.py b/synapse/storage/util/id_generators.py
index c92cd4a6ba..0aca78aa67 100644
--- a/synapse/storage/util/id_generators.py
+++ b/synapse/storage/util/id_generators.py
@@ -24,6 +24,7 @@ from typing_extensions import Deque
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.storage.database import DatabasePool, LoggingTransaction
+from synapse.storage.types import Cursor
from synapse.storage.util.sequence import PostgresSequenceGenerator
logger = logging.getLogger(__name__)
@@ -552,7 +553,7 @@ class MultiWriterIdGenerator:
# do.
break
- def _update_stream_positions_table_txn(self, txn):
+ def _update_stream_positions_table_txn(self, txn: Cursor):
"""Update the `stream_positions` table with newly persisted position.
"""
@@ -602,10 +603,13 @@ class _MultiWriterCtxManager:
stream_ids = attr.ib(type=List[int], factory=list)
async def __aenter__(self) -> Union[int, List[int]]:
+ # It's safe to run this in autocommit mode as fetching values from a
+ # sequence ignores transaction semantics anyway.
self.stream_ids = await self.id_gen._db.runInteraction(
"_load_next_mult_id",
self.id_gen._load_next_mult_id_txn,
self.multiple_ids or 1,
+ db_autocommit=True,
)
# Assert the fetched ID is actually greater than any ID we've already
@@ -636,10 +640,16 @@ class _MultiWriterCtxManager:
#
# We only do this on the success path so that the persisted current
# position points to a persisted row with the correct instance name.
+ #
+ # We do this in autocommit mode as a) the upsert works correctly outside
+ # transactions and b) reduces the amount of time the rows are locked
+ # for. If we don't do this then we'll often hit serialization errors due
+ # to the fact we default to REPEATABLE READ isolation levels.
if self.id_gen._writers:
await self.id_gen._db.runInteraction(
"MultiWriterIdGenerator._update_table",
self.id_gen._update_stream_positions_table_txn,
+ db_autocommit=True,
)
return False
diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py
index 40ba652248..eac7e4dcd2 100644
--- a/tests/storage/test_base.py
+++ b/tests/storage/test_base.py
@@ -56,6 +56,7 @@ class SQLBaseStoreTestCase(unittest.TestCase):
engine = create_engine(sqlite_config)
fake_engine = Mock(wraps=engine)
fake_engine.can_native_upsert = False
+ fake_engine.in_transaction.return_value = False
db = DatabasePool(Mock(), Mock(config=sqlite_config), fake_engine)
db._db_pool = self.db_pool
|
