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

queue events not triggered after worker pause and resume #423

Closed
aakashlpin opened this issue Sep 25, 2014 · 13 comments
Closed

queue events not triggered after worker pause and resume #423

aakashlpin opened this issue Sep 25, 2014 · 13 comments
Labels

Comments

@aakashlpin
Copy link

@behrad I figured out from the code that you are the author for pause and resume features (So directly addressing this to you.)

I have a use case where I am very tightly dependent on the queue events being received properly. However, even one pause and resume of the queue stops triggering these events. Could you look into it, please?

@behrad
Copy link
Collaborator

behrad commented Sep 25, 2014

on the queue events

Queue level events!? or Job level events!?
Are you pausing an specific worker? then It is not expected not to remove event pub/sub handler, However shutting down the kue, will stop all workers and remove the event pub/sub handler, so you won't receive any further events.

@aakashlpin
Copy link
Author

queue level events. I am not using job level events as reference to job level events is lost after server restart. For my use case, type is always of one single type with a single worker.
queue.process(type, function(job, done, ctx){})

I store a global reference to ctx when calling .process and then pause and resume when I need to use the server for other higher priority tasks. This causes queue level events to drop the pub/sub. Let me know if you would like to see more code!

@behrad
Copy link
Collaborator

behrad commented Sep 26, 2014

one single type with a single worker

Oh, the problem could be it. I'm not sure but Kue may want to cleanup if all workers are stopped!! I should check the code and solve it for pause/resume scenarios. I'd like you to write a test case :)

@aakashlpin
Copy link
Author

I'll do that in a few hours and let you know! Thanks :)

@aakashlpin
Copy link
Author

For the test case I have created a fresh repo at https://github.com/aakashlpin/kue-testcase

I have not been able to replicate the issue as-is from my existing codebase as the behaviour of kue has changed for this specific test case. However, the issue still exists in one form or the other. I have attached the screenshot of what happens when I run index.js. It always gets stuck at this point and does nothing more even though more jobs are in the queue. Could you please explain me this behaviour and guide me in the right direction? (In my original codebase though, the processing continues and events get lost. i.e I stop seeing job completed in the logs)

screenshot 2014-09-26 19 15 16

@behrad
Copy link
Collaborator

behrad commented Sep 27, 2014

as the behaviour of kue has changed for this specific test case

try to find what is being changed between your code/setup !?

Could you please explain me this behaviour and guide me in the right direction?

I should read/run your code, when I have time, but for now I can just say
When you are pausing workers, you would see no more jobs being processed, so make sure your resume is being called or you would feel no activity.
On the other hand, this is not logical to set different ctx from process of different workers into a single global variable. ctx is a per worker object.

@aakashlpin
Copy link
Author

Resume is being called as you would see in the logs in console. It even starts processing the next element in the queue but the success callback method done is never called. I am not sure why that would happen as I am not aware of kue internals. 

So even if I have one single worker, storing the ctx in a global variable is not logical?

@behrad
Copy link
Collaborator

behrad commented Sep 27, 2014

but the success callback method done is never called

You should call done not Kue! please double check your code if you have no logical errors on using kue.

If you have one job type (worker) that would be OK to globally store ctx :)

@aakashlpin
Copy link
Author

Yeah I understand! I am calling done not kue. Silly misunderstanding. I am sure my code is logically correct. Just whenever you have time, it would be great if you can go through the code and see if I am not doing things as it would be expected. 

Thanks in advance!

@aakashlpin
Copy link
Author

@behrad I have fixed the test case to reflect the initial issue.

try to find what is being changed between your code/setup !?

(Using setTimeout to simulate a delay while also using pause and resume was the issue earlier for the test case)

@behrad
Copy link
Collaborator

behrad commented Sep 30, 2014

nice, i should run your code when I have time

@behrad
Copy link
Collaborator

behrad commented Oct 1, 2014

this is definitely a bug in worker#resume which is not updating all flags properly. Thank you @aakashlpin

@behrad behrad added the Bug label Oct 1, 2014
@behrad behrad self-assigned this Oct 1, 2014
@behrad behrad closed this as completed in 23d2b20 Oct 1, 2014
@aakashlpin
Copy link
Author

Wuhoo! Fantastic. Thank you so much for fixing this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants