Comments (13)
@freman I've noticed one obvious mistake that you do not declare the local variables red
(and also err
) in your function _M.new(self)
, which means the red
variable is actually a global variable that is still shared by concurrent requests.
It is recommended to always use the lua-releng
tool to scan all your Lua code for such accidental use of Lua globals where you really want locals. See https://github.com/openresty/lua-nginx-module#lua-variable-scope
from lua-resty-redis.
@freman ngx_lua's cosocket objects can only be used by the request creating them while you're sharing the cosocket object in your module table across all the requests, thus leading to the "bad request" exception. See https://github.com/openresty/lua-resty-redis#limitations
The correct way is to create separate cosocket objects (hence separate resty.redis instances) for each request handler.
from lua-resty-redis.
I thought that's what I was doing by initing the module for every request.
from lua-resty-redis.
@freman Lua modules are cached and shared globally unless you turn off lua_code_cache
(turning off the code cache is not recommended to production use but just for quick development). See https://github.com/openresty/lua-nginx-module#data-sharing-within-an-nginx-worker
from lua-resty-redis.
@freman The correct way is to make your module initiate new objects for individual requests in an OO-style way, just like lua-resty-redis
itself. That way, the states can be cleanly separated and prevent unwanted state sharing.
from lua-resty-redis.
Here I was hoping to reduce the code duplication, now I'm on my way down the rabbit hole :D
from lua-resty-redis.
I think I'm still messing something up @agentzh - still newish at lua, this would be my first object orientated script (and previously my first module) - I pretty much resorted to copying what you had done.
local _M = {}
local mt = { __index = _M }
local REDIS_TIMEOUT = 250
local REDIS_HOST = '127.0.0.1'
local REDIS_PORT = 6379
local REDIS_POOLSIZE = 200
local REDIS_IDLETIME = 60
local MAP_EXPIRES = 3660
function _M.new(self)
local redis = require "resty.redis"
red = redis:new()
err = false
red:set_timeout(REDIS_TIMEOUT)
local ok, err = red:connect(REDIS_HOST, REDIS_PORT)
if not ok then
ngx.log(ngx.ERR, "Absolute failure of redis: " .. err)
return nil
end
return setmetatable({ red = red, err = err }, mt)
end
function _M.fetch(self)
local res, err = self.red:get(ngx.var.uid_got)
if not res then
ngx.log(ngx.ERR, "Failed to get value for key (" .. ngx.var.uid_got .. "): " .. err)
self.err = true
return
end
if type(res) == "string" then
ngx.var.httphost = res
local res, err = self.red:expire(ngx.var.uid_got, MAP_EXPIRES)
if not res then
ngx.log(ngx.ERR, "Failed to reset expiry for (" .. ngx.var.uid_got ..") to " .. MAP_EXPIRES .. ": " .. err)
self.err = true
return
end
end
return true
end
function _M.store(self)
local res, err = self.red:setex(ngx.var.uid_got, MAP_EXPIRES, ngx.var.httphost)
if not res then
ngx.log(ngx.ERR, "Failed to store value (" .. ngx.var.httphost .. ") for key (" .. ngx.var.uid_got .. "): " .. err)
self.err = true
return
end
-- if ngx.var.args then
-- ngx.log(ngx.ERR, "Not redirecting...")
-- return true
-- end
return "/" .. ngx.var.redirecturl
end
function _M.clear(self)
local res, err = self.red:del(ngx.var.uid_got)
if not res then
ngx.log(ngx.ERR, "Failed to clear value for key (" .. ngx.var.uid_got .. "): " .. err)
self.err = true
return
end
end
function _M.pool(self)
if self.err then
ngx.log(ngx.ERR, "Not sending closed connection back to pool")
else
self.red:set_keepalive(REDIS_IDLETIME, REDIS_POOLSIZE)
end
end
return _M
It's still spitting a few bad request errors out of redis.lua:292
2014/05/12 04:17:32 [error] 71#0: *31 lua entry thread aborted: runtime error: /usr/share/lua/5.1/resty/redis.lua:292: bad request
stack traceback:
coroutine 0:
[C]: in function 'send'
/usr/share/lua/5.1/resty/redis.lua:292: in function 'get'
/etc/nginx/conf.d/lua/hostmap.lua:28: in function 'fetch'
/etc/nginx/conf.d/lua/select_host.lua:5: in function </etc/nginx/conf.d/lua/select_host.lua:1>, client: 172.17.42.1, server: www.example.com, request: "GET /css/odds_and_rules.css HTTP/1.1", host: "www.yoda.lan", referrer: "https://www.yoda.lan/"
2014/05/12 04:17:32 [error] 71#0: *32 lua entry thread aborted: runtime error: /usr/share/lua/5.1/resty/redis.lua:292: bad request
stack traceback:
coroutine 0:
[C]: in function 'send'
/usr/share/lua/5.1/resty/redis.lua:292: in function 'get'
/etc/nginx/conf.d/lua/hostmap.lua:28: in function 'fetch'
/etc/nginx/conf.d/lua/select_host.lua:5: in function </etc/nginx/conf.d/lua/select_host.lua:1>, client: 172.17.42.1, server: www.example.com, request: "GET /css/index.css HTTP/1.1", host: "www.yoda.lan", referrer: "https://www.yoda.lan/"
2014/05/12 04:17:32 [error] 71#0: *30 lua entry thread aborted: runtime error: /usr/share/lua/5.1/resty/redis.lua:292: bad request
stack traceback:
coroutine 0:
[C]: in function 'send'
/usr/share/lua/5.1/resty/redis.lua:292: in function 'get'
/etc/nginx/conf.d/lua/hostmap.lua:28: in function 'fetch'
/etc/nginx/conf.d/lua/select_host.lua:5: in function </etc/nginx/conf.d/lua/select_host.lua:1>, client: 172.17.42.1, server: www.example.com, request: "GET /js/global.js HTTP/1.1", host: "www.yoda.lan", referrer: "https://www.yoda.lan/"
And select_host.lua
if ngx.var.uid_got then
local hostmap = require "hostmap"
if hostmap then
local map = hostmap:new()
if map:fetch() then
map:pool()
end
end
end
from lua-resty-redis.
@freman BTW, it's also better to move the line local redis = require "resty.redis"
to the toplevel of your Lua module scope because it does no good to introduce require()
calls into your hot Lua code paths.
from lua-resty-redis.
Thank you very much for your help and advice @agentzh, I believe it's working now without fault
from lua-resty-redis.
@freman Good to know :) I'm closing this.
from lua-resty-redis.
Hi @agentzh I seem to have this same bad request error when trying to get or set, and I can't figure out what to improve. I was also trying to use pcall to be able to serve an alternative if there is a bad request. I don't see where I would not have local. I thought it is. What am I not getting?
local imageserver = {}
local originalDir
local resizedDir
local testDir
local utils
local defaultWidth
local defaultHeight
local defaultQuality
local redis
local red
local magick
local ok, err
function imageserver.init()
utils = require "utils"
defaultWidth = 450
defaultHeight = 450
defaultQuality = 70
magick = require "magick"
redis = require "redis"
testDir = '/original/'
originalDir = '/mount/'
resizedDir = '/resized/'
imageserver.init_redis()
end
function imageserver.done()
-- put it into the connection pool of size 100,
-- with 10 seconds max idle time
ok, err = red:set_keepalive(5000, 2000)
if ok then
utils.log("redis","set keepalive success")
else
utils.log("redis","failed to set keepalive: ", err)
end
-- or just close the connection right away:
-- ok, err = red:close()
-- if not ok then
-- utils.log("failed to close: ", err)
-- end
return
end
function imageserver.resize_and_compress(image,width,height,quality)
local source = originalDir .. image
local key = imageserver.key_for_image(image,width,height,quality)
local destination = resizedDir .. key
local size
if not utils.file_exists(source) or not utils.file_is_valid(source) then
return utils.return_404()
end
if not width and not height then
width = defaultWidth
height = defaultHeight
end
if not width then
width = ""
end
if not height then
height = ""
end
if not quality then
quality = defaultQuality
end
size = width .. "x" .. height
magick.thumb(source, size, destination)
return destination
end
function imageserver.put_in_cache(key,value)
ok = false
ok, err = red:set(key, value)
if ok then
utils.log("redis","set " .. key)
else
utils.log("redis","failed to set " .. key .. " : " .. err)
end
return ok
end
function imageserver.init_redis()
timeout = 200;
red = redis:new()
red:set_timeout(timeout)
ok, err = red:connect("127.0.0.1", 6379)
if ok then
utils.log("redis","success connecting to redis")
else
utils.log("redis","failed to connect: " .. err)
end
return
end
function imageserver.get_from_cache(key)
-- return value matching key --
utils.log("redis","the connection reused:" .. red:get_reused_times())
if not red then
utils.log("redis","red not here?")
return nil
else
result, errr = red:get(key)
if not result then
utils.log("redis", "failed to get " .. key .. " : " .. errr)
return nil
end
if result == ngx.null then
utils.log("redis","no data for " .. key)
return nil
end
end
return result
end
-- Cache Handling --
function imageserver.clear_cache(key)
if key == nil then
-- clear all cache --
else
imageserver.put_in_cache(key,nil)
end
end
function imageserver.key_for_image_original(image)
return image
end
function imageserver.key_for_image(image,width,height,quality)
local file, ext = image:match("([^.]+).([^.]+)")
local key = string.gsub(file,"/","_")
if not key then
-- return nil
elseif width == nil and height == nil and quality == nil then
key = key .. "_0_0_0"
elseif width == nil and quality == nil then
key = key .. "_0_" .. height .. "_0"
elseif height == nil and quality == nil then
key = key .. "_" .. width .. "_0_0"
elseif width == nil and height == nil then
key = key .. "_0_0_" .. quality
elseif width == nil and quality == nil then
key = key .. "_0_" .. height .. "_0"
elseif width == nil then
key = key .. "_0_" .. height .. "_" .. quality
elseif height == nil then
key = key .. "_" .. width .. "_0_" .. quality
elseif quality == nil then
key = key .. "_" .. width .. "_" .. height .. "_0"
else
key = key .. "_" .. width .. "_" .. height .. "_" .. quality
end
if not not key then
key = key .. "." .. ext
end
return key;
end
function imageserver.parse_request(request,arguments)
parameters = {}
parameters['image'] = request
parameters['width'] = tonumber(arguments['w'])
parameters['height'] = tonumber(arguments['h'])
parameters['quality'] = tonumber(arguments['q'])
parameters['original'] = tonumber(arguments['o'])
return parameters
end
function imageserver.serve(image,width,height,quality)
local imageData
local key
key = imageserver.key_for_image(image,width,height,quality)
imageData = imageserver.get_from_cache(key)
if not key then
return utils.return_404()
elseif not imageData then
local filename = imageserver.resize_and_compress(image,width,height,quality)
imageData = imageserver.get_from_file(filename)
imageserver.put_in_cache(key,imageData)
imageserver.output_image(imageData)
else
imageserver.output_image(imageData)
end
end
function imageserver.serve_original(image)
local imageData = imageserver.get_from_file(originalDir .. image)
if not imageData then
utils.return_404()
else
imageserver.output_image(imageData)
end
end
function imageserver.output_image(imageData)
--[[
utils.output(imageData)
--]]
---[[
utils.output_image(imageData)
--]]
end
function imageserver.get_from_file(filename)
local data
local fileHandle
fileHandle = io.open (filename, 'rb')
if not fileHandle then
return nil
else
imageData = fileHandle:read('*a')
return imageData
end
end
function imageserver.validate_parameters(parameters)
if not parameters['image'] then
-- we need the image at least --
return false
elseif parameters['original'] and utils.table_count(parameters) > 2 then
-- if original is asked, we don't allow any other parameters --
return false
elseif not parameters['original'] and utils.table_count(parameters) > 4 then
-- if no original asked, but somehow there are more than 4 params, that's weird --
return false
elseif parameters['quality'] and (not utils.is_int(parameters['quality']) or parameters['quality'] > 100 or parameters['quality'] < 1 ) then
-- quality needs to be int between 0-100 --
return false
elseif parameters['width'] and ( not utils.is_int(parameters['width']) or parameters['width'] > 2550 or parameters['width'] < 1 )then
-- width needs to be int between 1-2550 --
return false
elseif parameters['height'] and ( not utils.is_int(parameters['height']) or parameters['height'] > 2550 or parameters['height'] < 1 ) then
-- height needs to be int between 1-2550 --
return false
else
return true
end
end
function imageserver.serve_from_request(request,arguments)
parameters = imageserver.parse_request(request,arguments)
if imageserver.validate_parameters(parameters) then
if utils.starts(parameters['image'],'test/') then
parameters['image'] = string.sub(parameters['image'],6)
originalDir = testDir
end
utils.log("serve",originalDir)
if not parameters['original'] then
imageserver.serve(parameters['image'],parameters['width'],parameters['height'],parameters['quality'])
else
imageserver.serve_original(parameters['image'])
end
else
utils.return_404()
end
end
return imageserver
from lua-resty-redis.
@morksinaanab As I've said earlier, this is a clear sign of misusing the cosocket API. Please read my previous comments in this ticket for more details. BTW, please do not paste lengthy examples since I will not have the time to read it. Always try to provide a minimal and standalone example that is easy to digest for others if you want to provide any. Thanks for your cooperation!
from lua-resty-redis.
i have jump into this bad hole,and thanks veriy much.
from lua-resty-redis.
Related Issues (20)
- Confusing instructions for installation on README.md
- redis:connect is taking exactly 60 seconds for connection
- SSL not working
- The performance of redis pipeline mode is unstable HOT 3
- CLIENT SETNAME?
- redis:get(key) question HOT 2
- opm package outdated HOT 1
- Connection pool not creating HOT 2
- How to create a single redis connection HOT 5
- this support two-way authentication?
- Add keepalive for redis client to make the connections reliable
- Nginx 1.24 HOT 2
- Nginx creates more connection than poolsize, backlog to redis HOT 5
- lua tcp socket read timed out, context: ssl_certificate_by_lua*, client: 172.69.58.204, server: 0.0.0.0:443 HOT 2
- redis set_keepalive doesn't work HOT 1
- lua entry thread aborted: runtime error: /usr/local/openresty/lualib/resty/redis.lua:357: bad request HOT 1
- no request found HOT 1
- Add the host:port to the error message
- Reading from client: error:0A000126:SSL routines::unexpected eof while reading
- red:smembers("key"), sometime not include the new member? HOT 6
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from lua-resty-redis.