tidwall / redcon

Redis compatible server framework for Go
MIT License
2.16k stars 159 forks source link

Redis CLI requires `COMMAND` command on connect #32

Closed blainsmith closed 3 years ago

blainsmith commented 4 years ago

While this is not technically a limitation of this library, but as I was implementing a server with redcon I noticed that my local redis-cli client v5.0.7 didn't seem to work with the redcon-based server I made. After doing some digging I noticed that the redis-cli issues a COMMAND command after a connection is established and it expects a response to work. Once I responded to that with redcon the official redis-cli worked fine. Not sure if you want to add this as a not in the README if people attempt to use it and it doesn't work as expected right away.

tidwall commented 4 years ago

I'm not able to reproduce. I just built 5.0.7 directly from github.com/antirez/redis and ran the redis-cli against the go example/clone.go in the redcon repo and it worked with no errors or warnings.

These were my steps.

Terminal 1

Redcon clone example

$ go get github.com/tidwall/redcon/example
$ example
2020/06/20 16:41:08 started server at :6380

Terminal 2

Build and run redis-cli 5.0.7

$ git clone https://github.com/antirez/redis
$ cd redis
$ git checkout 5.0.7
$ make
$ src/redis-cli -p 6380
127.0.0.1:6380>

This was from my Mac.

tidwall commented 4 years ago

I wonder if there's a build setting or a flag that adds that feature. Did you build your redis-cli or was it installed from a package manager?

blainsmith commented 4 years ago

Interesting I installed it via apt on Ubuntu. I will do some more digging based on what you found and see if I can come to a resolution. I had the same result you did with your clone example and my current redis-cli. Stay tuned!

blainsmith commented 4 years ago

Ah I found it. It wasn't the COMMAND command. If you remove the default case in the command switch redis-cli will not provide you with the prompt. I was able to reproduce this if you remove these lines from the clone example.

Missing default case

> redis-cli -p 6380           

With default case

> redis-cli -p 6380                                 
127.0.0.1:6380>
tidwall commented 4 years ago

Ah yes. That makes sense.

tidwall commented 3 years ago

I'm closing this for now, but feel free to reopen if needed. Thanks!