Fixing Job processing order without munging the job IDS #708
Conversation
Redis sorts zsets lexographically for members with the same priority. This causes jobs to run out of order. If you submit 11 jobs at the same priority level, they will run as 1,10,11,2,3,4,5,6,7,8,9. This adds a zid to the job that prepends the number of digits in the job id (001)1. This new zset value will be sorted correctly by redis. This is not desirable as a general purpose id, so it is hidden and the standard jobid can be used for all external purposes. The prepended value is stripped off when doing zpop.
Fixing range by and delay code so all tests pass again.
|
I know there have been a couple fixes for this submitted, but I wanted one that didn't permanently mess up the job id. This should preserve a FIFO job order up to a 999 digit job id while presenting the normal expected job id to the user of the library. I am happy to clean it up some if that would help get it merged. |
|
According to the docs for redis, INCR will max out at a 64bit signed integer: 9223372036854775807. So this method of job id sorting will outlast the redis server. A 2 digit length value would actually be sufficient. |
|
Thanks for this. Looks good. I think it's a good bet to keep id length to two digits. A job id of more than 99 digits is a bit too far fetched in my opinion :-) My suggestion is to use a |
The suggestion to remove 2 characters from the zid seemed reasonable. Also renaming parseID to stripFIFO to be consistent with the other file.
|
Those seem like good suggestions. Changes have been pushed. |
I didn't like defining the strip FIFO and create FIFO in separate files. This collocates them in one place next to the getKey call that is used everywhere.
|
Moved the stripFIFO and createFIFO calls to the client object so they can be updated in one place. Just like the getKey function. |
I was using tab characters, switching to spaces.
|
will check this soon, thank you guys... |
Expecting a string was causing a crash when undefined was passed in. Undefined doesn't resolve to an id, so we return null.
|
merged in 0.10.3 |
This is my attempt at fixing the FIFO issue.
I created an internal zid value that is used for zsets. This string value is in form: (id.length)id.
This allows for the ID to be parsed back out while maintaining the job order.
The length portion is fixed at 3 characters, which allow for ids to stay in order up to 999 digits long. This seemed sufficient to me, but is an upper limit. This could be easily increased if there is a need.