From a2085b1f41715f59757cdf580a714c3a49dbcbc9 Mon Sep 17 00:00:00 2001 From: shahar Date: Tue, 24 Sep 2024 14:21:48 +0300 Subject: [PATCH 1/2] chore: Forbid replicating a replica We do not support connecting a replica to a replica, but before this PR we allowed doing so. This PR disables that behavior. Fixes #3679 --- src/server/server_family.cc | 4 ++++ tests/dragonfly/replication_test.py | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/server/server_family.cc b/src/server/server_family.cc index 58a6fbce36fd..b4c8cdf9252b 100644 --- a/src/server/server_family.cc +++ b/src/server/server_family.cc @@ -2839,6 +2839,10 @@ void ServerFamily::ReplTakeOver(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::ReplConf(CmdArgList args, ConnectionContext* cntx) { + if (!ServerState::tlocal()->is_master) { + return cntx->SendError("Replicating a replica is unsupported"); + } + if (args.size() % 2 == 1) goto err; for (unsigned i = 0; i < args.size(); i += 2) { diff --git a/tests/dragonfly/replication_test.py b/tests/dragonfly/replication_test.py index 95cf423974ef..dfc63bcad46c 100644 --- a/tests/dragonfly/replication_test.py +++ b/tests/dragonfly/replication_test.py @@ -2663,3 +2663,25 @@ async def check_master_status(): assert await seeder.compare(capture, port=master.port) await disconnect_clients(c_master, c_replica) + + +@pytest.mark.asyncio +async def test_replica_of_replica(df_factory): + # Can't connect a replica to a replica, but OK to connect 2 replicas to the same master + master = df_factory.create(proactor_threads=2) + replica = df_factory.create(proactor_threads=2) + replica2 = df_factory.create(proactor_threads=2) + + df_factory.start_all([master, replica, replica2]) + + c_replica = replica.client() + c_replica2 = replica2.client() + + assert await c_replica.execute_command(f"REPLICAOF localhost {master.port}") == "OK" + + with pytest.raises(redis.exceptions.ResponseError): + await c_replica2.execute_command(f"REPLICAOF localhost {replica.port}") + + assert await c_replica2.execute_command(f"REPLICAOF localhost {master.port}") == "OK" + + await disconnect_clients(c_replica, c_replica2) From 116ce0902091307df6d9dcc82abca6cfcd9d31d0 Mon Sep 17 00:00:00 2001 From: shahar Date: Tue, 24 Sep 2024 16:11:15 +0300 Subject: [PATCH 2/2] `replicaof_mu_` --- src/server/server_family.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/server/server_family.cc b/src/server/server_family.cc index b4c8cdf9252b..e01cd2a9df95 100644 --- a/src/server/server_family.cc +++ b/src/server/server_family.cc @@ -2839,8 +2839,11 @@ void ServerFamily::ReplTakeOver(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::ReplConf(CmdArgList args, ConnectionContext* cntx) { - if (!ServerState::tlocal()->is_master) { - return cntx->SendError("Replicating a replica is unsupported"); + { + util::fb2::LockGuard lk(replicaof_mu_); + if (!ServerState::tlocal()->is_master) { + return cntx->SendError("Replicating a replica is unsupported"); + } } if (args.size() % 2 == 1)