Five call sites repeated the same choice: `execute_values` with a
`VALUES ?` statement on psycopg2, or `execute_batch` with a row of
placeholders elsewhere. `execute_batch_values` makes that choice once,
given the statement and the row width, so the store code no longer
needs to know which driver has psycopg2's extras.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
`make_pool` returned the Rust pool under a `type: ignore` because it is
not an `adbapi.ConnectionPool`. Name the slice of the adbapi interface
`DatabasePool` relies on as a Protocol, which both pools satisfy, and
type `make_pool` and the test harness fakes with it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
The pool tests were written with `inlineCallbacks`; trial runs `async`
test methods directly, so use `await`. Adds a test that `txn_limit`
replaces the pooled connection after the configured number of checkouts.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
Pool exhaustion was invisible until checkouts timed out. Expose
deadpool's occupancy from the Rust pool and publish it as gauges per
pool (max connections, open connections, idle connections, waiting
checkouts), plus a histogram of the time a checkout waits for a
connection.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
`database_engine` was the DBAPI module's name, which for the Rust backend
is its Rust module rather than anything recognisable. Report the
configured engine name there, as before for sqlite3 and psycopg2, and
add `database_driver` so deployments on the native driver can be told
apart.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
The Rust backend prepares and caches statements per connection. DDL that
changes a cached statement's result type, such as a schema delta applied
during a rolling restart, makes Postgres reject the stale plan with
SQLSTATE 0A000. Inside a transaction that error used to propagate as a
plain DatabaseError, which `new_transaction` does not retry, so other
workers saw transient failures until their caches turned over.
The cursor already drops the cached entry before the error propagates,
so treating 0A000 like a deadlock means the retried transaction prepares
a fresh plan.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
The Rust shim has no `psycopg2.extras`, so the bulk write helpers that Synapse
runs via `execute_values` need a shim-compatible path. Add a
`uses_psycopg2_extras` engine capability flag (True for psycopg2, False for the
Rust and SQLite engines) and branch the write paths on it:
- The bulk INSERT/DELETE helpers (`simple_insert_many_txn`,
`simple_upsert_many_txn`, `simple_delete_many_batch_txn`, and the
sliding-sync / relations / state-deletion inserts) use psycopg2's single
multi-row `execute_values` on psycopg2, and `execute_batch` (the shim's
pipelined `executemany`) on the Rust and SQLite backends.
- `execute_values` itself is now psycopg2-only (it asserts
`uses_psycopg2_extras`); no query path splices row values into SQL as
literals.
- The four VALUES-join / RETURNING queries that `execute_values(fetch=True)`
served (in `event_federation` and `end_to_end_keys`) can't be expressed by
`executemany`, so they use a parameterised `unnest(...)` form that works on
all three backends.
Also cast the sequence name in `nextval(?::text)`: the Rust driver binds typed
parameters and can't produce a `regclass` from a name, whereas a text argument
coerces (this is the only parameterised sequence call, and Postgres-only SQL, so
psycopg2 and sqlite are unaffected).
psycopg2 behaviour and performance are unchanged; the Rust and SQLite bulk paths
now match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d
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
The Rust `ConnectionPool` takes the homeserver's `RustRuntime`, whose
tokio runtime drives the pool. `make_pool`, `rust_dbapi.connect` and
`make_conn` now take it too, and `DatabasePool` and `Databases` pass
`hs.get_rust_runtime()`. `make_conn` falls back to a standalone runtime
on the global reactor for the scripts and tests that have no homeserver
to take one from.
The adapter and engine tests build their pools through the shared
`_make_pool` helper for the same reason.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
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
Match adbapi's `_runWithConnection` contract in `RustConnectionPool._run`:
a transaction the function leaves open is committed when it returns and
rolled back when it raises, before the connection goes back to the pool.
The commit makes callers that rely on the pool's implicit commit durable,
and is a no-op after functions that commit themselves. The rollback
matters more: without it every raising transaction function hands a
mid-transaction connection back to the shim, which discards it (tearing
down the TCP session) rather than returning it to the pool for reuse.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ti5yoaVfjHYZ3LmzXFxSwq
`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>
These things are not needed now that MSC2965 has stabilised. In any
case, they
should never have been exposed without a config flag in the first place.
This speeds up the /createRoom calls a tiny bit by not checking the
senders push rules.
This helps speed up a lot of test runs in Element Web which
runs a lot of tests in one-person rooms (and creates a heck of a lot of rooms)
## Summary
- default `request_token_inhibit_3pid_errors` to `true` to prevent
account enumeration
- document the new default and the explicit opt-out
- add an upgrade note and regression coverage for the default
Fixes#18415.
Part of: MSC4354
Part of: #19641
Follows: #20232 (which applies already-persisted redactions to Sticky
Events being persisted)
In some cases we batch events together during persistence.
If one of the events in the batch is a redaction of another event, we
should apply it, which provides the same benefit as #20232 (namely: we
avoid potentially dodgy event content being persisted & the event
naturally loses its stickiness
so is not written to `sticky_events`)
---------
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
Fixes#20309.
For endpoints that validate the request body with Pydantic, a validation
failure put Pydantic's raw verbose error text into the `error` field of
the standard error response, e.g.:
```json
{
"errcode": "M_MISSING_PARAM",
"error": "1 validation error for PutBody\\nroom_id\\n Field required [type=missing, input_value={}, input_type=dict]\\n For further information visit https://errors.pydantic.dev/2.12/v/missing"
}
```
This leaks internal details clients cannot act upon (model class names,
raw input values, pydantic.dev URLs). This PR adds a
`_format_pydantic_error` helper in `synapse/http/servlet.py` that builds
a short client-friendly message from the structured
`ValidationError.errors()` data instead of `str(e)`:
- `{}` against a model requiring `room_id` -> `Missing required field
'room_id'.`
- `{"room_id": 123}` -> `'room_id': Input should be a valid string.`
- nested models use dotted paths, e.g. `Missing required field
'put.room_id'.`
- multiple errors are joined: `Missing required field 'name'; Missing
required field 'put'.`
The `errcode` selection logic (`M_MISSING_PARAM` / `M_INVALID_PARAM` /
`M_BAD_JSON`) is unchanged.
Part of #19414 (Support Matrix 1.16).
⚠️ Before merging, wait for the following PRs to be merged first:
- https://github.com/element-hq/synapse/pull/20081
- https://github.com/element-hq/synapse/pull/20169
- https://github.com/element-hq/synapse/pull/20171
---
Since Matrix 1.16, the
[spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync)
defines the stable identifiers for
[MSC4222](https://github.com/matrix-org/matrix-spec-proposals/pull/4222):
the `use_state_after` query parameter on `GET /_matrix/client/v3/sync`,
and the `state_after` room response field:
> **`use_state_after`** — `boolean` — Controls whether to receive state
changes between the previous sync and the start of the timeline, or
between the previous sync and the end of the timeline. If this is set to
`true`, servers MUST respond with the state between the previous sync
and the end of the timeline in `state_after` and MUST omit `state`. […]
>
> **`state_after`** — […] Required if `use_state_after` was set to true
in the request, even if it is empty. Added in `v1.16`
>
> — [Matrix
Spec](https://spec.matrix.org/v1.19/client-server-api/#get_matrixclientv3sync)
Synapse so far only implemented the unstable identifiers
(`org.matrix.msc4222.use_state_after` /
`org.matrix.msc4222.state_after`), gated behind the `msc4222_enabled`
experimental flag and the per-user experimental features admin API.
This PR:
- accepts the stable `use_state_after` query parameter, unconditionally,
and responds with the stable `state_after` field. The unstable query
parameter is kept for a transition period; clients opting in with it get
the unstable response field name back, so unstable-era clients see an
unchanged wire format. If both parameters are set, the stable one wins.
- removes the `msc4222_enabled` experimental flag and the `msc4222`
per-user feature from the experimental features admin API, since the
behaviour is no longer gated.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Fixes https://github.com/element-hq/synapse/issues/18793
When the last local user leaves a room, Synapse clears
`current_state_events` and writes `null` `event_id` entries into the
`current_state_delta_stream`. The incremental sync code for MSC4222's
state_after skipped those `null` deltas entirely, and
`get_current_state_ids` also returned nothing (table already cleared).
Result: clients using `state_after` never learned about the leave.
Fix by falling back to state groups (which are preserved) to look up the
actual state at `end_token` for any cleared state keys.
### 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:
- 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: Paul Chobert <paul@chobert.fr>
Co-authored-by: Quentin Gliech <quenting@element.io>
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Several clients, including Element Web/Desktop and Element X, have
already implemented
[MSC4426](https://github.com/matrix-org/matrix-spec-proposals/pull/4426)
User Status profile fields. However, we have only implemented sending
profile fields down sync for local users
([MSC4429](https://github.com/matrix-org/matrix-spec-proposals/pull/4429)/[MSC4262](https://github.com/matrix-org/matrix-spec-proposals/pull/4262)),
since federation support for pushing profile field updates from server
to server has not yet been implemented
([MSC4259](https://github.com/matrix-org/matrix-spec-proposals/pull/4259)).
This has created user confusion. The before mentioned clients use the
profile and/or profile field fetch endpoints to fetch missing profile
information, regardless of the user being local or remote. Since the
sync response isn't delivering these profiles, the client does a
fallback fetch, causing a one time update of the profile information
over federation.
This causes stale data to be cached in the client for the remote user,
which in the case of EW for example is never cleared.
This filtering should be temporary until MSC4259 is implemented and the
remote profile data also syncs via the profile update stream to clients.
I suggest making this not configurable so we don't have to maintain a
configuration property, as hopefully this would only be a case for a few
months at most.
### Pull Request Checklist
<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
---------
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
TLDR: `PUT` on a server-default push rule now applies the same existence
check that `GET` already does. A rule that is not served cannot be
modified.
Synapse ships some server-default push rules behind experimental feature
flags, so the set of server-default rules a user has depends on the
server's configuration. Most flags add rules that only exist while the
flag is on.
[MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210)
works the other way: it removes the three legacy mention rules while its
flag is on. The flags that change the set are:
- `msc1767_enabled`:
[MSC1767](https://github.com/matrix-org/matrix-spec-proposals/pull/1767)
extensible events rules
- `msc3381_polls_enabled`:
[MSC3381](https://github.com/matrix-org/matrix-spec-proposals/pull/3381)
poll rules
- `msc3664_enabled`:
[MSC3664](https://github.com/matrix-org/matrix-spec-proposals/pull/3664)
reply rule
- `msc4028_push_encrypted_events`:
[MSC4028](https://github.com/matrix-org/matrix-spec-proposals/pull/4028)
encrypted event rule
- `msc4306_enabled`:
[MSC4306](https://github.com/matrix-org/matrix-spec-proposals/pull/4306)
thread subscription rules
- `msc4210_enabled`: removes `.m.rule.contains_display_name`,
`.m.rule.contains_user_name` and `.m.rule.roomnotif`
`GET /pushrules/` only lists the rules that exist for that
configuration. `GET` on a single rule that is not in the set returns
`404 M_NOT_FOUND`. The same set is used to evaluate notifications, and
to build the `m.push_rules` account data in `/sync`.
`PUT .../enabled` and `PUT .../actions` did not use that set. They
accepted any rule ID that Synapse knows about, whether or not the server
currently serves it. Clients could modify a rule the server had just
said did not exist. The request returned `200`, the change was stored,
and it was then ignored. Both endpoints now return `404 M_NOT_FOUND` for
such a rule, as [the spec
requires](https://spec.matrix.org/v1.19/client-server-api/#put_matrixclientv3pushrulesglobalkindruleidenabled)
for a rule that does not exist.
Before (`msc4210_enabled: true`):
```
GET /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif
404 {"errcode": "M_NOT_FOUND", "error": "..."}
PUT /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif/enabled
{"enabled": false}
200 {}
```
After:
```
GET /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif
404 {"errcode": "M_NOT_FOUND", "error": "..."}
PUT /_matrix/client/v3/pushrules/global/override/.m.rule.roomnotif/enabled
{"enabled": false}
404 {"errcode": "M_NOT_FOUND", "error": "Unknown rule"}
```
This is a no-op for default configurations. All of these flags are off
by default, and rules that are served are still modifiable as before.
ESS deployments of MatrixRTC are going to run `lk-jwt-service` as an
application service so that it can manage delayed events. However,
setting up an application service is particularly inefficient, as
Synapse iterates through all the events of the rooms it covers since
their creation.
When an application service is configured with `url: null`, there is no
need to traverse all the events for it, so we can set up such
application services much faster without having to decide what to do for
other application services. Discussions about those are still open:
element-hq/synapse#11629 (see also matrix-org/synapse#1834).
### 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))
Use a new, dedicated endpoint for scheduling delayed events. This should
deprecate the `delay` query parameter on the `/send` and `/state`
endpoints.
Co-authored-by: Paul Chobert <paul@chobert.fr>
Co-authored-by: Eric Eastwood <erice@element.io>
In a similar vein to #20315, this attempts to speed up Synapse when
delayed events aren't in use. Here we skip doing any work if the room
event stream has not advanced.
