Replication Stage(#HD5) Same listening ports used by 3 replicas?

Hello everyone, I am doubtful whether test implementation for Replication stage 12 (Multi Replica Command Propagation).

Implementation creates 3 replica connections to master and makes a handshake from each connection using the same listening-port parameter.
I wonder whether it’s correct to use the same port? Does real redis pass this test?
I don’t understand why would it replicate commands to the same replica multiple times, shouldn’t it “store” information about “replication targets” in a “hashmap”, where key is derived solely from listening address and not from address of opened tcp connection?

As i understand MASTER do not open any additional connection at least in situation of simple replication. Maybe it will be used in Cluster variant or Sentinel.
Anyway port is not enough, pair of incoming address and port should be unique.

1 Like

Yeah, that makes a lot of sense, if incoming address + ‘listening-address’ should be unique then test is defined incorrectly

We’ve got this noted here: Multiple replicas stage - all replicas advertise the same "listening" port · Issue #153 · codecrafters-io/build-your-own-redis · GitHub.

Note that Redis doesn’t use listening-port to identify a replica, it just uses the port / address of the replication TCP connection.

Listening port can change over a time (the port of a redis server can be changed at runtime), so it doesn’t really uniquely identify a replica. The replication TCP connection on the other hand does uniquely identify a replica as long as the replica only keeps one replication connection open at a time.

1 Like

I’ll keep this open until we’ve fixed the listening ports to be random!

1 Like

This is updated now! fix: set different listening-port for multi replicas spawned simultaneously. by ryan-gang · Pull Request #135 · codecrafters-io/redis-tester · GitHub

Note: I’ve updated the title of this post to include the stage ID (#HD5). You can learn about the stages rename here: Upcoming change: Stages overhaul