VimPerformanceState#purge_timer for old records

[kbrock]

Will purge for both orphans and records older than 6 months

fixes #11868

[kbrock]

update:

• added a spec
• no other changes

[miq-bot]

Checked commit kbrock@8e19738 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 👍

[Fryguy]

I'm ok to merge this as is, but I'm reading through the purging code, and I don't understand the purpose of purge_orphaned at all. The reason I'm saying that is that I started looking at supporting "multiple" purging strategies in a cleaner way after this PR, and then I noticed that some tables already do it using purge_by_scope, and it seems so much simpler. For example, binary blobs supports both older than or orphaned via a single scope:

manageiq/app/models/binary_blob/purging.rb

Line 20 in 33714b1

where(:resource_id => nil).where(arel_table[:created_at].lteq(older_than))

and then just uses [:scope, purge_date] as the purge mode.

[Fryguy]

Based on my comment above, I think this can just more succintly be:

Suggested change

	def purge_scope(older_than)
	where(arel_table[:timestamp].lt(older_than))
	end
	def purge_scope(older_than)
	where(:resource_id => nil).where(arel_table[:timestamp].lt(older_than))
	end

And then change the purge_mode_and_value to:

        def purge_mode_and_value
-         %w(orphaned resource)
+         [:scope, purge_date]
        end

[kbrock]

so after a discussion, we determined orphaned is saying that this record is owned by a record that was removed from the database.

In theory, I understand how this can happen, but it looked like all of the references were setup to destroy this record.

[Fryguy]

Oh right - we discussed this and there are 2 types of orphans

1. resource_id is nil
2. resource_id is not nil but the corresponding record not longer exists

The main way this can happen is whether the caller uses dependent => :nullify (1) vs nothing at all (2).

where(:resource_id => nil) would only handle the first case, whereas orphaned scope handles the second case.

BinaryBlobs in partucular does where(:resource_id => nil).where(arel_table[:timestamp].lt(older_than)), which partially handles the first case by applying a timestamp check AND and resource_id => nil check. If I recall, we can create BinaryBlobs without a resource_id in some cases, particularly in provisioning, and there was a bug where that BinaryBlob was getting removed during the provisioning and breaking things, which is why that one is "special"

[kbrock]

To further clarify what "nothing at all" means for future readers:

It means there is no dependent: ... clause defined. Or the association is not even mentioned in the source (deleted) model.

[kbrock]

aah, came across this example:

class ExtManagementSystem
  has_many    :vim_performance_states, :as => :resource  # Destroy will be handled by purger
end

So in this case, the ems will be deleted but the vim performance state will point to an ems_id that does not exist.

[Fryguy]

Going to merge anyway, but let's discuss my comments above.

[Fryguy]

Backported to petrosian in commit 64332eb.

commit 64332eb3280042c51987a25e4cd841918913fbc4
Author: Jason Frey <fryguy9@gmail.com>
Date:   Fri Jul 7 16:32:50 2023 -0400

    Merge pull request #22605 from kbrock/vps_purging
    
    VimPerformanceState#purge_timer for old records
    
    (cherry picked from commit 41dc875b7e59ec1209fa9b1413264ff093ec1ae8)