Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime error: attempt to yield across C-call boundary due to lua-resty-redis connect() invocation #5

Closed
un-def opened this issue Jul 8, 2016 · 3 comments

Comments

@un-def
Copy link

@un-def un-def commented Jul 8, 2016

lapis 1.5.0-2
lapis-redis dev-1
openresty 1.9.15.1

nginx.conf:

location / {
  default_type text/html;
  content_by_lua_block {
    require('lapis').serve('app')
  }
}

config.lua

config({'development', 'production'}, {
  redis = {
    host = '127.0.0.1',
    port = 6379,
  }
})

app.lua:

local redis = require('lapis.redis').get_redis()

exception:

[error] 655#0: *8 lua entry thread aborted: runtime error: attempt to yield across C-call boundary
stack traceback:
coroutine 0:
        [C]: in function 'require'
        /usr/local/openresty/luajit/share/lua/5.1/lapis/init.lua:15: in function 'serve'
        content_by_lua(nginx.conf.compiled:30):2: in function <content_by_lua(nginx.conf.compiled:30):1>, client: 127.0.0.1, server: , request: "GET / HTTP/1.1", host: "127.0.0.1:9000"

I found this issue, but I don't understand how this error can be fixed.

@un-def un-def changed the title runtime error: attempt to yield across C-call boundary due to connect() invokation runtime error: attempt to yield across C-call boundary due to lua-resty-redis connect() invokation Jul 8, 2016
@un-def
Copy link
Author

@un-def un-def commented Jul 8, 2016

Hmm, I've solved this issue with tricky workaround:

redis_wrapper.lua

local redis_wrapper_meta = {
  __index = function(self, method)
    if not rawget(self, 'redis_obj') then
      local redis_obj, err = require('lapis.redis').get_redis()
      if not redis_obj then
        error('lapis.redis error: ' .. err)
      end
      local config = require('lapis.config').get()
      if config.redis.db then
        redis_obj:select(config.redis.db)
      end
      self.redis_obj = redis_obj
    end
    return self.redis_obj[method]
  end
}

return setmetatable({}, redis_wrapper_meta)

app.lua

local redis = require('redis_wrapper')
redis:set('foo', 'bar')

I think I can implement same lazy connect trick in lapis-redis module with optional get_redis argument to prevent connect() call while application module is imported in lapis.init.serve() (https://github.com/leafo/lapis/blob/a1fd7d854bd8d859b02e0af32387b85cb8023e00/lapis/init.moon#L17).
require('lapis.redis').get_redis() — get connected redis object (throw errors immediately)
require('lapis.redis').get_redis(false) — get redis object, but connect only when it's needed (e.g., when get/set method is called)

In addition, It would be great to be able to specify db number in config along with host/port.

What do you think about these ideas?

@un-def un-def changed the title runtime error: attempt to yield across C-call boundary due to lua-resty-redis connect() invokation runtime error: attempt to yield across C-call boundary due to lua-resty-redis connect() invocation Jul 8, 2016
@leafo
Copy link
Owner

@leafo leafo commented Jul 8, 2016

You have to call get_redis within a request, not on the initialization of an app. The redis connection is managed by nginx's socket pooling so it needs to be handed back to nginx after the request finishes. Lapis will do this automatically if you call get_redis anywhere in the body of a action or before filter.

You can't have a global redis connection per worker because workers can be processing multiple http requests at once when they yield due to asynchronous operations.

@un-def
Copy link
Author

@un-def un-def commented Jul 9, 2016

Thanks for clarifying! Now I realized that the redis object (as a “singleton”) instantiating mechanism is already implemented in get_redis and multiple get_redis calls don't cause severe overhead.

@un-def un-def closed this Jul 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants