A checkout from the Rust connection pool (`ConnectionPool.connect`) previously
blocked indefinitely while waiting for a free slot or establishing a new
connection, so an unreachable or overloaded server could hang the worker
threads handling requests forever.
Bound the whole checkout with a configurable timeout: deadpool's `wait` and
`create` timeouts (enabled via its `rt_tokio_1` runtime feature, driven on our
own tokio runtime). Hitting it raises an OperationalError, which Synapse
already treats as a retryable operational error.
Configurable via the `pool_checkout_timeout` database option (milliseconds;
default 30s, `0` disables), read by the Rust engine and plumbed to both the
connection pool and the bootstrap (pool-of-one) connection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
The Rust backend connected with `NoTls`, so it couldn't talk to a Postgres that
requires (or, as libpq defaults, prefers) TLS — a blocker for most managed /
networked databases. Honour the same libpq keys psycopg2 already forwards, so an
existing `database.args` keeps working: `sslmode`, `sslrootcert`, `sslcert`,
`sslkey`, `sslpassword`.
Implemented on rustls (already in the tree via reqwest; pinned to the `ring`
provider since `aws-lc-rs` needs a C toolchain we don't have). `tokio-postgres`
only knows `sslmode` disable/prefer/require and does no verification itself, so a
new `postgres/tls.rs` drives both the handshake mode and the verifier:
disable -> no TLS
allow/prefer -> encrypt opportunistically, no verification (accept-any)
require -> encrypt, no verification (libpq's `require` does NOT verify)
verify-ca -> verify the chain, not the hostname (WebPKI, name bypassed)
verify-full -> verify chain + hostname (WebPKI)
`sslrootcert` (or the system trust store) supplies the roots; `sslcert`/`sslkey`
add a client certificate for mutual TLS. The cert-path keys and the `verify-*`
modes are stripped from the DSN and passed to the pool as explicit params
(`rust_dbapi.split_ssl_params`), since `tokio-postgres` can't parse them.
Behaviour change: the default (no `sslmode`) is now libpq's `prefer` — attempt
TLS, fall back to plaintext — where it was previously no TLS. This matches
psycopg2; plaintext servers still connect via the fallback.
Caveats vs libpq/openssl: rustls is stricter (requires SANs, rejects a
non-CA self-signed root), and an encrypted client key (`sslpassword`) isn't
supported and is rejected with a clear error.
Validated end-to-end against an SSL Postgres: require / verify-ca / verify-full
connect and use TLS, verify-full with only the system roots correctly rejects an
unknown CA, and hostname is checked (verify-full) / ignored (verify-ca). Unit
tests cover the sslmode->verifier mapping; the Rust-backend storage suites still
pass with the new default (prefer falls back against the plaintext test server).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
`new_transaction` sets a transaction's isolation level (for the receipts,
push-actions and purge paths, which ask for a specific level) by calling
`engine.attempt_to_set_isolation_level` before the transaction and resetting it
after. The Rust engine raised `NotImplementedError` there, so those transactions
errored out on the Rust backend.
The shim has no psycopg2-style `conn.set_isolation_level`, so implement it in
SQL: run `SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL <level>` on
the connection. That sets the session's default for *subsequent* transactions,
so it's committed before the caller's transaction begins (SET is transactional);
`new_transaction` resets it to the default (REPEATABLE READ, which the pool
already applies at connection setup) afterwards, scoping the override to the one
transaction — matching psycopg2's behaviour. The level name comes from a fixed
`IsolationLevel` map, not caller input.
Fixes the isolation-level failures in tests.storage.test_event_push_actions,
tests.storage.test_purge and tests.storage.test_receipts on the Rust backend
(20/20 pass); psycopg2 and sqlite are unaffected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Add SYNAPSE_TEST_RUST_DRIVER to run the Postgres test suite against the native
Rust driver (sets use_rust_driver on the test database config, off by default).
Teach the harness's make_fake_db_pool to build a RustConnectionPool for the Rust
engine, with its worker thread pool swapped for the threadless test pool so
queries run deterministically on the reactor during `pump` (the shim's block_on
waits there for the tokio workers). Test-database create/drop is admin work, so
it now goes directly over psycopg2 (always available in tests) rather than the
engine module's `connect`.
Running the suite this way surfaces a few real psycopg2-compatibility gaps:
- `fixup_config_defaults` (was `fixup_default_host`) now also fills libpq's
PGUSER / PGPASSWORD environment defaults, not just the host — tokio-postgres
reads none of them from the environment, but Synapse's test setup relies on
them (as psycopg2 does).
- `build_dsn` skips `None`-valued args, matching psycopg2's handling of unset
kwargs.
With these, a homeserver boots on the Rust backend under the test harness and
starts running real transactions and background updates. The remaining psycopg2
type-coercion / behaviour differences are addressed in the commits that follow;
psycopg2 and sqlite runs are unaffected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
`DatabasePool`'s `inner_func` calls `conn.reconnect()` when a connection is
found closed, or to recycle one that has exceeded the per-connection
transaction limit (`txn_limit`). The Rust DBAPI2 adapter had no `reconnect`, so
those paths (off by default, but real) would have raised.
Add `reconnect` to the adapter: it returns the current connection to the pool
(or discards it if unusable) and checks out a fresh one. The adapter now holds
the pool it was checked out of, with an `owns_pool` flag distinguishing a shared
pool (RustConnectionPool — reconnect from it, don't close it) from a bootstrap
pool-of-one (`rust_dbapi.connect` — closed together with the connection).
Tested: reconnect swaps in a working connection; runWithConnection and the
bootstrap/adapter paths are unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
A running homeserver exercises three shim capabilities beyond what the unit
tests reached; add them so it boots on the Rust backend end to end:
- `prepare_database` reads `db_conn.autocommit`; add a readable `autocommit`
getter to the shim `Connection` (and the adapter), mirroring psycopg2's
property.
- schema preparation runs multi-statement scripts via
`engine.execute_script_file` -> `cursor.executescript`; route
`LoggingTransaction.executescript` to the shim's multi-statement primitive
for the Rust backend (as it already does for sqlite), and expose
`executescript` on the DBAPI2 adapter cursor.
- store loading binds Python lists as Postgres arrays for `column = ANY($1)` /
`!= ALL($1)` (Synapse's `make_in_list_sql_clause`); add a `PgValue::Array`
variant that classifies a `list` and encodes it as an array of the element
column type, and accept array column types in `ToSql`.
With these, a homeserver boots on the Rust backend: it prepares the full schema
(176 tables), listens, `/health` returns OK, and DB-backed client endpoints
respond correctly.
Tested: array `to_sql` / classification and array-column `accepts` (Rust unit
tests); and, against a live Postgres, `= ANY($1)` list binding, adapter
`executescript`, and the `autocommit` getter. sqlite and psycopg2 homeserver
boots remain green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Turn the Rust backend on with `database.use_rust_driver: true` (alongside
`name: psycopg2`). Keeping `name: psycopg2` means the engine is still selected as
Postgres and all the `isinstance(engine, PostgresEngine)` dialect checks hold;
the flag only swaps the driver implementation.
- `create_engine` returns `RustPostgresEngine` instead of `Psycopg2Engine` when
the flag is set.
- `make_pool` gains a `clock` argument and, for the Rust engine, builds a
`RustConnectionPool` (via `_make_rust_pool`) instead of an adbapi pool:
it derives the libpq DSN from the config `args`, sizes threads/connections
to `cp_max`, passes the engine's synchronous_commit / statement_timeout,
starts the pool, and registers a shutdown hook via the clock. Its declared
return type stays `adbapi.ConnectionPool` — `RustConnectionPool` provides the
`_db_pool` subset `DatabasePool` uses (runWithConnection / threadID /
running / threadpool) — so callers that lean on adbapi-specific methods keep
type-checking.
- `RustConnectionPool` takes synchronous_commit / statement_timeout_ms and
threads them into its pooled connections' session setup.
- `DatabasePool` passes its clock to `make_pool`; the test harness's
`make_fake_db_pool` accepts the new argument.
Tested: create_engine honours the flag (and the Rust engine is still a
PostgresEngine, not a Psycopg2Engine); make_pool builds a started
RustConnectionPool that serves a query as `_db_pool`. psycopg2 and sqlite
homeserver boots remain green. A full homeserver boot on the Rust backend isn't
covered here because the test harness patches `make_pool` with a synchronous
adbapi fake.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Startup opens a bootstrap connection via `make_conn` (outside the runtime pool)
and validates it with `engine.check_database` before `prepare_database` runs.
Neither worked for the Rust backend: the Rust module has no `module.connect`,
and check_database/server_version were NotImplementedError stubs. Fill them in:
- `rust_dbapi.build_dsn` turns the libpq-style `args` into a DSN string, and
`rust_dbapi.connect` opens a standalone connection (a pool of one, kept
alive by the returned Connection) for one-off bootstrap use.
- `make_conn` routes the Rust engine through `rust_dbapi.connect` (with the
engine's synchronous_commit / statement_timeout) instead of
`engine.module.connect`; psycopg2 and sqlite are unchanged.
- `RustPostgresEngine.check_database` reads the server version over a cursor
(`SHOW server_version_num`) rather than psycopg2's `conn.server_version`,
and applies the same version / encoding / collation / ctype checks;
`server_version` is derived from the cached value.
Tested: build_dsn quoting; and, against a live Postgres, make_conn +
check_database + server_version + a query. psycopg2 and sqlite homeserver boots
(make_conn → check_database) remain green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Make RustConnectionPool a drop-in for DatabasePool._db_pool so real
runInteraction traffic can flow through it (once make_pool is switched):
- it now hands each function a DBAPI2-adapter connection (rust_dbapi.Connection)
wrapping the pooled shim, so LoggingDatabaseConnection.cursor() yields a
working LoggingTransaction and the engine's in_transaction / is_closed /
set_autocommit operate on it;
- the entry point is named `runWithConnection` (matching
twisted.enterprise.adbapi.ConnectionPool, which database.py calls by that
name), alongside `threadID` for the transaction-limit path. With
`threadpool` and `running` already present, the pool covers the slice of the
adbapi interface DatabasePool uses.
A new test drives a full transaction through the pool the way
DatabasePool.runWithConnection's inner_func does — engine.in_transaction check,
LoggingDatabaseConnection + LoggingTransaction, `?`→`$n` conversion, commit —
off the reactor thread, and gets the result back via the Deferred.
Still outstanding before make_pool can return it: `reconnect` on the connection
(transaction-limit / closed-connection paths) and the startup path
(make_conn / check_database).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Previously RustPostgresEngine subclassed the psycopg2 PostgresEngine, which
dragged psycopg2 into the Rust path and conflated "is Postgres" with "is
psycopg2". Split the hierarchy so both drivers are siblings:
- `PostgresEngine` (new `postgres_base` module) is now the driver-agnostic
base holding the shared SQL-dialect and config logic (single_threaded,
supports_using_any_list, row_id_name, get_db_locale, check_new_database,
lock_table, synchronous_commit / statement_timeout). It has no driver
dependency, so it always imports. Everything touching a live connection,
the DBAPI2 exception module, or the placeholder style is left abstract.
- `Psycopg2Engine(PostgresEngine)` holds the psycopg2 specifics (register_type
/ register_adapter, isolation-level map, conn.status/closed/server_version,
`%s` placeholders, psycopg2 execute path, `uses_psycopg2_extras = True`).
- `RustPostgresEngine(PostgresEngine)` is re-parented onto the base (no longer
inherits psycopg2). It passes the Rust DBAPI2 module to the base and gets
NotImplementedError stubs for the still-psycopg2-shaped check_database /
server_version (part of the deferred startup wiring).
The base keeps the name `PostgresEngine`, so all ~91
`isinstance(engine, PostgresEngine)` checks across the storage layer (which mean
"emit Postgres SQL") hold for both drivers unchanged. `create_engine` now
returns `Psycopg2Engine` for `name == "psycopg2"`.
Verified: full lint clean; Rust engine/adapter tests pass; a psycopg2 homeserver
boots and runs (test_room_search under Postgres) and the sqlite path is
unaffected (test_room_search / test_event_federation under sqlite).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Synapse's LoggingTransaction drives a cursor through the DBAPI2 spelling
(fetchone/fetchmany/fetchall, iteration, rowcount/description as properties),
but the Rust shim cursor exposes fetch_one/fetch_all/fetch_next_batch and
rowcount()/description() as methods. Rather than reshape the Rust API, add thin
Python `Connection`/`Cursor` wrappers that present the DBAPI2 shape and delegate
to the shim:
- `Cursor` maps fetchone/fetchmany/fetchall/__iter__ and rowcount/description
onto the shim, and tracks exhaustion so fetching past the end keeps
returning "no more rows" (the shim raises instead);
- `Connection.cursor()` returns the adapter cursor, and the transaction-control
and engine-facing methods (commit/rollback/close, set_autocommit, is_closed,
in_transaction) delegate straight through.
An end-to-end test drives a real LoggingTransaction backed by the adapter and
RustPostgresEngine: `?` placeholders are converted to `$n`, a query runs, and
rows come back via fetchone/iteration/description.
Not handled yet: execute_batch / execute_values (psycopg2 extras that
LoggingTransaction calls directly for PostgresEngine) still need a routing
change to reach a shim-backed implementation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
A database engine that drives the Rust Connection/Cursor shim instead of
psycopg2. It subclasses PostgresEngine to reuse the pure SQL-generation and
configuration behaviour, and overrides only the parts that touch a live
connection or are wired to psycopg2 internals:
- `module` points at the Rust backend's DBAPI2 exception hierarchy, which the
transaction driver catches on;
- `convert_param_style` rewrites `?` to `$1, $2, ...` (the shim binds libpq
positional placeholders, not psycopg2's `%s`);
- `in_transaction` / `is_connection_closed` / `attempt_to_set_autocommit` call
the shim's own methods;
- `is_deadlock` matches the Rust `DatabaseError` and its `pgcode`;
- `executescript` uses the shim's multi-statement primitive;
- `on_new_connection` is a no-op — session setup belongs in the Rust pool.
Per-transaction isolation-level overrides raise NotImplementedError for now, and
`check_database` / `server_version` still read psycopg2 attributes; the engine
is not yet selectable via `create_engine`, so neither is reached. Wiring it into
`make_pool` (which also feeds session config to the pool) is the next step.
Tested directly: param-style rewriting, deadlock/pgcode matching, the module
pointer, and — against a live shim connection — in_transaction, is_closed and
autocommit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
`RustConnectionPool` runs synchronous Synapse transaction functions off the
reactor thread against the native Rust connection pool. Each
`run_with_connection(func, *args)` call runs, on a dedicated Twisted thread
pool, `func(conn, *args)` with a `Connection` checked out of the Rust pool, and
returns a Deferred that fires on the reactor with the result (or an errback if
it raised). It builds on `defer_to_threadpool`, so log contexts are preserved
across the hop, and returns the connection to the pool afterwards (clean →
reused, mid-transaction/broken → discarded, per the shim's disposal rules).
The Rust pool sizes 1:1 with the thread count, and `PyConnectionPool.close()`
(new) closes all idle connections so the owner can drop the server connections
deterministically. The bridge is lifecycle-owner-managed (start/close) and
takes no clock, keeping it trivially testable; slotting it into
`DatabasePool.make_pool` — which also needs engine-level support for the shim
connection (in_transaction, is_connection_closed, autocommit/isolation,
reconnect) — is left to a follow-up.
Tested against a live Postgres over the real reactor (Synapse's in-memory test
reactor mocks the DB thread pool out, so a plain Twisted trial TestCase is used):
result/return-value, arg forwarding, exception-to-errback, connection reuse
across calls, and concurrent checkouts.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
The Python-facing `Connection`/`Cursor` shim now always wraps a connection
checked out of the `deadpool` pool. `ConnInner` holds a `PooledConnection` and
disposes of it correctly when the last reference (the `Connection` and all its
cursors) goes away, via a `Drop` impl plus `release`/`discard` helpers:
- a clean connection is returned to the pool for reuse;
- a connection is *discarded* (detached with `Object::take`, which also
shrinks the pool) whenever reuse would be unsafe — a failed COMMIT/ROLLBACK,
a poisoned mutex, or being dropped with a transaction still open (which
can't be rolled back synchronously from `Drop`, so the socket close makes
the server do it).
A plain query error still does *not* throw the connection away — it stays
open+aborted for the driver to `rollback()`, exactly as psycopg2 behaves.
The pool is the only way to obtain a connection: the standalone `connect(dsn)`
free function is replaced by a Python-facing `ConnectionPool` class (Rust
`PyConnectionPool`, exposed as `postgres.ConnectionPool`). Build it once from a
DSN, then check connections out with `pool.connect()`. Checkout failures map
onto the DBAPI2 hierarchy — a backend connect error reuses the query-error
mapping (and its `pgcode`), while a timeout / closed pool becomes
`OperationalError` — so `connect()` behaves like psycopg2's for callers.
Adds live-Postgres tests (gated on SYNAPSE_TEST_POSTGRES_DSN) asserting which
connections end up back in the pool, and drives the Python test suite through a
pool in `setUp`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
Implement DBAPI2's `executemany`, which Synapse uses for batched writes.
The statement is prepared once and run for each parameter set inside the
connection's (lazily opened) transaction, so a failure part-way aborts
the whole batch. The per-set executions are pipelined — their futures
are driven concurrently so tokio_postgres streams the batch onto the
connection in one round-trip rather than one per statement.
Afterwards `rowcount` reports the total rows affected across all
executions (as psycopg2 does) via a new fetchless "command complete"
cursor state; there is no result set to fetch or describe. An empty
parameter sequence is a no-op that leaves rowcount at -1.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The column names for a result set were already carried through the
cursor state machine but kept write-only, awaiting a reader. Add the
PEP-249 `Cursor.description` accessor on top of that plumbing: it
returns one 7-tuple per column (only the name populated, which is all
Synapse reads), or `None` when there is no row-returning result set —
before any query, after an error, or for a column-less statement such
as a bare INSERT, matching psycopg2.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Synapse's schema setup (`prepare_database.py`) runs `;`-separated SQL
scripts via the engine's `executescript`. `Cursor.execute` can't serve
those: it `prepare`s the query, and Postgres rejects multiple commands in
a prepared statement.
Add `Cursor.executescript`, which runs the whole script on the simple-query
protocol (`batch_execute`), which does allow multiple statements. It takes
no parameters and produces no fetchable rows.
The script runs inside the connection's *current* transaction (opened
lazily like `execute`) and is left open for the caller to commit. It
deliberately does NOT reproduce the commit-any-pending-transaction-first
behaviour of `sqlite3.executescript` (which psycopg2's engine mirrors with a
leading `COMMIT`). That forced commit actually undercuts the atomicity
`prepare_database` sets out to get — it opens a transaction so "upgrades are
either applied completely, or not at all", but the first script's implicit
commit ends it. Running the script within the ongoing transaction instead
lets successive scripts accumulate and be committed once, which is both
simpler and more correct. The only engine-level piece left to layer on top
is the auto-increment placeholder substitution.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously every `tokio_postgres` error became a bare `RuntimeError`. Synapse's
transaction driver, though, branches on the *type* of a database error and on
its `pgcode`: `new_transaction` retries `OperationalError`, retries deadlocks it
recognises via `is_deadlock` (which reads `pgcode`) on a `DatabaseError`, and
`simple_upsert` retries `IntegrityError`. With everything collapsed to
`RuntimeError` none of that fired.
Add just the distinctions Synapse acts on, rather than psycopg2's full PEP-249
hierarchy: `Error` -> `DatabaseError` -> {`OperationalError`, `IntegrityError`},
exposed on the `postgres` submodule, each instance tagged with `pgcode` (the
SQLSTATE string, or `None`). A small classifier maps the SQLSTATE class:
constraint violations (`23`) to `IntegrityError`, connection/resource classes
(`08`/`53`/`57`/`58`) to `OperationalError`, everything else (incl. `40*`
deadlocks, which retry via `pgcode`) to `DatabaseError`. Codeless errors are
split with `is_closed()`: a lost connection is operational, any other (a bad
parameter, a failed connect) is a plain `DatabaseError` so it isn't retried.
Errors surfacing while a result stream is drained (the usual case for an
`INSERT` constraint violation) now route through the same mapping, so they carry
the right class and `pgcode` too.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the end-to-end test suite for the Rust DBAPI2 backend, driving the real
tokio-postgres client against a live Postgres server. It is skipped unless
the suite is configured for Postgres (SYNAPSE_POSTGRES), the same switch the
rest of the suite uses.
These cover the behaviours that need a real server rather than the
in-memory fakes the Rust unit tests use: connect (good and bad DSN),
run_interaction (return value, arg/kwarg forwarding, commit on success,
rollback on a Python exception, and recovery after both a Python-raised and
a server-rejected statement, including a constraint violation), cursor
reuse across queries, the fetch_one/fetch_all/fetch_next_batch/rowcount
surface (including batching across a 1000-row result set and the
exhausted-vs-no-active-query error distinction), and value round-trips for
every supported type (NULL, bytea with NUL/high bytes, float4 lossiness,
and an out-of-range int bound to a real int4 column).
A separately-guarded case exercises the libpq default-host fixup by
connecting with a DSN that omits the host.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Impose limit of scheduled delayed events
- Update error codes to match latest draft of MSC4140
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.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>
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).
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 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
https://github.com/element-hq/synapse/pull/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`
The port of event serialization to Rust (#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>
### Summary
This moves the synchronous core of client event serialization out of
`synapse/events/utils.py` and into Rust
(`rust/src/events/serialize.rs`).
Event serialization is on the hot path for `/sync`, `/messages`, and
most client-facing endpoints. Previously it was a recursive pure-Python
routine (`_serialize_event` / `_inject_bundled_aggregations` /
`only_fields`) that interleaved
CPU-bound formatting with `async` DB/IO. This PR separates those two
concerns and moves the CPU-bound half to Rust:
- **Async prep stays in Python.**
`EventClientSerializer._prepare_serialization` does all DB/IO up front:
batch-fetching redaction events and running the registered module
`unsigned`-callback hooks, for the top-level events *and* every bundled
sub-event (edits and thread latest events, which are themselves
serialized). The admin/MSC4354 config is resolved once via
`_update_config`, rather than re-checked on every recursive call as the
old code did.
- **The synchronous core moves to Rust.** Given an event plus the
pre-fetched `redaction_map`, `unsigned_additions`, and
`bundle_aggregations`, the Rust code produces the client JSON entirely
in Rust — including the v1/v2 format transforms, `only_event_fields`
filtering, redaction handling, and recursive bundled aggregations.
### Details
- The Rust entry point is a single batch function, `serialize_events`,
taking a list of `(event, membership)` pairs. The three lookup maps are
shared across the whole batch, so they're read out of Python and
converted to Rust
structures **once** per batch rather than once per event.
`EventClientSerializer.serialize_event` (singular) is a thin wrapper
that calls it with a one-element list.
- `SerializeEventConfig` is now a Rust `pyclass`, and the old
`event_format` callable is replaced by the `EventFormat` enum (`Raw` /
`ClientV1` / `ClientV2` / `ClientV2WithoutRoomId`). Call sites in
`rest/admin/events.py` and
`rest/client/{notifications,room,sync}.py` are updated to pass the enum.
`make_config_for_admin` and MSC4354 enablement now go through
`SerializeEventConfig.for_admin()` / `with_msc4354()`.
- New accessors on `EventInternalMetadata` (`redacted_by`, `txn_id`,
`device_id`, `token_id`, `delay_id`, `soft_failed`,
`policy_server_spammy`) expose to Rust the fields the serializer reads.
- The `_split_field` unit tests move from `tests/events/test_utils.py`
to a Rust test in `serialize.rs`, since the implementation moved.
### Behaviour
This is intended to be a behaviour-preserving refactor — the Rust core
mirrors the previous Python output (field ordering, v1 key promotion,
redaction placement per room version, null-`redacts` handling,
transaction-ID gating).
Existing serialization, relations, and sync tests pass unchanged.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Co-authored-by: Eric Eastwood <erice@element.io>
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)
Introduced in: #17847
This 10-second wall-clock timeout was troublesome as it fails flakily on
slow/struggling CI runners, like the
default ones for private GitHub repositories.
The loop also silently relied on the reactor advance in `make_request`,
whereas we could just deterministically advance the reactor the known
amount of times
instead.
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Change `/org.matrix.msc3814.v1/dehydrated_device/[device_id]/events` to
accept GET requests instead of POST.
The original version of
[MSC3814](https://github.com/matrix-org/matrix-spec-proposals/pull/3814)
said we should delete keys after returning them from this endpoint, but
it is being updated to say we should not delete them, and therefore the
appropriate verb is GET.
Synapse already doesn't delete anything, so we just need to change to a
GET with a `next_batch` query param. (Currently it is a POST with
`next_batch` in the JSON content.)
This code was initially written by @ara4n and Claude, but both he and I
have read it and think it makes sense. I am far from a Synapse expert,
so feel free to tell me it's all wrong and point me in the right
direction.
I don't know what system tests will be affected by this, but I guess we
will see when the CI runs (right?).
This is a change to an unstable endpoint so no need for notifications
about breaking changes or similar.
Part of https://github.com/element-hq/element-meta/issues/2704
### Pull Request Checklist
* [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:
* [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: Matthew Hodgson <matthew@matrix.org>
This reports the total count of users (split by appservice) which is
meant to be the monthless counterpart to the MAU metric.
Context:
> So this is largely for billing purposes and wanting to know the change
in the number of users. If a user is deactivated then we no longer want
to count them. Consumers *might* want to count appservice users, and
maybe count them based on the service (perhaps you change more for users
under bridge X or bridge Y).
>
> *-- https://github.com/element-hq/synapse/pull/19848#discussion_r3402216234*
Follows: #19487
Part of: MSC4354 whose experimental feature tracking issue is #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
#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 #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>
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 https://github.com/matrix-org/sytest/pull/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>
Fixes: #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>
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>
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
