-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
We also process them in parallel like we do for PDUs.
This means that things like to device messages don't get blocked behind processing PDUs, which can potentially take *ages*.
2eee0b3
to
1269e28
Compare
Is this #5175 ? |
This still blocks responding to the |
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.
lgtm otherwise
run_in_background(self._handle_edus_in_txn, origin, transaction), | ||
], | ||
consumeErrors=True, | ||
).addErrback(unwrapFirstError) |
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'm not entirely sure how the error handling is working here. Will this raise an exception if one of the wrapped methods raise?
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 is basically a copy from: https://github.com/matrix-org/synapse/blob/master/synapse/util/async_helpers.py#L167-L186
The way defer.gatherResults
works is that:
consumeErrors=True
means that it won't report errors to the default "unhandled error" logginggatherResults
will raise aFirstError
that wrapse the actual exception. This is a bit annoying as we might be attempting to handle particular exceptions further up the stack, so we useunwrapFirstError
which does what it says on the tin.
Co-Authored-By: Andrew Morgan <[email protected]>
* commit 'b5ce7f587': Process EDUs in parallel with PDUs. (#6697)
This means that things like to device messages don't get blocked behind processing PDUs, which can potentially take ages if e.g. the server has been down for a while and has a lot to catch up on.