search

chrisboulton / php-resque-scheduler

An addon for php-resque that lets you queue jobs for execution some time in the future. Follows resque-scheduler.
MIT License
271 stars 129 forks source link

Fix bug where scheduler sleeps when processing a delayed queue #34

Closed mhagstrand closed 8 years ago

mhagstrand commented 8 years ago

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.

rolfvreijdenberger commented 8 years ago

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 commented 8 years ago

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.

rolfvreijdenberger commented 8 years ago

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