A minor optimization to avoid unnecessary copying/building
identical dictionaries when filtering private read receipts.
Also clarifies comments and cleans-up some tests.
Parse the `m.relates_to` event content field (which describes relations)
in a single place, this is used during:
* Event persistence.
* Validation of the Client-Server API.
* Fetching bundled aggregations.
* Processing of push rules.
Each of these separately implement the logic and each made slightly
different assumptions about what was valid. Some had minor / potential
bugs.
Enable cancellation of `GET /rooms/$room_id/members`,
`GET /rooms/$room_id/state` and
`GET /rooms/$room_id/state/$state_key/*` requests.
Signed-off-by: Sean Quah <seanq@element.io>
`BaseFederationServlet` wraps its endpoints in a bunch of async code
that has not been vetted for compatibility with cancellation.
Fail CI if a `@cancellable` flag is applied to a federation endpoint.
Signed-off-by: Sean Quah <seanq@element.io>
While `ReplicationEndpoint`s register themselves via `JsonResource`,
they pass a method that calls the handler, instead of the handler itself,
to `register_paths`. As a result, `JsonResource` will not correctly pick
up the `@cancellable` flag and we have to apply it ourselves.
Signed-off-by: Sean Quah <seanq@element.io>
Both `RestServlet`s and `BaseFederationServlet`s register their handlers
with `HttpServer.register_paths` / `JsonResource.register_paths`. Update
`JsonResource` to respect the `@cancellable` flag on handlers registered
in this way.
Although `ReplicationEndpoint` also registers itself using
`register_paths`, it does not pass the handler method that would have the
`@cancellable` flag directly, and so needs separate handling.
Signed-off-by: Sean Quah <seanq@element.io>
`DirectServeHtmlResource` and `DirectServeJsonResource` both inherit
from `_AsyncResource`. These classes expect to be subclassed with
`_async_render_*` methods.
This commit has no effect on `JsonResource`, despite inheriting from
`_AsyncResource`. `JsonResource` has its own `_async_render` override
which will need to be updated separately.
Signed-off-by: Sean Quah <seanq@element.io>
Refactor how the `EventContext` class works, with the intention of reducing the amount of state we fetch from the DB during event processing.
The idea here is to get rid of the cached `current_state_ids` and `prev_state_ids` that live in the `EventContext`, and instead defer straight to the database (and its caching).
One change that may have a noticeable effect is that we now no longer prefill the `get_current_state_ids` cache on a state change. However, that query is relatively light, since its just a case of reading a table from the DB (unlike fetching state at an event which is more heavyweight). For deployments with workers this cache isn't even used.
Part of #12684
All async request processing goes through `_AsyncResource`, so this is
the only place where a `Deferred` needs to be captured for cancellation.
Unfortunately, the same isn't true for determining whether a request
can be cancelled. Each of `RestServlet`, `BaseFederationServlet`,
`DirectServe{Html,Json}Resource` and `ReplicationEndpoint` have
different wrappers around the method doing the request handling and they
all need to be handled separately.
Signed-off-by: Sean Quah <seanq@element.io>
Also expose the `SynapseRequest` from `FakeChannel` in tests, so that
we can call `Request.connectionLost` to simulate a client disconnecting.
Signed-off-by: Sean Quah <seanq@element.io>
* Move `_condition_checker` into `PushRuleEvaluatorForEvent`.
* Move the condition cache into `PushRuleEvaluatorForEvent`.
* Improve docstrings.
* Inline a method which is only called once.
There's no guarantee that module callbacks will handle cancellation
appropriately. Protect module callbacks with read semantics from
cancellation and avoid swallowing `CancelledError`s that arise.
Other module callbacks, such as the `on_*` callbacks, are presumed to
live on code paths that involve writes and aren't cancellation-friendly.
These module callbacks have been left alone.
Signed-off-by: Sean Quah <seanq@element.io>
* Move `pympler` back into the `all` extras
Undoes a change I made in #12381. I can't fully remember my reasoning,
but this changed the contents of the debian packages in a backwards
incompatible way. We're not aware of anyone who's been bitten by this,
but we still want to fix it.
To the reviewer: please be convinced that the debian packages will still
contain pympler after this change.
* Debian changelog entry to keep the linter happy
Update the "Build docker images" GitHub Actions workflow to use
`docker/metadata-action` to generate docker image tags, instead of a
custom shell script.
Signed-off-by: Henry <97804910+henryclw@users.noreply.github.com>
Fixes a regression from 8b309adb43 (#11660)
and b65acead42 (#11752) where events which
themselves were an edit or an annotation could have bundled aggregations calculated,
which is not allowed.
* Add mau_appservice_trial_days
* Add a test
* Tweaks
* changelog
* Ensure we sync after the delay
* Fix types
* Add config statement
* Fix test
* Reinstate logging that got removed
* Fix feature name
getClientIP was deprecated in Twisted 18.4.0, which also added
getClientAddress. The Synapse minimum version for Twisted is
currently 18.9.0, so all supported versions have the new API.
* Changes hidden read receipts to be a separate receipt type
(instead of a field on `m.read`).
* Updates the `/receipts` endpoint to accept `m.fully_read`.
* `m.login.jwt`, which was never specced and has been deprecated
since Synapse 1.16.0. (`org.matrix.login.jwt` can be used instead.)
* `uk.half-shot.msc2778.login.application_service`, which was
stabilized as part of the Matrix spec v1.2 release.
The `latest_event` field of the bundled aggregations for `m.thread` relations
did not include bundled aggregations itself. This resulted in clients needing to
immediately request the event from the server (and thus making it useless that
the latest event itself was serialized instead of just including an event ID).
I've seen a few errors which can only plausibly be explained by the calculated
event id for an event being different from the ID of the event in the
database. It should be cheap to check this, so let's do so and raise an
exception.
Check we're on the right branch before tagging, and on the right tag before uploading
* Abort if we're on the wrong branch
* Check we have the right tag checked out
* Clarify that `publish` only releases to GitHub
This works by taking a row level lock on the `rooms` table at the start of both transactions, ensuring that they don't run at the same time. In the event persistence transaction we also check that there is an entry still in the `rooms` table.
I can't figure out how to do this in SQLite. I was just going to lock the table, but it seems that we don't support that in SQLite either, so I'm *really* confused as to how we maintain integrity in SQLite when using `lock_table`....
This was originally added when we first added a `MemoryHandler` to the default
log config back in https://github.com/matrix-org/synapse/pull/8040, to ensure
that we didn't explode with an infinite loop if there was an error formatting
the logs.
Since then, we made additional improvements to logging which make this
workaround redundant. In particular:
* we no longer attempt to log un-UTF8-decodable byte sequences, which were the
most likely cause of an error in the first place.
* https://github.com/matrix-org/synapse/pull/8268 ensures that in the unlikely
case that there *is* an error, it won't cause an infinite loop.
* Allow unused ignores in "bleeding edge" CI
Where "bleeding edge" means the Twisted Trunk and Latest Deps jobs.
Follow up from #12531.
Resolves#12574.
* Use `--extras all` in latest deps mypy CI
Twisted trunk job already does this.
Missed in #12531.
* changelog
The status code of requests must always be set, regardless of client
disconnection, otherwise they will always be logged as 200!.
Broken for `respond_with_json` in
f48792eec4.
Broken for `respond_with_json_bytes` in
3e58ce72b4.
Broken for `respond_with_html_bytes` in
ea26e9a98b.
Signed-off-by: Sean Quah <seanq@element.io>
When configuring the return values of mocks, prefer awaitables from
`make_awaitable` over `defer.succeed`. `Deferred`s are only awaitable
once, so it is inappropriate for a mock to return the same `Deferred`
multiple times.
Also update `run_in_background` to support functions that return
arbitrary awaitables.
Signed-off-by: Sean Quah <seanq@element.io>
Over time we've begun to use newer versions of mypy, typeshed, stub
packages---and of course we've improved our own annotations. This makes
some type ignore comments no longer necessary. I have removed them.
There was one exception: a module that imports `select.epoll`. The
ignore is redundant on Linux, but I've kept it ignored for those of us
who work on the source tree using not-Linux. (#11771)
I'm more interested in the config line which enforces this. I want
unused ignores to be reported, because I think it's useful feedback when
annotating to know when you've fixed a problem you had to previously
ignore.
* Installing extras before typechecking
Lacking an easy way to install all extras generically, let's bite the bullet and
make install the hand-maintained `all` extra before typechecking.
Now that https://github.com/matrix-org/backend-meta/pull/6 is merged to
the release/v1 branch.
==============================
This release candidate fixes bugs related to Synapse 1.58.0rc1's logic for handling device list updates.
Bugfixes
--------
- Fix a bug introduced in Synapse 1.58.0rc1 where the main process could consume excessive amounts of CPU and memory while handling sentry logging failures. ([\#12554](https://github.com/matrix-org/synapse/issues/12554))
- Fix a bug introduced in Synapse 1.58.0rc1 where opentracing contexts were not correctly sent to whitelisted remote servers with device lists updates. ([\#12555](https://github.com/matrix-org/synapse/issues/12555))
Internal Changes
----------------
- Reduce unnecessary work when handling remote device list updates. ([\#12557](https://github.com/matrix-org/synapse/issues/12557))
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE1508oLYUKainYFJakD7OEIo53t0FAmJoINsACgkQkD7OEIo5
3t0ETA//VANpmz0ykLeBA0XRW0eifQkHnQyBoZQtXL+IUmV9FZcX2q/akuQ5toJL
p+AAJ6bKfYSwxyxgw3bJhftOVsIpG546auqVHvRXznlHk9vXKnuYevOz1ifnYSih
j58fYTWnYZYv04uADbTX4fnJjiPDhiRYCZF6RiB5u1HGl1gr+Q1kJI7CL289/d1V
MgBsJfzjHw+M7Vl2doddINnG65oPzrntMysv+PAma+WFxjQvLQoiaui96kaDM/2W
XdFQh4k3OeyhBHKVk3+KrV9zPEh16JuymKDWoAnaMeIBBPwtngZp8tkEr4rSW8gP
fOGDOMfqu896WqyT20fVhWzrsi24UjCw2TxY1C2UdCOoNqVJvMMMZ/aB129+8S/5
65FOI77Oqmn2KGQ5+75hun3a9UcsRD+9iitg2wajxlle5yk6ZFi095aZVf94ls8d
uSWakqvFr6PAY8TnvthQNsAj00LJVqKKH3RrNw7pDOX7QWE1qbcbPHFrtCMHC5Bz
Z1/LDqNCxhwruPEghmg+0ToRama5uYdeCMzqECZ8GmzhlouezMaFK6bFH1SGwNbo
Idx60qwCXK3vHka3MVSqLg15gBGj1dQEJt8q/FVrYDemEFlNshPydJTbtbIOfj/R
VEZa12gbtqFenPMQyDuzbFqgwirbeYQQHIRltQB3yDeggg8j4ng=
=FAvF
-----END PGP SIGNATURE-----
gpgsig -----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE1508oLYUKainYFJakD7OEIo53t0FAmJoJswACgkQkD7OEIo5
3t3u0RAApTbw1tPIFXN9aQWVzBfaYRlw4zFKJlAl4JF6J7QT5qDny9NnhLTKqUPh
bMj+IfcPN2nlUM6U6F9Fnu0HQXP9/hyVNLIauvqCnRa2GCb6F7fymSTJSsyUGSZW
0i3FXaR+OaIOqhjphGYx6oaVtGpgg7cKsp8VhVMTZrt0/sjFXxDV6oeaKfe2OpIE
6D4wYh13zS9CzzoNoMUkkqwUr2e+DbsCp5BkjbU8F2abRRpC14QSF4/qH0dfAQ4i
A6Lj02Gzv/AcE5e6e3bRYQkt4L8+/di+aY3vbj4jsCqJYFTP4yvdc6iScFALyxXn
QNGTZlctuNbR/v6WVQbiFtSyF1GnLos1+7jpEBpGRUk30H68zOwsi+qLZh9r98mB
vQ0G2HA/klzdnIvq9DxWLL0PxQZKocdu5CT2ujV+6J7vkKAXmC/0rkDRWdi2qooU
uRb3CbAHo4A6GnoqsrhY1i1JuUNbX7Nu2BZsLaiIdp2JTt6OzS/0zfUgCZzPw0u0
1586SlmiSdtXNMOlgpHPVtD71CCq/48tbccsomI1hk5dmiqlZX4hqsHSQY97k422
NV9LNpPo0K5l2Qmegera94Qz0FzCv4TCSDEAXn//lHCZ6eAVARk6Wym0UCOXToqw
Xg1igJEkGOkIcwXj6GpG+r8yMdYD52xG6Ud8ZbYTX1glDfVWN4U=
=YYmi
-----END PGP SIGNATURE-----
Merge tag 'v1.58.0rc2' into develop
Synapse 1.58.0rc2 (2022-04-26)
==============================
This release candidate fixes bugs related to Synapse 1.58.0rc1's logic for handling device list updates.
Bugfixes
--------
- Fix a bug introduced in Synapse 1.58.0rc1 where the main process could consume excessive amounts of CPU and memory while handling sentry logging failures. ([\#12554](https://github.com/matrix-org/synapse/issues/12554))
- Fix a bug introduced in Synapse 1.58.0rc1 where opentracing contexts were not correctly sent to whitelisted remote servers with device lists updates. ([\#12555](https://github.com/matrix-org/synapse/issues/12555))
Internal Changes
----------------
- Reduce unnecessary work when handling remote device list updates. ([\#12557](https://github.com/matrix-org/synapse/issues/12557))
Try to avoid an OOM by checking fewer extremities.
Generally this is a big rewrite of _maybe_backfill, to try and fix some of the TODOs and other problems in it. It's best reviewed commit-by-commit.
Multiple calls to `EventsWorkerStore._get_events_from_cache_or_db` can
reuse the same database fetch, which is initiated by the first call.
Ensure that cancelling the first call doesn't cancel the other calls
sharing the same database fetch.
Signed-off-by: Sean Quah <seanq@element.io>
This will mainly be useful when dealing with module callbacks, which are
all typed as returning `Awaitable`s instead of coroutines or
`Deferred`s.
Signed-off-by: Sean Quah <seanq@element.io>
When we join a room via the faster-joins mechanism, we end up with "partial
state" at some points on the event DAG. Many parts of the codebase need to
wait for the full state to load. So, we implement a mechanism to keep track of
which events have partial state, and wait for them to be fully-populated.
MSC2314 has now been closed, so we're backing out its implementation, which
originally happened in #6176.
Unfortunately it's not a direct revert, as that PR mixed in a bunch of
unrelated changes to tests etc.
* Use `poetry` to build venv in debian packages
Co-authored-by: Dan Callahan <danc@element.io>
Co-authored-by: Shay <hillerys@element.io>
* Changelog
* Only pull in from requirements.txt
Addresses the same problem as #12439.
* Include `test` and `all` extras
`poetry export` helpfully silently ignores an unknown extra
Haven't seen this before because it's the only place we export `all` and
`test`. I could have __sworm__ that the syntax `--extra "all test"`
worked for `poetry install`...
* Clean up requirements file on subsequence builds
* Fix shell syntax
Co-authored-by: Dan Callahan <danc@element.io>
Co-authored-by: Shay <hillerys@element.io>
When we run a worker-mode synapse under docker, everything gets logged to stdout. Currently, output from the workers is tacked with a worker name, for example:
```
2022-04-13 15:27:56,810 - worker:frontend_proxy1 - synapse.util.caches.lrucache - 154 - INFO - LruCache._expire_old_entries-0 - Dropped 0 items from caches
```
- note `worker:frontend_proxy1`. No such tag is applied to log lines from the master, which makes somewhat confusing reading.
To fix this, we generate a dedicated log config file for the master in the same way that we do for the workers, and use that.
In trying to use the MSC3026 busy presence status, the user's status
would be set back to 'online' next time they synced. This change makes
it so that syncing does not affect a user's presence status if it
is currently set to 'busy': it must be removed through the presence
API.
The MSC defers to implementations on the behaviour of busy presence,
so this ought to remain compatible with the MSC.
* Run "main" trial tests under poetry
Olddeps and twisted trunk tests are handled in separate PRs.
The PyPy config is a best-effort only; it's completely untested.
Pulled out from #12337.
* Changelog
This was missed when initially stabilising room version 8 and was
left in as a compatibility shim. Most homeservers have upgraded
to a version which expects the proper field name, and the failure
mode is reasonable (a user on an older server may have to attempt
joining the room twice with an obscure error message the first time).
We work through all the events with partial state, updating the state at each
of them. Once it's done, we recalculate the state for the whole room, and then
mark the room as having complete state.
* Add some type hints to datastore
* newsfile
* change `Collection` to `List`
* refactor return type of `select_users_txn`
* correct type hint in `stream.py`
* Remove `Optional` in `select_users_txn`
* remove not needed return type in `__init__`
* Revert change in `get_stream_id_for_event_txn`
* Remove import from `Literal`
* Specify `tls` extra for Twisted dependency.
It was already pulled in for us by `treq`, but we should be explicit
that we do use the `tls` functionality of Twisted directly.
* Mark `idna` as dev-dependency
This doesn't actually change anything, as `Twisted[tls]` will put it in
as a main dependency anyway.
The requirements file generated by `poetry export` isn't correctly processed by `pip install -r requirements.txt`. It contains twisted and treq, both pinned to 22.2.0.
When `pip` installs treq, it notices that `Twisted[tls]` is required. It then tries to acquire the latest twisted release, only to fail (because this hash isn't listed in the requirements file).From e.g. https://github.com/matrix-org/synapse/runs/5977154990?check_suite_focus=true
> ```
> #15 9.204 Collecting Twisted[tls]>=18.7.0
> #15 9.205 ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
> #15 9.205 Twisted[tls]>=18.7.0 from 38622ff95b/Twisted-22.4.0-py3-none-any.whl (sha256)=f9f7a91f94932477a9fc3b169d57f54f96c6e74a23d78d9ce54039a7f48928a2 (from treq==22.2.0->-r /synapse/requirements.txt (line 724))
> #15 ERROR: executor failed running [/bin/sh -c pip install --prefix="/install" --no-warn-script-location -r /synapse/requirements.txt]: exit code: 1
> ```
The underlying pip issue is https://github.com/pypa/pip/issues/9644. A comment notes that one can avoid this behaviour with by `pip install`ing with the `--no-deps` flag. Let us do so.
(At first glance, the problem looks like https://github.com/python-poetry/poetry/issues/5311, but that was a bug in `poetry install`; this is `poetry export`, whose behaviour is fine AFAICS).
Consider the requester's ignored users when calculating the
bundled aggregations.
See #12285 / 4df10d3214
for corresponding changes for the `/relations` endpoint.
Of note:
* No untyped defs in `register_new_matrix_user`
This one might be contraversial. `request_registration` has three
dependency-injection arguments used for testing. I'm removing the
injection of the `requests` module and using `unitest.mock.patch` in the
test cases instead.
Doing `reveal_type(requests)` and `reveal_type(requests.get)` before the
change:
```
synapse/_scripts/register_new_matrix_user.py:45: note: Revealed type is "Any"
synapse/_scripts/register_new_matrix_user.py:46: note: Revealed type is "Any"
```
And after:
```
synapse/_scripts/register_new_matrix_user.py:44: note: Revealed type is "types.ModuleType"
synapse/_scripts/register_new_matrix_user.py:45: note: Revealed type is "def (url: Union[builtins.str, builtins.bytes], params: Union[Union[_typeshed.SupportsItems[Union[builtins.str, builtins.bytes, builtins.int, builtins.float], Union[builtins.str, builtins.bytes, builtins.int, builtins.float, typing.Iterable[Union[builtins.str, builtins.bytes, builtins.int, builtins.float]], None]], Tuple[Union[builtins.str, builtins.bytes, builtins.int, builtins.float], Union[builtins.str, builtins.bytes, builtins.int, builtins.float, typing.Iterable[Union[builtins.str, builtins.bytes, builtins.int, builtins.float]], None]], typing.Iterable[Tuple[Union[builtins.str, builtins.bytes, builtins.int, builtins.float], Union[builtins.str, builtins.bytes, builtins.int, builtins.float, typing.Iterable[Union[builtins.str, builtins.bytes, builtins.int, builtins.float]], None]]], builtins.str, builtins.bytes], None] =, data: Union[Any, None] =, headers: Union[Any, None] =, cookies: Union[Any, None] =, files: Union[Any, None] =, auth: Union[Any, None] =, timeout: Union[Any, None] =, allow_redirects: builtins.bool =, proxies: Union[Any, None] =, hooks: Union[Any, None] =, stream: Union[Any, None] =, verify: Union[Any, None] =, cert: Union[Any, None] =, json: Union[Any, None] =) -> requests.models.Response"
```
* Drive-by comment in `synapse.storage.types`
* No untyped defs in `synapse_port_db`
This was by far the most painful. I'm happy to break this up into
smaller pieces for review if it's not managable as-is.
Fixesmatrix-org/complement#330 (or it will, once we remove the old files).
It's not quite a lift-and-shift: I've also taken the opportunity to get rid of the custom CA that we used to use to sign the TLS certs, which has been superceded by the CA exposed by Complement.
* Pull out query param types to `synapse.http.types`
* Use QueryParams everywhere
* Simplify `encode_query_args`
* Add annotation which would have caught #12410
Principally, `prometheus_client.REGISTRY.register` now requires its argument to
extend `prometheus_client.Collector`.
Additionally, `Gauge.set` is now annotated so that passing `Optional[int]`
causes an error.
Just after a task acquires a contended `Linearizer` lock, it sleeps.
If the task is cancelled during this sleep, we need to release the lock.
Signed-off-by: Sean Quah <seanq@element.io>
`StreamToken.from_string` and `RoomStreamToken.parse` are both async
methods that could be cancelled. These methods must not replace
`CancelledError`s with `SynapseError`s.
Signed-off-by: Sean Quah <seanq@element.io>
Refactor and convert `Linearizer` to async. This makes a `Linearizer`
cancellation bug easier to fix.
Also refactor to use an async context manager, which eliminates an
unlikely footgun where code that doesn't immediately use the context
manager could forget to release the lock.
Signed-off-by: Sean Quah <seanq@element.io>
This is a first step in dealing with #7721.
The idea is basically that rather than calculating the full set of users a device list update needs to be sent to up front, we instead simply record the rooms the user was in at the time of the change. This will allow a few things:
1. we can defer calculating the set of remote servers that need to be poked about the change; and
2. during `/sync` and `/keys/changes` we can avoid also avoid calculating users who share rooms with other users, and instead just look at the rooms that have changed.
However, care needs to be taken to correctly handle server downgrades. As such this PR writes to both `device_lists_changes_in_room` and the `device_lists_outbound_pokes` table synchronously. In a future release we can then bump the database schema compat version to `69` and then we can assume that the new `device_lists_changes_in_room` exists and is handled.
There is a temporary option to disable writing to `device_lists_outbound_pokes` synchronously, allowing us to test the new code path does work (and by implication upgrading to a future release and downgrading to this one will work correctly).
Note: Ideally we'd do the calculation of room to servers on a worker (e.g. the background worker), but currently only master can write to the `device_list_outbound_pokes` table.
There are a bunch of places we call get_success on an immediate value, which is unnecessary. Let's rip them out, and remove the redundant functionality in get_success and friends.
Switching to a sequence means there's no need to track `last_txn` on the
AS state table to generate new TXN IDs. This also means that there is
no longer contention between the AS scheduler and AS handler on updates
to the `application_services_state` table, which will prevent serialization
errors during the complete AS txn transaction.
It seems like calling `_get_state_group_for_events` for an event where the
state is unknown is an error. Accordingly, let's raise an exception rather than
silently returning an empty result.
If we're missing most of the events in the room state, then we may as well call the /state endpoint, instead of individually requesting each and every event.
The intention here is to avoid doing state lookups for outliers in
`/_matrix/federation/v1/event`. Unfortunately that's expanded into something of
a rewrite of `filter_events_for_server`, which ended up trying to do that
operation in a couple of places.
The PaginationChunk class attempted to bundle some properties
together, but really just caused callers to jump through hoops and
hid implementation details.
This endpoint was removed from MSC2675 before it was approved.
It is currently unspecified (even in any MSCs) and therefore subject to
removal. It is not implemented by any known clients.
This also changes the bundled aggregation format for `m.annotation`,
which previously included pagination tokens for the `/aggregations`
endpoint, which are no longer useful.
