If presence is disabled after having been enabled, the presence states
in the database (and hence on clients) are frozen at whatever they were
when presence was last enabled: nothing writes to the presence stream
any more and /sync omits the presence section entirely, so clients show
the old presence states forever.
Fix this in two parts:
1. At startup, if presence is disabled but the database still contains
non-offline presence states, the presence writer sends out one final
round of updates marking those users as offline.
2. /sync no longer unconditionally omits presence when presence is
disabled: incremental syncs whose since token is behind the presence
stream still get the straggling updates. As the stream doesn't advance
while presence is disabled, clients catch up once and the check then
short-circuits to a token comparison.
Remote servers already handle this themselves by timing out our users
([`FEDERATION_TIMEOUT`](https://github.com/element-hq/synapse/blob/4d8905a15a417ed0054ec2533d243932d890bbbd/synapse/handlers/presence.py#L194-L198)),
so no federation changes are needed.
Note that this only fixes the issue if presence is fully disabled. If
set to `untracked` we still have the same issue, however since modules
would still write to presence we can't just clobber everything like we
do in this patch.
This does two things, first it adds a config flag to ignore rooms for
the purposes of presence routing.
Secondly, it changes the caching behaviour to try and improve the cache
hit ratio. Previously, the size of the `do_users_share_a_room` cache
(which stores pairs of users) needs to `O(n²)` for the number of online
users, which is infeasible for large servers.
Instead, we call `get_users_in_room` for both the syncing and updated
users. This sounds more expensive, but a) we will already have cached
the syncing user's rooms, and b) we will only calculate the updated
user's rooms once (rather than once per syncing user).
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sync workers proxied a full `ReplicationPresenceSetState` call to the
presence writer on every sync request with `affect_presence=True` (via
`user_syncing`), and a `ReplicationBumpPresenceActiveTime` call on every
user action, even though the writer's presence timers only need feeding
every `SYNC_ONLINE_TIMEOUT` / `LAST_ACTIVE_GRANULARITY`. On busy clients
this amounts to tens of no-op replication calls per user per minute, and
the resulting per-update work is the dominant CPU cost on saturated
presence writers.
Track the last relayed `(state, timestamp)` per `(user, device)` on the
worker and suppress unchanged sync-driven repeats within a 25s relay
interval - deliberately below `SYNC_ONLINE_TIMEOUT` (30s) so the
writer's device `last_sync_ts`/`last_active_ts` timers stay fed and
users neither flap offline nor bounce `currently_active`. Genuine state
changes are relayed immediately, explicit (non-sync) set_state calls
always go through and reset the throttle, bumps that might un-idle a
device bypass it, and entries are evicted when a `USER_SYNC` stop is
sent so reconnecting devices are relayed afresh.
This gives the writer-CPU benefit that deployments currently obtain by
tightening `rc_presence` to ~1/29s, without dropping real presence
transitions.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Quentin Gliech <quenting@element.io>
Adds `last_active_granularity`, `sync_online_timeout` and `idle_timeout`
options to the `presence` config section, controlling the previously
hard-coded `LAST_ACTIVE_GRANULARITY`, `SYNC_ONLINE_TIMEOUT` and
`IDLE_TIMER` constants (which remain as the defaults).
This is mainly useful on deployments that ratelimit how often syncs can
affect presence (`rc_presence`): the sync timeout must exceed the
ratelimit interval or users flap offline between syncs, so tuning down
presence traffic currently requires squeezing under the fixed 30s limit.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
`test_lock_contention` is a performance-regression canary (#16840): the
pathological behaviour it guards against spent ~30s spinning the CPU, vs
~0.5s when healthy. The 5s wall-clock alarm it used was calibrated on
SQLite, but against PostgreSQL a healthy run already takes 3-4s of
wall-clock time (500 sequential acquire/release cycles, each a real
database round-trip), so any CI load pushed it over the limit.
Add a `cpu_time` mode to `tests/utils.py`'s test_timeout, implemented
with
[`setitimer(ITIMER_PROF)`](https://docs.python.org/3/library/signal.html#signal.setitimer),
which budgets process CPU time instead of wall-clock time. Time spent
blocked on the database or lost to a loaded CI runner no longer counts,
while a regression to CPU-spinning still trips the alarm mid-spin. A
healthy run costs <1s of CPU on either database engine; the budget is
10s.
This also subsumes the RISC-V wall-clock carve-out from #18430, which is
removed.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This means you can use `get_success(...)` anywhere regardless
of what kind of work needs to be done.
Spawning from adding some more async Rust things in
https://github.com/element-hq/synapse/pull/19846 and wanting something
more standard instead of the custom `till_deferred_has_result(...)` that
has crept in to a few files.
Alternative to https://github.com/element-hq/synapse/pull/19867 spurred
on by [this
comment](https://github.com/element-hq/synapse/pull/19867#discussion_r3441774685)
from @erikjohnston
### How does this work?
Previously, `get_success(...)` just ran in a hot-loop advancing the
Twisted reactor clock which didn't give any time for other threads to do
some work or acquire the GIL if necessary (whenever there is a hand-off
from Rust to Python, we need the GIL).
Now, `get_success(...)` loops until we see a result (until we hit the
~0.1s real-time timeout). In the loop, we call
[`time.sleep(0)`](https://docs.python.org/3/library/time.html#time.sleep)
which will "Suspend execution of the calling thread [...]" (CPU and GIL)
to allow other threads to do some work. Then like before, we advance the
Twisted reactor clock to run any scheduled callbacks which includes
anything the other threads may have scheduled.
### Does this slow down the entire test suite?
Seems just as fast as before. There is minutes variance in what we had
before and after but both are within the same range of each other.
(see PR for actual before/after timings)
I.e. `default_config("test")` and `default_config("test", False)` are so
opaque and hard to connect the dots with.
Spawning from trying to figure out what the `server_name` is set as for
our `HomeserverTestCase` in order to reference it in
https://github.com/element-hq/synapse/pull/19848#discussion_r3397455309
Follow on from https://github.com/element-hq/synapse/pull/19701.
Unfortunately serde has a bug when using `#[serde(flatten)]` with
`arbitrary-precision` feature when handling integers that fit in a i128
when doing `serde_json::from_value`. See
https://github.com/serde-rs/serde/issues/2230.
The `depythonize` hits the same issue. To fix this we make it so we only
parse events from strings and not values.
This is based on https://github.com/element-hq/synapse/pull/18416, which
got reverted (#19614) due to it incorrectly rejecting to-device messages
to users with many devices (and thus breaking message sending).
Fix https://github.com/element-hq/synapse/issues/17035
A to-device message content looks like:
```jsonc
{
"@user:domain": {"device1": {...}, "device2": {...}},
...
}
```
The previous PR would split up into multiple EDUs, each with a subset of
the users. However, if one user's entry was too large it would not
further split it up and then error out.
The main change in this PR is to allow splitting up a single user into
multiple EDUs.
Other changes:
1. Rename to `SOFT_MAX_EDU_SIZE` to indicate that we sometimes send EDUs
with larger size than that, and its more a target than a hard limit.
2. Check early if any to-device message (to a specific device) is too
large to send, even if we're not going to send it over federation. This
ensures that we catch issues where clients try to send too large
to-device.
This still means that if a client send a large individual to-device
message it will fail, but I don't believe we ever send such large
to-device messages (normally they're in the range of a few KB).
---
I ended up changing the implementation a bunch to make it easy to reuse
the code to split up dictionaries. Instead of repeatedly splitting up
the EDU until each bit fits into the size, we instead record the size of
each entry in the dict and instead split up based on cumulative size.
This means we call `encode_canonical_json` on each entry rather than
once on the entire struct, but its not significantly slower to do so.
--
cc @MatMaul @MadLittleMods
---------
Co-authored-by: Mathieu Velten <matmaul@gmail.com>
Co-authored-by: mcalinghee <mcalinghee.dev@gmail.com>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Ports the event class to Rust.
The main difference here are:
1. There is now a single event class
2. We now validate a lot more at event construction time than we
previously did (we basically checked nothing before). This required some
changes to the tests, including
https://github.com/matrix-org/sytest/pull/1423
Reviewable commit-by-commit.
### Overview of Event Rust structure
The format of the event struct in Rust is quite different than that in
Python.
The top-level looks like:
```rust
pub struct Event {
/// The parsed event JSON.
fields: FormattedEvent,
/// The event ID. For format v1 this is read directly from the JSON;
/// for v2+ it is computed from the canonical-JSON hash at
/// construction time and cached here.
event_id: Arc<str>,
/// Synapse-internal per-event state that lives outside the federated
/// JSON (e.g. outlier flag, soft-failure, stream positions).
#[pyo3(get)]
internal_metadata: EventInternalMetadata,
/// The room version this event was parsed for.
#[pyo3(get)]
room_version: &'static RoomVersion,
/// `None` for accepted events; otherwise a short reason set by auth
/// when the event was rejected.
rejected_reason: Option<Box<str>>,
}
```
which includes the actual parsed event in `FormattedEvent`, plus the
rest of the event metadata.
```rust
pub struct FormattedEvent<E = Arc<EventFormatEnum>> {
#[serde(default)]
pub signatures: Signatures,
#[serde(default)]
pub unsigned: Unsigned,
#[serde(flatten)]
pub specific_fields: E,
#[serde(flatten)]
pub common_fields: Arc<EventCommonFields>,
}
```
The struct is further split into the common fields, format specific
fields, plus the signatures and unsigned. We split out the signature and
unsigned fields as they are mutable, so when we clone the event we can
still share the common and specific fields and only copy signature and
unsigned.
The `specific_fields` are the fields that depend on the format version.
They can either be a specific format (e.g. `E = EventFormatV1`) or a
type-erased enum `EventFormatEnum` that is across all room versions:
```rust
pub enum EventFormatEnum {
V1(EventFormatV1),
V2V3(EventFormatV2V3),
V4(EventFormatV4),
VMSC4242(EventFormatVMSC4242),
}
```
For example:
```rust
/// Shared flat-list encoding of `auth_events` and `prev_events`, reused
/// by every format from v2/v3 onwards.
#[derive(Serialize, Deserialize)]
pub struct SimpleAuthPrevEvents {
pub auth_events: Vec<String>,
pub prev_events: Vec<String>,
}
/// Version-specific fields for room versions 3-10.
#[derive(Serialize, Deserialize)]
pub struct EventFormatV2V3 {
pub room_id: Box<str>,
#[serde(flatten)]
pub auth_prev_events: SimpleAuthPrevEvents,
}
```
### Dev notes
As discussed in
[`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$3gTjDO440GbAz57cXcCawwiyFLiD0crrarvS1uhzKOY?via=jki.re&via=element.io&via=matrix.org)
---------
Co-authored-by: Eric Eastwood <erice@element.io>
When we port the `Event` class to Rust, the constructor will check for
the existence of required fields. To support that, we tidy up the test
code where we construct fake events to add all the required fields.
There should be no behavioural changes.
Review commit-by-commit.
This is another stepping stone in porting the event class fully to Rust.
The new `Signatures` class is relatively simple, as we actually don't
interact with it that much in the code. It does *not* implement
`Mapping` or `MutableMapping` as that takes quite a lot of effort that
we don't need, even though it would be more ergonomic.
Fixes the symptoms of https://github.com/element-hq/synapse/issues/19315
/ https://github.com/element-hq/synapse/issues/19588 but not the
underlying reason causing the number to grow so large in the first
place.
```
ValueError: Exceeds the limit (4300 digits) for integer string conversion; use sys.set_int_max_str_digits() to increase the limit
```
Copied from the original pull request on [Famedly's Synapse
repo](https://github.com/famedly/synapse/pull/221) (with some edits):
Basing the time interval around a 5 seconds leaves a big window of
waiting especially as this window is doubled each retry, when another
worker could be making progress but can not.
Right now, the retry interval in seconds looks like `[0.2, 5, 10, 20,
40, 80, 160, 320, (continues to double)]` after which logging should
start about excessive times and (relatively quickly) end up with an
extremely large retry interval with an unrealistic expectation past the
heat death of the universe. 1 year in seconds = 31,536,000.
With this change, retry intervals in seconds should look more like:
```
[
0.2,
0.4,
0.8,
1.6,
3.2,
6.4,
12.8,
25.6,
51.2,
60, < never goes higher than this
]
```
Logging about excessive wait times will start at 10 minutes.
<details>
<summary>Previous breakdown when we were using 15 minutes</summary>
```
[
0.2,
0.4,
0.8,
1.6,
3.2,
6.4,
12.8,
25.6,
51.2,
102.4, # 1.7 minutes
204.8, # 3.41 minutes
409.6, # 6.83 minutes
819.2, # 13.65 minutes < logging about excessive times will start here, 13th iteration
900, # 15 minutes < never goes higher than this
]
```
</details>
Further suggested work in this area could be to define the cap, the
retry interval starting point and the multiplier depending on how
frequently this lock should be checked. See data below for reasons why.
Increasing the jitter range may also be a good idea
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Currently synapse returns `M_FORBIDDEN` when trying to use the account
deactivation API, if the server admin disabled displayname changes. This
is undesirable, since it prevents GDPR erasure without admin
interaction. The admin API seems to work fine though. This also only
seems to affect the deactivate API, when the erase flag is true.
Relevant endpoint:
https://spec.matrix.org/latest/client-server-api/#post_matrixclientv3accountdeactivate
This change only removes the checked for condition that the displayname
and profile avatar are allowed to be changed per the configuration
setting. If a user is deleting themselves, why is that denied?
There did not seem to be a basic test for this endpoint that checks the
`erase` usage, so that was added as well as checking the above mentioned
behavior.
Fixes#13043
The usages of the table mostly already correctly handled if we don't
have old entries, as that was needed when we first added the table.
I arbitrarily set the prune time to 30 days. The only use for old
entries is for sync streams that haven't synced since then, and we
should very rarely see sync streams that haven't been used in 30 days.
Reviewable commit-by-commit.
---------
Co-authored-by: Olivier 'reivilibre' <oliverw@element.io>
Co-authored-by: Olivier 'reivilibre' <olivier@librepush.net>
When we return events to clients we need to annotate them with the
membership of the user at the time of the event, in the `unsigned`
section. We already check the membership at the event during the
visibility checks, and so we annotate events there. However, since this
a per-user field we end up having to clone the event in question.
Instead, let's add a `FilteredEvent` class that is returned by the
visibility checks, which allows returning the membership without editing
the event. This has three benefits:
1. Avoids the clones of the event.
2. Allows us to statically check that we have filtered events before
returning them to clients.
3. We no longer edit `unsigned` data after event deserialization, this
makes it easier to port the event class to Rust.
The last benefit is why we're doing this *now*, however IMV it shouldn't
affect whether we want this change or not.
Reviewable commit-by-commit
---------
Co-authored-by: Olivier 'reivilibre' <oliverw@element.io>
This is to make it easier to port to Rust, as well as making things
conceptually simpler.
Two changes:
1. Remove the `__getitem__` interface on events
2. Remove `.user_id` as an alias of `.sender`.
Reverts element-hq/synapse#18416
Unfortunately, this causes failures on `/sendToDevice` endpoint in
normal circumstances. If a single user has, say, a hundred devices then
we easily go over the limit. This blocks message sending entirely in
encrypted rooms.
cc @MadLittleMods @MatMaul
Fixes: #8088
Previously we would perform OIDC discovery on startup,
which involves making HTTP requests to the identity provider(s).
If that took a long time, we would block startup.
If that failed, we would crash startup.
This commit:
- makes the loading happen in the background on startup
- makes an error in the 'preload' non-fatal (though it logs at CRITICAL
for visibility)
- adds a templated error page to show on failed redirects (for
unavailable providers), as otherwise you get a JSON response in your
navigator.
- This involves introducing 2 new exception types to mark other
exceptions and keep the error handling fine-grained.
The machinery was already there to load-on-demand the discovery config,
so when the identity provider
comes back up, the discovery is reattempted and login can succeed.
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Fixes https://github.com/element-hq/synapse/issues/19494
MSC4284 policy servers
This:
* removes the old `/check` (recommendation) support because it's from an
older design. Policy servers should have updated to `/sign` by now. We
also remove optionality around the policy server's public key because it
was only optional to support `/check`.
* supports the stable `m.room.policy` state event and `/sign` endpoints,
falling back to unstable if required. Note the changes between unstable
and stable:
* Stable `/sign` uses errors instead of an empty signatures block to
indicate refusal.
* Stable `m.room.policy` nests the public key in an object with explicit
key algorithm (always ed25519 for now)
* does *not* introduce tests that the above fallback to unstable works.
If it breaks, we're not going to be sad about an early transition. Tests
can be added upon request, though.
* fixes a bug where the policy server was asked to sign policy server
state events (the events were correctly skipped in `is_event_allowed`,
but `ask_policy_server_to_sign_event` didn't do the same).
* fixes a bug where the original event sender's signature can be deleted
if the sending server is the same as the policy server.
* proxies Matrix-shaped errors from the policy server to the
Client-Server API as `SynapseError`s (a new capability of the stable
API).
Membership event handling (from the issue) is expected to be a different
PR due to the size of changes involved (tracked by
https://github.com/element-hq/synapse/issues/19587).
### 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: turt2live <1190097+turt2live@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Fixes#19347
This deprecates MSC2697 which has been closed since May 2024. As per
#19347 this seems to be a thing we can just rip out. The crypto team
have moved onto MSC3814 and are suggesting that developers who rely on
MSC2697 should use MSC3814 instead.
MSC2697 implementation originally introduced by https://github.com/matrix-org/synapse/pull/8380
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>
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.
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.
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](https://github.com/element-hq/synapse/commit/38da9884e70e8e44bde14c67a7a8a9d49a8b87ac).
Instead of fixing the implementation which would just be a copy of our
normal `Clock`, we can just remove `MockClock`
