The current query supports passing in a list of users, which generates a
query using `user_id = ANY(..)`. This is generates a less efficient
query plan that is notably slower than a simple `user_id = ?` condition.
Note: The new function is mostly a copy and paste and then a
simplification of the existing function.
The crux of the change is to try and make the queries simpler and pull
out fewer rows. Before, there were quite a few joins against subqueries,
which caused postgres to pull out more rows than necessary.
Instead, let's simplify the query and do some of the filtering out in
Python instead, letting Postgres do better optimizations now that it
doesn't have to deal with joins against subqueries.
Review note: this is a complete rewrite of the function, so not sure how
useful the diff is.
---------
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
In previous versions of authlib using `client_secret_basic` without a
`client_secret` would result in an invalid auth header. Since authlib
1.3 it throws an exception.
The configuration may be accepted in by very lax servers, so we don't
want to deny it outright. Instead, let's default the
`client_auth_method` to `none`, which does the right thing. If the
config specifies `client_auth_method` and no `client_secret` then that
is going to be bogus and we should reject it
Sometimes we fail to fetch events during backfill due to missing state,
and we often end up querying the same bad events periodically (as people
backpaginate). In such cases its likely we will continue to fail to get
the state, and therefore we should try *before* loading the state that
we have from the DB (as otherwise it's wasted DB and memory).
---------
Co-authored-by: reivilibre <oliverw@matrix.org>
There are two changes here:
1. Only pull out the required state when handling the request.
2. Change the get filtered state return type to check that we're only
querying state that was requested
---------
Co-authored-by: reivilibre <oliverw@matrix.org>
Instead of persisting outliers in a bunch of batches, let's just do them
all at once.
This is fine because all `_auth_and_persist_outliers_inner` is doing is
checking the auth rules for each event, which requires the events to be
topologically sorted by the auth graph.
The idea here being that the directory server shouldn't advertise rooms
to a requesting server is the requesting server would not be allowed to
join or participate in the room.
<!--
Fixes: # <!-- -->
<!--
Supersedes: # <!-- -->
<!--
Follows: # <!-- -->
<!--
Part of: # <!-- -->
Base: `develop` <!-- git-stack-base-branch:develop -->
<!--
This pull request is commit-by-commit review friendly. <!-- -->
<!--
This pull request is intended for commit-by-commit review. <!-- -->
Original commit schedule, with full messages:
<ol>
<li>
Pass `from_federation_origin` down into room list retrieval code
</li>
<li>
Don't cache /publicRooms response for inbound federated requests
</li>
<li>
fixup! Don't cache /publicRooms response for inbound federated requests
</li>
<li>
Cap the number of /publicRooms entries to 100
</li>
<li>
Simplify code now that you can't request unlimited rooms
</li>
<li>
Filter out rooms from federated requests that don't have the correct ACL
</li>
<li>
Request a handful more when filtering ACLs so that we can try to avoid
shortchanging the requester
</li>
</ol>
---------
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
There are a couple of things we need to be careful of here:
1. The current python code does no validation when loading from the DB,
so we need to be careful to ignore such errors (at least on jki.re there
are some old events with internal metadata fields of the wrong type).
2. We want to be memory efficient, as we often have many hundreds of
thousands of events in the cache at a time.
---------
Co-authored-by: Quentin Gliech <quenting@element.io>
We remove these fields as they're just duplicating data the event
already stores, and (for reasons 🤫) I'd like to simplify
the class to only store simple types.
I'm not entirely convinced that we shouldn't instead add helper methods
to the event class to generate stream tokens, but I don't really think
that's where they belong either
This is an extra response parameter just added to MSC3981. In the
current impl, the recursion depth is always 3, so this just returns a
static 3 if the recurse parameter is supplied.
Previously, the response status of `HTMLResource` was hardcoded as
`200`. However, for proper redirection after the user verifies their
email, we require the status to be `302`. This PR addresses that issue
by using `code` as response status.
Added in https://github.com/matrix-org/synapse/pull/16533, this workflow
was intended to be run once to add the version picker to all historical
versions of the https://matrix-org.github.io/synapse documentation
website.
Note that the latest version of the docs built from this repo now exist
at https://element-hq.github.io/synapse/.
The workflow has been run successfully and the version picker was added
to the documentation. Thus we can now delete this workflow.
---
Note: Do not confuse this PR with
https://github.com/matrix-org/synapse/issues/9453. This PR was made
while we were populating this repo with "Dummy issues" after the
changeover from matrix-org/synapse to element-hq/synapse - therefore
referencing this PR may cause some confusion.
Closes:
- https://github.com/matrix-org/synapse/issues/10397
- #10397
An administrator should know whether he wants to set a password or not.
There are many uses cases where a blank password is required.
- Use of only some users with SSO.
- Use of bots with password, users with SSO
* Add `ALTER TABLE ... REPLICA IDENTITY ...` for individual tables
We can't combine them into one file as it makes it likely to hit a deadlock
if Synapse is running, as it only takes one other transaction to access two
tables in a different order to the schema delta.
* Add notes
* Newsfile
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
* Re-introduce REPLICA IDENTITY test
---------
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
poetry-core 1.8.x includes a fix which properly moves the generate
synapse_rust.abi3.so file to the synapse directory when using an
editable install.
Without this change developers are left with a confusing experience
of the synapse.synapse_rust module not being found after installation.
Implement MSC3860 to follow redirects for federated media downloads.
Note that the Client-Server API doesn't support this (yet) since the media
repository in Synapse doesn't have a way of supporting redirects.
* Describe `insert_client_ip`
* Pull out client_ips and MAU tracking to BaseAuth
* Define HAS_AUTHLIB once in tests
sick of copypasting
* Track ips and token usage when delegating auth
* Test that we track MAU and user_ips
* Don't track `__oidc_admin`
Keeping track of a lower bound of stream ID where we've deleted everything below makes the queries much faster. Otherwise, every time we scan for rows to delete we'd re-scan across all the rows that have previously deleted (until the next table VACUUM).
If a worker reconnects to Redis we send out the current positions of all our streams. However, if we're also trying to send out a backlog of RDATA at the same time then we can end up sending a `POSITION` with the current token *before* we've sent all the RDATA before the current token.
This doesn't cause actual bugs as the receiving servers see the POSITION, fetch the relevant rows from the DB, and then ignore the old RDATA as they come in. However, this is inefficient so it'd be better if we didn't send out-of-order positions
* Fix the CI query that did not detect all cases of missing primary keys
* Add more missing REPLICA IDENTITY entries
* Newsfile
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
---------
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
* Add Postgres replica identities to tables that don't have an implicit one
Fixes#16224
* Newsfile
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
* Move the delta to version 83 as we missed the boat for 82
* Add a test that all tables have a REPLICA IDENTITY
* Extend the test to include when indices are deleted
* isort
* black
* Fully qualify `oid` as it is a 'hidden attribute' in Postgres 11
* Update tests/storage/test_database.py
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
* Add missed tables
---------
Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
If simple_{insert,upsert,update}_many_txn is called without any data
to modify then return instead of executing the query.
This matches the behavior of simple_{select,delete}_many_txn.
Fetch information needed for push rule evaluation in parallel.
Ideally this would use query pipelining, but this is not
available in psycopg2.
Due to the database thread pool this may result in little
to no parallelization.
Previously only Twisted's EPollReactor was compatible with the
reactor timing metric, notably not working when asyncio was used.
After this change, the following configurations support the reactor
timing metric:
* poll, epoll, or select reactors
* asyncio reactor with a poll, epoll, select, /dev/poll, or kqueue event loop.
The event persistence code used to handle multiple rooms
at a time, but was simplified to only ever be called with a
single room at a time (different rooms are now handled in
parallel). The code is still generic to multiple rooms causing
a lot of work that is unnecessary (e.g. unnecessary loops, and
partitioning data by room).
This strips out the ability to handle multiple rooms at once, greatly
simplifying the code.
Just to standardize on the normal helpers, it might also have
a slight perf improvement on PostgreSQL which will now use
`ANY (?)` instead of `IN (?, ?, ...)`.
* complement: enable dirty runs
* Add changelog
* Set a low connpool limit when running in Complement
Dirty runs can cause many containers to be running concurrently,
which seems to easily exhaust resources on the host. The increased
speedup from dirty runs also seems to use more db connections on
workers, which are misconfigured currently to have
`SUM(workers * cp_max) > max_connections`, causing
```
FATAL: sorry, too many clients already
```
which results in tests failing.
* Try p=2 concurrency to restrict slowness of servers which causes partial state join tests to flake
* Debug logging
* Only run flakey tests
* Only adjust connection pool limits in worker mode
* Move cp vars to somewhere where they get executed in CI
* Move cp values back to where they actually work
* Debug logging
* Try p=1 to see if this makes worker mode happier
* Remove debug logging
This is mostly useful for federated rooms where some users
would get stuck in the invite or knock state when the room
was purged from their homeserver.
This adds a module API which allows a module to update a user's
presence state/status message. This is useful for controlling presence
from an external system.
To fully control presence from the module the presence.enabled config
parameter gains a new state of "untracked" which disables internal tracking
of presence changes via user actions, etc. Only updates from the module will
be persisted and sent down sync properly).
Twisted trunk makes a change to the `TLSMemoryBIOFactory` where
the underlying protocol is changed from `TLSMemoryBIOProtocol` to
`BufferingTLSTransport` to improve performance of TLS code (see
https://github.com/twisted/twisted/issues/11989).
In order to properly hook this code up in tests we need to pass the test
reactor's clock into `TLSMemoryBIOFactory` to avoid the global (trial)
reactor being used by default.
Twisted does something similar internally for tests:
157cd8e659/src/twisted/web/test/test_agent.py (L871-L874)
* Fix bug where a new writer advances their token too quickly
When starting a new writer (for e.g. persisting events), the
`MultiWriterIdGenerator` doesn't have a minimum token for it as there
are no rows matching that new writer in the DB.
This results in the the first stream ID it acquired being announced as
persisted *before* it actually finishes persisting, if another writer
gets and persists a subsequent stream ID. This is due to the logic of
setting the minimum persisted position to the minimum known position of
across all writers, and the new writer starts off not being considered.
* Fix sending out POSITIONs when our token advances without update
Broke in #14820
* For replication HTTP requests, only wait for minimal position
This could happen if the last rows in the account data stream were inserted into `account_data`. After a restart the max account ID would be calculated without looking at the `account_data` table, and so have an old ID.
If using the script remotely, there's no particularly convincing reason
to disable certificate verification, as this makes the connection
interceptible.
If on the other hand, the script is used locally (the most common use
case), you can simply target the HTTP listener and avoid TLS altogether.
This is what the script already attempts to do if passed a homeserver
configuration YAML file.
This splits thinsg into two queries, but most of the time we won't have
new event backwards extremities so this shouldn't actually add an extra
RTT for the majority of cases.
Note this removes the check for events with no prev events, but that was
part of MSC2716 work that has since been removed.
