Prevent account_data content from being sent over TCP replication

[anoadragon453]

There was an issue with changes to account_data not being reflected upon a /sync request from a client, specifically changes to m.direct. The client would send a PUT /.../m.direct, but never receive the new version down /sync.

This seemed to only happen on servers running in worker mode for large user accounts. It turns out that m.direct for large user accounts is huge, literally a dictionary for every user_id and room_id for a direct chat that your account has.

When a change is made on the master process, the updated account_data entry is replicated over to synchrotron workers, to tell them that they need to send down a new copy to clients when they ask for it. Unfortunately, the content of this replication was so large that twisted would deny it and drop the request:

Traceback (most recent call last):
  File "/home/ops/.synapse3/env3/lib/python3.5/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
    result = g.send(result)
StopIteration: ([(505374, ('@andrewm:amorgan.xyz', None, 'm.direct', '{"@xxx:matrix.org": ["!WUAgSOAjsAMjDlDkhf:amorgan.xyz", "!uLpJrldJgpBjerWFJD:matrix.org", "!aSlnrTeGrTkKwziiij:matrix.org"], "@yyy:matrix.org": ["!yZHTGeDKZUeKaqeTeU:matrix.org"], <truncated>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/ops/.synapse3/env3/lib/python3.5/site-packages/synapse/replication/tcp/resource.py", line 225, in _run_notifier_loop
    conn.stream_update(stream.NAME, token, row)
  File "/home/ops/.synapse3/env3/lib/python3.5/site-packages/synapse/replication/tcp/protocol.py", line 544, in stream_update
    self.send_command(RdataCommand(stream_name, token, data))
  File "/home/ops/.synapse3/env3/lib/python3.5/site-packages/synapse/replication/tcp/protocol.py", line 293, in send_command
    % (cmd.NAME, len(encoded_string), self.MAX_LENGTH)
Exception: Failed to send command RDATA as too long (21544 > 16384)

This means the synchrotron would never get the update, and never know to send the updated account_data entry to clients.

Instead, since the account_data cache on the synchrotron is already invalidated through replication, we can simply stop sending the content of account_data.m.direct over instead. Replications will, the synchrotron will know to pull m.direct data out of the database a fresh, and everything works again.

[erikjohnston]

To be honest I'd be tempted to only send the account_data key name, and not the contents. Workers should hopefully be able to pull that from the database as necessary. Ideally each item sent down the stream should be well bounded.

Throwing huge JSON blobs to workers is probably not going to help the performance any.

[anoadragon453]

To be honest I'd be tempted to only send the account_data key name, and not the contents. Workers should hopefully be able to pull that from the database as necessary. Ideally each item sent down the stream should be well bounded.

Throwing huge JSON blobs to workers is probably not going to help the performance any.

Not a bad point. I'll check what's involved in that.

[anoadragon453]

Looks like:

synapse/synapse/replication/tcp/streams/_base.py

Lines 169 to 173 in 998f7fe

	Returns:
	Deferred[Tuple[List[Tuple[int, Any]], int]:
	Resolves to a pair ``(updates, current_token)``, where ``updates`` is a
	list of ``(token, row)`` entries. ``row`` will be json-serialised and
	sent over the replication steam.

is where the data is taken out of the database. This seems like it's fundamental to a lot of things that get sent over replication, and might be kinda scary to change. Will investigate more tomorrow.