Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jobs are not in order or not shown in order when displayed in the UI #33

Closed
techwraith opened this issue Jul 7, 2011 · 27 comments
Closed

Comments

@techwraith
Copy link

See what I mean here: http://twitpic.com/5mpuqo

Looks like it's being shown in alpha order instead of numerical.

@tj
Copy link
Contributor

tj commented Jul 7, 2011

haha yeah i've definitely noticed that too. but i was testing something else so i didn't look into it much yet

@tj
Copy link
Contributor

tj commented Jul 8, 2011

hmm it seems like they are ordered correct, however without a refresh they seem to be tacked on the bottom regardless of priority, kinda funky, looking into it

@tj
Copy link
Contributor

tj commented Jul 8, 2011

oh nvm it's because the dom elements are not sorted so existing ones stay where they are

@tj
Copy link
Contributor

tj commented Jul 8, 2011

verified that they are processed in proper order as well

@alvinsw
Copy link

alvinsw commented Sep 27, 2012

As of version 0.4.1, they are still not processed in proper order. A queue is intuitively expected to be FIFO, but redis sorted set is sorting the elements numerically only for the score. Kue implementation is to use the priority number for redis score. This means that a job with the same priority is sorted based on the id as STRING instead of NUMBER. A job with id 10 will be processed before job with id 2. I think this is unintuitive and a violation of the queue principle. This bug is clearly demonstrated in this screencast: http://nodetuts.com/tutorials/27-kue-jobs.html#video

@sebicas
Copy link

sebicas commented Oct 9, 2012

I have the same problem... any ideas of how can this be fixed?

@imZack
Copy link

imZack commented May 8, 2014

I still can't get the correct order of jobs. (version 0.7.5)
Something wrong with me?
gut

@behrad
Copy link
Collaborator

behrad commented May 8, 2014

First, Jobs are ordered by their priority not job ids.
Secondly, New jobs are added at the bottom of your loaded page, you may refresh your page then to see them in priority order.

@imZack
Copy link

imZack commented May 8, 2014

Yep, I got it. But I noticed the commits above provides sorting function by id, it works well as it should be therefore when the UI web scrolling down, loading more data(by increasing ends range) the job list did't sorted correctly(the json api's output is always ordered by id) that's what I mentioned.

@behrad
Copy link
Collaborator

behrad commented May 8, 2014

That sort function has been merged long ago...

it's because the dom elements are not sorted so existing ones stay where they are

Anyone can help providing a patch?

@imZack
Copy link

imZack commented May 8, 2014

So this issue should be reopen? Because the issue just had been solved partially (JSON API).
I will trying to make a patch in next few days.

@behrad behrad reopened this May 8, 2014
@behrad
Copy link
Collaborator

behrad commented May 8, 2014

This should be handled in client in an efficient way, any patch is welcome.

@nisaacson
Copy link

I am still seeing an issue of jobs processing in the incorrect order. I want to process earlier jobs first. I understand that priority has precedence over id but i need to process an arbitrary amount of jobs where the only sorting criteria is earlier jobs get processed before later jobs.

If I set all jobs to have the same priority, then the order jobs get processed is 1, 10...2

Is there a way I can force FIFO behavior on the queue so that jobs get processed with ids 1, 2, 3...10?


Perhaps the kue code needs to be changed to something like this answer http://stackoverflow.com/a/13820749 to maintain the proper sorting in redis.

Right now when I look at the list of inactive test_job_type in redis I see

127.0.0.1:6379> zrange  q:jobs:test_job_type:inactive 0 -1 withscores
 1) "1"
 2) "-10"
 3) "10"
 4) "-10"
 5) "11"
 6) "-10"
 7) "12"
 8) "-10"
 9) "13"
10) "-10"
11) "14"
12) "-10"
13) "15"
14) "-10"
15) "2"
16) "-10"
17) "3"
18) "-10"
19) "4"
20) "-10"
21) "5"
22) "-10"
23) "6"
24) "-10"
25) "7"
26) "-10"
27) "8"
28) "-10"
29) "9"
30) "-10"

