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

Why TrackedStateEvent might not be worth it. #6

Closed
Gnuxie opened this issue Jan 26, 2024 · 1 comment
Closed

Why TrackedStateEvent might not be worth it. #6

Gnuxie opened this issue Jan 26, 2024 · 1 comment

Comments

@Gnuxie
Copy link
Owner

Gnuxie commented Jan 26, 2024

Currently the RoomStateRevision can be configured to store a minimal representation of events provided they fit the TrackedStateEvent interface. This can be done by setting events as InformOnly within a StateTrackingMeta object and then giving StateTrackingMeta object to the RoomStateRevision either at initialization or later. In theory this will mean that the RoomStateRevision will use up less memory because events will be stored with most keys redacted. However, this does mean that PolicyListRevisions and RoomMembershipRevisions cannot be instantiated from a RoomStateRevision, only a blank revision that is incrementally built by listening to a RoomStateRevisionIssuer. It's also unclear whether there is going to be much benefit from the reduced memory footprint of some events, given that for Draupnir, all rooms protected will need their membership tracked. Which preserves the event content and most top level fields.

It's going to be expensive to run Draupnir to protected a room with 100k members, and this sort of saving is going to be linear. If we needed to think seriously about this for whatever reason, homeservers probably will reach the limit first. Telegram for instance only supports 200k members per group.

Let's pretend the average membership event will be 2000bytes for Draupnir to hold in memory (my estimates are more in the region of 800bytes, but lets up it just to be conservative, and the real serialized size is probably less still, in the region of 500-600bytes per event). And now let's say there's a million of them. That would mean there only needs to be 2GB to represent them. Which is quite reasonable. Even if we double or quadruple that.

So I think it's fair to say for simplicity, we're not going to have to worry about this quite yet and if we did, we would need to change the architecture of MPS regardless of the linear savings for TrackedStateEvent. We'd reach the point where we have to be able to change the cached membership to be a partial understanding of room state based on who is active in the room at around the same time in either case.

Gnuxie added a commit that referenced this issue Jan 26, 2024
Handling redactions of events that have keys preserved in auth rules
made it difficult to continue. We would have had to add key counts to
`TrackedStateEvent` or remove it entirely.

Since removing it entirely was an option, I decided to look into what
the benefits of `TrackedStateEvent` were since the surrounding code
is a little complex and unimplemented (we didn't copy fields from
state events to create minimal tracked events, as was the intention).

#6

So after looking at the merits it's clear that it's probably not worth
having, because it isn't saving very much and there's a good chance
if it did work as intended, people would make nasty hidden mistakes.
@Gnuxie
Copy link
Owner Author

Gnuxie commented Jan 26, 2024

It's gone

@Gnuxie Gnuxie closed this as completed Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant