Closed socket error

[subnetmarco]

I'm trying to understand what is the best practice for handling Redis connections.

The doc suggests to avoid having a global connection to the Redis server in order to avoid any concurrency problems. This means opening a new connection to Redis for every incoming connection to Nginx, isn't this very inefficient?

Also, I'm trying to reuse the same connection in both access_by_lua and log_by_lua, but I get a attempt to send data on a closed socket error in the log_by_lua timer callback.

init_by_lua:

redis = require "resty.redis"

access_by_lua:

ngx.ctx.redis = redis:new()
local ok, err = ngx.ctx.redis:connect("127.0.0.1", 6379)
if not ok then
  ngx.log(ngx.ERR, "Cant connect to Redis")
end

ngx.ctx.redis:incr("incoming_requests")

log_by_lua:

local function log(premature, redis)
  redis:incr("requests_completed")
end

local ok, err = ngx.timer.at(0, log, ngx.ctx.redis)
if not ok then
    ngx.log(ngx.ERR, "failed to create timer: ", err)
    return
end

And I'm getting the following error in log_by_lua:

[error] 16644#0: attempt to send data on a closed socket: u:00007FCA22600B78, c:0000000000000000, ft:0 eof:0, context: ngx.timer

[agentzh]

@thefosk To answer your questions:

1. connect() method will always look up the connection pool for idle connections first. So if you have enabled the connection pool properly, then connect() should be very cheap.
2. Don't try to share the same resty.redis object across different request handlers like rewrite_by_lua and access_by_lua because the lifetime of the cosocket object cannot go across the boundary of the request handler (it will be automatically shut down). The correct way is to put the connection back to pool as soon as you finish using it, via calling the set_keepalive method. By putting back connections into the pool can also allow other concurrent requests to reuse these connections, which is also more efficient.
3. Try hard not to scatter your Lua code across various phases wherever possible because it's less efficient than putting all the Lua code into the same request handler (i.e., in the same phase).
4. The cosocket API used by lua-resty-redis and others cannot be used in the context of log_by_lua. But you can workaround this limitation by using the ngx.timer.at API, just as in the lua-resty-logger-socket library: https://github.com/cloudflare/lua-resty-logger-socket

[subnetmarco]

1. When you say to enable the connection pool, you mean to properly use the set_keepalive method?
2. I'm confused on this. Because the Redis client is instantiated on every request with redis:new(), the pool would be available only for the current request and not for every other request right? Because I'm using Redis in both the access and log handlers, should I put the connection back into the pool:

• In both the access and log handlers, after I've done my Redis operations?
• Only in the access handler, so I can reconnect to Redis during the log phase and get back the connection from the pool? (see code below) It wouldn't make sense to put the connection back into the pool after the log handler because it's not going to be used anymore in the current requests's scope, and the Redis client is instantiated on every request with redis:new() which creates a new object and a new pool on every subsequent request.
• In both handlers?

This is my updated, working, code, that puts the connection back in the pool in the access handler, and then reconnects during the log phase (but doesn't put the connection back to the pool after that):

access_by_lua:

ngx.ctx.redis = redis:new()
local ok, err = ngx.ctx.redis:connect("127.0.0.1", 6379)
if not ok then
  ngx.log(ngx.ERR, "Cant connect to Redis")
end

ngx.ctx.redis:incr("incoming_requests")

local ok, err = ngx.ctx.redis:set_keepalive(10000, 100)
if not ok then
  ngx.log(ngx.ERR, "Failed to set keepalive")
end

log_by_lua:

local function log(premature, redis)
  local ok, err = redis:connect("127.0.0.1", 6379)
  if not ok then
    ngx.log(ngx.ERR, "Cant connect to Redis")
  end
  redis:incr("requests_completed")
end

local ok, err = ngx.timer.at(0, log, ngx.ctx.redis)
if not ok then
  ngx.log(ngx.ERR, "failed to create timer: ", err)
  return
end

[agentzh]

Hello!

On Tue, Nov 19, 2013 at 5:17 PM, Marco Palladino
notifications@github.com wrote:

When you say to enable the connection pool, you mean to properly use the
set_keepalive method?

Yes.

I'm confused on this. Because the Redis client is instantiated on every
request with redis:new(), the pool would be available only for the current
request and not for every other request right?

Nope. The connection pool is implemented as an underlying service in
the ngx_lua core. It is not tight to a particular cosocket object nor
a particular request handled by Nginx at all. The pools are shared on
the Nginx worker process level.

Because I'm using Redis in
both the access and log handlers, should I put the connection back into the
pool:

Sure. Also, as I've said, you cannot use Redis (or other
cosocket-based libraries) in the log phase (if you're using ngx.timer,
then your cosocket operations are performed in the context of the
timer callback handler).

In both the access and log handlers, after I've done my Redis operations?

Sure. But for the log handlers, you'll have to use the timer callback.

You should always call set_keepalive() after you finish using the
redis connection (unless there is an error), otherwise the connection
is always automatically closed. No matter in what context you're
using. The connection pool is global (on the Nginx worker process
level) and has a different lifetime as requests.

log_by_lua:

local function log(premature, redis)
local ok, err = redis:connect("127.0.0.1", 6379)
if not ok then
ngx.log(ngx.ERR, "Cant connect to Redis")
end
redis:incr("requests_completed")

Call redis:set_keepalive() here to put the connection back to the
pool. otherwise the connection will get closed.

BTW, you should also check the return values of redis:incr() here and
handle the error if there is any (in case of errors, the connection is
automatically closed and you should not call set_keepalive in this
case).

Regards,
-agentzh

[agentzh]

It's worth mentioning that you can use the systemtap-based ngx-lua-conn-pools tool to trace live Nginx worker processes to watch the status of the (global) ngx_lua connection pools on Linux:

https://github.com/agentzh/nginx-systemtap-toolkit#ngx-lua-conn-pools

[subnetmarco]

One more question:

In the examples above, I'm using Redis in both the access and log handlers. I've initialized the Redis client with redis:new() in the access handler and then set it to a context variable ngx.ctx.redis, so that I can reuse it later in the log_by_lua handler.

In the log handler I'm reading the ngx.ctx.redis object without invoking redis:new() again. Is this recommended?

[agentzh]

Hello!

On Wed, Dec 4, 2013 at 3:51 PM, Marco Palladino wrote:

In the examples above, I'm using Redis in both the access and log handlers.

You cannot use cosockets in the log phase handlers. Unless you use
timers (i.e., ngx.timer.at) as a workaround.

I've initialized the Redis client with redis:new() in the access handler and then set it to a context variable ngx.ctx.redis.

Don't share the resty.redis objects (i.e., the thing returned by
redis:new()) across the boundary of the current request handler (for
example, access_by_lua is a request handler).

-agentzh

[subnetmarco]

Thanks, I'll close the issue.