-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle large chain calc better #17291
Conversation
We calculate the auth chain links outside of the main persist event transaction to ensure that we do not block other event sending during the calculation.
So this doesn't seem to have helped too much in this case, example is from HQ I keep seeing this in my logs as well, but not sure if it's relevant |
Hi @morguldir, yup, this PR won't cause the event itself to get persisted faster. Instead it ensures other events (in other rooms) don't get blocked from being sent down to the client. |
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.
Just a small optimisation and a couple documentation changes, otherwise LGTM!
@@ -100,6 +99,14 @@ def is_noop(self) -> bool: | |||
return not self.to_delete and not self.to_insert and not self.no_longer_in_room | |||
|
|||
|
|||
@attr.s(slots=True, auto_attribs=True) | |||
class NewEventChainLinks: |
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.
Could you add a docstring to this object? I don't think it's particularly grokkable at a glance without going and seeing how it's used.
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.
Struggled with the wording, LMK if that makes sense.
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 had to re-read a couple times, but it's understandable 👍
It's certainly a fairly abstract concept to describe without a diagram.
@@ -100,6 +99,14 @@ def is_noop(self) -> bool: | |||
return not self.to_delete and not self.to_insert and not self.no_longer_in_room | |||
|
|||
|
|||
@attr.s(slots=True, auto_attribs=True) | |||
class NewEventChainLinks: |
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 had to re-read a couple times, but it's understandable 👍
It's certainly a fairly abstract concept to describe without a diagram.
This reverts commit bdf82ef.
We calculate the auth chain links outside of the main persist event transaction to ensure that we do not block other event sending during the calculation.
We calculate the auth chain links outside of the main persist event transaction to ensure that we do not block other event sending during the calculation.