Skip to content

Commit

Permalink
Add random string to process identity.
Browse files Browse the repository at this point in the history
In environments with deterministic hostnames and PIDs (i.e. Heroku), the
current system for constructing a process's identity string produces
collisions (e.g. a new process can assume the identity of one that has
died). In order to address this, we add a random string to the identity string.
Issue sidekiq#2071.
  • Loading branch information
michaeldiscala committed Dec 31, 2014
1 parent a6709a9 commit ebd06ec
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Changes.md
@@ -1,6 +1,6 @@
HEAD
-----------

- Add random integer to process identity [#2113, michaeldiscala]
- Log Sidekiq Pro's Batch ID if available [#2076]
- Refactor Processor Redis usage to avoid redis/redis-rb#490 [#]
- Add better usage text for `sidekiqctl`.
Expand Down
3 changes: 2 additions & 1 deletion lib/sidekiq/api.rb
Expand Up @@ -616,6 +616,7 @@ def size
# 'queues' => ['default', 'low'],
# 'busy' => 10,
# 'beat' => <last heartbeat>,
# 'identity' => <unique string identifying the process>,
# }
class Process
def initialize(hash)
Expand Down Expand Up @@ -655,7 +656,7 @@ def signal(sig)
end

def identity
@id ||= "#{self['hostname']}:#{self['pid']}"
self['identity']
end
end

Expand Down
1 change: 1 addition & 0 deletions lib/sidekiq/launcher.rb
Expand Up @@ -75,6 +75,7 @@ def start_heartbeat
'concurrency' => @options[:concurrency],
'queues' => @options[:queues].uniq,
'labels' => Sidekiq.options[:labels],
'identity' => identity,
}
# this data doesn't change so dump it to a string
# now so we don't need to dump it every heartbeat.
Expand Down
7 changes: 6 additions & 1 deletion lib/sidekiq/util.rb
@@ -1,4 +1,5 @@
require 'socket'
require 'securerandom'
require 'sidekiq/exception_handler'
require 'sidekiq/core_ext'

Expand Down Expand Up @@ -30,8 +31,12 @@ def hostname
ENV['DYNO'] || Socket.gethostname
end

def process_nonce
@@process_nonce ||= SecureRandom.hex(6)
end

def identity
@@identity ||= "#{hostname}:#{$$}"
@@identity ||= "#{hostname}:#{$$}:#{process_nonce}"
end

def fire_event(event)
Expand Down
4 changes: 2 additions & 2 deletions lib/sidekiq/web.rb
Expand Up @@ -45,8 +45,8 @@ def custom_tabs
end

post "/busy" do
if params['hostname']
p = Sidekiq::Process.new('hostname' => params["hostname"], 'pid' => params['pid'])
if params['identity']
p = Sidekiq::Process.new('identity' => params['identity'])
p.quiet! if params[:quiet]
p.stop! if params[:stop]
else
Expand Down
15 changes: 12 additions & 3 deletions test/test_api.rb
Expand Up @@ -374,7 +374,15 @@ class ApiWorker
end

it 'can enumerate processes' do
odata = { 'pid' => 123, 'hostname' => hostname, 'key' => "#{hostname}:123", 'started_at' => Time.now.to_f - 15 }
identity_string = "identity_string"
odata = {
'pid' => 123,
'hostname' => hostname,
'key' => identity_string,
'identity' => identity_string,
'started_at' => Time.now.to_f - 15,
}

time = Time.now.to_f
Sidekiq.redis do |conn|
conn.multi do
Expand All @@ -392,8 +400,9 @@ class ApiWorker
assert_equal 123, data['pid']
data.quiet!
data.stop!
assert_equal "TERM", Sidekiq.redis{|c| c.lpop("#{hostname}:123-signals") }
assert_equal "USR1", Sidekiq.redis{|c| c.lpop("#{hostname}:123-signals") }
signals_string = "#{odata['key']}-signals"
assert_equal "TERM", Sidekiq.redis{|c| c.lpop(signals_string) }
assert_equal "USR1", Sidekiq.redis{|c| c.lpop(signals_string) }
end

it 'can enumerate workers' do
Expand Down
18 changes: 12 additions & 6 deletions test/test_web.rb
Expand Up @@ -50,17 +50,23 @@ def perform(a, b)
end

it 'can quiet a process' do
assert_nil Sidekiq.redis { |c| c.lpop "host:pid-signals" }
post '/busy', 'quiet' => '1', 'hostname' => 'host', 'pid' => 'pid'
identity = 'identity'
signals_key = "#{identity}-signals"

assert_nil Sidekiq.redis { |c| c.lpop signals_key }
post '/busy', 'quiet' => '1', 'identity' => identity
assert_equal 302, last_response.status
assert_equal 'USR1', Sidekiq.redis { |c| c.lpop "host:pid-signals" }
assert_equal 'USR1', Sidekiq.redis { |c| c.lpop signals_key }
end

it 'can stop a process' do
assert_nil Sidekiq.redis { |c| c.lpop "host:pid-signals" }
post '/busy', 'stop' => '1', 'hostname' => 'host', 'pid' => 'pid'
identity = 'identity'
signals_key = "#{identity}-signals"

assert_nil Sidekiq.redis { |c| c.lpop signals_key }
post '/busy', 'stop' => '1', 'identity' => identity
assert_equal 302, last_response.status
assert_equal 'TERM', Sidekiq.redis { |c| c.lpop "host:pid-signals" }
assert_equal 'TERM', Sidekiq.redis { |c| c.lpop signals_key }
end
end

Expand Down
3 changes: 1 addition & 2 deletions web/views/busy.erb
Expand Up @@ -39,8 +39,7 @@
<td>
<div class="btn-group pull-right">
<form method="POST">
<input type="hidden" name="hostname" value="<%= process['hostname'] %>"/>
<input type="hidden" name="pid" value="<%= process['pid'] %>"/>
<input type="hidden" name="identity" value="<%= process['identity'] %>"/>
<button class="btn btn-warn" type="submit" name="quiet" value="1"><%= t('Quiet') %></button>
<button class="btn btn-danger" type="submit" name="stop" value="1"><%= t('Stop') %></button>
</form>
Expand Down

0 comments on commit ebd06ec

Please sign in to comment.