Ericsson / ered

An Erlang client library for Valkey/Redis Cluster
MIT License
16 stars 8 forks source link

Don't use blocking recv #2

Open zuiderkwast opened 2 years ago

zuiderkwast commented 2 years ago

When gen_tcp:recv (or ssl:recv) is used, the process can't respond to supervisor events. We should use {active, once} or {active, N} instead.

We use one process to send data to a socket and another to receive the responses. Currently the sending process is the controlling process. We should make the receive process the controlling process instead, because gen_tcp:send can block.

drmull commented 2 years ago

Need to see if this has any performance impact when reading big bulk strings and similar. I am not sure if the reading process really needs to be part of the supervision tree or if it is ok to just put the gen_servers under supervision

zuiderkwast commented 2 years ago

Yes, we should try this in an isolated test: A minimal example program just sending data to a server that's streaming it back. Three versions would be nice:

  1. One process to send and passive receive from gen_tcp
  2. Two processes, active recv
  3. Two processes, passive recv

Supervision is not a must for any process, but it makes the design more robust. If it turns out version 2 is faster, I think we can live with spawn_link for the receiving process.

Btw, how likely is it than gen_tcp:send will block in our pipeline scenarios? It happens when the send buffer is full. Redis may be busy or the network is slow. If it's likely, it's another reason for using passive mode and the receiving process be controlling process.

drmull commented 2 years ago

Btw, how likely is it than gen_tcp:send will block in our pipeline scenarios?

The note on gen_tcp:send gives the impression that it will not block, at least not in the send buffer full scenario.

https://www.erlang.org/doc/man/gen_tcp.html#send-2

zuiderkwast commented 2 years ago

True, in only blocks if the socket backend is used. Maybe we want to support that in the future though.

ghost commented 2 years ago

The note on gen_tcp:send gives the impression that it will not block, at least not in the send buffer full scenario.

but

the function can hang even when using 'inet' backend if the internal buffers are full