Merge pull request #17144 from Ladas/stop_unbounded_growth_of_queued_…

…targets

Stop unbounded growth of targets

app/models/ems_refresh.rb

@@ -173,7 +173,7 @@ 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
+      targets = uniq_targets(targets)
 
       # If we are merging with an existing queue item we don't need a new
       # task, just use the original one
@@ -217,8 +217,22 @@ def self.create_refresh_task(ems, targets)
       :message => "Queued the action: [#{task_options[:action]}] being run for user: [#{task_options[:userid]}]"
     )
   end
+
   private_class_method :create_refresh_task
 
+  def self.uniq_targets(targets)
+    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) }
+
+      application_record_targets + manager_refresh_targets
+    else
+      targets
+    end
+  end
+  private_class_method :uniq_targets
+
   #
   # Helper methods for advanced debugging
   #

spec/models/ems_refresh_spec.rb

@@ -45,6 +45,56 @@
     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) }
+
+    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