From 4a6e6eb5e697ec38da6cb07a0fe4e1e38baf99e6 Mon Sep 17 00:00:00 2001 From: Hauke Klement Date: Fri, 20 Feb 2015 14:59:51 +0100 Subject: [PATCH] refactored refill of Docker container pool --- config/docker.yml.erb | 7 ++- lib/docker_container_pool.rb | 24 +++++----- spec/lib/docker_container_pool_spec.rb | 62 +++++++++++++++++--------- 3 files changed, 59 insertions(+), 34 deletions(-) diff --git a/config/docker.yml.erb b/config/docker.yml.erb index 79b8da17..ea234022 100644 --- a/config/docker.yml.erb +++ b/config/docker.yml.erb @@ -13,8 +13,11 @@ production: <<: *default pool: active: true - interval: 30 - maximum_refill_count: 30 + refill: + async: false + batch_size: 32 + interval: 30 + timeout: 60 workspace_root: <%= Rails.root.join('tmp', 'files', Rails.env) %> test: diff --git a/lib/docker_container_pool.rb b/lib/docker_container_pool.rb index b0ff2bc8..8b8338ab 100644 --- a/lib/docker_container_pool.rb +++ b/lib/docker_container_pool.rb @@ -7,10 +7,8 @@ class DockerContainerPool def self.clean_up @refill_task.try(:shutdown) - @containers.each do |key, value| - while !value.empty? do - DockerClient.destroy_container(value.shift) - end + @containers.values.each do |containers| + DockerClient.destroy_container(containers.shift) until containers.empty? end end @@ -35,18 +33,22 @@ class DockerContainerPool end def self.refill - ExecutionEnvironment.all.each do |execution_environment| - refill_count = [execution_environment.pool_size - @containers[execution_environment.id].length, config[:maximum_refill_count]].min - if refill_count > 0 - Concurrent::Future.execute do - @containers[execution_environment.id] += refill_count.times.map { create_container(execution_environment) } - end + ExecutionEnvironment.where('pool_size > 0').each do |execution_environment| + if config[:refill][:async] + Concurrent::Future.execute { refill_for_execution_environment(execution_environment) } + else + refill_for_execution_environment(execution_environment) end end end + def self.refill_for_execution_environment(execution_environment) + refill_count = [execution_environment.pool_size - @containers[execution_environment.id].length, config[:refill][:batch_size]].min + @containers[execution_environment.id] += refill_count.times.map { create_container(execution_environment) } + end + def self.start_refill_task - @refill_task = Concurrent::TimerTask.new(execution_interval: config[:interval], run_now: true) { refill } + @refill_task = Concurrent::TimerTask.new(execution_interval: config[:refill][:interval], run_now: true, timeout_interval: config[:refill][:timeout]) { refill } @refill_task.execute end end diff --git a/spec/lib/docker_container_pool_spec.rb b/spec/lib/docker_container_pool_spec.rb index c1125c25..aadc9ff6 100644 --- a/spec/lib/docker_container_pool_spec.rb +++ b/spec/lib/docker_container_pool_spec.rb @@ -80,34 +80,47 @@ describe DockerContainerPool do end describe '.refill' do - let(:config) { double } - let(:maximum_refill_count) { 5 } - - before(:each) do - expect(described_class).to receive(:config).and_return(config) - expect(config).to receive(:[]).with(:maximum_refill_count).and_return(maximum_refill_count) - end - + before(:each) { @execution_environment.update(pool_size: 10) } after(:each) { described_class.refill } - it 'regards all execution environments' do - ExecutionEnvironment.all.each do |execution_environment| - expect(described_class.instance_variable_get(:@containers)).to receive(:[]).with(execution_environment.id).and_call_original + context 'when configured to work synchronously' do + before(:each) do + expect(described_class).to receive(:config).and_return(refill: {async: false}) + end + + it 'works synchronously' do + expect(described_class).to receive(:refill_for_execution_environment) end end + context 'when configured to work asynchronously' do + before(:each) do + expect(described_class).to receive(:config).and_return(refill: {async: true}) + end + + it 'works asynchronously' do + expect_any_instance_of(Concurrent::Future).to receive(:execute) do |future| + expect(described_class).to receive(:refill_for_execution_environment) + future.instance_variable_get(:@task).call + end + end + end + end + + describe '.refill_for_execution_environment' do + let(:batch_size) { 5 } + + before(:each) do + expect(described_class).to receive(:config).and_return(refill: {batch_size: batch_size}) + end + + after(:each) { described_class.refill_for_execution_environment(@execution_environment) } + context 'with something to refill' do before(:each) { @execution_environment.update(pool_size: 10) } it 'complies with the maximum batch size' do - expect_any_instance_of(Concurrent::Future).to receive(:execute) do |future| - expect(described_class).to receive(:create_container).with(@execution_environment).exactly(maximum_refill_count).times - future.instance_variable_get(:@task).call - end - end - - it 'works asynchronously' do - expect(Concurrent::Future).to receive(:execute) + expect(described_class).to receive(:create_container).with(@execution_environment).exactly(batch_size).times end end @@ -115,16 +128,23 @@ describe DockerContainerPool do before(:each) { @execution_environment.update(pool_size: 0) } it 'does nothing' do - expect(Concurrent::Future).not_to receive(:execute) + expect(described_class).not_to receive(:create_container) end end end describe '.start_refill_task' do + let(:interval) { 30 } + let(:timeout) { 60 } + + before(:each) do + expect(described_class).to receive(:config).at_least(:once).and_return(refill: {interval: interval, timeout: timeout}) + end + after(:each) { described_class.start_refill_task } it 'creates an asynchronous task' do - expect(Concurrent::TimerTask).to receive(:new).and_call_original + expect(Concurrent::TimerTask).to receive(:new).with(execution_interval: interval, run_now: true, timeout_interval: timeout).and_call_original end it 'executes the task' do