Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconcile or unify RoomEvents event chunks with AllEventsCache events #3886

Open
jmartinesp opened this issue Aug 23, 2024 · 5 comments
Open
Assignees

Comments

@jmartinesp
Copy link
Contributor

jmartinesp commented Aug 23, 2024

In EventCache::save_event there is this comment that says:

This doesn't insert the event into the linked chunk. In the future there'll be no distinction between the linked chunk and the separate cache.

And we have 2 separate caches now: one for chunks of events for timelines of rooms (RoomEvents), which come from room timeline updates during syncs and back pagination, and another one for any event in any room and their related events (AllEventsCache), which may come from any source.

I believe the end goal here is using a persistent storage to hold a cache that unifies both of these, and it's probably related to the 'implement a basic reconciliation of events; don't handle separate connected components of events yet' point in #3058. I'm opening this issue to both discuss if this is the way we want to proceed and, if it is, keep some recording of it.

@bnjbvr
Copy link
Member

bnjbvr commented Aug 26, 2024

After we've implemented proper persistent storage for events, I'd expect that the second AllEventsCache would read from the database, instead of storing any information in memory, so we can likely fold this as one item of #3280?

@bnjbvr bnjbvr self-assigned this Jan 14, 2025
@bnjbvr
Copy link
Member

bnjbvr commented Jan 14, 2025

For what it's worth, we might even not need migrations for the sqlite backend, if we consider using sqlite's json_extract function to inspect the event's content. This might be kind of equivalent to the parsing done by Ruma, though, so we might as well add new columns to the events table (one for the relation type and another one for the related event id).

@Hywan
Copy link
Member

Hywan commented Jan 14, 2025

I know it's not something easy, but I think we may want to bench having a new column for the event type vs. using json_extract. We've talk about this in the past already, but I didn't get time to do this yet on my side.

On the paper, it's not particularly elegant to use json_extract as it involves to recompute data every time, and possibly on a large dataset.

@bnjbvr
Copy link
Member

bnjbvr commented Jan 15, 2025

json_extract would work for most cases, because we could get the relation type and the related event using two calls:

  • the relation type is mostly readable from $.content.`m.relates_to`.rel_type
  • the related event id is mostly readable from $.content.`m.relates_to`.event_id

Unfortunately, this is not sufficient: the AllEventsCache also wants to know about redacted events. Redacted events may store the redacted event id in either $.content.redacts or $.redacts, according to the room version (redacted redactions seem to only store it in $.content.redacts).

But we have a tangential problem, that redacted events are still stored in clear in the database, and they shouldn't be. Otherwise, a user may see the content of a redacted event, if they're tech savvy enough to find the sqlite database. Probably, an event that the event cache is aware of and then that is redacted should be replaced in the database by the redacted for. We can keep the redaction event around too, in case we want to display the reason later. This would avoid the special-casing for the AllEventsCache above.

@bnjbvr
Copy link
Member

bnjbvr commented Jan 21, 2025

We actually can't use json_extract, because the database might be encrypted, and we wouldn't have access to the clear-text JSON event in this case. So we need the extra two columns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants