Skip to content

properly handle delayed queues when using RESET_TO_HIGHEST_PRIORITY s…#133

Merged
gresrun merged 2 commits into
gresrun:masterfrom
uberall:fix-fromMultiplePriorityQueues
Jan 16, 2018
Merged

properly handle delayed queues when using RESET_TO_HIGHEST_PRIORITY s…#133
gresrun merged 2 commits into
gresrun:masterfrom
uberall:fix-fromMultiplePriorityQueues

Conversation

@bp-FLN

@bp-FLN bp-FLN commented Jan 2, 2018

Copy link
Copy Markdown
Contributor

…trategy

fixes #126

@ofirnk ofirnk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than small question, LGTM 👍


if queueType == 'zset' then
local firstMsg = redis.call('ZRANGE', queueName, '0', '0')
local firstMsg = redis.call('ZRANGEBYSCORE', queueName, '-inf', now, 'WITHSCORES', 'LIMIT', '0', '1')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I forgot already, but why WITHSCORES is needed here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not needed indeed, nice catch

i also removed it from the jesque_pop lua script

@gresrun
gresrun merged commit f67c8ff into gresrun:master Jan 16, 2018
@bp-FLN
bp-FLN deleted the fix-fromMultiplePriorityQueues branch January 23, 2018 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reset to highest priority next queue strategy ignores delayed tasks

3 participants