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

Remove no-op for Redis. #15274

Merged
merged 3 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion changelog.d/15272.misc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Remove unused class `DirectTcpReplicationClientFactory`.
Clean-up direct TCP replication code.
1 change: 1 addition & 0 deletions changelog.d/15274.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Clean-up direct TCP replication code.
26 changes: 1 addition & 25 deletions synapse/replication/tcp/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -625,23 +625,6 @@ def on_REMOTE_SERVER_UP(

self._notifier.notify_remote_server_up(cmd.data)

# We relay to all other connections to ensure every instance gets the
# notification.
#
# When configured to use redis we'll always only have one connection and
# so this is a no-op (all instances will have already received the same
# REMOTE_SERVER_UP command).
#
# For direct TCP connections this will relay to all other connections
# connected to us. When on master this will correctly fan out to all
# other direct TCP clients and on workers there'll only be the one
# connection to master.
#
# (The logic here should also be sound if we have a mix of Redis and
# direct TCP connections so long as there is only one traffic route
# between two instances, but that is not currently supported).
self.send_command(cmd, ignore_conn=conn)
Comment on lines -628 to -643
Copy link
Member Author

Choose a reason for hiding this comment

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

My reading of this comment is:

  • On Redis we only have a single connection.
  • When we get a REMOTE_SERVER_UP message we echo it back out to other connections, except ourselves.
  • From that I deduce this is a no-op (which the comment says).

We can then remove this code, the code testing this situation and the additional parameter to send_command.


def new_connection(self, connection: IReplicationConnection) -> None:
"""Called when we have a new connection."""
self._connections.append(connection)
Expand Down Expand Up @@ -689,21 +672,14 @@ def connected(self) -> bool:
"""
return bool(self._connections)

def send_command(
self, cmd: Command, ignore_conn: Optional[IReplicationConnection] = None
) -> None:
def send_command(self, cmd: Command) -> None:
"""Send a command to all connected connections.

Args:
cmd
ignore_conn: If set don't send command to the given connection.
Used when relaying commands from one connection to all others.
"""
if self._connections:
for connection in self._connections:
if connection == ignore_conn:
continue

try:
connection.send_command(cmd)
except Exception:
Expand Down
63 changes: 0 additions & 63 deletions tests/replication/tcp/test_remote_server_up.py

This file was deleted.