feat(event cache): rework the SQL schema #4849

bnjbvr · 2025-03-26T15:42:41Z

This rejiggers the event cache's store schema so that events can be saved independently of their position in a linked chunk. Also, it makes it possible to extract an event's relation metadata, so we can get rid of the AllEventsCache along the way.

Fixes #4841.
Fixes #3886.

Part of #3280.

Remaining steps:

benchmark performance hit of extracting the relationship + join request
add save_event to the event cache store trait + impl
add event_with_relations to the event cache store trait + impl
remove AllEventsCache
94626ae is the wrong abstraction: instead there should be a way to retrieve only related events with the given filters; then, the transitive closure of relationships can be achieved in the caller, instead of the store impl
rejigger find_event_with_relations event cache API so it doesn't return redacted events (there's no reason to do that)
add test for the store save_event() method
add test for the store find_event_relations() method
add method to clear all events

Kind of depending on ruma/ruma#2052, although we don't really have to.

…ir position in a chunk

…t into the database

codecov · 2025-03-26T15:56:44Z

Codecov Report

Attention: Patch coverage is 94.73684% with 15 lines in your changes missing coverage. Please review.

Project coverage is 86.50%. Comparing base (8c988be) to head (c1cb324).
Report is 78 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/matrix-sdk-common/src/linked_chunk/relational.rs	85.00%	6 Missing ⚠️
...rates/matrix-sdk-base/src/event_cache/store/mod.rs	78.57%	3 Missing ⚠️
crates/matrix-sdk-sqlite/src/event_cache_store.rs	96.00%	3 Missing ⚠️
...rix-sdk-base/src/event_cache/store/memory_store.rs	90.90%	2 Missing ⚠️
crates/matrix-sdk/src/event_cache/room/mod.rs	97.95%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4849      +/-   ##
==========================================
+ Coverage   86.47%   86.50%   +0.02%     
==========================================
  Files         297      297              
  Lines       34636    34756     +120     
==========================================
+ Hits        29951    30065     +114     
- Misses       4685     4691       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bnjbvr · 2025-03-26T16:12:57Z

