Fixes https://github.com/element-hq/synapse/issues/19175
This PR moves tracking of what lazy loaded membership we've sent to each
room out of the required state table. This avoids that table from
continuously growing, which massively helps performance as we pull out
all matching rows for the connection when we receive a request.
The new table is only read when we have data in a room to send, so we
end up reading a lot fewer rows from the DB. Though we now read from
that table for every room we have events to return in, rather than once
at the start of the request.
For an explanation of how the new table works, see the
[comment](https://github.com/element-hq/synapse/blob/erikj/sss_better_membership_storage2/synapse/storage/schema/main/delta/93/02_sliding_sync_members.sql#L15-L38)
on the table schema.
The table is designed so that we can later prune old entries if we wish,
but that is not implemented in this PR.
Reviewable commit-by-commit.
---------
Co-authored-by: Eric Eastwood <erice@element.io>
Related to https://github.com/element-hq/synapse/issues/17035, when
Synapse receives a request that is larger than the maximum size allowed,
it aborts the connection without ever sending back a HTTP response.
I dug into our usage of twisted and how best to try and report such an
error and this is what I came up with.
It would be ideal to be able to report the status from within
`handleContentChunk` but that is called too early on in the twisted http
handling code, before things have been setup enough to be able to
properly write a response.
I tested this change out locally (both with C-S and S-S apis) and they
do receive a 413 response now in addition to the connection being
closed.
Hopefully this will aid in being able to quickly detect when
https://github.com/element-hq/synapse/issues/17035 is occurring as the
current situation makes it very hard to narrow things down to that
specific issue without making a lot of assumptions.
This PR also responds with more meaningful error codes now in the case
of:
- multiple `Content-Length` headers
- invalid `Content-Length` header value
- request content size being larger than the `Content-Length` value
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [X] Pull request is based on the develop branch
* [X] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [X] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Eric Eastwood <erice@element.io>
Bumps the minor-and-patches group with 3 updates:
[mypy](https://github.com/python/mypy),
[mypy-zope](https://github.com/Shoobx/mypy-zope) and
[phonenumbers](https://github.com/daviddrysdale/python-phonenumbers).
Updates `mypy` from 1.17.1 to 1.18.2
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/python/mypy/blob/master/CHANGELOG.md">mypy's
changelog</a>.</em></p>
<blockquote>
<h3>Mypy 1.18.2</h3>
<ul>
<li>Fix crash on recursive alias (Ivan Levkivskyi, PR <a
href="https://redirect.github.com/python/mypy/pull/19845">19845</a>)</li>
<li>Add additional guidance for stubtest errors when runtime is
<code>object.__init__</code> (Stephen Morton, PR <a
href="https://redirect.github.com/python/mypy/pull/19733">19733</a>)</li>
<li>Fix handling of None values in f-string expressions in mypyc
(BobTheBuidler, PR <a
href="https://redirect.github.com/python/mypy/pull/19846">19846</a>)</li>
</ul>
<h3>Acknowledgements</h3>
<p>Thanks to all mypy contributors who contributed to this release:</p>
<ul>
<li>Ali Hamdan</li>
<li>Anthony Sottile</li>
<li>BobTheBuidler</li>
<li>Brian Schubert</li>
<li>Chainfire</li>
<li>Charlie Denton</li>
<li>Christoph Tyralla</li>
<li>CoolCat467</li>
<li>Daniel Hnyk</li>
<li>Emily</li>
<li>Emma Smith</li>
<li>Ethan Sarp</li>
<li>Ivan Levkivskyi</li>
<li>Jahongir Qurbonov</li>
<li>Jelle Zijlstra</li>
<li>Joren Hammudoglu</li>
<li>Jukka Lehtosalo</li>
<li>Marc Mueller</li>
<li>Omer Hadari</li>
<li>Piotr Sawicki</li>
<li>PrinceNaroliya</li>
<li>Randolf Scholz</li>
<li>Robsdedude</li>
<li>Saul Shanabrook</li>
<li>Shantanu</li>
<li>Stanislav Terliakov</li>
<li>Stephen Morton</li>
<li>wyattscarpenter</li>
</ul>
<p>I’d also like to thank my employer, Dropbox, for supporting mypy
development.</p>
<h2>Mypy 1.17</h2>
<p>We’ve just uploaded mypy 1.17 to the Python Package Index (<a
href="https://pypi.org/project/mypy/">PyPI</a>).
Mypy is a static type checker for Python. This release includes new
features and bug fixes.
You can install it as follows:</p>
<pre><code>python3 -m pip install -U mypy
</code></pre>
<p>You can read the full documentation for this release on <a
href="http://mypy.readthedocs.io">Read the Docs</a>.</p>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="df05f05555"><code>df05f05</code></a>
remove +dev from version</li>
<li><a
href="01a7a1285d"><code>01a7a12</code></a>
Update changelog for 1.18.2 (<a
href="https://redirect.github.com/python/mypy/issues/19873">#19873</a>)</li>
<li><a
href="ca5abf09f3"><code>ca5abf0</code></a>
Typeshed cherry-pick: Make type of <code>unitest.mock.Any</code> a
subclass of <code>Any</code> (<a
href="https://redirect.github.com/python/mypy/issues/1">#1</a>...</li>
<li><a
href="9d794b57d9"><code>9d794b5</code></a>
[mypyc] fix: inappropriate <code>None</code>s in f-strings (<a
href="https://redirect.github.com/python/mypy/issues/19846">#19846</a>)</li>
<li><a
href="2c0510c848"><code>2c0510c</code></a>
stubtest: additional guidance on errors when runtime is
object.<strong>init</strong> (<a
href="https://redirect.github.com/python/mypy/issues/19733">#19733</a>)</li>
<li><a
href="2f3f03c3e3"><code>2f3f03c</code></a>
Bump version to 1.18.2+dev for point release</li>
<li><a
href="76698412bc"><code>7669841</code></a>
Fix crash on recursive alias in indirection.py (<a
href="https://redirect.github.com/python/mypy/issues/19845">#19845</a>)</li>
<li><a
href="03fbaa941b"><code>03fbaa9</code></a>
bump version to 1.18.1 due to wheels failure</li>
<li><a
href="b44a1fbf0c"><code>b44a1fb</code></a>
removed +dev from version</li>
<li><a
href="7197a99d1a"><code>7197a99</code></a>
Removed Unreleased in the Changelog for Release 1.18 (<a
href="https://redirect.github.com/python/mypy/issues/19827">#19827</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/python/mypy/compare/v1.17.1...v1.18.2">compare
view</a></li>
</ul>
</details>
<br />
Updates `mypy-zope` from 1.0.13 to 1.0.14
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/Shoobx/mypy-zope/blob/master/CHANGELOG.md">mypy-zope's
changelog</a>.</em></p>
<blockquote>
<h2>1.0.14 (2025-12-01)</h2>
<hr />
<ul>
<li>Support mypy-1.19</li>
<li>Support mypy-1.18</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="38d22f3f4f"><code>38d22f3</code></a>
Preparing release 1.0.14</li>
<li><a
href="76762ec861"><code>76762ec</code></a>
Maintain changelog</li>
<li><a
href="4971d98ab8"><code>4971d98</code></a>
Merge pull request <a
href="https://redirect.github.com/Shoobx/mypy-zope/issues/134">#134</a>
from Shoobx/dependabot/pip/mypy-gte-1.0.0-and-lt-1.20.0</li>
<li><a
href="47af89d2c7"><code>47af89d</code></a>
Update mypy requirement from <1.19.0,>=1.0.0 to
>=1.0.0,<1.20.0</li>
<li><a
href="0c596ff804"><code>0c596ff</code></a>
Maintain changelog</li>
<li><a
href="dcaa27841d"><code>dcaa278</code></a>
Merge pull request <a
href="https://redirect.github.com/Shoobx/mypy-zope/issues/132">#132</a>
from Shoobx/dependabot/pip/mypy-gte-1.0.0-and-lt-1.19.0</li>
<li><a
href="8f7b6778df"><code>8f7b677</code></a>
Update mypy requirement from <1.18.0,>=1.0.0 to
>=1.0.0,<1.19.0</li>
<li><a
href="91b275b364"><code>91b275b</code></a>
Back to development: 1.0.14</li>
<li>See full diff in <a
href="https://github.com/Shoobx/mypy-zope/compare/1.0.13...1.0.14">compare
view</a></li>
</ul>
</details>
<br />
Updates `phonenumbers` from 9.0.18 to 9.0.19
<details>
<summary>Commits</summary>
<ul>
<li><a
href="38f2ffe1e8"><code>38f2ffe</code></a>
Prep for 9.0.19 release</li>
<li><a
href="cd7f0cc64f"><code>cd7f0cc</code></a>
Generated files for metadata</li>
<li><a
href="40ae18f50a"><code>40ae18f</code></a>
Merge metadata changes from upstream 9.0.19</li>
<li>See full diff in <a
href="https://github.com/daviddrysdale/python-phonenumbers/compare/v9.0.18...v9.0.19">compare
view</a></li>
</ul>
</details>
<br />
**Does not** update `pysaml2` from 7.5.0 to 7.5.4 since this would
downgrade pyOpenSSL
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/IdentityPython/pysaml2/releases">pysaml2's
releases</a>.</em></p>
<blockquote>
<h2>Version v7.5.4</h2>
<h2>v7.5.4 (2025-10-07)</h2>
<ul>
<li>Minor refactor to handle <code>shelve.open</code> and
<code>dbm</code> errors</li>
<li>Remove import of deprecated <code>cgi</code> module</li>
<li>Replace deprecated <code>datetime.utcnow()</code> by
<code>datetime.now(timezone.utc)</code></li>
<li>deps: Remove the <code>importlib_metadata</code> dependency</li>
<li>deps: Remove the <code>importlib_resources</code> dependency</li>
<li>deps: Update dependency versions and lockfile</li>
<li>build: Update pyproject and lockfile to be compatible with PEP
621</li>
<li>docs: Correct spelling mistakes</li>
<li>docs: Fix interal references/links</li>
<li>docs: Clarify units for accepted_time_diff config param</li>
<li>docs: Correct documentation for contact_person</li>
</ul>
<h2>Version 7.5.3</h2>
<h2>7.5.3 (2025-10-04)</h2>
<ul>
<li><a
href="https://redirect.github.com/IdentityPython/pysaml2/issues/973">#973</a>
Fix prepare_for_negotiated_authenticate to avoid double signing redirect
requests</li>
</ul>
<h2>Version 7.5.2</h2>
<h2>7.5.2 (2025-02-10)</h2>
<ul>
<li>Include the XSD of the XML Encryption Syntax and Processing Version
1.1 to the schema validator</li>
</ul>
<h2>Version 7.5.1</h2>
<h2>7.5.1 (2025-02-10)</h2>
<ul>
<li>deps: restrict pyOpenSSL up to v24.2.1 until it is replaced</li>
<li>deps: update dependncies for the lockfile and examples</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/IdentityPython/pysaml2/blob/master/CHANGELOG.md">pysaml2's
changelog</a>.</em></p>
<blockquote>
<h2>v7.5.4 (2025-10-07)</h2>
<ul>
<li>Minor refactor to handle <code>shelve.open</code> and
<code>dbm</code> errors</li>
<li>Remove import of deprecated <code>cgi</code> module</li>
<li>Replace deprecated <code>datetime.utcnow()</code> by
<code>datetime.now(timezone.utc)</code></li>
<li>deps: Remove the <code>importlib_metadata</code> dependency</li>
<li>deps: Remove the <code>importlib_resources</code> dependency</li>
<li>deps: Update dependency versions and lockfile</li>
<li>build: Update pyproject and lockfile to be compatible with PEP
621</li>
<li>docs: Correct spelling mistakes</li>
<li>docs: Fix interal references/links</li>
<li>docs: Clarify units for accepted_time_diff config param</li>
<li>docs: Correct documentation for contact_person</li>
</ul>
<h2>7.5.3 (2025-10-04)</h2>
<ul>
<li><a
href="https://redirect.github.com/IdentityPython/pysaml2/issues/973">#973</a>
Fix prepare_for_negotiated_authenticate to avoid double signing redirect
requests</li>
</ul>
<h2>7.5.2 (2025-02-10)</h2>
<ul>
<li>Include the XSD of the XML Encryption Syntax and Processing Version
1.1 to the schema validator</li>
</ul>
<h2>7.5.1 (2025-02-10)</h2>
<ul>
<li>deps: restrict pyOpenSSL up to v24.2.1 until it is replaced</li>
<li>deps: update dependencies for the lockfile and examples</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="9cf71f7f9e"><code>9cf71f7</code></a>
Release version 7.5.4</li>
<li><a
href="c3ec7199d1"><code>c3ec719</code></a>
Refactor _shelve_compat</li>
<li><a
href="1d6ea6024e"><code>1d6ea60</code></a>
Remove import of deprecated cgi module</li>
<li><a
href="c45eb9df82"><code>c45eb9d</code></a>
Replace deprecated datetime.utcnow() by datetime.now(timezone.utc)</li>
<li><a
href="178f6d12b4"><code>178f6d1</code></a>
Remove unneeded dependencies</li>
<li><a
href="1f0a25a5cf"><code>1f0a25a</code></a>
remove importlib_metadata import</li>
<li><a
href="099f716ae7"><code>099f716</code></a>
remove importlib_resources imports</li>
<li><a
href="3fa11ee15d"><code>3fa11ee</code></a>
spelling updates.</li>
<li><a
href="4b7887f59a"><code>4b7887f</code></a>
update link.</li>
<li><a
href="bc8d3b4ecc"><code>bc8d3b4</code></a>
update link.</li>
<li>Additional commits viewable in <a
href="https://github.com/IdentityPython/pysaml2/compare/v7.5.0...v7.5.4">compare
view</a></li>
</ul>
</details>
<br />
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions
</details>
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Devon Hudson <devonhudson@librem.one>
Fix#19233
Synapse fails to handle events in v12 rooms when the server is run with
the `{use_frozen_dicts: True}` config.
This PR fixes the issue, and adds tests which cover room creation,
joining, and joining over federation, with both frozen and not frozen
config settings, by extending the existing `test_send_join` federation
tests.
This approach to testing was chosen as it is a simple way to get high
level integration style test coverage, without going through all our
existing tests and trying to retroactively add in coverage when using
frozen dicts.
This should provide an easy place for future room versions to extend the
suite of tests and reduce the chance of introducing subtle bugs like
this in the future.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
This changes the arguments in clock functions to be `Duration` and
converts call sites and constants into `Duration`. There are still some
more functions around that should be converted (e.g.
`timeout_deferred`), but we leave that to another PR.
We also changes `.as_secs()` to return a float, as the rounding broke
things subtly. The only reason to keep it (its the same as
`timedelta.total_seconds()`) is for symmetry with `as_millis()`.
Follows on from https://github.com/element-hq/synapse/pull/19223
MSC4380 aims to be a simplified implementation of MSC4155; the hope is
that we can get it specced and rolled out rapidly, so that we can
resolve the fact that `matrix.org` has enabled MSC4155.
The implementation leans heavily on what's already there for MSC4155.
It has its own `experimental_features` flag. If both MSC4155 and MSC4380
are enabled, and a user has both configurations set, then we prioritise
the MSC4380 one.
Contributed wearing my 🎩 Spec Core Team hat.
We have various constants to try and avoid mistyping of durations, e.g.
`ONE_HOUR_SECONDS * MILLISECONDS_PER_SECOND`, however this can get a
little verbose and doesn't help with typing.
Instead, let's move towards a dedicated `Duration` class (basically a
[`timedelta`](https://docs.python.org/3/library/datetime.html#timedelta-objects)
with helper methods).
This PR introduces the new types and converts all usages of the existing
constants with it. Future PRs may work to move the clock methods to also
use it (e.g. `call_later` and `looping_call`).
Reviewable commit-by-commit.
We add some logic to expire sliding sync connections if they get old or
if there is too much pending data to return.
The values of the constants are picked fairly arbitrarily, these are
currently:
1. More than 100 rooms with pending events if the connection hasn't been
used in over an hour
2. The connection hasn't been used for over a week
Reviewable commit-by-commit
---------
Co-authored-by: Eric Eastwood <erice@element.io>
As per recent proposals in MSC4140, remove authentication for
restarting/cancelling/sending a delayed event, and give each of those
actions its own endpoint. (The original consolidated endpoint is still
supported for backwards compatibility.)
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Half-Shot <will@half-shot.uk>
Spawning a background process comes with a bunch of overhead, so let's
try to reduce the number of background processes we need to spawn when
handling inbound fed.
Currently, we seem to be doing roughly one per command. Instead, lets
keep the background process alive for a bit waiting for a new command to
come in.
I noticed this in some profiling. Basically, we prune the ratelimiters
by copying and iterating over every entry every 60 seconds. Instead,
let's use a wheel timer to track when we should potentially prune a
given key, and then we a) check fewer keys, and b) can run more
frequently. Hopefully this should mean we don't have a large pause
everytime we prune a ratelimiter with lots of keys.
Also fixes a bug where we didn't prune entries that were added via
`record_action` and never subsequently updated. This affected the media
and joins-per-room ratelimiter.
Fix lost logcontext when using `timeout_deferred(...)` and things
actually timeout.
Fix https://github.com/element-hq/synapse/issues/19087 (our HTTP client
times out requests using `timeout_deferred(...)`
Fix https://github.com/element-hq/synapse/issues/19066 (`/sync` uses
`notifier.wait_for_events()` which uses `timeout_deferred(...)` under
the hood)
### When/why did these lost logcontext warnings start happening?
```
synapse.logging.context - 107 - WARNING - sentinel - Expected logging context call_later but found POST-2453
synapse.logging.context - 107 - WARNING - sentinel - Expected logging context call_later was lost
```
In https://github.com/element-hq/synapse/pull/18828, we switched
`timeout_deferred(...)` from using `reactor.callLater(...)` to
[`clock.call_later(...)`](3b59ac3b69/synapse/util/clock.py (L224-L313))
under the hood. This meant it started dealing with logcontexts but our
`time_it_out()` callback didn't follow our [Synapse logcontext
rules](3b59ac3b69/docs/log_contexts.md).
This is a normal
problem where we `await` a deferred without wrapping it in
`make_deferred_yieldable(...)`. But I've opted to replace the usage of
`deferLater` with something more standard for the Synapse codebase.
Part of https://github.com/element-hq/synapse/issues/18905
It's unclear why we're only now seeing these failures happen with the
changes from https://github.com/element-hq/synapse/pull/19057
Example failures seen in
https://github.com/element-hq/synapse/actions/runs/18477454390/job/52645183606?pr=19057
```
builtins.AssertionError: Expected `looping_call` callback from the reactor to start with the sentinel logcontext but saw task-_resumable_task-0-IBzAmHUoepQfLnEA. In other words, another task shouldn't have leaked their logcontext to us.
```
It is often useful when investigating a space to get information about
that space and it's children. This PR adds an Admin API to return
information about a space and it's children, regardless of room
membership. Will not fetch information over federation about remote
rooms that the server is not participating in.
This means we
can move the open registration config validation from `setup()` to
`HomeServerConfig.validate_config()` (much more sane).
Spawning from looking at this area of code in
https://github.com/element-hq/synapse/pull/19015
### Background
As part of Element's plan to support a light form of vhosting (virtual
host) (multiple instances of Synapse in the same Python process), we're
currently diving into the details and implications of running multiple
instances of Synapse in the same Python process.
"Clean tenant provisioning" tracked internally by
https://github.com/element-hq/synapse-small-hosts/issues/221
### Partial startup problem
In the context of Synapse Pro for Small Hosts, since the Twisted reactor
is already running (from the `multi_synapse` shard process itself), when
provisioning a homeserver tenant, the `reactor.callWhenRunning(...)`
callbacks will be invoked immediately. This includes the Synapse's
[`start`](0615b64bb4/synapse/app/homeserver.py (L418-L429))
callback which sets up everything (including listeners, background
tasks, etc). If we encounter an error at this point, we are partially
setup but the exception will [bubble back to
us](8be122186b/multi_synapse/app/shard.py (L114-L121))
without us having a handle to the homeserver yet so we can't call
`hs.shutdown()` and clean everything up.
### What does this PR do?
Structures Synapse so we split creating the homeserver instance from
setting everything up. This way we have access to `hs` if anything goes
wrong during setup and can subsequently `hs.shutdown()` to clean
everything up.
Revert https://github.com/element-hq/synapse/pull/18849
Go back to our custom `LogContextScopeManager` after trying
OpenTracing's `ContextVarsScopeManager`.
Fix https://github.com/element-hq/synapse/issues/19004
### Why revert?
For reference, with the normal reactor, `ContextVarsScopeManager` worked
just as good as our custom `LogContextScopeManager` as far as I can tell
(and even better in some cases). But since Twisted appears to not fully
support `ContextVar`'s, it doesn't work as expected in all cases.
Compounding things, `ContextVarsScopeManager` was causing errors with
the experimental `SYNAPSE_ASYNC_IO_REACTOR` option.
Since we're not getting the full benefit that we originally desired, we
might as well revert and figure out alternatives for extending the
logcontext lifetimes to support the use case we were trying to unlock
(c.f. https://github.com/element-hq/synapse/pull/18804).
See
https://github.com/element-hq/synapse/issues/19004#issuecomment-3358052171
for more info.
### Does this require backporting and patch releases?
No. Since `ContextVarsScopeManager` operates just as good with the
normal reactor and was only causing actual errors with the experimental
`SYNAPSE_ASYNC_IO_REACTOR` option, I don't think this requires us to
backport and make patch releases at all.
### Maintain cross-links between main trace and background process work
In order to maintain the functionality introduced in https://github.com/element-hq/synapse/pull/18932 (cross-links between the background process trace and currently active trace), we also needed a small change.
Previously, when we were using `ContextVarsScopeManager`, it tracked the tracing scope across the logcontext changes without issue. Now that we're using our own custom `LogContextScopeManager` again, we need to capture the active span from the logcontext before we reset to the sentinel context because of the `PreserveLoggingContext()` below.
Added some tests to ensure we maintain the `run_as_background` tracing behavior regardless of the tracing scope manager we use.
Spawning from adding some logcontext debug logs in
https://github.com/element-hq/synapse/pull/18966 and since we're not
logging at the `set_current_context(...)` level (see reasoning there),
this removes some usage of `set_current_context(...)`.
Specifically, `MockClock.call_later(...)` doesn't handle logcontexts
correctly. It uses the calling logcontext as the callback context
(wrong, as the logcontext could finish before the callback finishes) and
it didn't reset back to the sentinel context before handing back to the
reactor. It was like this since it was [introduced 10+ years
ago](38da9884e7).
Instead of fixing the implementation which would just be a copy of our
normal `Clock`, we can just remove `MockClock`
### Background
As part of Element's plan to support a light form of vhosting (virtual
host) (multiple instances of Synapse in the same Python process), we're
currently diving into the details and implications of running multiple
instances of Synapse in the same Python process.
"Per-tenant logging" tracked internally by
https://github.com/element-hq/synapse-small-hosts/issues/48
### Prior art
Previously, we exposed `server_name` by providing a static logging
`MetadataFilter` that injected the values:
205d9e4fc4/synapse/config/logger.py (L216)
While this can work fine for the normal case of one Synapse instance per
Python process, this configures things globally and isn't compatible
when we try to start multiple Synapse instances because each subsequent
tenant will overwrite the previous tenant.
### What does this PR do?
We remove the `MetadataFilter` and replace it by tracking the
`server_name` in the `LoggingContext` and expose it with our existing
[`LoggingContextFilter`](205d9e4fc4/synapse/logging/context.py (L584-L622))
that we already use to expose information about the `request`.
This means that the `server_name` value follows wherever we log as
expected even when we have multiple Synapse instances running in the
same process.
### A note on logcontext
Anywhere, Synapse mistakenly uses the `sentinel` logcontext to log
something, we won't know which server sent the log. We've been fixing up
`sentinel` logcontext usage as tracked by
https://github.com/element-hq/synapse/issues/18905
Any further `sentinel` logcontext usage we find in the future can be
fixed piecemeal as normal.
d2a966f922/docs/log_contexts.md (L71-L81)
### Testing strategy
1. Adjust your logging config to include `%(server_name)s` in the format
```yaml
formatters:
precise:
format: '%(asctime)s - %(server_name)s - %(name)s - %(lineno)d -
%(levelname)s - %(request)s - %(message)s'
```
1. Start Synapse: `poetry run synapse_homeserver --config-path
homeserver.yaml`
1. Make some requests (`curl
http://localhost:8008/_matrix/client/versions`, etc)
1. Open the homeserver logs and notice the `server_name` in the logs as
expected. `unknown_server_from_sentinel_context` is expected for the
`sentinel` logcontext (things outside of Synapse).
Related to https://github.com/element-hq/synapse/issues/18905
These fixes were split off from
https://github.com/element-hq/synapse/pull/18828 where @devonh was
seeing some test failures because `timeout_deferred(...)` is being
updated to use `Clock` utilities instead of raw `reactor` methods. This
test was failing in that branch/PR until we made this new version that
handles the logcontexts properly.
While the previous version of this test does pass on `develop`, it was
using what appears completely wrong assertions, assumptions, and bad
patterns to make it happen (see diff comments below)
---
Test originally introduced in https://github.com/matrix-org/synapse/pull/4407
Spawning from
https://github.com/matrix-org/synapse/pull/12588#discussion_r865843321
> It turns out `Deferred.cancel()` is a lot like
`Deferred.callback()`/`errback()` in that it will trash the logging
context:
> it can resume a coroutine, which will restore its own logging context,
then run:
>
> - until it blocks, setting the sentinel context
> - or until it terminates, setting the context it was started with
>
> So we need to wrap it in `with PreserveLoggingContext():`, like we do
with `.callback()`:
>
> ```python
> with PreserveLoggingContext():
> self.render_deferred.cancel()
> ```
>
> *-- @squahtx,
https://github.com/matrix-org/synapse/pull/12588#discussion_r865843321*
