diff --git a/app/models/ems_refresh.rb b/app/models/ems_refresh.rb index 434a05d7d180..605d0f674e44 100644 --- a/app/models/ems_refresh.rb +++ b/app/models/ems_refresh.rb @@ -173,7 +173,13 @@ def self.queue_merge(targets, ems, create_task = false) # Items will be naturally serialized since there is a dedicated worker. MiqQueue.put_or_update(queue_options) do |msg, item| targets = msg.nil? ? targets : msg.data.concat(targets) - targets.uniq! if targets.size > 1_000 + if targets.size > 1_000 + manager_refresh_targets, application_record_targets = targets.partition { |key, _| key == "ManagerRefresh::Target" } + application_record_targets.uniq! + manager_refresh_targets.uniq! { |_, value| value.values_at(:manager_id, :association, :manager_ref) } + + targets = application_record_targets + manager_refresh_targets + end # If we are merging with an existing queue item we don't need a new # task, just use the original one diff --git a/spec/models/ems_refresh_spec.rb b/spec/models/ems_refresh_spec.rb index 01b719fb5f70..3bc0da84dbfd 100644 --- a/spec/models/ems_refresh_spec.rb +++ b/spec/models/ems_refresh_spec.rb @@ -45,6 +45,65 @@ end end + context "stopping targets unbounded growth" do + before(:each) do + _guid, _server, zone = EvmSpecHelper.create_guid_miq_server_zone + @ems = FactoryGirl.create(:ems_vmware, :zone => zone) + end + + let(:targets) do + targets = [] + (0..996).each do |i| + targets << ManagerRefresh::Target.load( + :manager_id => @ems.id, + :association => :vms, + :manager_ref => {:ems_ref => "vm_1"}, + :event_id => i, + :options => {:opt1 => "opt#{i}", :opt2 => "opt2"} + ) + end + + targets << vm_target + targets << host_target + targets << @ems + targets + end + + let(:vm_target) { FactoryGirl.create(:vm_vmware, :ext_management_system => @ems) } + let(:host_target) { FactoryGirl.create(:host_vmware, :ext_management_system => @ems) } + let(:manager_refresh_target) do + ManagerRefresh::Target.load( + :manager_id => @ems.id, + :association => :vms, + :manager_ref => {:ems_ref => "vm_1"}, + :event_id => 0, + :options => {:opt1 => "opt0", :opt2 => "opt2"} + ) + end + + it "doesn't call uniq on targets if size is <= 1000" do + described_class.queue_refresh(targets) + + expect(MiqQueue.last.data.size).to eq(1_000) + end + + it "uniques targets if next queuing breaches size 1000" do + described_class.queue_refresh(targets) + described_class.queue_refresh([host_target, vm_target, @ems]) + + expect(MiqQueue.last.data.size).to eq(4) + described_class.queue_refresh(targets) + expect(MiqQueue.last.data.size).to eq(4) + end + + it "uniques targets if queuing breaches size 1000" do + # We need different Vm, since targets are uniqued before queueing + described_class.queue_refresh(targets << FactoryGirl.create(:vm_vmware, :ext_management_system => @ems)) + + expect(MiqQueue.last.data.size).to eq(5) + end + end + context ".queue_refresh_task" do before do _guid, _server, zone = EvmSpecHelper.create_guid_miq_server_zone