Since the server will reject any duplicate one-time keys forever,
clients which encounter such an error will spam sentry with such
reports.
This patch ensures that we only send the sentry report once.
This patch updates our `Raw` API usage since the newly added `JsonCastable` that disallows Raw casts that are known to fail deserialization.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch changes the query used by
`SqliteEventCacheStore::load_all_chunks_metadata`. It was the cause of
severe slowness. The new query improves the throughput by +1140% and the
time by -91.916%. The benchmark will follow in the next patch.
Metrics for 10'000 events (with 1 gap every 80 events).
- Before:
- throughput: 20.686 Kelem/s,
- time: 483.43 ms.
- After:
- throughput: 253.52 Kelem/s,
- time: 39.478 ms.
This query will visit all chunks of a linked chunk with ID
`hashed_linked_chunk_id`. For each chunk, it collects its ID
(`ChunkIdentifier`), previous chunk, next chunk, and number of
events (`num_events`). If it's a gap, `num_events` is equal to 0,
otherwise it counts the number of events in `event_chunks` where
`event_chunks.chunk_id = linked_chunks.id`.
Why not using a `(LEFT) JOIN` + `COUNT`? Because for gaps, the entire
`event_chunks` will be traversed every time. It's extremely inefficient.
To speed that up, we could use an `INDEX` but it will consume more
storage space. Finally, traversing an `INDEX` boils down to traverse a
B-tree, which is O(log n), whilst this `CASE` approach is O(1). This
solution is nice trade-off and offers great performance.
This patch adds `timer!` logs in each method from `EventCacheStore` for
`SqliteEventCacheStore`. It will help to know the execution duration of
each of these methods.
Until now, `SqliteEventCacheStore` manages a pool of connections. A
connection is fetched from this pool and operations are executed on it,
regardless whether these are read operations or write operations.
We are seeing more and more _database is busy_ errors. We believe this
is because too many write operations are executed concurrently.
The solution to solve this is to use multiple connections for read
operations, and a single connection for write operations. That way,
concurrent writings are no longer a thing, and we hope it will reduce
the number of _database is busy_ errors to zero. That's our guess.
This patch does that. When the pool of connections is created, a
connection is elected as the `write_connection`. To get a connection for
read operations, one has to use the new `SqliteEventCacheStore::read`
method (it replaces the `acquire` method). To get a connection for
write operations, one has to use the new `SQliteEventCacheStore::write`
method. It returns a `OwnedMutexGuard` from an async `Mutex`. All
callers that want to do write operations on this store have to wait
their turn, this `Mutex` is fair, and the first to wait on the lock is
the first that will take the lock (FIFO). It guarantees the execution
ordering the code expects.
The rest of the patch updates all spots where `acquire` was used and
replaces them by `read()` or `write()`. A particular care was made to
see if other places are using `SqliteEventCacheStore::pool` directly. No
place remains except in `read()` and `write()`.
This patch updates `SqliteStoreConfig::pool_size` to be at least 2. We
need 2 connections: one for write operations, one for read operations.
This behaviour is coming in the next patches.
The SQLite implementation for the
EventCache::find_event_with_relations() the relation type list isn't
inserted using SQL placeholders.
The relation types are inserted manually using a format!() call. The
usage of the format!() call can lead to SQL injection if a
RelationType::Custom variant is used which contains SQL expressions.
This patch modifies the, query logic which retrieves the related events,
to use two separate queries which use SQL placeholders to insert all
the dynamic variables.
Security-Impact: Moderate
CVE: CVE-2025-53549
GitHub-Advisory: GHSA-275g-g844-73jh
This reorders methods so that they're grouped in "dual" pairs
(encode/decode, serialize/deserialize). Also adds a doc comment to
`encode_key`, as I've wondered in the past what it did.
These are not included in Element's main privacy policy, and may
constitute PII (because the homeserver may include the name of some
user). We keep the information as separate log lines, so that
rageshakes/manual reports still include those.
It avoids using different versions in several places for consistency. It
also allows to be able to change it in a single place when needed.
This also bumps the fallback to v11 everywhere, since it is the default
version for new rooms since Matrix 1.14 and it has the sanest redaction
rules.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch adds optional thread root event id parameters to the drafting functions exposed on the room level
allowing unfinished messages to be managed for the main room as well as any inner thread.
Internally it uses the room id or a tuple of the room id and the thread as keys for the various backing stores.
Internal Sentry reports tell us that enabling the busy_timeout seems to
have *increased* the number of "database is busy" errors, instead of
lowering those. As a result, we're going to disable the pragmas in all
the places where we enabled it before, and observe how the number of
"database is busy" errors evolves.
In a "soon" future, threads have their own linked chunk. All our code
has been written with the fact that a linked chunk belong to *a room* in
mind, so it needs some biggish update. Fortunately, most of the changes
are mechanical, so they should be rather easy to review.
Part of #4869, namely #5122.
