failed to set keepalive: connection in dubious state

[hcaihao]

trafficstars

local redis = require "resty.redis" local red = redis:new()

red:set_timeout(1000) -- 1 sec

local ok, err = red:connect("127.0.0.1", 6379) if not ok then ngx.say("failed to connect: ", err) return end

--ngx.say("connected to redis.")

local key = "testkey" red:set(key, false)

local ok, err = red:set_keepalive(10000, 100) if not ok then ngx.say("failed to set keepalive: ", err) return end

The code bellow throw an error: "failed to set keepalive: connection in dubious state"

I know redis set support string value only, But "red:set(key, true)" will be ok, why?

[agentzh]

@hcaihao You should always check the return values of the set() method call and handle any errors since this call can fail.

[hcaihao]

The problem is what's the difference between "red:set(key, true)" and "red:set(key, false)"?

[doujiang24]

@hcaihao

1. redis only have string type in kv, so you'd better to use red:set(key, "false") instead
2. false is not valid in resty.redis now, so redis return err: "ERR Protocol error: invalid bulk length" and send tcp close frame immediately, then the sock is now readable, this is why you got "connection in dubious state".

@agentzh May be we can just raise an error here? Seems we don't need to support this in send. Or tostring first? https://github.com/openresty/lua-resty-redis/blob/master/lib/resty/redis.lua#L260

[agentzh]

@hcaihao This is because the Lua false value maps to the redis NULL value on the protocol level, which is not supported by the redis set command at all and thus results in an error returned. That's why you should always handle any errors properly yourself.

@doujiang24 I'm not sure about that. We could raise an exception for this particular case but I still think people should always check return values of the redis queries anyway.

[doujiang24]

@agentzh Total agree people should always check return values. But I think the error ERR Protocol error: invalid bulk length is a higher level ERROR in redis, so redis close the connection. Other errors like ERR wrong number of arguments for 'set' command, redis won't close the connection.

So, maybe just always tostring first can be acceptable? https://github.com/openresty/lua-resty-redis/pull/79

[hcaihao]

@agentzh @doujiang24 Thk for your explanation! I see it.

[agentzh]

@doujiang24 Yeah, I'm fine with tostring when the input argument is not a string.