synku / HiredisProxy

Embedded Redis client for Redis
MIT License
2 stars 0 forks source link

Use asynchronous RedisModule API #1

Open dvirsky opened 7 years ago

dvirsky commented 7 years ago

Hi. I'm Dvir from Redis Labs. Very nice module idea, are you submitting it to the hackathon?

One comment though: right now you are using the synchronous hiredis API and blocking the entire slave when you are proxying commands to the master. This is less than ideal of course.

You should:

  1. Use the hiredis async API, or use a side thread for this. (the first option is better, the latter is simpler to implement)

  2. Use the RedisModule_BlockClient and RedisModule_UnblockClient APIs to suspend the client issuing the proxy command, while keeping redis itself working on other tasks. You can read about it here: https://github.com/antirez/redis/blob/unstable/src/modules/BLOCK.md

Let me know if you need other help.

Keep up the good work and spread the Modules gospel!

Dvir

cc: @itamarhaber

synku commented 7 years ago

Hey Dvir - Yes async support would be a great improvement. Thank you very much for taking the time to provide those pointers, it will make this task much easier. I was not aware of the BLOCK/UNBLOCK mechanism, this is very helpful. Also unaware of the hackathon and will check it out ;-)

Best Jerome

dvirsky commented 7 years ago

@synku thanks.

I just wrote a POC of a cross-shard command framework for modules. It's not really ready, but if you want an example of using the block/unblock API, plus using hiredis with a libuv io loop on a background thread - it's all there! :)

Pitfalls to avoid:

  1. Use -Bsymbolic -Bsymbolic-functions as linker flags. otherwise symbols from redis' internal hiredis lib will cause a complete mess and your code will crash. See the Makefile. this cost me a lot of time :)
  2. do not trigger redis async stuff from the module's thread, it's not thread safe. see how I used uv_queue_work to synchronize the two threads.
  3. notice that when you unblock the client, if you send the unblock handler the hiredis response, it won't work - hiredis deletes the response object, you need to copy it.

https://github.com/RedisLabs/rmr

itamarhaber commented 7 years ago

@synku wrt hackathon, the submission's deadline's 4 days away, tick tock, no rush :clock1130: :)