As expected, this is a small performance hit when reading (because there's an extra join) around 7% of a time increase at the limit; when writing it's a speedup (time reduced by 5%), and the only reason I can fathom is that the events_chunks entries may be more compact and cause faster writes.

Benchmark

writing/sqlite store/10 time:   [98.738 µs 100.02 µs 102.24 µs]
                        thrpt:  [97.806 Kelem/s 99.977 Kelem/s 101.28 Kelem/s]
                 change:
                        time:   [+2.6943% +4.8981% +7.1514%] (p = 0.00 < 0.05)
                        thrpt:  [-6.6741% -4.6694% -2.6237%]
                        Performance has regressed.
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high mild
writing/sqlite store/100
                        time:   [344.28 µs 348.38 µs 351.99 µs]
                        thrpt:  [284.10 Kelem/s 287.04 Kelem/s 290.46 Kelem/s]
                 change:
                        time:   [-0.4169% +2.1253% +4.7940%] (p = 0.13 > 0.05)
                        thrpt:  [-4.5747% -2.0811% +0.4186%]
                        No change in performance detected.
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high severe
writing/sqlite store/1000
                        time:   [3.6115 ms 3.6858 ms 3.8293 ms]
                        thrpt:  [261.14 Kelem/s 271.31 Kelem/s 276.90 Kelem/s]
                 change:
                        time:   [-5.5496% -2.0645% +1.6455%] (p = 0.30 > 0.05)
                        thrpt:  [-1.6189% +2.1081% +5.8757%]
                        No change in performance detected.
writing/sqlite store/10000
                        time:   [37.588 ms 37.931 ms 38.342 ms]
                        thrpt:  [260.81 Kelem/s 263.64 Kelem/s 266.04 Kelem/s]
                 change:
                        time:   [-8.2726% -7.5524% -6.7257%] (p = 0.00 < 0.05)
                        thrpt:  [+7.2106% +8.1693% +9.0187%]
                        Performance has improved.
writing/sqlite store/100000
                        time:   [444.65 ms 445.37 ms 446.12 ms]
                        thrpt:  [224.16 Kelem/s 224.53 Kelem/s 224.90 Kelem/s]
                 change:
                        time:   [-5.0624% -4.8849% -4.7099%] (p = 0.00 < 0.05)
                        thrpt:  [+4.9427% +5.1357% +5.3323%]
                        Performance has improved.

reading/sqlite store/10 time:   [98.191 µs 100.13 µs 102.35 µs]
                        thrpt:  [97.700 Kelem/s 99.868 Kelem/s 101.84 Kelem/s]
                 change:
                        time:   [-4.5647% +0.0695% +4.7387%] (p = 0.98 > 0.05)
                        thrpt:  [-4.5243% -0.0695% +4.7830%]
                        No change in performance detected.
Found 3 outliers among 10 measurements (30.00%)
  1 (10.00%) low mild
  2 (20.00%) high mild
reading/sqlite store/100
                        time:   [339.02 µs 344.72 µs 351.37 µs]
                        thrpt:  [284.60 Kelem/s 290.09 Kelem/s 294.97 Kelem/s]
                 change:
                        time:   [-1.4143% +1.5665% +3.9775%] (p = 0.30 > 0.05)
                        thrpt:  [-3.8254% -1.5423% +1.4346%]
                        No change in performance detected.
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high mild
reading/sqlite store/1000
                        time:   [2.9736 ms 3.0122 ms 3.1078 ms]
                        thrpt:  [321.77 Kelem/s 331.98 Kelem/s 336.30 Kelem/s]
                 change:
                        time:   [+4.2030% +13.069% +29.372%] (p = 0.06 > 0.05)
                        thrpt:  [-22.704% -11.559% -4.0334%]
                        No change in performance detected.
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high severe
reading/sqlite store/10000
                        time:   [32.343 ms 32.536 ms 32.941 ms]
                        thrpt:  [303.57 Kelem/s 307.35 Kelem/s 309.19 Kelem/s]
                 change:
                        time:   [+6.4924% +7.7863% +9.2892%] (p = 0.00 < 0.05)
                        thrpt:  [-8.4996% -7.2238% -6.0966%]
                        Performance has regressed.
Found 2 outliers among 10 measurements (20.00%)
  1 (10.00%) high mild
  1 (10.00%) high severe
reading/sqlite store/100000
                        time:   [427.41 ms 433.37 ms 439.85 ms]
                        thrpt:  [227.35 Kelem/s 230.75 Kelem/s 233.97 Kelem/s]
                 change:
                        time:   [+5.1884% +6.7795% +8.5055%] (p = 0.00 < 0.05)
                        thrpt:  [-7.8388% -6.3491% -4.9325%]
                        Performance has regressed.

…n handle indexed items This is necessary to save out-of-band items into the relational linked chunk. I'm not quite sure of the value to keep it generic, at this point, but at least it makes testing easy.

Getting the position when reading an event is no longer required: - the only use case for reading the position out of the event cache was when we wanted to replace a redacted item into the linked chunk; now with save_event(), we can replace it without having to know its position. As an extra measure of caution, I've also included the room_id in the `events` table, next to the event_id, so that looking for an event is still restricted to a single room.

…pers

…id, with or without its relations

…getting an event's relations

…plies It's unclear whether it's useful, especially in the case where it would return an entire reply chain. It's not possible to filter in replies only, using the function either, which is a sign that replies shouldn't be indexed, IMO. In any case, that's something we can add back in the future, if we want to.

…daction events A redaction event would be either applied a priori (by the server, when returning the sync response), or the event cache would handle it, and redact it in the database; in any case, we'd never see the original event in its non-redacted form, so there's no point in returning the redaction event itself.

…with_relations` And rename it `find_event_relations`.

… delete all the events' contents

And fix the sql backend \o/

Hywan

Really great job, thanks for all this, pretty exciting to see AllEventCache being removed, finally!

I've a couple of feedback, but nothing fancy. The biggest question is about a cascading deletion between events_chunks and events.

crates/matrix-sdk-sqlite/migrations/event_cache_store/007_events_chunks.sql

crates/matrix-sdk-sqlite/src/event_cache_store.rs

crates/matrix-sdk/src/event_cache/room/mod.rs

benchmarks/benches/room_bench.rs

crates/matrix-sdk/src/event_cache/mod.rs

crates/matrix-sdk/src/event_cache/room/mod.rs

crates/matrix-sdk-sqlite/src/event_cache_store.rs

Hywan

Approving in advance knowing all questions will be addressed.

…chunks And some comments have been tweaked too.

bnjbvr added 2 commits March 26, 2025 16:15

refactor(event cache): store the events' content independently of the…

398c84e

…ir position in a chunk

feat(event cache): extract an event's relationship before inserting i…

ca258ad

…t into the database

bnjbvr added 10 commits March 27, 2025 14:36

refactor(linked chunk): rejigger the relational linked chunk so it ca…

d2e3101

…n handle indexed items This is necessary to save out-of-band items into the relational linked chunk. I'm not quite sure of the value to keep it generic, at this point, but at least it makes testing easy.

feat(event cache): allow to persist an out-of-band event into storage

c88be28

refactor(event cache): move relation extraction into common store hel…

389460c

…pers

feat(event cache): allow retrieving an event and all its relations

5bd2af7

refactor(event cache): use the store methods to retrieve an event by …

c508595

…id, with or without its relations

refactor(event cache): get rid of the AllEventsCache

942fe22

feat(event cache): get the transitive closure of related events when …

f380f00

…getting an event's relations

bnjbvr force-pushed the bnjbvr/rework-sql-schema branch from 531e3e1 to 4b0a55a Compare March 31, 2025 15:14

bnjbvr added 4 commits April 1, 2025 11:20

refactor(event cache): don't return the event itself, in `find_event_…

7a84f2f

…with_relations` And rename it `find_event_relations`.

refactor(event cache): regroup code to compute the filter strings

5ca0a4f

refactor(event cache): have EventCacheStore::clear_all_rooms_chunks…

ee5fa77

… delete all the events' contents

test(event cache): add tests for save_event() and find_event_relations()

f19be33

And fix the sql backend \o/

bnjbvr marked this pull request as ready for review April 1, 2025 10:01

bnjbvr requested a review from a team as a code owner April 1, 2025 10:01

bnjbvr requested review from Hywan and removed request for a team April 1, 2025 10:01

Hywan requested changes Apr 1, 2025

View reviewed changes

Hywan self-requested a review April 1, 2025 15:44

Hywan approved these changes Apr 1, 2025

View reviewed changes

refactor(sqlite): rename gaps to gap_chunks / events_chunks to event_…

c1cb324

…chunks And some comments have been tweaked too.

bnjbvr enabled auto-merge (rebase) April 2, 2025 11:12

bnjbvr merged commit d30dc71 into main Apr 2, 2025
42 checks passed

bnjbvr deleted the bnjbvr/rework-sql-schema branch April 2, 2025 11:26

Hywan mentioned this pull request May 5, 2025

fix(sqlite): Fix a UNIQUE constraint violation with Update::RemoveItem #5001

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(event cache): rework the SQL schema #4849

feat(event cache): rework the SQL schema #4849

Uh oh!

bnjbvr commented Mar 26, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 26, 2025 •

edited

Loading

Uh oh!

bnjbvr commented Mar 26, 2025

Uh oh!

Hywan left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hywan left a comment

Uh oh!

Uh oh!

Uh oh!

feat(event cache): rework the SQL schema #4849

feat(event cache): rework the SQL schema #4849

Uh oh!

Conversation

bnjbvr commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bnjbvr commented Mar 26, 2025

Uh oh!

Hywan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hywan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bnjbvr commented Mar 26, 2025 •

edited

Loading

codecov bot commented Mar 26, 2025 •

edited

Loading