Vivek Goel
Vivek Goel

Reputation: 24160

Redis optimize time taken by ZRANGEBYSCORE

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.

    1. [Number of keys]
  1. foo1 sorted set name having 6k.
  2. foo2 sorted set name.
  3. foo3 Sorted set. Where member from foo1 to foo2 are moved.
  4. Current timestamp.
  5. Current timestamp - 6 min.

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:

  1. foo1 is contains member having score as timestamp, at which they need to be fired.
  2. Whenever an event need to fired in future. It is put in foo1 having score as schedule time.
  3. My script read all member (one by one) having score less than or equal current timestamp from foo1 and move them to foo3. This is done one by one and elements are moved. This element is assigned to assigned to some worker. (Hidden)
  4. If worker completes the work. It remove member from foo3.
  5. After x seconds timeout if member was not remove foo3. Script moves it back to foo1. So it can be reassigned to some other worker.

Upvotes: 3

Views: 3875

Answers (1)

catwell
catwell

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 something wrong with your Redis setup, or
  • you are not really doing this, or
  • your measurements are wrong.

There is not much more I can say unless you can provide a .rdb with data that exhibits this issue...

Upvotes: 6

Related Questions