Add support for Unix sockets #4
Conversation
|
This is a neat idea. I tend to use either hosted or containerized Redis instances in production so I hadn't thought about using Unix sockets. Is running Redis on the same box a common production use case for you? |
|
You can also use Unix sockets with local Docker containers. That is a scenario that I faced in production. I have not done any benchmarks with the Crystal client, but generally it should bring a small performance improvement. At least with redis-cli and NodeJs (ioredis), I could confirm a slightly improved throughput. Unfortunately, I don't have a minimal example to share, but this article describes how Unix Sockets can be used with Docker: https://medium.com/@jonbaldie/how-to-connect-to-redis-with-unix-sockets-in-docker-9e94e01b7acd |
There was a problem hiding this comment.
Was thinking about this PR again this morning randomly. It looks like the union of types becomes exactly what the Postgres driver uses, so that's comforting. Just a couple questions about the implementation.
| # Check whether we should use SSL | ||
| if uri.scheme == "rediss" | ||
| socket = OpenSSL::SSL::Socket::Client.new(socket) | ||
| if uri.scheme == "socket" |
There was a problem hiding this comment.
Thinking about this a bit more, I think the socket URL scheme may be a bit too generic a name — they're all sockets. 😄 Do you think it might make more sense to use unix as the URL scheme?
There was a problem hiding this comment.
I see the point. Unfortunately, there is not a standard among the Redis clients to build on. Redis itself (e.g. in redis.conf) refers to it as "unixsocket". Maybe that is the clearest?
It is also more consistent with the other existing Redis client for Crystal: https://github.com/stefanwille/crystal-redis/blob/009e35fa40bbd518798afcc32c397402c6c6acb2/src/redis/connection.cr#L8
There was a problem hiding this comment.
Apologies for the wildly delayed reply here. I seem to have pulled this move:
If the Redis server recommends unixsocket, I imagine that's what most Redis clients also use, and consistency can be pretty important. Admittedly, since I almost always use TCP sockets for IPC even when Unix sockets are available, I don't have many personal opinions on how to use Unix sockets so my only real recommendation is least-surprise. 🙂
| if uri.scheme == "socket" | ||
| socket = UNIXSocket.new(uri.path) | ||
| socket.sync = false | ||
| db = "0" |
There was a problem hiding this comment.
I see why you did it this way, but it seems like it would be impossible to use a DB slot other than 0 while using a Unix socket.
There was a problem hiding this comment.
Have to admit I only learned later what DB slots are. But agreed that it does not make sense to exclude the functionality depending on the connection type without any need.
I'll change it when I have some time.
There was a problem hiding this comment.
Following along the same lines as the other reply, if other Redis clients have opinions on how to select a different DB with a Unix socket URL, I'm happy to follow suit here.
URIs like this
socket:/path/to/redis.sockwill be treated as Unix Sockets.