more updates and merge in develop#11
Open
jason-famedly wants to merge 91 commits into
Open
Conversation
…f them fails. (element-hq#19842) When building v1.155.0rc1, flaky deb builds combined with fail-fast behaviour meant that I had to press the retry button 5 times to get a full set. Without fail-fast, I suspect 1 or 2 retries would have done the job. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> Co-authored-by: Eric Eastwood <erice@element.io>
…ge (element-hq#19849) 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 element-hq#19848 (comment)
…9834) The sytest `After /purge_history users still get pushed for new messages` is flaky. The flakiness exposes a real bug rather than a test-timing issue. Notification counts are stored in two places: `event_push_actions` (one row per unread event) and `event_push_summary` (aggregate counts populated periodically by `_rotate_notifs`, which runs on a 30-second timer). `_purge_history_txn` deletes the purged events' rows from `event_push_actions` but never adjusts `event_push_summary` (only the full-room `purge_room` drops that table). So the result depends on a race: if rotation hasn't fired, counts come live from `event_push_actions`, the purge removes the right rows, and the count is correct. If rotation fires before the purge — more likely under the slower multi-postgres/workers/asyncio CI config — the events get folded into `event_push_summary`, the purge then deletes the underlying `event_push_actions` rows but leaves the summary untouched, and the count comes out inflated. ### Fix Before deleting the rotated rows from `event_push_actions`, decrement `event_push_summary` by the amount attributable to the events being deleted. The decrement mirrors the counting logic in `_rotate_notifs_before_txn`: only rows that were already rotated (`stream_ordering <= event_push_summary_stream_ordering`) and that fall after the summary's receipt are subtracted, so it stays correct in the presence of read receipts and unread/highlight rows. The SQL avoids `UPDATE ... FROM` and CTEs so it works on both SQLite and Postgres. End-of-purge cache invalidation already covers `get_unread_event_push_actions_by_room_for_user`. ### Tests Adds `test_count_aggregation_after_purge`, which forces a rotation before purging and asserts the aggregate count reflects only the surviving events, covering read receipts and a subsequent re-rotation. It fails (`3 != 1`) without the fix. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…element-hq#19843) When building the v1.155.0rc1, the release notification said the workflow was successful but it had actually failed. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…during the remote join handshake (element-hq#19390)
…ement-hq#19840) The Complement integration-test job prints the full raw `go test -json` stream straight to the GitHub Actions build log. That's an enormous, unreadable wall of JSON, and the GitHub web UI renders very large logs poorly — making the run hard to view and slow to load. Instead, let's store the raw JSON and upload it as an artefact. We still render failing test output as before. We will still stream when tests finish (i.e. PASS / FAIL) so that people can see that things are actually running. However, all other output (such as logs) are hidden. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: dependabot[bot] <support@github.com>
Running `scripts-dev/lint.sh` locally revealed a failure, so here are the fixes that the linter applied + a manual change to address an issue that couldn't be auto-fixed.
…ation`. (element-hq#19845) Fixes: element-hq#19844 Concretely, this changes `ResponseCache` to unset cache entries once they resolve to a `Failure`. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> Co-authored-by: Eric Eastwood <erice@element.io>
…s, as per MSC2409 (element-hq#19758) Part of element-hq#18118 [MSC2409](matrix-org/matrix-spec-proposals#2409) was merged into the spec on Oct 28, 2024 and is part of Matrix Spec 1.13 This adds the stable identifiers for MSC2409 while maintaining the existing unstable identifier for backwards compatibility. The existing `to_device` identifiers borrowed from MSC2409 for [MSC4203](matrix-org/matrix-spec-proposals#4203) are also maintained. (Includes one drive-by removal of a FIXME comment that was no longer needed after [May 16, 2022](element-hq@83be72d)) ### 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: Quentin Gliech <quenting@element.io>
When I run `ruff` locally, either `ruff` or one of the linting scripts adds `.ruff_cache` to the `.gitignore` file. This PR adds that line so that running linters doesn't result in a dirty git working tree (and to ignore ruff's cache, of course). ### 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))
…in` gap (element-hq#19856) To better explain that this new thing we're doing in element-hq#19390 is a good citizen thing, not a requirement. Follow-up to element-hq#19390
…c to reuse `get_users_which_can_issue_invite`. (element-hq#19732)
… room versions other than 1 and 2. (element-hq#19850) Not aware of an open ticket for this. I came across it when I accidentally broke the feature even more (as part of another piece of work), then discovered there weren't tests for this. So this is overall a low-priority drive-by fix. Requires a fix to SyTest matrix-org/sytest#1426 (as it depended on the bug). <ol> <li> Add a test for purging rooms with `delete_local_events=False` \ Parameterised by room version, this test currently succeeds on v2 but fails on v12. This is because the condition checking for local events relies on the old event ID format, which has not been used since v2. </li> <li> Fix delete_local_events=False for room versions above v2 \ The event ID format changing means that we have to rely on `sender` to know the origin of an event </li> </ol> --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
--------- Signed-off-by: Sami Olmari <sami@olmari.fi>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
…response for rooms with restricted join rules, as required by Matrix 1.15. (element-hq#19762) Currently the `/summary` endpoint on the client and federation APIs does not include the allowed_room_ids field, that is present on the /hierarchy API. This is required by the 1.15 spec and this is what this PR does. Complement test: matrix-org/complement#873 Part of element-hq#18731
…nt-hq#19794) Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…lement-hq#19705) Co-authored-by: Andrew Morgan <andrew@amorgan.xyz>
…ion/configuration (element-hq#19853) Implements matrix-org/matrix-spec-proposals#1763 Tracking issue element-hq#19852 This only implements the MSC's configuration endpoint and does not add new configuration fields or options. The reason for including this is to allow clients to run global retention against locally stored events, and to be able to offer configuration UI. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Bringing it up to parity with the other 404 StoreErrors naming the table name already. More helpful response when debugging issues with incomplete relations. Concretly: a user that got partly removed and then reinstated, missing an entry in 'profiles' table. This should also contribute to a better understanding of element-hq#2807 and element-hq#2173
…lement-hq#19868) Spawning from element-hq#19824 / element-hq#19846 and wanting to use `create_deferred` in more than just the `http_client.rs`
…19835) Signed-off-by: dependabot[bot] <support@github.com>
…lement-hq#19591) Follows: element-hq#19487 Part of: MSC4354 whose experimental feature tracking issue is element-hq#19409 This PR implements the Sliding Sync (MSC4186) extension described in MSC4354, allowing sliding sync clients to receive sticky events in a reliable way. The logic is much the same as for oldschool sync (implementation in element-hq#19487), although in the sliding sync extension, the client can choose their own limit and must control their own pagination through an extra token in the extension request/response bodies. Note this does not yet send down existing sticky events in the room when the room has been newly-joined. This newly-discovered gap is tracked at element-hq#19662 and will be addressed for both current sync and MSC4186 SSS soon. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org> Co-authored-by: Eric Eastwood <erice@element.io>
After looking into it, just a couple of things to pick a bone at in the old wording, which I thought could be clarified for when I next come to look at this again. - the claim that there's a fundamental difference; I'd argue there isn't really, it's just by convention on some mainstream distros. So I have changed this to 'typically' - statements that some distros fetch dependencies at build time (probably does happen, but traditional distros make a point of not doing this for the reasons you'd expect). - This was probably meant to be talking about Debian, but my observation based on sample size of 3 is that some crates are packaged natively, others are vendored in the respective application's source package (like they do for us) and sometime they patch the bounds a bit There could probably be room to talk about how distros vendoring packages is a maintenance burden on them, but I guess it's a bit moot as we would struggle to conform to wide enough bounds to make everyone happy (and anyway; I expect the distros that vendor packages have the tooling to make this easy to update and we do keep on top of security updates and release frequently...) --- Spawning from discussion in [`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$VttYPPUevn2S_W_rrzg2ZOXWI6aKebk2ganTgrLEWUc?via=jki.re&via=element.io&via=matrix.org) --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…at (element-hq#19922) The port of event serialization to Rust (element-hq#19837) removed `format_event_raw`, `format_event_for_client_v1`, `format_event_for_client_v2` and `format_event_for_client_v2_without_room_id` from `synapse.events.utils`, but there are modules in the wild that import them from there. Reimplement them as standalone pyfunctions in Rust, operating directly on the Python dict so the original semantics are preserved exactly (in-place mutation, returning the same dict, arbitrary non-JSON values passing through, KeyError on a missing `unsigned` in the v1 format), and re-export them from `synapse.events.utils`. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…hq#19878) This is a stepping stone before we can go full Rust everywhere. We're providing a generic interface as we want database access to work in Synapse and `synapse-rust-apps`. In `synapse-rust-apps`, we will use a `tokio-postgres` based database connection pool so it's full Rust. We want to avoid the situation where we have two database connection pools (one for Python, one for Rust) as we've run into connection exhaustion problems on Matrix.org before. As an example of using it and sanity check for all this work (including tests), I've also ported over the `/versions` handler to the Rust side with database access. The `/versions` endpoint is the simplest endpoint I could find that still had some database access. Hopefully the refactor on `/versions` isn't that controversial as it's not really the point of this PR. We can always remove it from this PR but it's just here as a sanity check that all of this works. ### Why `runInteraction(...)`? Using the same `runInteraction` pattern that we already have in Synapse means we can port over existing Synapse code/endpoints without much thought. But this pattern also makes sense because we want[^1] transactions to have repeatable-read isolation (easy to think about, less foot-guns). Having everything thappen in a function callback means we can do retries for serialization/deadlock errors. [^1]: To note: Ideally, we'd want the least isolation possible but the problem is that there is no tooling to yell at you when your queries/logic is wrong so repeatable-read isolation is a great balance. > When an application receives this error message, it should abort the current transaction and retry the whole transaction from the beginning. The second time through, the transaction will see the previously-committed change as part of its initial view of the database, so there is no logical conflict in using the new version of the row as the starting point for the new transaction's update. > > Note that only updating transactions might need to be retried; read-only transactions will never have serialization conflicts. > > *-- https://www.postgresql.org/docs/current/transaction-iso.html#XACT-REPEATABLE-READ* As a note, this strategy is less of an impedance mismatch (aligns more closely) with Synapse so the glue code for the `python_db_pool` should also be simpler. ### How does this interact with logcontext (`LoggingContext`)? See [docs on log contexts](https://github.com/element-hq/synapse/blob/4e9f7757f17ba81b8747b7f8f9646d17df145aa3/docs/log_contexts.md) for more background. We already support normal logging from Rust -> Python with `pyo3-log` and `log` but as soon as we pass a thread boundary, everything is logged against the `sentinel` log context. Normally, we want logs and CPU/DB usage correlated with the request that spawned the work. You can see how I took a stab at fixing this in element-hq#19846 by capturing the logcontext in a Tokio task local and re-activating as necessary. For example, in that PR, I reactivated the logcontext in `run_python_awaitable(...)` which we use to call `runInteraction(...)` from the Rust side which means all of the database usage is correlated with the request as expected. It also means any `log:info!(...)` done in `run_interaction(...)` is correlated correctly. But there needs to be a better story for when you want to log everywhere else. I haven't explored tracking CPU usage on the Rust side. I've left all of this out of this PR as I think it will be better to tackle this as a dedicated follow-up. For example, I'm thinking about instead creating a new `LoggingContext` with the `parent_context` set to the calling context and try to avoid needing to call `set_current_context(...)` on the Python side where possible (like tracking CPU). ### Testing strategy Added some tests that exercise some `async` Rust handlers for the `/versions` endpoint: ``` SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.rest.client.test_versions.VersionsTestCase ``` Real-world: 1. `poetry run synapse_homeserver --config-path homeserver.yaml` 1. `GET http://localhost:8008/_matrix/client/versions`
…iness (element-hq#19929) `test_lock_contention` is a performance-regression canary (element-hq#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 element-hq#18430, which is removed. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… pollution caused by nginx upstreams being temporarily unavailable) (element-hq#19936) Fix element-hq#19907 The flake manifested as a failure pointing at `TestMessagesOverFederation/Backfill_from_nearby_backward_extremities_past_token` but was actually caused by some cross-test pollution from an earlier test (`TestOIDCProviderUnavailable`) causing some workers to be temporarily unavailable. As explained in element-hq#19907 (comment), > ### Cross-test pollution > > When I point an LLM at the logs, it points to `TestOIDCProviderUnavailable` being the culprit because of the server restarts polluting this test. When this flake happens, we can indeed see that `TestOIDCProviderUnavailable` runs before `TestMessagesOverFederation`. > > ``` > PASS TestEventBetweenMakeJoinAndSendJoinIsNotLost 15.08s > PASS TestFederation/parallel/HS2_->_HS1 1.5s > PASS TestFederation/parallel/HS1_->_HS2 1.51s > PASS TestFederation/parallel 0s > PASS TestFederation 15.12s > PASS TestOIDCProviderUnavailable//login/sso/redirect_shows_HTML_error 0.02s > PASS TestOIDCProviderUnavailable 8.62s > FAIL TestMessagesOverFederation/Backfill_from_nearby_backward_extremities_past_token 0.89s > FAIL TestMessagesOverFederation 1.1s > PASS TestSynapseVersion/Synapse_version_matches_current_git_checkout 0.97s > PASS TestSynapseVersion 0.97s > ``` > > The pollution happens because we enable [`COMPLEMENT_ENABLE_DIRTY_RUNS`](https://github.com/element-hq/synapse/blob/c63d77a79d7157f26f849684520ba9e99f4d07c0/scripts-dev/complement.sh#L309-L311) ([docs](https://github.com/matrix-org/complement/blob/0e6f8552ff0c99fddb97222399efed3e1f0cb91a/ENVIRONMENT.md#complement_enable_dirty_runs)) which means Complement will reuse deployments (shares homeservers between tests). > > During the `TestOIDCProviderUnavailable` test, there are some stray federation requests that hit the homeserver while it's still booting which marks the nginx upstream as unavailable for 10 seconds. nginx has a default of [`max_fails=1`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#max_fails) and [`fail_timeout=10s`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#fail_timeout). Then when `TestMessagesOverFederation` starts, we're still in the 10 second unavailable window and nginx doesn't even try to connect at all. > > <details> > <summary>LLM summary of the logs and how this happens in practice</summary> > > 1. **19:52:10–11** — the previous federation test finishes: `user-3:hs2` does a faster-join (`send_join?omit_members=true`) to `!YnyCRpCLIpimppIreR:hs1`, so `hs2` kicks off a `sync_partial_state_room` background resync that is still running when the test ends. > 2. **19:52:11.6** — `TestOIDCProviderUnavailable` starts and calls `deployment.StopServer(t, "hs1")` / `StartServer` (`complement/tests/oidc_test.go:78-80`) to apply an OIDC config fragment. `hs1` gets `SIGTERM`; new supervisord at 19:52:13.3. `hs2` is not restarted and keeps retrying its unfinished work. > 3. **19:52:14** — `hs1`'s nginx is up, but the Synapse workers aren't: `federation_inbound` only listens on `18015` at 19:52:18.7, `federation_reader` on `18016` at 19:52:19.0. > 4. **19:52:15–16** — `hs2`'s retries arrive in that gap: `PUT /_matrix/federation/v1/send/…` → `18015` refused; `GET /state_ids/!YnyCRpCLIpimppIreR:hs1` → `18016` refused. With nginx defaults (`max_fails=1`, `fail_timeout=10s`) and only one server per upstream block, both upstreams are now marked down until ~19:52:25/26. (Side casualty: `hs2`'s partial-state resync gives up — "We can't get valid state history" — and puts `hs1` on a 10-minute federation backoff.) > 5. **19:52:21.5** — the failing test's `make_join` for `@user-5-bob:hs2` reaches `hs1`'s nginx → `no live upstreams` → `502` → the join fails, even though the worker has been listening for 2.5 seconds by then. > 6. **19:52:22** — `TestSynapseVersion`'s `GET /_matrix/federation/v1/version` hits the same dead upstream → `502` → it fails too. > > So it's a flake caused by a race between the OIDC test's container restart, hs2's background federation retries, and nginx's passive health-check — not anything wrong with the backfill logic under test. > > </details> > > > We can indeed confirm this suspicion with these logs > > :x: https://github.com/element-hq/synapse/actions/runs/28888070856/job/85701936736 (archive: [85701936736.log](https://github.com/user-attachments/files/29809986/85701936736.log)): > ``` > Error: 2026/07/07 19:52:20 [error] 34#34: *1 no live upstreams while connecting to upstream, client: 172.18.0.3, server: localhost, request: "PUT /_matrix/federation/v1/send/1783453927718 HTTP/1.1", upstream: "http://federation_inbound/_matrix/federation/v1/send/1783453927718", host: "hs1" > ... > > Error: 2026/07/07 19:52:21 [error] 33#33: *4 no live upstreams while connecting to upstream, client: 172.18.0.3, server: localhost, request: "GET /_matrix/federation/v1/make_join/%21scAbyTQDdaauGYicpU%3Ahs1/%40user-5-bob%3Ahs2?ver=1&ver=2&ver=3&ver=4&ver=5&ver=6&ver=7&ver=8&ver=9&ver=10&ver=11&ver=12&ver=org.matrix.msc3757.10&ver=org.matrix.msc3757.11&ver=org.matrix.hydra.11 HTTP/1.1", upstream: "http://federation_reader/_matrix/federation/v1/make_join/%21scAbyTQDdaauGYicpU%3Ahs1/%40user-5-bob%3Ahs2?ver=1&ver=2&ver=3&ver=4&ver=5&ver=6&ver=7&ver=8&ver=9&ver=10&ver=11&ver=12&ver=org.matrix.msc3757.10&ver=org.matrix.msc3757.11&ver=org.matrix.hydra.11", host: "hs1" > ``` > > The fix here would be to disable nginx's unavailable upstream behavior by configuring `max_fails=0` in the upstream block: https://github.com/element-hq/synapse/blob/c63d77a79d7157f26f849684520ba9e99f4d07c0/docker/configure_workers_and_start.py#L374-L378
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>
…sition` (element-hq#19923) This speeds up the cascading delete from `sliding_sync_connection_positions`, which without an index on `connection_position` requires a sequential scan of the whole table for each deleted position. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…pes (element-hq#19911) A handful of places in the storage layer bound a value whose Python type didn't match the declared column type — an `int` into a `TEXT` column, or a `str` into a `BIGINT` column — and relied on psycopg2's loose coercion to paper over the mismatch. These are latent correctness bugs: they only work because the driver silently converts, and a stricter driver that binds typed parameters rejects them outright. Found during a Rust port of the database pool where the driver does not coerce automatically. Each fix binds the value with the type the column actually declares, rather than depending on driver-specific coercion. All changes are behaviour-preserving on psycopg2 and sqlite. There is also a fix to the multi-writer id-gen tests where we forgot to commit. This is tangential, but was found during the same effort. ### Changes - **`device_lists_remote_extremeties.stream_id (TEXT)`** — `_update_remote_device_list_cache_txn` (typed `stream_id: int`) bound an int; store it as a string, matching the column and the sibling `_update_remote_device_list_cache_entry_txn` (typed `stream_id: str`). The old mismatch, when rejected, was swallowed inside the device-list resync and hung `query_devices`. - **`user_filters.filter_id (BIGINT)`** — `get_user_filter` bound the raw `int | str` (a string, from sync requests). It already validates via `int(filter_id)`; bind that int so it matches the column. - **`rejections.last_check (TEXT)`** — both writers stored `clock.time_msec()` (an int); store the timestamp as a string. - **user-directory temp position** — the `populate_user_directory` background update's staging column `_temp_populate_user_directory_position.position` was `TEXT` but held an int (read back into a `BIGINT` column). Declare it `BIGINT`. The temp table is created and dropped within the background update, so there's no migration. - **`test_batched_state_group_storing`** — selected from `state_group_edges` with a stringified `state_group`; bind the int directly (the column is an integer). - **Multi-writer id-generator tests** — constructing a `MultiWriterIdGenerator` prunes stale `stream_positions` rows, but the harness never committed, so the cleanup only survived because adbapi keeps one connection per thread with its transaction open. Commit after construction so it persists regardless of pool semantics (the delete is legitimate work that should be committed anyway).
…lement-hq#19941) 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>
- Impose limit of scheduled delayed events - Update error codes to match latest draft of MSC4140 --------- Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
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>
…ment-hq#19948) 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.
…saction gets retried. (element-hq#19947) Introduced in: element-hq#19473 Noticed in: element-hq#19556 (comment) I have not experienced the bug in the real world, it's just something I noticed by reading. -- Fix bug in `_prune_device_lists_changes_in_room` when transaction is retried The `nonlocal` variable is a footgun as it increments the counter even though the transaction did not commit yet and may still be retried. --------- Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
…q#19663) Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
…sertion message explaining error
…nsert_many() instead of leaning on abstraction
Note here that 'false' implies that the next value to be called is *this* value and not the next
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.