From a01a78080ccea6252e34e46b61f1bd66674418d3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Mar 2023 15:54:56 -0400 Subject: [PATCH 1/3] Remove no-op for Redis. --- synapse/replication/tcp/handler.py | 17 ----- .../replication/tcp/test_remote_server_up.py | 63 ------------------- 2 files changed, 80 deletions(-) delete mode 100644 tests/replication/tcp/test_remote_server_up.py diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index d03a53d76429..ad024380afb9 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -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) - def new_connection(self, connection: IReplicationConnection) -> None: """Called when we have a new connection.""" self._connections.append(connection) diff --git a/tests/replication/tcp/test_remote_server_up.py b/tests/replication/tcp/test_remote_server_up.py deleted file mode 100644 index b75fc05fd55b..000000000000 --- a/tests/replication/tcp/test_remote_server_up.py +++ /dev/null @@ -1,63 +0,0 @@ -# Copyright 2020 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from typing import Tuple - -from twisted.internet.address import IPv4Address -from twisted.internet.interfaces import IProtocol -from twisted.test.proto_helpers import MemoryReactor, StringTransport - -from synapse.replication.tcp.resource import ReplicationStreamProtocolFactory -from synapse.server import HomeServer -from synapse.util import Clock - -from tests.unittest import HomeserverTestCase - - -class RemoteServerUpTestCase(HomeserverTestCase): - def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: - self.factory = ReplicationStreamProtocolFactory(hs) - - def _make_client(self) -> Tuple[IProtocol, StringTransport]: - """Create a new direct TCP replication connection""" - - proto = self.factory.buildProtocol(IPv4Address("TCP", "127.0.0.1", 0)) - transport = StringTransport() - proto.makeConnection(transport) - - # We can safely ignore the commands received during connection. - self.pump() - transport.clear() - - return proto, transport - - def test_relay(self) -> None: - """Test that Synapse will relay REMOTE_SERVER_UP commands to all - other connections, but not the one that sent it. - """ - - proto1, transport1 = self._make_client() - - # We shouldn't receive an echo. - proto1.dataReceived(b"REMOTE_SERVER_UP example.com\n") - self.pump() - self.assertEqual(transport1.value(), b"") - - # But we should see an echo if we connect another client - proto2, transport2 = self._make_client() - proto1.dataReceived(b"REMOTE_SERVER_UP example.com\n") - - self.pump() - self.assertEqual(transport1.value(), b"") - self.assertEqual(transport2.value(), b"REMOTE_SERVER_UP example.com\n") From e60596d3f89d946ed2e5415488cfb87fc8e546cd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Mar 2023 15:57:39 -0400 Subject: [PATCH 2/3] Remove unused argument to send_command. --- synapse/replication/tcp/handler.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index ad024380afb9..2290b3e6fe7d 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -672,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: From 2f3c1c56e56a4fb27cbc5d3064e50d47fbac3797 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Mar 2023 16:01:02 -0400 Subject: [PATCH 3/3] Newsfragment --- changelog.d/15272.misc | 2 +- changelog.d/15274.misc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/15274.misc diff --git a/changelog.d/15272.misc b/changelog.d/15272.misc index 7a3ef323e9aa..f7c0276ecc22 100644 --- a/changelog.d/15272.misc +++ b/changelog.d/15272.misc @@ -1 +1 @@ -Remove unused class `DirectTcpReplicationClientFactory`. +Clean-up direct TCP replication code. diff --git a/changelog.d/15274.misc b/changelog.d/15274.misc new file mode 100644 index 000000000000..f7c0276ecc22 --- /dev/null +++ b/changelog.d/15274.misc @@ -0,0 +1 @@ +Clean-up direct TCP replication code.