Vikashvisu: Issue with #ec3 (List) Blocking retrieval

GitHub PR: Add delay in client connection to ensure order of connections by andy1li · Pull Request #217 · codecrafters-io/redis-tester · GitHub

@vikashvisu GitHub - vikashvisu/redis-trial: REDIS

Hey @vikashvisu, could you briefly walk me through how your code ensures the pushed value is delivered to the first BLPOP connection?

I added some logs and confirmed that your program is reading the commands in the correct order, but the two BLPOPs can be out of order:

Looks like there’s a race condition at play here:

Hey @andy1li Yes, there is a race condition. I have handled it using a queue. Check the code and the log below:

You can clearly see in the log that the client3 is being added first in the queue.

@vikashvisu Looks like the way popQueue is handled also contains a race condition:

@andy1li Does it enforce us to use the event loop and not one thread per client? or can this race be handled while using multithreading? If it can be handled, can you tell me, how?

I am able to achieve consistency by putting by the ‘BLPOP‘ command in a queue as soon as I read in the command. But it seems with multithreading, only a workaround is possible. and that too only if the time difference between two commands is a bit significant. If two clients simultaneously fire the blpop command or if some other variables come into play, then we cannot assert the order. In that case, implementing an event loop is a better option.

1 Like

@vikashvisu Unfortunately, your recent submissions did not pass the flaky test, which indicates there are concurrency issues remaining:

For reference, here’s an example solution that passes this stage consistently:

@andy1li I think you tested a previous push. I have pushed latest. can you check?

@vikashvisu Oh, I didn’t manually test any push today. The flaky tests are automatically run for every user submission.

Feel free to push a few more times. The concurrency issue will eventually surface.

@andy1li I have changed my complete implementation to use an event loop. Can you let me know the results of the flaky test?

@vikashvisu None of your submissions have failed the flaky test in the past four days! :tada:

Thanks @andy1li !!
I still have one issue. I am still using a separate thread for the communication between the master server and the replica server. But I would like to use the same event loop for the comm between the replica and the master. Do you have any good reference from where I can learn how to achieve this?

@vikashvisu What specific issues did you run into when trying to move the communication into the same event loop?

@andy1li Thanks for the help. All my issues are resolved. We can close this issue.

1 Like

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