openstack/zaqar
Code Issues Proposed changes

Fix fail of claim after some message expire(Redis)

Browse Source 
If some messages in Redis are already expired, it's impossible to claim
any new messages.

Zaqar internally catches this error from running "claim_messages.lua":
"ResponseError: Error running script: @user_script:59: user_script:59:
attempt to compare nil with number". And returns 503 response code to
the user.

This happens because if some message is expired (it's <message_id>
record was automatically deleted from database), it's ID is still listed
in set "<project_id>.<queue_name>.messages" and Zaqar uses this ID. In
this case on attempt to get some values from <message_id>'s fields, it
gets 'nil' values.

This patch makes Zaqar check if returned field values from <message_id>
record are 'nil'. If values are not 'nil', process the record normally.
If values are 'nil', remember message ID. In the end of the script
"claim_messages.lua" garbage collect all such message IDs, because it's
a good opportunity to do so.

Closes-Bug: 1548135
Change-Id: Iedd2b82f6da30dd38bfdbb837bf9d0d4c282e222
This commit is contained in:
Eva Balycheva 2016-05-08 04:42:53 +03:00 committed by wangxiyuan
parent 454d4acf01
commit 48f3e9f9a2
1 changed files with 29 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
zaqar/storage/redis/scripts/claim_messages.lua
View File

@ -32,6 +32,7 @@ local BATCH_SIZE = 100
local start = 0
local claimed_msgs = {}
local msg_ids_to_cleanup = {}
local found_unclaimed = false
@ -55,34 +56,46 @@ while (#claimed_msgs < limit) do
if not found_unclaimed then
local msg = redis.call('HMGET', mid, 'c', 'c.e')
if msg[1] == '' or tonumber(msg[2]) <= now then
if msg[1] == false and msg[2] == false then
-- NOTE(Eva-i): It means the message expired and does not
-- actually exist anymore, we must later garbage collect it's
-- ID from the set and move on.
msg_ids_to_cleanup[#msg_ids_to_cleanup + 1] = mid
elseif msg[1] == '' or tonumber(msg[2]) <= now then
found_unclaimed = true
end
end
if found_unclaimed then
-- Found an unclaimed message, so claim it.
local msg_expires_prev = redis.call('HGET', mid, 'e')
-- Found an unclaimed message, so claim it
redis.call('HMSET', mid,
'c', claim_id,
'c.e', claim_expires)
-- Will the message expire early?
if tonumber(msg_expires_prev) < claim_expires then
if msg_expires_prev ~= false then
-- NOTE(Eva-i): Condition above means the message is not
-- expired and we really can claim it.
redis.call('HMSET', mid,
't', msg_ttl,
'e', msg_expires)
end
'c', claim_id,
'c.e', claim_expires)
claimed_msgs[#claimed_msgs + 1] = mid
-- Will the message expire early?
if tonumber(msg_expires_prev) < claim_expires then
redis.call('HMSET', mid,
't', msg_ttl,
'e', msg_expires)
end
if (#claimed_msgs == limit) then
break
claimed_msgs[#claimed_msgs + 1] = mid
if (#claimed_msgs == limit) then
break
end
end
end
end
end
if (#msg_ids_to_cleanup ~= 0) then
-- Garbage collect expired message IDs stored in msgset_key.
redis.call('ZREM', msgset_key, unpack(msg_ids_to_cleanup))
end
return claimed_msgs