From bd59becebd4fa80d6ff4d692f852202639e9e8a3 Mon Sep 17 00:00:00 2001 From: Dmitry Krasnoukhov Date: Mon, 14 Apr 2014 13:30:59 +0300 Subject: [PATCH] Do not stop heartbeat even when manager is already stopped * Heartbeat should never stop (discussion: https://github.com/mperham/sidekiq/commit/ba8c8a57b962489333ce2cc01bdcc0aa381572b8#commitcomment-5996837) * Test that heartbeat stores data in Redis with ttl --- lib/sidekiq/manager.rb | 2 -- test/test_manager.rb | 63 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/lib/sidekiq/manager.rb b/lib/sidekiq/manager.rb index c0c50d25..19d6afb0 100644 --- a/lib/sidekiq/manager.rb +++ b/lib/sidekiq/manager.rb @@ -139,8 +139,6 @@ module Sidekiq proctitle << 'stopping' if stopped? $0 = proctitle.join(' ') - return if stopped? - ❤(key) after(5) do heartbeat(key, data) diff --git a/test/test_manager.rb b/test/test_manager.rb index 6d78af02..3d131429 100644 --- a/test/test_manager.rb +++ b/test/test_manager.rb @@ -1,10 +1,13 @@ require 'helper' require 'sidekiq/manager' -require 'sidekiq/util' class TestManager < Sidekiq::Test describe 'manager' do + before do + Sidekiq.redis {|c| c.flushdb } + end + it 'creates N processor instances' do mgr = Sidekiq::Manager.new(options) assert_equal options[:concurrency], mgr.ready.size @@ -79,25 +82,63 @@ class TestManager < Sidekiq::Test end describe 'heartbeat' do - describe 'proctitle' do - it 'sets useful info' do - mgr = Sidekiq::Manager.new(options) - mgr.heartbeat('identity', heartbeat_data) + before do + uow = Object.new + @processor = Minitest::Mock.new + @processor.expect(:async, @processor, []) + @processor.expect(:process, nil, [uow]) + + @mgr = Sidekiq::Manager.new(options) + @mgr.ready << @processor + @mgr.assign(uow) + + @processor.verify + end + + describe 'when manager is active' do + before do + @mgr.heartbeat('identity', heartbeat_data) + end + + it 'sets useful info to proctitle' do proctitle = $0 - assert_equal $0, "sidekiq #{Sidekiq::VERSION} myapp [0 of 3 busy]" + assert_equal "sidekiq #{Sidekiq::VERSION} myapp [1 of 3 busy]", $0 $0 = proctitle end - it 'indicates when stopped' do - mgr = Sidekiq::Manager.new(options) - mgr.stop - mgr.heartbeat('identity', heartbeat_data) + it 'stores process info in redis' do + info = Sidekiq.redis { |c| c.hmget('identity', 'busy') } + assert_equal ["1"], info + expires = Sidekiq.redis { |c| c.pttl('identity') } + assert_in_delta 60000, expires, 1 + end + end + describe 'when manager is stopped' do + before do + @processor.expect(:alive?, []) + @processor.expect(:terminate, []) + + @mgr.stop + @mgr.processor_done(@processor) + @mgr.heartbeat('identity', heartbeat_data) + + @processor.verify + end + + it 'indicates status in proctitle' do proctitle = $0 - assert_equal $0, "sidekiq #{Sidekiq::VERSION} myapp [0 of 3 busy] stopping" + assert_equal "sidekiq #{Sidekiq::VERSION} myapp [0 of 3 busy] stopping", $0 $0 = proctitle end + + it 'stores process info in redis' do + info = Sidekiq.redis { |c| c.hmget('identity', 'busy') } + assert_equal ["0"], info + expires = Sidekiq.redis { |c| c.pttl('identity') } + assert_in_delta 60000, expires, 1 + end end end