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

Introduce configuration override #1

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,45 @@ environment variables can be used to determine environment). For example in
production:
"foo,bar,baz": 4

### Custom Configuration

Sometimes, a static YAML file doesn't cut it. At high levels of stress, you
may want to update Resque::Pool's configuration settings on the go, maybe via
other YAML files on the server or a database-backed solution. This is
accomplished by creating a custom configuration object.

class QueueConfigurator
def call(config)
config.merge { 'b' => 3 }
end
end

pool = Resque::Pool.new({ 'a' => 1 }, QueueConfigurator.new)
pool.config
# => { 'a' => 1, 'b' => 3 }

The configuration object can be any sort of `call`-able object, and it must
return a Resque::Pool compatible hash configuration.

If you are running Resque pool from the command line, you will need to do two
things:

1.) Create a configuration file passing your config override.

# ./config/resque_pool_config.rb
Resque::Pool.config_override = lambda { |config| { "foo,bar,baz" => 100 } }

2.) Run `resque-pool` and pass it the `--config-manager` option

$ resque-pool --config-manager ./config/resque_pool_config.rb

Your pool's configuration will now pull from your override.

**Warning**: *Using a custom configuration manager is risky. Only attempt to use
this feature if you are absolutely sure that your configuration manager does
not break your systems. You can completely override your pool configuration if
you are not careful!*

### Rake task config

Require the rake tasks (`resque/pool/tasks`) in your `Rakefile`, load your
Expand Down
37 changes: 31 additions & 6 deletions lib/resque/pool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@

module Resque
class Pool
SIG_QUEUE_MAX_SIZE = 5
DEFAULT_OVERRIDE_PROC = lambda { |config| config }
SIG_QUEUE_MAX_SIZE = 5
DEFAULT_WORKER_INTERVAL = 5
QUEUE_SIGS = [ :QUIT, :INT, :TERM, :USR1, :USR2, :CONT, :HUP, :WINCH, ]
CHUNK_SIZE = (16 * 1024)
Expand All @@ -20,12 +21,27 @@ class Pool
attr_reader :config
attr_reader :workers

def initialize(config)
init_config(config)
def initialize(configuration, config_proc = nil)
init_config(configuration)
@config_proc = config_proc || self.class.config_override
@workers = Hash.new { |workers, queues| workers[queues] = {} }
procline "(initialized)"
end

# Config Override:
#
def self.config_override
@config_override || DEFAULT_OVERRIDE_PROC
end

def self.config_override=(override)
if override.respond_to? :call
@config_override = override
else
procline "Config override #{override.inspect} is not a callable object."
end
end

# Config: after_prefork {{{

# The `after_prefork` hook will be run in workers if you are using the
Expand Down Expand Up @@ -77,7 +93,7 @@ def self.run
if GC.respond_to?(:copy_on_write_friendly=)
GC.copy_on_write_friendly = true
end
Resque::Pool.new(choose_config_file).start.join
Resque::Pool.new(choose_config_file, config_override).start.join
end

# }}}
Expand All @@ -103,8 +119,8 @@ def load_config
else
@config ||= {}
end
environment and @config[environment] and config.merge!(@config[environment])
config.delete_if {|key, value| value.is_a? Hash }
environment and @config[environment] and @config.merge!(@config[environment])
@config.delete_if {|key, value| value.is_a? Hash }
end

def environment
Expand Down Expand Up @@ -340,6 +356,14 @@ def signal_all_workers(signal)
# }}}
# ???: maintain_worker_count, all_known_queues {{{

def refresh_config

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be refresh_config!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also noticed that if I new up a Resque::Pool instance and specify a configuration manager, that if I call pool.config, I get the original configuration, not the configuration from the configuration manager. I wonder if in specifying a config manager we call refresh_config at that point in time to make sure the config has changed as one might expect. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did notice that. We would need to change the :config attr reader into a plain method call that refreshed everything. Sound like a reasonable approach?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried implementing it, and it dawned on me. If we do a lot of calls to config in a loop, we will be calling the configuration override a lot. I don't know if that is exactly what we want. Maybe a proxy that only will reload it if it hasn't been loaded in the last five seconds or something?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets look/talk about it in person.

cloned = @config.dup
@config = @config_proc.call(@config)
rescue => e
log "There was an issue updating the configuration: #{e.message} #{e.backtrace.join("\n")}"
cloned
end

