-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Split out federation transaction sending to a worker #1635
Conversation
e2e94e3
to
88d85eb
Compare
|
||
# There should be only one reader, so lets delete everything its | ||
# acknowledged its seen. | ||
self._clear_queue_before_pos(token) |
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.
Might be worth trying to use a separate parameter for controlling the acks. For example there's a script in scripts-dev that tails all replication rows, if you ran it againtst synapse it might inadvertently delete all the federation queues. That could be awkward.
If you used a separate parameter for acknowledging like ack_federation
or something and didn't report it in the list of streams then I don't think this would be such a problem.
Although it might be that case that we are better off not exposing the federation
in the list of streams.
else: | ||
self.edus[pos] = edu | ||
|
||
def send_presence(self, destination, states): |
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 wonder if we send lots of identical messages to multiple dests. I wonder if it might be possible to reduce the duplication here if that is the case.
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.
Probably, but I think that can be dealt with later if necessary.
@NegativeMjark PTAL |
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
Or events that are sent via the federation "send_join" API. This should match the behaviour from before v0.18.5 and #1635 landed.
No description provided.