-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add support for sharding the client reader. #7830
Conversation
daa2c27
to
bae66a7
Compare
bae66a7
to
c11f7ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, could you update the docs/workers.md
too please?
Co-authored-by: Erik Johnston <erik@matrix.org>
@@ -40,7 +40,8 @@ def __init__(self, hs): | |||
|
|||
# Start the user parter loop so it can resume parting users from rooms where | |||
# it left off (if it has work left to do). | |||
hs.get_reactor().callWhenRunning(self._start_user_parting) | |||
if hs.config.worker_app is None: | |||
hs.get_reactor().callWhenRunning(self._start_user_parting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is worth pointing out also that we don't seem to use this pattern of callWhenRunning
anywhere else for background processes. I wonder if that method can just go away and we can directly call the code from it?
Not really related to this PR, just found it odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we tend to do loopings calls, which also won't run until we're started? I think it makes sense to only start work once we've finished starting up
@erikjohnston Not sure if you meant to approve this or not. 😄 |
Oh, I was waiting on words in |
* commit '457096e6d': Support handling registration requests across multiple client readers. (#7830)
This is mostly just tests, but does make one change to ensure that a background process only runs on a single process. I'm not 100% sure this is necessary, but I think it is.
Fixes #7672