Split out federation transaction sending to a worker #1635

Merged
merged 22 commits into from Nov 23, 2016

Projects

None yet

2 participants

@erikjohnston
Member
erikjohnston commented Nov 21, 2016 edited

No description provided.

erikjohnston added some commits Nov 4, 2016
@erikjohnston erikjohnston Add initial cut of federation send queue
1587b5a
@erikjohnston erikjohnston Rename transaction queue functions to send_* 0e830d3
@erikjohnston erikjohnston Move logic into transaction_queue daec6fc
@erikjohnston erikjohnston Add transaction queue and transport layer to DI 847d5db
@erikjohnston erikjohnston Use new federation_sender DI 59ef517
@erikjohnston erikjohnston Hook up the send queue and create a federation sender worker ed787cf
@erikjohnston erikjohnston Handle sending events and device messages over federation f8ee662
@erikjohnston erikjohnston Store federation stream positions in the database 7c9cdb2
@erikjohnston erikjohnston Fix tests
524d61b
@erikjohnston erikjohnston changed the title from Erikj/split out fed txn to Split out federation transaction sending to a worker Nov 21, 2016
erikjohnston added some commits Nov 21, 2016
@erikjohnston erikjohnston Remove explicit calls to send_pdu
9687e03
@erikjohnston erikjohnston Comments
50934ce
@erikjohnston erikjohnston Add some metrics
88d85eb
@erikjohnston erikjohnston Add federation-sender to sytest
73dc099
synapse/federation/send_queue.py
+
+ # There should be only one reader, so lets delete everything its
+ # acknowledged its seen.
+ self._clear_queue_before_pos(token)
@NegativeMjark
NegativeMjark Nov 21, 2016 Contributor

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.

@erikjohnston erikjohnston Comments
51e8970
+ else:
+ self.edus[pos] = edu
+
+ def send_presence(self, destination, states):
@NegativeMjark
NegativeMjark Nov 21, 2016 Contributor

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.

@erikjohnston
erikjohnston Nov 23, 2016 Member

Probably, but I think that can be dealt with later if necessary.

erikjohnston added some commits Nov 22, 2016
@erikjohnston erikjohnston Invalidate retry cache in both directions
90565d0
@erikjohnston erikjohnston Fix tests and flake8
54fed21
@erikjohnston erikjohnston Explicit federation ack
4c79a63
@erikjohnston erikjohnston Comment
4d9b5c6
@erikjohnston erikjohnston Merge branch 'develop' of github.com:matrix-org/synapse into erikj/sp…
…lit_out_fed_txn
b69f76c
@erikjohnston erikjohnston Ensure only main or federation_sender process can send federation tra…
…ffic
26072df
@erikjohnston erikjohnston Fix tests
ee5e8d7
@erikjohnston
Member
@erikjohnston erikjohnston Shuffle receipt handler around so that worker apps don't need to load it
feec718
@NegativeMjark

LGTM

@erikjohnston erikjohnston merged commit 302fbd2 into develop Nov 23, 2016

8 of 10 checks passed

Sytest SQLite (Commit) Build #2035 origin/erikj/split_out_fed_txn failed in 8 min 50 sec
Details
Sytest Dendron (Merged PR) Build finished.
Details
Flake8 + Packaging (Commit) Build #2086 origin/erikj/split_out_fed_txn succeeded in 40 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Dendron (Commit) Build #1152 origin/erikj/split_out_fed_txn succeeded in 10 min
Details
Sytest Postgres (Commit) Build #1989 origin/erikj/split_out_fed_txn succeeded in 6 min 50 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #2117 origin/erikj/split_out_fed_txn succeeded in 2 min 18 sec
Details
Unit Tests (Merged PR) Build finished.
Details
@richvdh richvdh deleted the erikj/split_out_fed_txn branch Dec 1, 2016
@NegativeMjark NegativeMjark added a commit that referenced this pull request Jan 5, 2017
@NegativeMjark NegativeMjark Only send events that originate on this server.
Or events that are sent via the federation "send_join" API.

This should match the behaviour from before v0.18.5 and #1635 landed.
f784980
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment