Incorrect number of replica count in #na2

Challenges
1

Hi.

For some reason, I am returning an incorrect number of replicas.

The canonical implementation redis/src/replication.c at unstable · redis/redis · GitHub returns immediately if the stored replicas offset equals to the current master offset.

I’m doing the same:

 if replicas_in_sync >= numreplicas  {
         // we can return immediately
         info!("{} > {}, returning immediately.", replicas_in_sync, numreplicas);
         let _ = respond_to.send(Some(vec![
                (RespValue::Integer(replicas_in_sync as i64)),
          ]));
}

however, I see the tester only replies with 1 replica and expects 1 as a reply:

remote: [tester::#NA2] [test] client: Received bytes: “:4\r\n”
remote: [tester::#NA2] [test] client: Received RESP integer: 4
remote: [tester::#NA2] Expected 1, got 4
remote: [tester::#NA2] Test failed

so, is there a difference between real redis and how the testers handle this scenario?

2

Hey @devfire! Just for context, our tester has been tested thoroughly against the official redis, so it’s possible but very unlikely that it handles things differently.

I tried to run your code using codecrafters test --previous, and it could not pass a previous stage ACKs with commands #yd3 anymore:

image
image2110×742 298 KB

Since this could impact later stages, it might be better to fix #yd3 first.

3

Oh this is a good call out – absolutely.

4

@andy1li - there’s no way to do a full regression suite, is there? Even if targeting specific stage one at a time? I’m always worried about accidentally breaking something that used to work before.

5

Sorry, but our CLI doesn’t currently support running tests for specific stages. There are only two modes for now:

  1. codecrafters test runs through the stages in the descending order.
  2. codecrafters test --previous in the ascending order. This could be used as a substitute for a full regression suite.

For the Redis challenge, there might be too many stages to test with the --previous flag, but you can temporarily disable some of the extensions that you don’t need at the moment.

1 Like
6

This topic was automatically closed 5 days after the last reply. New replies are no longer allowed.