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

Support per-process options & better per-role options #141

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

metavida
Copy link

Note, this pull builds on top of pull #140, so includes all of its commits.

Per-process Options

Previously, if you wanted to specify different options for multiple sidekiq processes, you'd use :sidekiq_options_per_process. However, because pull #140 relies knowing the path in the :sidekiq_config, I needed to be able to specify a different :sidekiq_config value for each process.

With this pull request, it's now possible to pass an Array of values to most of the sidekiq options. Any option with a non-Array value will be applied to all processes.

An example from the README

set :sidekiq_processes,  2
set :sidekiq_log, 'log/background.log'
set :sidekiq_config, [
                       'config/sidekiq/high.yml',
                       'config/sidekiq/low.yml'
                     ]
set :sidekiq_queue,  [
                       [:high],
                       [:default, :low]
                     ]

In this example the first process will start with the following options:

  • log: 'log/background.log'
  • config: 'config/sidekiq/high.yml'
  • queue: 'high'

And the second sidekiq process will start with the following options:

  • log: 'log/background.log'
  • config: 'config/sidekiq/low.yml'
  • queue: 'low' AND 'default'

Better Per-role Options

Previously, if a user provided an Array of :sidekiq_role values, they could customize the :sidekiq_processes number per-role. However, I needed support for a different number of workers and various other config differences between roles.

With this pull request, it's now possible to set per-role options for most of the sidekiq options. If both a non-role-specific value and a role-specific value are provided, the role-specific value will be preferred. Also note that when using a role that does not contain the prefix :sidekiq_, the option names still retain the :sidekiq_ prefix to avoid potential naming conflicts with other caistrano plugins.

An example from the README

set :sidekiq_role, [:web, :sidekiq_worker]
set :sidekiq_log, 'log/background.log'

set :sidekiq_web_processes, 1
set :sidekiq_web_config, 'config/sidekiq/web.yml'

set :sidekiq_worker_processes, 2
set :sidekiq_worker_config, [
                              'config/sidekiq/worker_one.yml',
                              'config/sidekiq/worker_two.yml'
                            ]

server 'web1.example.com', roles: [:web]
server 'bg1.example.com',  roles: [:sidekiq_worker]

In this example the web1 host will have one process with the following options:

  • log: 'log/background.log'
  • config: 'config/sidekiq/web.yml'

The bg1 host will have two processes. The first will have the following options:

  • log: 'log/background.log'
  • config: 'config/sidekiq/worker_one.yml'

The second sidekiq process on bg1 will start with the following options:

  • log: 'log/background.log'
  • config: 'config/sidekiq/worker_two.yml'

...because presumably the developer is already smart enough to not put conflicting pidfile paths in their sidekiq config files.
Also update the default value to reflect the change made in seuros#116
For example

```
set :sidekiq_role, [:sidekiq_small, :sidekiq_big]
set :sidekiq_small_config 'config/sidekiq/small.yml'
set :sidekiq_big_processes 2
set :sidekiq_big_config ['config/sidekiq/first_big.yml', 'config/sidekiq/second_big.yml']
```
Example:

```
set :sidekiq_role,  [:web, :sidekiq_background]
set :sidekiq_web_config, 'config/sidekiq_web.yml'
set :sidekiq_background_config, 'config/sidekiq_bg.yml'
```
Also mark the sidekiq_options_per_process option as deprecated,
since a user can simply set sidekiq_options to an Array
...instead of reinventing the wheel.

Also, add more explicit documentation about the hack we have in place for backwards compatability.
While I think it's somewhat unlikely for a developer to want per-process env values, it's virtually free to add, so why not?
...and group options based on whether they're passed as options to sidekiq vs whether they're used to configure how capistrano behaves
This applies when stopping sidekiq
@itsmechlark
Copy link

PING

@vnazarenko
Copy link

Ping

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.

3 participants