refactor!(event cache): introduce LinkedChunkId in the backends#5182
refactor!(event cache): introduce LinkedChunkId in the backends#5182
LinkedChunkId in the backends#5182Conversation
bnjbvr
left a comment
bnjbvr
left a comment
There was a problem hiding this comment.
(Self-review, since I thought about the code this morning…)
There'll be one per room and then one per thread (which is the thread + root event id). Also introduce a few methods to help comparing them, converting from one to the other, etc.
…for the in-memory impl
…s` to `clear_all_linked_chunks` Since it also clears the thread linked chunks.
bnjbvr
force-pushed
the
bnjbvr/migrate-ec-store-apis-for-threads
branch
from
e8e66ea to
7fbb83e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5182 +/- ##
==========================================
- Coverage 85.09% 85.05% -0.04%
==========================================
Files 328 328
Lines 36877 36943 +66
==========================================
+ Hits 31381 31423 +42
- Misses 5496 5520 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
poljar
left a comment
poljar
left a comment
There was a problem hiding this comment.
I left a couple of nits. This looks mostly good.
I guess you'll squash merge this PR since individual commits don't build on their own?
My main gripe here is that we now have two things that are called a chunk identifier.
| async fn handle_linked_chunk_updates( | ||
| &self, | ||
| room_id: &RoomId, | ||
| lcid: LinkedChunkId<'_>, |
There was a problem hiding this comment.
Can we rename this to chunk_id or linked_chunk_id like the rest of the patch does?
lcid is a bit opaque on first sight.
|
Thanks for the review! Good point about the new dual |
2 participants
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.