Fix a bug and add a timeout test for takeover. (#1512)

src/server/replica.cc

@@ -235,12 +235,8 @@ std::error_code Replica::TakeOver(std::string_view timeout) {
   sock_->proactor()->Await(
       [this, &ec, timeout] { ec = SendNextPhaseRequest(absl::StrCat("TAKEOVER ", timeout)); });
 
-  if (ec) {
-    // TODO: Handle timeout more gracefully.
-    return cntx_.ReportError(ec);
-  }
-  // If we successfully taken over, return and let server_family stop us.
-  return {};
+  // If we successfully taken over, return and let server_family stop the replication.
+  return ec;
 }
 
 void Replica::MainReplicationFb() {

tests/dragonfly/replication_test.py

@@ -1179,3 +1179,41 @@ async def test_take_over_seeder(df_local_factory, df_seeder_factory, master_thre
 
         capture = await seeder.capture()
         assert await seeder.compare(capture, port=replica.port)
+
+
+@pytest.mark.asyncio
+async def test_take_over_timeout(df_local_factory, df_seeder_factory):
+    master = df_local_factory.create(proactor_threads=2,
+                                     port=BASE_PORT,
+                                     logtostderr=True)
+    replica = df_local_factory.create(
+        port=BASE_PORT+1, proactor_threads=2)
+    df_local_factory.start_all([master, replica])
+
+    seeder = df_seeder_factory.create(
+        port=master.port, keys=1000, dbcount=5, stop_on_failure=False)
+    async with (
+        master.client() as c_master,
+        replica.client() as c_replica,
+    ):
+        await c_replica.execute_command(f"REPLICAOF localhost {master.port}")
+        await wait_available_async(c_replica)
+
+        async def seed():
+            await seeder.run(target_ops=3000)
+
+        fill_task = asyncio.create_task(seed())
+
+        # Give the seeder a bit of time.
+        await asyncio.sleep(1)
+        try:
+            await c_replica.execute_command(f"REPLTAKEOVER 0.0001")
+        except redis.exceptions.ResponseError as e:
+            assert str(e) == "Couldn't execute takeover"
+        else:
+            assert False, "Takeover should not succeed."
+        seeder.stop()
+        await fill_task
+
+        assert await c_master.execute_command("role") == [b'master', [[b'127.0.0.1', bytes(str(replica.port), 'ascii'), b'stable_sync']]]
+        assert await c_replica.execute_command("role") == [b'replica', b'localhost', bytes(str(master.port), 'ascii'), b'stable_sync']

tests/dragonfly/server_family_test.py

@@ -40,7 +40,7 @@ change to match the fact that we supporting this operation.
 For now we are expecting to get an error
 '''
 
-@pytest.skip("Skip until we decided on correct behaviour of eval inside multi")
+@pytest.mark.skip("Skip until we decided on correct behaviour of eval inside multi")
 async def test_multi_eval(async_client: aioredis.Redis):
     try:
         pipeline = async_client.pipeline()