Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Split out federation transaction sending to a worker #1635

Merged
merged 22 commits into from
Nov 23, 2016

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Nov 21, 2016

No description provided.

@erikjohnston erikjohnston changed the title Erikj/split out fed txn Split out federation transaction sending to a worker Nov 21, 2016
@erikjohnston erikjohnston force-pushed the erikj/split_out_fed_txn branch from e2e94e3 to 88d85eb Compare November 21, 2016 17:37

# There should be only one reader, so lets delete everything its
# acknowledged its seen.
self._clear_queue_before_pos(token)
Copy link
Contributor

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):
Copy link
Contributor

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.

Copy link
Member Author

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.

@erikjohnston
Copy link
Member Author

@NegativeMjark PTAL

Copy link
Contributor

@NegativeMjark NegativeMjark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@erikjohnston erikjohnston merged commit 302fbd2 into develop Nov 23, 2016
@richvdh richvdh deleted the erikj/split_out_fed_txn branch December 1, 2016 14:09
NegativeMjark pushed a commit that referenced this pull request Jan 5, 2017
Or events that are sent via the federation "send_join" API.

This should match the behaviour from before v0.18.5 and #1635 landed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants