Version 1.0.0 seems to be ignoring the sidekiq_role that is set.

[codebycliff]

After upgrading from 0.20.0 to 1.0.0, Sidekiq is now being started on all of our app servers, regardless of the role set. Downgrading to 0.20.0 fixes it so that it only starts Sidekiq on the server that has that role specified.

Our configuration is as follows:

config/deploy.rb

set :sidekiq_role, :worker

config/deploy/production.rb

server 'app01.example.com', user: 'deployer', roles: %w{app web db}
server 'app02.example.com', user: 'deployer', roles: %w{app worker}
server 'app04.example.com', user: 'deployer', roles: %w{app web}

Thanks in advance.

[Looooong]

It is because sidekiq_role has been changed to plural form sidekiq_roles.

This should have been documented in the CHANGELOG as a breaking change.

[codebycliff]

Sounds good. Should it be added to the changelog as a breaking change now for people that haven't upgraded?

[rhomeister]

This was pretty painful for me to find and fix. A deprecation warning or something would have been appreciated....

[mensfeld]

Despite changing that it still does not work :/

Sidekiq actions are not running at all for deployment flow...

[mensfeld]

Guys any news on that one? I could help if needed

[Looooong]

Mine works fine. Do you set the role on the server properly?

[jsantos]

The same is happening to me. Other gems recognize my roles just fine, but this one is not recognizing them, which turns out to be a nightmare since the process number is not interpreted correctly.

@Looooong I'm defining the roles using the server-based syntax, are you doing it in a different way by any chance?

[Looooong]

I use Capistrano 3.10.1 and Capistrano Sidekiq 1.0.0. Here is how I config the Sidekiq:

set :sidekiq_roles, :sidekiq
set :sidekiq_config, -> { File.join(current_path, 'config', 'sidekiq.yml') }

server 'host', user: fetch(:user), roles: %w[app migration sidekiq], primary: true

[jsantos]

@Looooong yeah I'm using the exact same versions.

I believe the issue only occurs when there is more than one role (I control the number of sidekiq processes based on the role by using <role>_processes.

[mensfeld]

@jsantos same here - exactly the same case.

[Looooong]

You guys should take a look at sidekiq.rake:142 to see if there is any problem with the code. I don't see any problem with it, perhaps you can.

[jsantos]

Guess I've found the issue. After much debugging I figured out the issue lies exactly where @Looooong pointed out.

The .select! on sidekiq_roles.select! { |role| host.roles.include?(role) } is mutating :sidekiq_roles. So, for the first role the list of pidfiles is correctly returned, but the same won't happen to further method calls.

Example:

set :sidekiq_pid, File.join(shared_path, 'tmp', 'pids', 'sidekiq.pid')
set :sidekiq_roles, [:master, :slave]
set :master_processes, 1
set :slave_processes, 2

# On the first host (host.roles = [:master]): 
> pid_files
[
    [0] "app/shared/tmp/pids/sidekiq-0.pid"
]

# On the second host (roles = [:slave]):
> pid_files
[]

Without mutating the sidekiq_roles variable, we get the following:

# On the first host (host.roles = [:master]): 
> pid_files
[
    [0] "app/shared/tmp/pids/sidekiq-0.pid"
]

# On the second host (roles = [:slave]):
> pid_files
[
    [0] "app/shared/tmp/pids/sidekiq-0.pid",
    [1] "app/shared/tmp/pids/sidekiq-1.pid"
]

Since the method is used every time we call each_process_with_index, this kinda screws things up.

So on this line, we should use:

sidekiq_roles = sidekiq_roles.select { |role| host_roles.include?(role) }

To avoid the mutation of sidekiq_roles.

I'll issue a PR to fix this.

[Looooong]

Good finding there! I have never thought that sidekiq_roles = Array(fetch(:sidekiq_roles)) would return the original array.

[jsantos]

@Looooong Yeah, that's why it took so long to debug.

For reference:

[1] pry(main)> sidekiq_roles = [:master, :slave]
[
    [0] :master,
    [1] :slave
]
[2] pry(main)> roles = Array(sidekiq_roles)
[
    [0] :master,
    [1] :slave
]
[3] pry(main)> roles.select! {|r| r == :master}
[
    [0] :master
]
[4] pry(main)> roles
[
    [0] :master
]
[5] pry(main)> sidekiq_roles
[
    [0] :master
]
``

[mensfeld]

@jsantos any chance on releasing 1.0.1?

[jsantos]

@mensfeld I just issued the PR, but don't have the power to merge it... @seuros might need to weight in