Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Faster joins: audit code paths that do state queries #12643

Closed
richvdh opened this issue May 5, 2022 · 4 comments
Closed

Faster joins: audit code paths that do state queries #12643

richvdh opened this issue May 5, 2022 · 4 comments
Assignees
Labels
A-Federated-Join joins over federation generally suck T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@richvdh
Copy link
Member

richvdh commented May 5, 2022

Check that all the code paths that do state queries are happy with partial state. There are a bunch of things that call get_filtered_current_state_ids which really aren’t; there may well be others.

@anoadragon453 anoadragon453 added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label May 5, 2022
@erikjohnston
Copy link
Member

What do people think of indirecting all calls to DataStore.get_current_state_ids and co via the StateStorage, and then updating PartialStateEventsTracker to also track current state?

The downside to this approach is that it'll be easy for people to muscle memory using store.get_current_state_ids() and so bypassing the checks. That is true of the equivalent state group functions today I guess

@richvdh
Copy link
Member Author

richvdh commented May 25, 2022

To summarise what was said in chat:

  • when erik says StateStorage, he means StateGroupStorage.
  • PartialStateEventsTracker feels like the wrong place (better put it in a separate class)
  • indirecting current_state etc through StateGroupStorage (and renaming it to StateStorage) seems sensible.

@MadLittleMods MadLittleMods added the A-Federated-Join joins over federation generally suck label Jun 3, 2022
@erikjohnston
Copy link
Member

#12872 and #12964 will have sorted out a bunch of places that we check current_state_events, but someone should go through and audit if there are any other queries

@erikjohnston erikjohnston removed their assignment Jun 6, 2022
@richvdh richvdh self-assigned this Jul 13, 2022
@richvdh
Copy link
Member Author

richvdh commented Jul 15, 2022

I've been through the things that call methods in StateGroupWorkerStore and RoomMemberWorkerStore and opened a few issues, but mostly it's just "presence and device lists are going to be a nightmare", and they are covered by #13008 and #12993 respectively. I don't think further staring is going to be particularly fruitful and instead we should just file new issues as we find them.

@richvdh richvdh closed this as completed Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Federated-Join joins over federation generally suck T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

No branches or pull requests

4 participants