@behrad
Copy link
Collaborator

behrad commented Jul 2, 2014

Yes, I was aware of this. The source is lexicographic ordering of set id's in redis. That link provides an implementation which is simple but ugly, let me think more on this, Thank you for the link.

@nisaacson
Copy link

One option is to zero pad the to the front of the job id when saving to the sorted set in redis, and then call parseInt when fetching them back to get an actual numerical id.

One downside is there an upper bound on how many jobs can be stored that is controlled by the amount of zeros prepended to the front

var id = 1

var redisJobID = 000000001
// add job to redis inactive sorted set
...

/// Then change the zpop to call parseInt on the reply from redis
self.zpop(client.getKey('jobs:' + self.type + ':inactive'), function (err, id) {
  if (err || !id) {
    self.job = null;
    return fn(err /*|| "No job to pop!"*/);
  }
  id = parseInt(id, 10)
  Job.get(id, fn);

@nisaacson
Copy link

I put together a sample implementation here https://github.com/nisaacson/kue/tree/padded-job-id

@bblack
Copy link

bblack commented Jul 22, 2014

@nisaacson : Instead of appending zeroes, we can avoid the upper limit by pre-pending to the string a byte such as ? that is ordered lexicographically after the characters 1 through 9, in a quantity equal to the length of the job id in base 10.

Additionally, if we require a very recent version of redis, we can specify a substitute string for lexicographic ordering on zadd, that will be used on zrangebylex: http://redis.io/commands/zrangebylex

@behrad behrad added this to the 0.9.0 milestone Jul 25, 2014
@behrad behrad removed this from the 0.3.0 milestone Jul 25, 2014
@dweinstein
Copy link

getting sorting to work right on the UI would be awesome 👍

@behrad
Copy link
Collaborator

behrad commented Feb 18, 2015

the sorting issue is not related to UI and is builtin in regard with how Redis sorting works. Please read linked issues above in detail...
This will be fixed in later versions

@invernizzie
Copy link

The fact that this bug is still open means kue does not actually behave as a priority queue?
I'd appreciate if someone would let me know before I actually try it, and save me some precious time.

This sounds weird, however, since kue is avertised as a "priority job queue"...

@dweinstein
Copy link

The problem isn't in the job queue or the priority of those jobs, but in the display of the jobs in a web based UI. Often the jobs are not displayed in a sorted (numeric) order, which has been an annoyance for using kue at least for me.

@behrad
Copy link
Collaborator

behrad commented Mar 6, 2015

this is sure a bug within kue, the source is how redis behaves on Sorted Set key collations, However this wont affect the prioritization of jobs, but sorting them.
This will be solved in future minor version bump.

@invernizzie
Copy link

Thanks for the clarification.

El jue, mar 5, 2015 21:11, Behrad notifications@github.com escribió:

this is sure a bug within kue, the source is how redis behaves on Sorted
Set key collations, However this wont affect the prioritization of jobs,
but sorting them.
This will be solved in future minor version bump.


Reply to this email directly or view it on GitHub
#33 (comment).

@behrad
Copy link
Collaborator

behrad commented Mar 6, 2015

and this happens just for job ids when most significant digit is moved forward

1
10
100
1000
...

@behrad
Copy link
Collaborator

behrad commented Mar 22, 2015

this is moved to 1.0.0

@behrad behrad modified the milestones: 1.0.0, 0.9.0 Mar 22, 2015
@behrad behrad changed the title Jobs are either not completed in order or not shown in order when displayed in the UI. Jobs are not in order or not shown in order when displayed in the UI Apr 24, 2015
@behrad
Copy link
Collaborator

behrad commented Jul 30, 2015

#678

@behrad
Copy link
Collaborator

behrad commented Nov 20, 2015

Fixed in 0.10.3

@behrad behrad closed this as completed Nov 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants