matrix-org / matrix-rust-sdk

Matrix Client-Server SDK for Rust
Apache License 2.0
1.26k stars 246 forks source link

Multi-process support: Implement finer-grained in-memory caching for the crypto store #2624

Open bnjbvr opened 1 year ago

bnjbvr commented 1 year ago

Currently, our cross-process lock blows up the entire crypto state (OlmMachine) whenever we notice another process has written to underlying storage. This was implemented because we really wanted a working implementation for decrypting notifications on iOS, but this really is a shortcut. In particular, short-lived crypto procedures (like verification etc) could be interrupted if another process holds the lock in the middle; or we don't currently have any locking when writing a new message (and this might happen in another process too, see also #1960).

This issue is about fixing and implementing that properly, by having some sort of "cross-process lock guard" representing all the crypto-store data that can be put in a cache. Acquiring this lock may invalidate the cache if another process has written to the underlying database, transparently for the users of the lock. For the point of view of any user of that cache, it's "just" another async fallible operation.


The idea is to introduce a new CryptoStoreCache data structure, which maintains two properties:

The overall "migration" process is thus the following:

Alternatively, we could also drop in-memory caching support for some of these fields, if the performance impact is deemed low enough.

Once that's done for all those fields, the final step would be to move back the crypto-store lock into the OlmMachine and simplify public APIs again.

A good testing strategy should also be established to make sure we can invalidate the cache at any time, and that it would not fiddle with any of the function calls that may be running in the background.


Tasklist

Bootstrapping

Migrating

Take each field that's cached and move it to the transactional API and store cache.

The final touch

manuroe commented 7 months ago

Just discussed it with @BillCarsonFr. In the quest of killing UTDs, the crypto team is mostly interested by the Move Olm Sessions into the cache migration. This is a task they may attack a Q1 but they still want a proof (logs and tests) that one problem is coming from here. We just released UTDs analytics on EX. We need to wait a bit more to get more data.