Reputation: 24160
I am having a queue system using redis sorted set. My lua script look likes:
local moveElement = function(source, dest , score, destscore)
local element = redis.pcall('ZRANGEBYSCORE', source, '-inf',score, 'WITHSCORES' , 'LIMIT' , '0' , '1')
if element ~= false and #element ~= 0 then
redis.call('ZADD' , dest , destscore , element[1])
redis.call('ZREM' , source , element[1])
end
end
local temp = moveElement(KEYS[2], KEYS[1] , ARGV[2])
local temp = moveElement(KEYS[3], KEYS[1] , ARGV[2])
local score= redis.call('ZRANGEBYSCORE', KEYS[1], '-inf',ARGV[1], 'WITHSCORES' , 'LIMIT' , '0' , '10')
if score ~= false and #score ~= 0 then
local i = 1
while i<=#score do
redis.call('ZREM', KEYS[1] , score[i])
redis.call('ZREM', KEYS[2] , score[i])
redis.call('ZADD', KEYS[3], ARGV[1] , score[i])
i=i+2
end
end
return score
This lua script is taking 24 seconds with 6K members in sorted set.
SLOWLOG GET 10
1) 1) (integer) 5937
2) (integer) 1385993558
3) (integer) 24446
4) 1) "EVAL"
2) "local moveElement = function(source, dest , score, destscore) local element = redis.pcall('ZRANGEBYSCORE', sourc... (937 more bytes)"
My code approach is:
I call this script with following arguments.
Is there way to optimize time taken ? By caching lau script or something ?
As I am always fetching value from lower to higher will using ZRANGE help in place of ZRANGEBYSCORE?
More details:
I am trying to implement some type of queue system. Logic goes here:
Upvotes: 3
Views: 3875
Reputation: 7020
The script you posted has some issues. Here it is in Lua (please don't post quoted strings, it makes the code hard to read):
local moveElement = function(source, dest , score, destscore)
local element = redis.pcall(
'ZRANGEBYSCORE', source, '-inf', score, 'WITHSCORES' , 'LIMIT' , '0' , '1'
)
if element ~= false and #element ~= 0 then
redis.call('ZADD' , dest , destscore , element[1] )
redis.call('ZREM' , source , element[1])
end
end
local temp = moveElement(KEYS[2], KEYS[1] , ARGV[2])
local temp = moveElement(KEYS[3], KEYS[1] , ARGV[2])
local score= redis.call(
'ZRANGEBYSCORE', KEYS[1], '-inf', ARGV[1], 'WITHSCORES' , 'LIMIT' , '0' , '10'
)
if score ~= false and #score ~= 0 then
local i = 1
while i<=#score do
redis.call('ZREM', KEYS[1] , score[i])
redis.call('ZREM', KEYS[2] , score[i])
redis.call('ZADD', KEYS[3], ARGV[1] , score[i])
i=i+2
end
end
return score
First, line 5, element will never be false
. I suspect you are trying to catch an error but in this case it will be a table with an err
field. Same thing for score
further down.
Then, you assign two local temp
variables and never use them later on.
Your while
loop is better written with a for
:
for i=1,#score,2 do
redis.call('ZREM', KEYS[1] , score[i])
redis.call('ZREM', KEYS[2] , score[i])
redis.call('ZADD', KEYS[3], ARGV[1] , score[i])
end
Why are you using WITHSCORES
in moveElement? You do not use it.
Other than that, can you explain more clearly what you want to achieve so that I can help you further? An operation like that should certainly not take 24s anyway (except if you're running on an antique machine perhaps).
EDIT
Here is a simpler version of the script taking into account what you told me so far:
local now = ARGV[1]
local timed_out = ARGV[2]
local pending_jobs = KEYS[1]
local running_jobs = KEYS[2]
local jobs
local not_empty = function(x)
return (type(x) == "table") and (not x.err) and (#x ~= 0)
end
-- check if some jobs have timed out
-- if yes, take the oldest one and queue it again
jobs = redis.pcall(
'ZRANGEBYSCORE', timed_out, '-inf', timed_out, 'LIMIT', '0', '1'
)
if not_empty(jobs) then
redis.call('ZADD', pending_jobs, now, jobs[1])
redis.call('ZREM', running_jobs, jobs[1])
end
-- check if there are jobs ready
-- if yes, take the 10 oldest ones and run them
jobs = redis.pcall(
'ZRANGEBYSCORE', KEYS[1], '-inf', ARGV[1], 'LIMIT', '0', '10'
)
if not_empty(jobs) then
for i=1,#jobs do
redis.call('ZREM', pending_jobs, jobs[i])
redis.call('ZADD', running_jobs, now, jobs[i])
end
end
return jobs
Looking at this script I can see absolutely no reason why it would take as much as 24s. So either:
There is not much more I can say unless you can provide a .rdb with data that exhibits this issue...
Upvotes: 6