From 0019d1afc70272a270de3f2564b2534ee0758fed Mon Sep 17 00:00:00 2001 From: shushen Date: Fri, 18 Sep 2026 17:50:29 +0900 Subject: [PATCH 1/3] feature: connect() options db/password/username/tcp_keepalive, hmget() with a table, and two bug fixes. * bugfix: integer arguments with 15 or more digits were sent in scientific notation because tostring() formats numbers with %.14g (#135). * bugfix: a connection with an open MULTI could be put back into the connection pool; set_keepalive() now returns nil and "in transaction" until EXEC or DISCARD (#176). * feature: connect() options "db", "password" and "username", applied to newly established connections only, with a separate connection pool per database and ACL user (#53). * feature: connect() option "tcp_keepalive" enabling SO_KEEPALIVE (#263). * feature: hmget() accepts a table of fields, like hmset() (#29). * doc: error handling and connection lifecycle, ACL authentication, multi-word commands, pool size semantics (#147, #150, #151, #173, #174, #180, #206, #220, #234, #256, #265). * bumped _VERSION to 0.34 (v0.33 is already tagged). --- README.markdown | 82 ++++++++++- lib/resty/redis.lua | 143 ++++++++++++++++++- t/bugs.t | 97 +++++++++++++ t/connect-opts.t | 338 ++++++++++++++++++++++++++++++++++++++++++++ t/count.t | 2 +- t/hmset.t | 51 +++++++ t/transaction.t | 217 ++++++++++++++++++++++++++++ 7 files changed, 920 insertions(+), 10 deletions(-) create mode 100644 t/connect-opts.t diff --git a/README.markdown b/README.markdown index 8ddfecf..abf31b2 100644 --- a/README.markdown +++ b/README.markdown @@ -22,6 +22,7 @@ Table of Contents * [commit_pipeline](#commit_pipeline) * [cancel_pipeline](#cancel_pipeline) * [hmset](#hmset) + * [hmget](#hmget) * [array_to_hash](#array_to_hash) * [read_reply](#read_reply) * [add_commands](#add_commands) @@ -30,6 +31,7 @@ Table of Contents * [Load Balancing and Failover](#load-balancing-and-failover) * [Debugging](#debugging) * [Automatic Error Logging](#automatic-error-logging) +* [Error Handling and Connection Lifecycle](#error-handling-and-connection-lifecycle) * [Check List for Issues](#check-list-for-issues) * [Limitations](#limitations) * [Installation](#installation) @@ -96,6 +98,9 @@ Synopsis ok, err = red:set("dog", "an animal") if not ok then ngx.say("failed to set dog: ", err) + -- the connection is closed automatically when the + -- request ends, see "Error Handling and Connection + -- Lifecycle" below return end @@ -183,6 +188,16 @@ Similarly, the "LRANGE" redis command accepts three arguments, then you should c For example, "SET", "GET", "LRANGE", and "BLPOP" commands correspond to the methods "set", "get", "lrange", and "blpop". +The methods are generated on demand, so every Redis command works this way, including commands newer than this library and multi-word commands (the sub-command is just the first argument): + +```lua + -- CLIENT SETNAME myapp + local res, err = red:client("setname", "myapp") + + -- BITFIELD mykey GET u8 0 + local res, err = red:bitfield("mykey", "get", "u8", 0) +``` + Here are some more examples: ```lua @@ -249,7 +264,25 @@ The optional `options_table` argument is a Lua table holding the following keys: * `pool` - Specifies a custom name for the connection pool being used. If omitted, then the connection pool name will be generated from the string template `:` or ``. + Specifies a custom name for the connection pool being used. If omitted, then the connection pool name will be generated from the string template `:` or ``, followed by `/` when the `username` option is in effect and `/` when the `db` option is given (for example `127.0.0.1:6379/1` or `127.0.0.1:6379/alice/2`), so that connections to different databases or ACL users never share a pool. The `password` is never part of the pool name: every caller sharing a pool must authenticate the same way, or specify its own `pool`. + +* `db` + + Selects the given Redis database (a number) with the `SELECT` command right after a *new* connection is established. Connections reused from the connection pool are left untouched, which is why such connections get their own pool (see `pool` above). Omit this option for the default database `0`. If `SELECT` fails, `connect` closes the connection and returns `nil` plus the error string `"failed to select database : "`. + +* `password` + + Authenticates a *new* connection with the `AUTH` command before `SELECT` (if any). Connections reused from the connection pool are left untouched. An empty string means no authentication. If `AUTH` fails, `connect` closes the connection and returns `nil` plus the error string `"failed to authenticate: "`. See also [Redis Authentication](#redis-authentication). + +* `username` + + The Redis ACL user name to authenticate as (`AUTH `, Redis 6.0+). Only used together with `password`; an empty string means the default user. + +* `tcp_keepalive` + + If set to true, enables TCP keepalive (`SO_KEEPALIVE`) on the connection, whether newly established or reused from the pool. This lets an idle connection, typically one blocked in [read_reply](#read_reply) for Pub/Sub messages, notice a silently dropped peer. Only `SO_KEEPALIVE` is set: the probe timing comes from the operating system (on Linux `net.ipv4.tcp_keepalive_time`, 7200 seconds by default), so tune those kernel parameters or send a periodic `PING` if you need faster detection. Requires the `setoption` cosocket method ([ngx_lua 0.10.18](https://github.com/openresty/lua-nginx-module/tags) with lua-resty-core, the default since OpenResty 1.15.8.1). If it cannot be enabled, `connect` closes the connection and returns `nil` plus the error string `"failed to enable tcp keepalive: "`. + +Note that these keys are read from the whole `options_table`: a table that already carries a `db`, `password`, `username` or `tcp_keepalive` key for other purposes now has that meaning here. * `pool_size` @@ -298,6 +331,8 @@ In case of success, returns `1`. In case of errors, returns `nil` with a string Only call this method in the place you would have called the `close` method instead. Calling this method will immediately turn the current redis object into the `closed` state. Any subsequent operations other than `connect()` on the current object will return the `closed` error. +A connection with an open transaction (after `multi` but before `exec` or `discard`) cannot be put into the pool: this method returns `nil` and the error string `"in transaction"` and the connection stays open. Call `exec`, `discard` or `close` first. Note that `WATCH` is not tracked: `unwatch` (or `close`) before keeping such a connection alive. + [Back to TOC](#table-of-contents) get_reused_times @@ -371,6 +406,19 @@ itself), then the last argument must be a Lua table holding all the field/value [Back to TOC](#table-of-contents) +hmget +----- +`syntax: res, err = red:hmget(myhash, field1, field2, ...)` + +`syntax: res, err = red:hmget(myhash, { field1, field2, ... })` + +Special wrapper for the Redis "hmget" command. + +When there are only three arguments (including the "red" object +itself) and the last one is a Lua table, the table is taken as the list of fields. The values are returned as a Lua table in the same order as the fields, exactly as for the plain form. + +[Back to TOC](#table-of-contents) + array_to_hash ------------- `syntax: hash = red:array_to_hash(array)` @@ -519,7 +567,17 @@ password `foobared` in the `redis.conf` file: If the password specified is wrong, then the sample above will output the following to the HTTP client: - failed to authenticate: ERR invalid password + failed to authenticate: WRONGPASS invalid username-password pair or user is disabled. + +(`ERR invalid password` on Redis servers older than 6.0.) + +To authenticate as a Redis ACL user (Redis 6.0+), pass the user name first: + +```lua + local res, err = red:auth("alice", "foobared") +``` + +Instead of calling `auth` yourself on every new connection, you can also pass the `password` (and `username`) options to [connect](#connect), which runs `AUTH` only for connections that do not come from the connection pool. [Back to TOC](#table-of-contents) @@ -575,6 +633,8 @@ Then the output will be set ans: "QUEUED" exec ans: ["OK",[false,"ERR Operation against a key holding the wrong kind of value"]] +A connection cannot be put back into the connection pool while a transaction is open, see [set_keepalive](#set_keepalive). + [Back to TOC](#table-of-contents) Redis Module @@ -651,6 +711,24 @@ handling in your own Lua code, then you are recommended to disable this automati [Back to TOC](#table-of-contents) +Error Handling and Connection Lifecycle +======================================= + +The two error shapes returned by the command methods mean different things: + +* `false, err` is a Redis error reply (for example `ERR wrong number of arguments`). The connection is fine and can still be used or put into the pool with [set_keepalive](#set_keepalive), unless Redis itself closed it, as it does after protocol errors or when `maxclients` is exceeded. +* `nil, err` from a command method is a connection failure (`closed`, `timeout`, `connection reset by peer`, ...). Do not reuse the connection: call [close](#close) and connect again. After a read timeout this library has already closed the socket for you, so a subsequent [set_keepalive](#set_keepalive) returns `closed`; that is expected. + +Two exceptions to the second rule: a `timeout` from [read_reply](#read_reply) in subscribe mode is a normal polling result and the connection stays usable; after a timed-out `blpop`/`brpop` the connection must be closed and never kept alive, because the late reply would be read by the next user. Errors describing this library's own state (`in transaction`, `subscribed state`, `not subscribed`, `no pipeline`, `not initialized`) are not connection failures. + +A connection that is neither closed nor kept alive is closed automatically when the current request (or timer) finishes. This is not a leak, but the connection is not reused either; return early from error paths as in the [Synopsis](#synopsis) if that is acceptable to you. + +When setting the `max_idle_timeout` of [set_keepalive](#set_keepalive), keep it below the idle timeout of anything between NGINX and Redis (load balancers, NAT, firewalls) and below Redis' own `timeout` setting, otherwise pooled connections come back dead and the next command fails with `closed`, `connection reset by peer` or `timeout`. A `max_idle_timeout` of `0` means unlimited. + +The `pool_size` of [set_keepalive](#set_keepalive) (or of [connect](#connect)) bounds the number of *idle* connections kept in the pool, not the number of concurrent connections; use the `backlog` option of [connect](#connect) to limit the latter. + +[Back to TOC](#table-of-contents) + Check List for Issues ===================== diff --git a/lib/resty/redis.lua b/lib/resty/redis.lua index b0c72f9..4f34cdc 100644 --- a/lib/resty/redis.lua +++ b/lib/resty/redis.lua @@ -3,6 +3,8 @@ local sub = string.sub local byte = string.byte +local str_fmt = string.format +local floor = math.floor local tab_insert = table.insert local tab_remove = table.remove local tcp = ngx.socket.tcp @@ -27,9 +29,11 @@ end local tab_pool_len = 0 local tab_pool = new_tab(16, 0) +-- Redis integers are 64-bit; larger values keep tostring()'s form +local MAX_INT = 2^63 local _M = new_tab(0, 55) -_M._VERSION = '0.32' +_M._VERSION = '0.34' local common_cmds = { @@ -37,7 +41,7 @@ local common_cmds = { "del", "incr", "decr", -- Strings "llen", "lindex", "lpop", "lpush", "lrange", "linsert", -- Lists - "hexists", "hget", "hset", "hmget", + "hexists", "hget", "hset", --[[ "hmget", ]] --[[ "hmset", ]] "hdel", -- Hashes "smembers", "sismember", "sadd", "srem", "sdiff", "sinter", "sunion", -- Sets @@ -63,6 +67,8 @@ local unsub_commands = { local mt = { __index = _M } +local _do_cmd + local function get_tab_from_pool() if tab_pool_len > 0 then @@ -92,6 +98,7 @@ function _M.new(self) end local redis = setmetatable({ _sock = sock, _subscribed = false, + _in_multi = false, _n_channel = { unsubscribe = 0, punsubscribe = 0, @@ -177,15 +184,54 @@ function _M.connect(self, host, port_or_opts, opts) end + if unix and port_or_opts ~= nil then + opts = port_or_opts + end + + local db, username, password, tcp_keepalive + if opts then + db = opts.db + tcp_keepalive = opts.tcp_keepalive + + password = opts.password + if password == "" then + password = nil + end + + username = opts.username + if not password or username == "" then + username = nil + end + + if (db or username) and not opts.pool then + -- a pool per database / ACL user, otherwise a pooled connection + -- would carry its SELECT/AUTH state over to the next user (issue #53) + local pool = unix and host or (host .. ":" .. port_or_opts) + if username then + pool = pool .. "/" .. username + end + if db then + pool = pool .. "/" .. db + end + + local copy = {} + for k, v in pairs(opts) do + copy[k] = v + end + copy.pool = pool + opts = copy + end + end + self._subscribed = false + self._in_multi = false local ok, err if unix then -- second argument of sock:connect() cannot be nil - if port_or_opts ~= nil then - ok, err = sock:connect(host, port_or_opts) - opts = port_or_opts + if opts ~= nil then + ok, err = sock:connect(host, opts) else ok, err = sock:connect(host) end @@ -204,6 +250,47 @@ function _M.connect(self, host, port_or_opts, opts) end end + if tcp_keepalive then + local res, kerr = sock:setoption("keepalive", true) + if not res then + sock:close() + return nil, "failed to enable tcp keepalive: " .. tostring(kerr) + end + end + + if (password or db) and sock:getreusedtimes() == 0 then + -- send AUTH/SELECT directly even if a pipeline was opened before connect() + local reqs = rawget(self, "_reqs") + self._reqs = nil + + local res, cerr + if password then + if username then + res, cerr = _do_cmd(self, "auth", username, password) + else + res, cerr = _do_cmd(self, "auth", password) + end + + if not res then + self._reqs = reqs + sock:close() + return nil, "failed to authenticate: " .. tostring(cerr) + end + end + + if db then + res, cerr = _do_cmd(self, "select", db) + if not res then + self._reqs = reqs + sock:close() + return nil, "failed to select database " .. tostring(db) .. ": " + .. tostring(cerr) + end + end + + self._reqs = reqs + end + return ok, err end @@ -218,6 +305,10 @@ function _M.set_keepalive(self, ...) return nil, "subscribed state" end + if rawget(self, "_in_multi") then + return nil, "in transaction" + end + return sock:setkeepalive(...) end @@ -340,7 +431,15 @@ local function _gen_req(args) for i = 1, nargs do local arg = args[i] - if type(arg) ~= "string" then + local typ = type(arg) + if typ == "number" and (arg >= 1e14 or arg <= -1e14) + and arg == floor(arg) and arg <= MAX_INT and arg >= -MAX_INT + then + -- tostring() uses %.14g, which turns integers of 15+ digits + -- into scientific notation (issue #135) + arg = str_fmt("%.0f", arg) + + elseif typ ~= "string" then arg = tostring(arg) end @@ -365,7 +464,7 @@ local function _check_msg(self, res) end -local function _do_cmd(self, ...) +_do_cmd = function (self, ...) local args = {...} local sock = rawget(self, "_sock") @@ -643,13 +742,43 @@ function _M.hmset(self, hashname, ...) end +function _M.multi(self) + -- the connection must not be reused until EXEC or DISCARD (issue #176) + self._in_multi = true + return _do_cmd(self, "multi") +end + + +function _M.exec(self) + self._in_multi = false + return _do_cmd(self, "exec") +end + + +function _M.discard(self) + self._in_multi = false + return _do_cmd(self, "discard") +end + + +function _M.hmget(self, hashname, ...) + if select("#", ...) == 1 and type((...)) == "table" then + return _do_cmd(self, "hmget", hashname, unpack((...))) + end + + return _do_cmd(self, "hmget", hashname, ...) +end + + function _M.init_pipeline(self, n) self._reqs = new_tab(n or 4, 0) + self._in_multi_saved = rawget(self, "_in_multi") end function _M.cancel_pipeline(self) self._reqs = nil + self._in_multi = rawget(self, "_in_multi_saved") end diff --git a/t/bugs.t b/t/bugs.t index cdcae2b..b82739e 100644 --- a/t/bugs.t +++ b/t/bugs.t @@ -243,3 +243,100 @@ dog: hello world --- no_error_log [error] + + + +=== TEST 5: github issue #135: big integer arguments are not mangled +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local nums = { + 1824192940134586, + -1824192940134586, + 2^53, + 1e15, + 2^63, + } + + for i, num in ipairs(nums) do + local ok, err = red:set("big-int", num) + if not ok then + ngx.say("failed to set: ", err) + return + end + + local res, err = red:get("big-int") + if not res then + ngx.say("failed to get: ", err) + return + end + + ngx.say(i, ": ", res) + end + + red:del("big-int") + red:close() + } +--- response_body +1: 1824192940134586 +2: -1824192940134586 +3: 9007199254740992 +4: 1000000000000000 +5: 9223372036854775808 +--- no_error_log +[error] + + + +=== TEST 6: github issue #135: other number arguments are unchanged +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:del("pi", "beyond", "big-hash") + + red:set("pi", 3.14) + ngx.say("pi: ", (red:get("pi"))) + + -- beyond 64-bit integers, tostring() form is kept + red:set("beyond", 2^64) + ngx.say("beyond: ", (red:get("beyond"))) + + red:expire("pi", 60) + ngx.say("ttl: ", (red:ttl("pi"))) + + -- the hmset table form goes through the same encoder + red:hmset("big-hash", { id = 123456789012345 }) + ngx.say("id: ", (red:hget("big-hash", "id"))) + + red:del("pi", "beyond", "big-hash") + red:close() + } +--- response_body +pi: 3.14 +beyond: 1.844674407371e+19 +ttl: 60 +id: 123456789012345 +--- no_error_log +[error] diff --git a/t/connect-opts.t b/t/connect-opts.t new file mode 100644 index 0000000..b8935c5 --- /dev/null +++ b/t/connect-opts.t @@ -0,0 +1,338 @@ +# vim:set ft= ts=4 sw=4 et: + +use t::Test; + +repeat_each(2); + +plan tests => repeat_each() * (3 * blocks()); + +run_tests(); + +__DATA__ + +=== TEST 1: db option selects on fresh connections and uses its own pool +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local opts = { db = 1 } + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, opts) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:del("db-opt") + red:set("db-opt", "in db 1") + ngx.say("opts.pool: ", opts.pool) + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("db 0: ", (red:get("db-opt"))) + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, opts) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reused: ", (red:get_reused_times())) + ngx.say("db 1: ", (red:get("db-opt"))) + + red:del("db-opt") + red:close() + } +--- response_body +opts.pool: nil +db 0: null +reused: 1 +db 1: in db 1 +--- no_error_log +[error] + + + +=== TEST 2: an explicit pool name wins over the db isolation +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { db = 1, pool = "shared" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { pool = "shared" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reused: ", (red:get_reused_times())) + red:close() + } +--- response_body +reused: 1 +--- no_error_log +[error] + + + +=== TEST 3: nothing is re-applied on a reused connection +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, { db = 1 }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:select(2) + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, { db = 1 }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reused: ", (red:get_reused_times())) + + local info = red:client("info") + ngx.say("db: ", string.match(info, "db=(%d+)")) + red:close() + } +--- response_body +reused: 1 +db: 2 +--- no_error_log +[error] + + + +=== TEST 4: username + password authenticate as an ACL user, with their own pool +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local res, err = red:acl("setuser", "resty-opts", "reset", "on", + ">s3cret", "~*", "+@all") + if not res then + ngx.say("failed to create the acl user: ", err) + return + end + + red:set_keepalive(0, 1024) + + local opts = { username = "resty-opts", password = "s3cret" } + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, opts) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("whoami: ", (red:acl("whoami"))) + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, opts) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reused: ", (red:get_reused_times())) + ngx.say("whoami: ", (red:acl("whoami"))) + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("whoami: ", (red:acl("whoami"))) + red:acl("deluser", "resty-opts") + red:close() + } +--- response_body +whoami: resty-opts +reused: 1 +whoami: resty-opts +whoami: default +--- no_error_log +[error] + + + +=== TEST 5: a wrong password fails the connect and closes the socket +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:acl("setuser", "resty-opts", "reset", "on", ">s3cret", "~*", "+@all") + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { username = "resty-opts", password = "wrong" }) + ngx.say("connect: ", ok, " ", err) + + local times, err = red:get_reused_times() + ngx.say("reused: ", times, " ", err) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:acl("deluser", "resty-opts") + red:close() + } +--- response_body_like chop +^connect: nil failed to authenticate: WRONGPASS .* +reused: nil closed +$ +--- no_error_log +[error] + + + +=== TEST 6: password on a server without one fails; an empty password is ignored +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { password = "x" }) + ngx.say("connect: ", ok, " ", err) + + local times, err = red:get_reused_times() + ngx.say("reused: ", times, " ", err) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { password = "", username = "" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("ping: ", (red:ping())) + red:close() + } +--- response_body_like chop +^connect: nil failed to authenticate: ERR AUTH .* without any password .* +reused: nil closed +ping: PONG +$ +--- no_error_log +[error] + + + +=== TEST 7: an invalid db fails the connect and closes the socket +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { db = 99999 }) + ngx.say("connect: ", ok, " ", err) + + local times, err = red:get_reused_times() + ngx.say("reused: ", times, " ", err) + } +--- response_body +connect: nil failed to select database 99999: ERR DB index is out of range +reused: nil closed +--- no_error_log +[error] + + + +=== TEST 8: tcp_keepalive enables SO_KEEPALIVE on the connection +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { tcp_keepalive = true }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + -- nginx strips PATH down to /usr/local/bin:/usr/bin + local p = io.popen("PATH=$PATH:/usr/sbin:/sbin ss -tno state established " + .. "'( dport = :$TEST_NGINX_REDIS_PORT )'") + local out = p:read("*a") + p:close() + + ngx.say("keepalive timer: ", + string.find(out, "timer:(keepalive", 1, true) and "yes" or "no") + ngx.say("ping: ", (red:ping())) + red:close() + } +--- response_body +keepalive timer: yes +ping: PONG +--- no_error_log +[error] diff --git a/t/count.t b/t/count.t index 18514bd..3563c93 100644 --- a/t/count.t +++ b/t/count.t @@ -22,6 +22,6 @@ __DATA__ ngx.say("size: ", n) '; --- response_body -size: 58 +size: 61 --- no_error_log [error] diff --git a/t/hmset.t b/t/hmset.t index 03b2bbd..50c3508 100644 --- a/t/hmset.t +++ b/t/hmset.t @@ -126,3 +126,54 @@ hmget animals: barkmeowmoo --- internal_server_error --- error_log table expected, got string + + + +=== TEST 4: hmget with a field table (github issue #29) +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local cjson = require "cjson" + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:del("animals") + + local res, err = red:hmset("animals", { dog = "bark", cat = "meow" }) + if not res then + ngx.say("failed to set animals: ", err) + return + end + + local res, err = red:hmget("animals", { "dog", "cat", "cow" }) + if not res then + ngx.say("failed to get animals: ", err) + return + end + + ngx.say("hmget table: ", cjson.encode(res)) + + local res, err = red:hmget("animals", "dog") + if not res then + ngx.say("failed to get animals: ", err) + return + end + + ngx.say("hmget single: ", cjson.encode(res)) + + red:del("animals") + red:close() + } +--- response_body +hmget table: ["bark","meow",null] +hmget single: ["bark"] +--- no_error_log +[error] diff --git a/t/transaction.t b/t/transaction.t index 4651838..b3361e1 100644 --- a/t/transaction.t +++ b/t/transaction.t @@ -117,3 +117,220 @@ exec ans: \["OK",\[false,"(?:ERR|WRONGTYPE) Operation against a key holding the $ --- no_error_log [error] + + + +=== TEST 3: github issue #176: set_keepalive is refused while MULTI is open +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local ok, err = red:multi() + if not ok then + ngx.say("failed to run multi: ", err) + return + end + + ngx.say("set: ", (red:set("txn", 1))) + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + else + ngx.say("kept alive") + end + + ngx.say("discard: ", (red:discard())) + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + return + end + + ngx.say("ok") + } +--- response_body +set: QUEUED +refused: in transaction +discard: OK +ok +--- no_error_log +[error] + + + +=== TEST 4: exec ends the transaction state +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:multi() + red:set("txn", 2) + + local res, err = red:exec() + if not res then + ngx.say("failed to exec: ", err) + return + end + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + return + end + + ngx.say("ok") + } +--- response_body +ok +--- no_error_log +[error] + + + +=== TEST 5: connect resets the transaction state +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:multi() + red:close() + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + return + end + + ngx.say("ok") + } +--- response_body +ok +--- no_error_log +[error] + + + +=== TEST 6: a pipelined MULTI is tracked too +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local cjson = require "cjson" + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:init_pipeline() + red:multi() + red:set("txn", 3) + + local results, err = red:commit_pipeline() + if not results then + ngx.say("failed to commit: ", err) + return + end + + ngx.say("results: ", cjson.encode(results)) + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + else + ngx.say("kept alive") + end + + ngx.say("discard: ", (red:discard())) + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + return + end + + ngx.say("ok") + } +--- response_body +results: ["OK","QUEUED"] +refused: in transaction +discard: OK +ok +--- no_error_log +[error] + + + +=== TEST 7: cancel_pipeline forgets a MULTI that was never sent +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:init_pipeline() + red:multi() + red:cancel_pipeline() + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + return + end + + ngx.say("ok") + } +--- response_body +ok +--- no_error_log +[error] From 463026d69910dfe0d5c82c2700a27c332d9e67b7 Mon Sep 17 00:00:00 2001 From: shushen Date: Fri, 18 Sep 2026 19:24:58 +0900 Subject: [PATCH 2/3] bugfix: transaction state now follows confirmed replies; unambiguous pool names for db/username. * The transaction guard for set_keepalive() is set by MULTI and cleared only by an acknowledged EXEC or DISCARD; error replies (EXECABORT, NOPERM) keep the connection out of the pool, and pipelined MULTI/EXEC/DISCARD are settled when commit_pipeline() reads their replies. Previously a pipelined or rejected DISCARD, or cancel_pipeline() without a pipeline, could put a connection with an open transaction back into the pool. * The default pool name for the "db" and "username" connect() options is now ":/db=/user=", and "db" must be numeric, so that {db = 1} and {username = "1"} can no longer share a pool. * hmget() and the transaction methods go through the module-prefix dispatch again, so red:():hmget() keeps working. --- README.markdown | 6 +- lib/resty/redis.lua | 92 ++++++++++++++++++++------- t/connect-opts.t | 89 ++++++++++++++++++++++++++ t/hmset.t | 31 +++++++++ t/transaction.t | 150 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 343 insertions(+), 25 deletions(-) diff --git a/README.markdown b/README.markdown index abf31b2..88d534c 100644 --- a/README.markdown +++ b/README.markdown @@ -264,11 +264,11 @@ The optional `options_table` argument is a Lua table holding the following keys: * `pool` - Specifies a custom name for the connection pool being used. If omitted, then the connection pool name will be generated from the string template `:` or ``, followed by `/` when the `username` option is in effect and `/` when the `db` option is given (for example `127.0.0.1:6379/1` or `127.0.0.1:6379/alice/2`), so that connections to different databases or ACL users never share a pool. The `password` is never part of the pool name: every caller sharing a pool must authenticate the same way, or specify its own `pool`. + Specifies a custom name for the connection pool being used. If omitted, then the connection pool name will be generated from the string template `:` or ``, followed by `/db=` when the `db` option is given and `/user=` when the `username` option is in effect (for example `127.0.0.1:6379/db=1` or `127.0.0.1:6379/db=2/user=alice`), so that connections to different databases or ACL users never share a pool. The `password` is never part of the pool name: every caller sharing a pool must authenticate the same way, or specify its own `pool`. * `db` - Selects the given Redis database (a number) with the `SELECT` command right after a *new* connection is established. Connections reused from the connection pool are left untouched, which is why such connections get their own pool (see `pool` above). Omit this option for the default database `0`. If `SELECT` fails, `connect` closes the connection and returns `nil` plus the error string `"failed to select database : "`. + Selects the given Redis database with the `SELECT` command right after a *new* connection is established. The value must be a number (or a numeric string); anything else raises a Lua error. Connections reused from the connection pool are left untouched, which is why such connections get their own pool (see `pool` above). Omit this option for the default database `0`. If `SELECT` fails, `connect` closes the connection and returns `nil` plus the error string `"failed to select database : "`. * `password` @@ -331,7 +331,7 @@ In case of success, returns `1`. In case of errors, returns `nil` with a string Only call this method in the place you would have called the `close` method instead. Calling this method will immediately turn the current redis object into the `closed` state. Any subsequent operations other than `connect()` on the current object will return the `closed` error. -A connection with an open transaction (after `multi` but before `exec` or `discard`) cannot be put into the pool: this method returns `nil` and the error string `"in transaction"` and the connection stays open. Call `exec`, `discard` or `close` first. Note that `WATCH` is not tracked: `unwatch` (or `close`) before keeping such a connection alive. +A connection with an open transaction cannot be put into the pool: from the `multi` call until Redis has acknowledged an `exec` or `discard`, this method returns `nil` and the error string `"in transaction"` and the connection stays open. If `exec` or `discard` fails with a Redis error reply (for example `EXECABORT` or `NOPERM`), the connection is still considered to be in a transaction; `close` it. Pipelined `multi`, `exec` and `discard` take effect when their replies are read by [commit_pipeline](#commit_pipeline). Note that `WATCH` is not tracked: `unwatch` (or `close`) before keeping such a connection alive. [Back to TOC](#table-of-contents) diff --git a/lib/resty/redis.lua b/lib/resty/redis.lua index 4f34cdc..ec80238 100644 --- a/lib/resty/redis.lua +++ b/lib/resty/redis.lua @@ -191,6 +191,14 @@ function _M.connect(self, host, port_or_opts, opts) local db, username, password, tcp_keepalive if opts then db = opts.db + if db ~= nil then + local typ = type(db) + db = tonumber(db) + if db == nil then + error("bad option db: number expected, got " .. typ, 2) + end + end + tcp_keepalive = opts.tcp_keepalive password = opts.password @@ -205,13 +213,14 @@ function _M.connect(self, host, port_or_opts, opts) if (db or username) and not opts.pool then -- a pool per database / ACL user, otherwise a pooled connection - -- would carry its SELECT/AUTH state over to the next user (issue #53) + -- would carry its SELECT/AUTH state over to the next user (issue #53); + -- the numeric db comes first so that no user name can impersonate it local pool = unix and host or (host .. ":" .. port_or_opts) - if username then - pool = pool .. "/" .. username - end if db then - pool = pool .. "/" .. db + pool = pool .. "/db=" .. db + end + if username then + pool = pool .. "/user=" .. username end local copy = {} @@ -742,43 +751,75 @@ function _M.hmset(self, hashname, ...) end -function _M.multi(self) - -- the connection must not be reused until EXEC or DISCARD (issue #176) - self._in_multi = true - return _do_cmd(self, "multi") +function _M.hmget(self, hashname, ...) + if select("#", ...) == 1 and type((...)) == "table" then + return do_cmd(self, "hmget", hashname, unpack((...))) + end + + return do_cmd(self, "hmget", hashname, ...) end -function _M.exec(self) - self._in_multi = false - return _do_cmd(self, "exec") +-- the connection must not be put back into the pool while a transaction is +-- open on the server (issue #176): MULTI marks it, and only a confirmed EXEC +-- or DISCARD reply clears it. Pipelined commands are settled when their +-- replies are read in commit_pipeline(). +local function update_multi_state(self, cmd, res) + if cmd == "multi" then + self._in_multi = true + + elseif res then + self._in_multi = false + end end -function _M.discard(self) - self._in_multi = false - return _do_cmd(self, "discard") -end +local function do_multi_cmd(self, cmd) + local reqs = rawget(self, "_reqs") + if reqs then + local res, err = do_cmd(self, cmd) + local txn_reqs = rawget(self, "_txn_reqs") + if not txn_reqs then + txn_reqs = {} + self._txn_reqs = txn_reqs + end + txn_reqs[#reqs] = cmd -function _M.hmget(self, hashname, ...) - if select("#", ...) == 1 and type((...)) == "table" then - return _do_cmd(self, "hmget", hashname, unpack((...))) + return res, err end - return _do_cmd(self, "hmget", hashname, ...) + local res, err = do_cmd(self, cmd) + update_multi_state(self, cmd, res) + + return res, err +end + + +function _M.multi(self) + return do_multi_cmd(self, "multi") +end + + +function _M.exec(self) + return do_multi_cmd(self, "exec") +end + + +function _M.discard(self) + return do_multi_cmd(self, "discard") end function _M.init_pipeline(self, n) self._reqs = new_tab(n or 4, 0) - self._in_multi_saved = rawget(self, "_in_multi") + self._txn_reqs = nil end function _M.cancel_pipeline(self) self._reqs = nil - self._in_multi = rawget(self, "_in_multi_saved") + self._txn_reqs = nil end @@ -790,6 +831,9 @@ function _M.commit_pipeline(self) self._reqs = nil + local txn_reqs = rawget(self, "_txn_reqs") + self._txn_reqs = nil + local sock = rawget(self, "_sock") if not sock then return nil, "not initialized" @@ -809,6 +853,10 @@ function _M.commit_pipeline(self) local vals = new_tab(nreqs, 0) for i = 1, nreqs do local res, err = _read_reply(self, sock) + if txn_reqs and txn_reqs[i] then + update_multi_state(self, txn_reqs[i], res) + end + if res then nvals = nvals + 1 vals[nvals] = res diff --git a/t/connect-opts.t b/t/connect-opts.t index b8935c5..859e175 100644 --- a/t/connect-opts.t +++ b/t/connect-opts.t @@ -336,3 +336,92 @@ keepalive timer: yes ping: PONG --- no_error_log [error] + + + +=== TEST 9: db and username never share a pool +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local res, err = red:acl("setuser", "1", "reset", "on", ">s3cret", "~*", "+@all") + if not res then + ngx.say("failed to create the acl user: ", err) + return + end + + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, { db = 1 }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { username = "1", password = "s3cret" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("whoami: ", (red:acl("whoami"))) + ngx.say("db: ", string.match(red:client("info"), "db=(%d+)")) + red:close() + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:acl("deluser", "1") + red:close() + } +--- response_body +whoami: 1 +db: 0 +--- no_error_log +[error] + + + +=== TEST 10: db must be a number +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = pcall(red.connect, red, "127.0.0.1", $TEST_NGINX_REDIS_PORT, + { db = "1/user=alice" }) + ngx.say("connect: ", ok, " ", err) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, { db = "1" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("db: ", string.match(red:client("info"), "db=(%d+)")) + red:close() + } +--- response_body +connect: false bad option db: number expected, got string +db: 1 +--- no_error_log +[error] diff --git a/t/hmset.t b/t/hmset.t index 50c3508..a22bb24 100644 --- a/t/hmset.t +++ b/t/hmset.t @@ -177,3 +177,34 @@ hmget table: ["bark","meow",null] hmget single: ["bark"] --- no_error_log [error] + + + +=== TEST 5: hmget honours a pending module prefix +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local res, err = red:review():hmget("animals", "dog") + ngx.say("hmget: ", res, " ", string.match(tostring(err), "review%.hmget")) + ngx.say("ping: ", (red:ping())) + + red:close() + } +--- response_body +hmget: false review.hmget +ping: PONG +--- no_error_log +[error] diff --git a/t/transaction.t b/t/transaction.t index b3361e1..2106603 100644 --- a/t/transaction.t +++ b/t/transaction.t @@ -334,3 +334,153 @@ ok ok --- no_error_log [error] + + + +=== TEST 8: a pipelined DISCARD only ends the transaction once it is confirmed +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local cjson = require "cjson" + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:multi() + red:init_pipeline() + red:discard() + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + else + ngx.say("kept alive") + end + + local results, err = red:commit_pipeline() + if not results then + ngx.say("failed to commit: ", err) + return + end + + ngx.say("results: ", cjson.encode(results)) + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + return + end + + ngx.say("ok") + } +--- response_body +refused: in transaction +results: ["OK"] +ok +--- no_error_log +[error] + + + +=== TEST 9: a rejected DISCARD leaves the transaction open +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + local res, err = red:acl("setuser", "resty-txn", "reset", "on", + ">s3cret", "~*", "+@all", "-discard") + if not res then + ngx.say("failed to create the acl user: ", err) + return + end + + red:set_keepalive(0, 1024) + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT, + { username = "resty-txn", password = "s3cret" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:multi() + red:set("txn", 4) + + local res, err = red:discard() + ngx.say("discard: ", res, " ", string.match(err, "^%u+")) + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + else + ngx.say("kept alive") + end + + red:close() + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:acl("deluser", "resty-txn") + red:close() + } +--- response_body +discard: false NOPERM +refused: in transaction +--- no_error_log +[error] + + + +=== TEST 10: cancel_pipeline without a pipeline keeps the transaction state +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + local red = redis:new() + + red:set_timeout(1000) -- 1 sec + + local ok, err = red:connect("127.0.0.1", $TEST_NGINX_REDIS_PORT) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:multi() + red:cancel_pipeline() + + local ok, err = red:set_keepalive(0, 1024) + if not ok then + ngx.say("refused: ", err) + else + ngx.say("kept alive") + end + + red:discard() + red:close() + } +--- response_body +refused: in transaction +--- no_error_log +[error] From d30279d9d022bed23f41e68db2246b754685d765 Mon Sep 17 00:00:00 2001 From: lijunlong Date: Fri, 18 Sep 2026 21:04:19 +0800 Subject: [PATCH 3/3] bugfix: preserve module command arguments and transaction state. Dispatch prefixed multi, exec, and discard commands without changing Redis transaction state. Forward all arguments through the wrappers so module commands retain their original call semantics. Add regression tests for direct and pipelined module commands, argument encoding, and connection pooling with and without an open transaction. The tests use a mock Redis server without requiring an external module. Validated 372 assertions across HTTP and stream, including RedisBloom integration tests, plus real module probes for mixed pipelines, cancellation, rejected commands, and connection reuse. Lua lint and whitespace checks passed. --- lib/resty/redis.lua | 23 ++-- t/transaction-module.t | 286 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+), 9 deletions(-) create mode 100644 t/transaction-module.t diff --git a/lib/resty/redis.lua b/lib/resty/redis.lua index ec80238..0ee8d1e 100644 --- a/lib/resty/redis.lua +++ b/lib/resty/redis.lua @@ -774,10 +774,15 @@ local function update_multi_state(self, cmd, res) end -local function do_multi_cmd(self, cmd) +local function do_multi_cmd(self, cmd, ...) + -- Prefixed commands do not change Redis transaction state. + if rawget(self, "_module_prefix") then + return do_cmd(self, cmd, ...) + end + local reqs = rawget(self, "_reqs") if reqs then - local res, err = do_cmd(self, cmd) + local res, err = do_cmd(self, cmd, ...) local txn_reqs = rawget(self, "_txn_reqs") if not txn_reqs then @@ -789,25 +794,25 @@ local function do_multi_cmd(self, cmd) return res, err end - local res, err = do_cmd(self, cmd) + local res, err = do_cmd(self, cmd, ...) update_multi_state(self, cmd, res) return res, err end -function _M.multi(self) - return do_multi_cmd(self, "multi") +function _M.multi(self, ...) + return do_multi_cmd(self, "multi", ...) end -function _M.exec(self) - return do_multi_cmd(self, "exec") +function _M.exec(self, ...) + return do_multi_cmd(self, "exec", ...) end -function _M.discard(self) - return do_multi_cmd(self, "discard") +function _M.discard(self, ...) + return do_multi_cmd(self, "discard", ...) end diff --git a/t/transaction-module.t b/t/transaction-module.t new file mode 100644 index 0000000..2dd6bcf --- /dev/null +++ b/t/transaction-module.t @@ -0,0 +1,286 @@ +# vim:set ft= ts=4 sw=4 et: + +use t::Test; + +repeat_each(2); + +plan tests => repeat_each() * (4 * blocks()); + +run_tests(); + +__DATA__ + +=== TEST 1: module multi preserves arguments and permits keepalive +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reply: ", (red:review():multi("one", "two"))) + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + } +--- tcp_listen: 1922 +--- tcp_query eval +"*3\r\n\$12\r\nreview.multi\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" +--- tcp_reply eval +"+OK\r\n" +--- tcp_no_close +--- response_body +reply: OK +keepalive: 1 nil +--- no_error_log +[error] + + + +=== TEST 2: module exec preserves arguments and permits keepalive +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reply: ", (red:review():exec("one", "two"))) + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + } +--- tcp_listen: 1922 +--- tcp_query eval +"*3\r\n\$11\r\nreview.exec\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" +--- tcp_reply eval +"+OK\r\n" +--- tcp_no_close +--- response_body +reply: OK +keepalive: 1 nil +--- no_error_log +[error] + + + +=== TEST 3: module discard preserves arguments and permits keepalive +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("reply: ", (red:review():discard("one", "two"))) + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + } +--- tcp_listen: 1922 +--- tcp_query eval +"*3\r\n\$14\r\nreview.discard\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" +--- tcp_reply eval +"+OK\r\n" +--- tcp_no_close +--- response_body +reply: OK +keepalive: 1 nil +--- no_error_log +[error] + + + +=== TEST 4: a direct module exec does not end a transaction +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("multi: ", (red:multi())) + ngx.say("module reply: ", (red:review():exec())) + + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + red:close() + } +--- tcp_listen: 1922 +--- tcp_query eval +"*1\r\n\$5\r\nmulti\r\n" +# The first reply acknowledges MULTI; the second is the module command's reply. +--- tcp_reply eval +"+OK\r\n+QUEUED\r\n" +--- tcp_no_close +--- response_body +multi: OK +module reply: QUEUED +keepalive: nil in transaction +--- no_error_log +[error] + + + +=== TEST 5: a direct module discard does not end a transaction +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("multi: ", (red:multi())) + ngx.say("module reply: ", (red:review():discard())) + + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + red:close() + } +--- tcp_listen: 1922 +--- tcp_query eval +"*1\r\n\$5\r\nmulti\r\n" +# The first reply acknowledges MULTI; the second is the module command's reply. +--- tcp_reply eval +"+OK\r\n+QUEUED\r\n" +--- tcp_no_close +--- response_body +multi: OK +module reply: QUEUED +keepalive: nil in transaction +--- no_error_log +[error] + + + +=== TEST 6: pipelined module commands preserve arguments and do not open a transaction +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:init_pipeline() + red:review():exec("one", "two") + red:review():discard("one", "two") + red:review():multi("one", "two") + red:ping() + + local results, err = red:commit_pipeline() + if not results then + ngx.say("failed to commit: ", err) + return + end + + ngx.say("replies: ", require("cjson").encode(results)) + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + } +--- tcp_listen: 1922 +--- tcp_query eval +"*3\r\n\$11\r\nreview.exec\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" . +"*3\r\n\$14\r\nreview.discard\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" . +"*3\r\n\$12\r\nreview.multi\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" . +"*1\r\n\$4\r\nping\r\n" +--- tcp_reply eval +"+OK\r\n+OK\r\n+OK\r\n+PONG\r\n" +--- tcp_no_close +--- response_body +replies: ["OK","OK","OK","PONG"] +keepalive: 1 nil +--- no_error_log +[error] + + + +=== TEST 7: pipelined module exec and discard leave a transaction open +--- global_config eval: $::GlobalConfig +--- server_config + content_by_lua_block { + local redis = require "resty.redis" + redis.register_module_prefix("review") + + local red = redis:new() + red:set_timeout(1000) + + local ok, err = red:connect("127.0.0.1", 1922) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + red:init_pipeline() + red:multi() + red:review():exec("one", "two") + red:review():discard("one", "two") + + local results, err = red:commit_pipeline() + if not results then + ngx.say("failed to commit: ", err) + return + end + + ngx.say("replies: ", require("cjson").encode(results)) + local ok, err = red:set_keepalive(0, 1) + ngx.say("keepalive: ", ok, " ", err) + red:close() + } +--- tcp_listen: 1922 +--- tcp_query eval +"*1\r\n\$5\r\nmulti\r\n" . +"*3\r\n\$11\r\nreview.exec\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" . +"*3\r\n\$14\r\nreview.discard\r\n\$3\r\none\r\n\$3\r\ntwo\r\n" +--- tcp_reply eval +"+OK\r\n+QUEUED\r\n+QUEUED\r\n" +--- tcp_no_close +--- response_body +replies: ["OK","QUEUED","QUEUED"] +keepalive: nil in transaction +--- no_error_log +[error]