Add concept of StatelessEventContext #2999
Conversation
This is in preparation for using contexts that may or may not have the current_state_ids set. This will allow us to avoid unnecessarily pulling out state for an event on the master process when using workers.
The master process (usually) doesn't need the state at an event when it has been created by a worker process, so let's not automatically load the state in that case.
| @@ -763,9 +764,18 @@ def is_inviter_member_event(e): | |||
| e.sender == event.sender | |||
| ) | |||
|
|
|||
| # We get the current state at the event. If we have a full | |||
| # EventContext, use it, otherwise we hit the DB. | |||
| if isinstance(context, EventContext): | |||
There was a problem hiding this comment.
isintance is a bit nasty. Can we do something like get_state_ids() which is defined to return None if they are unknown?
| if ctx.state_group in state_groups_map: | ||
| continue | ||
|
|
||
| if isinstance(ctx, EventContext) and ctx.current_state_ids: |
There was a problem hiding this comment.
Isn't this largely used on the master? Will this do anything on a system with a split-out event creator worker? a comment here might be useful.
| # state group from its context. | ||
| # Otherwise we need to pull the state group from the database. | ||
| missing_event_ids = [] # List of events we need to fetch groups for | ||
| state_groups_to_resolve = set() # State groups of new_latest_event_ids |
There was a problem hiding this comment.
looks like it's only a subset of the state groups of new_latest_event_ids?
| missing_event_ids.append(event_id) | ||
|
|
||
| if not was_updated: | ||
| return | ||
|
|
||
| if missing_event_ids: | ||
| # Now pull out the state for any missing events from DB |
There was a problem hiding this comment.
we now only get the state group ids here
| @@ -504,61 +504,68 @@ def _get_new_state_after_events(self, room_id, events_context, new_latest_event_ | |||
| defer.returnValue({}) | |||
|
|
|||
| # map from state_group to ((type, key) -> event_id) state map | |||
| state_groups = {} | |||
There was a problem hiding this comment.
I'm slightly failing to follow how the new code relates to the old in this file, but it generally looks sane, and a cleanup which may well be worth shipping independently of StatelessEventContext?
|
(Superseded by #3550) |
The master process (usually) doesn't need the state at an event when it
has been created by a worker process, so let's not automatically load
the state in that case.