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

use listen gem to monitor config file and dynamic adjust resque workers count #62

Closed
wants to merge 4 commits into from

Conversation

comme
Copy link

@comme comme commented Sep 16, 2012

in my project, i must keep resque queues don't have queueing job,so the workers count shoud greater or equal jobs count, i kone send -HUP to resque-pool-manager can adjust workers count but i think resque-pool self supoort dynamic adjust workers count is better!

@nevans
Copy link
Collaborator

nevans commented Sep 17, 2012

I like it! I should take a closer look at it tomorrow, and I expect I'll merge it and roll it into the next release then. Thanks!

@nevans
Copy link
Collaborator

nevans commented Oct 24, 2012

I'm finally getting the chance to look into this. Two minor concerns:

  1. When the listen gem is running in non-blocking mode, it executes the callback in a different thread. Therefore the callback should be 100% threadsafe. Since all we're doing is calling the HUP code anyway, I'm going to fudge that by changing the callback to add a :HUP onto the sig_queue... and change the sig_queue from an Array into a thread-safe Queue.
  2. People who don't use that command-line option shouldn't need to have the gem. I'll move the require inside the :dynamic_adjust_workers_count= setter, and I'll move the dynamic_adjust_workers_count? accessor forward in its if statement for short-circuit purposes.

@nevans
Copy link
Collaborator

nevans commented Oct 24, 2012

Sorry, I didn't get around to making my stated changes last night. If you make those changes, I'll merge it in. Otherwise, I'll get around to it the next time i have a spare moment.

@nevans
Copy link
Collaborator

nevans commented Mar 25, 2015

I do think this is obsoleted by #85. I hope to investigate that PR and merge it (or something very like it) into master soon. This code could then be relatively easily reimplemented more cleanly on top of that code (and I'd gladly include an example config in the docs, or a --listen command line flag to automatically enable).

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.

2 participants