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

Move USER_IP replication command to a separate redis pub/sub channel. #12460

Closed
Tracked by #12463 ...
erikjohnston opened this issue Apr 13, 2022 · 0 comments · Fixed by #12809
Closed
Tracked by #12463 ...

Move USER_IP replication command to a separate redis pub/sub channel. #12460

erikjohnston opened this issue Apr 13, 2022 · 0 comments · Fixed by #12809
Assignees
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@erikjohnston
Copy link
Member

erikjohnston commented Apr 13, 2022

Currently all replication commands get sent to all workers via the single pub/sub stream. This can be a lot of traffic, roughly evenly split between USER_IP and RDATA commands.

However, only a subset of workers care about USER_IP commands, and the rest just immediately drop them on receipt:

if self._is_master or self._should_insert_client_ips:
# We make a point of only returning an awaitable if there's actually
# something to do; on_USER_IP is not an async function, but
# _handle_user_ip is.
# If on_USER_IP returns an awaitable, it gets scheduled as a
# background process (see `BaseReplicationStreamProtocol.handle_command`).
return self._handle_user_ip(cmd)
else:
# Returning None when this process definitely has nothing to do
# reduces the overhead of handling the USER_IP command, which is
# currently broadcast to all workers regardless of utility.
return None

Instead of sending them to the same pub/sub channel, we should send USER_IP commands to a separate channel that is only subscribed to by workers that need them.

This would involve changing the stream name we send the USER_IP command to here:

self.synapse_outbound_redis_connection.publish(
self.synapse_stream_name, encoded_string
)

And change the channels we subscribe to:

await make_deferred_yieldable(self.subscribe(self.synapse_stream_name))

@erikjohnston erikjohnston changed the title Move USER_IP replication command to a separate Move USER_IP replication command to a separate redis stream. Apr 13, 2022
@erikjohnston erikjohnston added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Apr 13, 2022
@erikjohnston erikjohnston changed the title Move USER_IP replication command to a separate redis stream. Move USER_IP replication command to a separate redis pub/sub channel. Apr 13, 2022
@reivilibre reivilibre self-assigned this May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
2 participants