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.
richvdh
left a comment
There was a problem hiding this comment.
Looks plausible I guess. I'm surprised that there aren't going to be more places that get upset about these fields suddenly disappearing.
|
|
||
| # 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?
| 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
| 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.