def maintain_worker_count
all_known_queues.each do |queues|
delta = worker_delta_for(queues)
Expand All @@ -349,6 +373,7 @@ def maintain_worker_count
end

def all_known_queues
refresh_config
config.keys | workers.keys
end

Expand Down
15 changes: 15 additions & 0 deletions lib/resque/pool/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ def run
redirect opts
setup_environment opts
set_pool_options opts
set_pool_configuration_override opts
start_pool
end

Expand All @@ -38,6 +39,7 @@ def parse_options
opt :nosync, "Don't sync logfiles on every write"
opt :pidfile, "PID file location", :type => String, :short => "-p"
opt :environment, "Set RAILS_ENV/RACK_ENV/RESQUE_ENV", :type => String, :short => "-E"
opt :config_manager, "Path to configuration override definition", :type => String, :short => "-C"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sold on the name of this option. Having a -c and a -C flag that do completely different things is frustrating and a bit confusing. What about something like -o and --override?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im fine with changing it, not sure --override is the best option. Really, this file is wiring plugins to extension points, of which one exists currently (a config manager). So maybe -p --plugin? Also, this is dependency injection (has some negative connotations in the ruby world, but thats what this is). I wonder if -i --inject would work? Open to other ideas.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely not sold on --config-manager, but I like the idea of -p or -i. Needs some more thought, I think...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I'll wait to see if anyone else has stronger/better opinions.

opt :term_graceful_wait, "On TERM signal, wait for workers to shut down gracefully"
opt :term_graceful, "On TERM signal, shut down workers gracefully"
opt :term_immediate, "On TERM signal, shut down workers immediately (default)"
Expand Down Expand Up @@ -115,6 +117,19 @@ def set_pool_options(opts)
end
end

def set_pool_configuration_override(opts)
require 'pathname'
if opts[:config_manager]
file = Pathname.new(opts[:config_manager]).expand_path
if file.exist?
require file
Resque::Pool.log "Resque Pool configuration is overriden in #{opts[:config_manager]}"
else
Resque::Pool.log "--config-manager #{file} error: File does not exist"
end
end
end

def setup_environment(opts)
Resque::Pool.app_name = opts[:appname] if opts[:appname]
ENV["RACK_ENV"] = ENV["RAILS_ENV"] = ENV["RESQUE_ENV"] = opts[:environment] if opts[:environment]
Expand Down
3 changes: 3 additions & 0 deletions spec/resque_pool_override.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Resque::Pool.config_override = ->(config) {
{ "foo" => 1, "bar" => 2, "baz" => 3, "foo,bar,baz" => 4 }
}
50 changes: 50 additions & 0 deletions spec/resque_pool_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,56 @@
}
end

describe Resque::Pool, "when using a custom configuration manager" do
let(:config) do
{ 'foo' => 1, 'bar' => 2, 'foo,bar' => 3, 'bar,foo' => 4, }
end
subject { Resque::Pool.new(config, manager) }
before { subject.all_known_queues }

context "when no errors are raised" do
let(:manager) do
lambda { |config| config.merge( "fooey" => 10 ) }
end
it "should merge the other values into the pool's config" do
subject.config["fooey"].should == 10
subject.config["foo"].should == 1
subject.config["bar"].should == 2
subject.config["foo,bar"].should == 3
subject.config["bar,foo"].should == 4
end
end

context "when an error is raised" do
let(:manager) do
lambda { |config| raise "config error was raised" }
end

it "should replace the config of the original on an error" do
subject.config["foo"].should == 1
subject.config["bar"].should == 2
subject.config["foo,bar"].should == 3
subject.config["bar,foo"].should == 4
end
end

context "when a config override is globally set" do
around do |e|
Resque::Pool.config_override = lambda { |config|
{ "foo,bar,baz" => 100 }
}
e.run
Resque::Pool.config_override = nil
end
let(:manager) { nil }

it "should use the global configuration manager" do
subject.config.should == { "foo,bar,baz" => 100 }
end
end

end

describe Resque::Pool, "when loading a simple pool configuration" do
let(:config) do
{ 'foo' => 1, 'bar' => 2, 'foo,bar' => 3, 'bar,foo' => 4, }
Expand Down