Skip to content

Conversation

@mhagstrand
Copy link
Contributor

The function handleDelayedItems() was overwriting the parameter
$timestamp from the call nextDelayedTimestamp() with timestamp of
the most delayed job. Therefor it would only process 1 seconds
worth of items before sleeping. A slighty delayed queue could
fall very far behind.

The function handleDelayedItems() was overwriting the parameter
$timestamp from the call nextDelayedTimestamp() with timestamp of
the most delayed job. Therefor it would only process 1 seconds
worth of items before sleeping. A slighty delayed queue could
fall very far behind.
@mhagstrand mhagstrand force-pushed the SleepingWithDelayInQueue branch from 0135413 to 073f25f Compare November 18, 2016 03:26
@rolfvreijdenberger
Copy link

rolfvreijdenberger commented Nov 18, 2016

duplicate of #33
I think it's wise to review both this patch (which keeps the $timestamp parameter as input for the method) and #33 which does not keep it (seems to be not needed in the called method itself).
In case of the same functionality, I'd prefer the simplest (#33) but then we would also need to change the called method ResqueScheduler::nextDelayedTimestamp($timestamp) to not use $timestamp.

@chrisboulton can you update me on this, I'd be more than happy to get alter the code appropiately
@danhunsaker @chrisboulton will this repository also be maintained by a group of people?

@chrisboulton
Copy link
Owner

I think I actually prefer the fix here vs in #33. #33 doesn't respect whatever timestamp was passed, which I know while isn't used today, is somewhat of a breaking compatibility change.

@chrisboulton chrisboulton merged commit 5954c98 into chrisboulton:master Nov 28, 2016
@rolfvreijdenberger
Copy link

fair enough. thanks for merging a fix.
the discussion over removal of dead code can be done elsewhere 👍

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.

3 participants