Rueidis does not work with kvrocks 2.2.0

[FZambia]

Rueidis worked with kvrocks before, but does not work with its new release:

docker run -it --rm -p 6666:6666 apache/kvrocks:2.2.0

Test fails:

go test -run TestKvrocksSingleClientIntegration
panic: redis message type * is not a RESP3 map

goroutine 22 [running]:
github.com/rueian/rueidis.(*RedisMessage).ToMap(0xc0000e39e0?)
    /Users/fz/projects/FZambia/rueidis/message.go:725 +0xce
github.com/rueian/rueidis.RedisResult.ToMap({{0x0, 0x0}, {0x2a, {0x0, 0x0}, {0xc000226000, 0x6, 0x6}, 0x0, 0x0}})
    /Users/fz/projects/FZambia/rueidis/message.go:286 +0x11f
github.com/rueian/rueidis._newPipe(0xc0000acab0, 0xc00011a000, 0x0)
    /Users/fz/projects/FZambia/rueidis/pipe.go:142 +0x1a3b
github.com/rueian/rueidis.newPipe(...)
    /Users/fz/projects/FZambia/rueidis/pipe.go:73
github.com/rueian/rueidis.makeMux.func1()
    /Users/fz/projects/FZambia/rueidis/mux.go:56 +0xc6
github.com/rueian/rueidis.(*mux)._pipe(0xc00011c000, 0x0)
    /Users/fz/projects/FZambia/rueidis/mux.go:113 +0x38b
github.com/rueian/rueidis.(*mux).Dial(0x0?)
    /Users/fz/projects/FZambia/rueidis/mux.go:141 +0x1b
github.com/rueian/rueidis.(*clusterClient).init.func1({0x13970ed, 0xe}, {0x1449620, 0xc00011c000})
    /Users/fz/projects/FZambia/rueidis/cluster.go:59 +0x48
created by github.com/rueian/rueidis.(*clusterClient).init
    /Users/fz/projects/FZambia/rueidis/cluster.go:58 +0x305
exit status 2
FAIL    github.com/rueian/rueidis   0.182s

Redigo works fine with new kvrocks release. I suppose sth goes wrong with RESP2/RESP3 detection.

If the automatic detection becomes too tricky – then probably an option to force RESP2 or RESP3 could make sense.

[rueian]

Hi @FZambia,

Yes, it went wrong with RESP2/RESP3 detection.

Thankfully, the automatic detection won't be too tricky. I had fixed it in https://github.com/rueian/rueidis/commit/9e18fa61686f5c83e12575906e554c182cfa674e and would release it this weekend.

[FZambia]

Thanks!

One more thing with it is that for some reason:

func (r *RedisError) IsNoScript() bool {
    return strings.HasPrefix(r.string, "NOSCRIPT")
}

does not work, the r.string is ERR NOSCRIPT No matching script. Please use EVAL in case of kvrocks. For real Redis it's NOSCRIPT No matching script. Please use EVAL.

[FZambia]

Yeah... :(

kvrocks:

❯ redis-cli
127.0.0.1:6379> evalsha 1212 1 t
(error) ERR NOSCRIPT No matching script. Please use EVAL

Redis:

❯ redis-cli
127.0.0.1:6379> evalsha 1212 1 t
(error) NOSCRIPT No matching script. Please use EVAL.

[FZambia]

It's actually interesting that Redigo works fine, even though it uses the same HasPrefix check https://github.com/gomodule/redigo/blob/d6854479365f0307560fa28e18e2bd0634b05229/redis/script.go#L82

Maybe it strips ERR prefix somewhere...

[FZambia]

https://github.com/apache/incubator-kvrocks/issues/485 mentions this.

[FZambia]

It's actually interesting that Redigo works fine, even though it uses the same HasPrefix check

Found the reason - that's because in Redigo implementation I preloaded scripts manually on start.

[FZambia]

I was linked that in go-redis error messages are stripped to work with kvrocks: https://github.com/go-redis/redis/blob/4bda6ec2fb4a836bbffc0032ad3a0f2f79528ccb/error.go#L23. Not sure whether this should be added to rueidis too or I better fix this on the application level for now?

[rueian]

Hi @FZambia,

I think just removing the ERR prefix is a good idea. It is not only fast but also unified error message handling. I believe no one would rely on the meaningless ERR prefix to identify redis error, therefore just removing it is probably safe.

This has been addressed in https://github.com/rueian/rueidis/commit/e0b466d32a46e56d9d0b1537831922e08f511119 along with new Lua integration tests.

[FZambia]

Think it's safe yep, thanks - this will simplify my life a bit.

[FZambia]

Checked - everything works fine now, many thanks!