Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Purging of ContainerQuota & ContainerQuotaItem #17167

Merged
merged 1 commit into from
Mar 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/models/container_quota.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class ContainerQuota < ApplicationRecord
# - ContainerQuotaItems can be added/deleted/changed, and we use archiving to
# record changes too!
include ArchivedMixin
include_concern 'Purging'

belongs_to :ext_management_system, :foreign_key => "ems_id"
belongs_to :container_project
Expand Down
25 changes: 25 additions & 0 deletions app/models/container_quota/purging.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
class ContainerQuota < ApplicationRecord
module Purging
extend ActiveSupport::Concern
include PurgingMixin

module ClassMethods
def purge_date
::Settings.container_entities.history.keep_archived_quotas.to_i_with_method.seconds.ago.utc
end

def purge_window_size
::Settings.container_entities.history.purge_window_size
end

def purge_scope(older_than)
where(arel_table[:deleted_on].lteq(older_than))
end

def purge_associated_records(ids)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reminder: adding purge_associated_records currently causes us to retrieve all ids that are deleted from container quota. If the number of records is low, then this is not a problem.

If there are a bunch of records to be deleted, we may want to come up with another way to clear out these associated 2 tables.

Copy link
Contributor Author

@cben cben Mar 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect few ContainerQuota records purged — only when admins delete the whole quota definition in a project (*) — but possibly many child ContainerQuotaItem records to purge (these may be archived on inventory changes, may become comparable to ContainerGroup table).

(*) Technically it's possible to have an arbitrary number of quotas in a project, but in practice 0 or 1 will be common, unlikely to be over 4.

I'm doing this to avoid leaving orphan items. Since items should get archived when parent quota gets archived, we could also avoid orphans by giving items a lower lifetime, e.g. 5.month vs 6.month. But I prefer ensuring DB consistency by explicit code over "it happens to work if tuned correctly" :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch that, purge_associated_records is also needed to avoid leaving orphan ContainerQuotaScopes, no alternative there.

ContainerQuotaScope.where(:container_quota_id => ids).delete_all
ContainerQuotaItem.where(:container_quota_id => ids).delete_all
end
end
end
end
1 change: 1 addition & 0 deletions app/models/container_quota_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class ContainerQuotaItem < ApplicationRecord
# This model is unusual in using archiving not only to record deletions but also changes in quota_desired, quota_enforced, quota_observed.
# Instead of updating in-place, we archive the old record and create a new one.
include ArchivedMixin
include_concern 'Purging'

belongs_to :container_quota
has_many :container_quota_scopes, :through => :container_quota
Expand Down
20 changes: 20 additions & 0 deletions app/models/container_quota_item/purging.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
class ContainerQuotaItem < ApplicationRecord
module Purging
extend ActiveSupport::Concern
include PurgingMixin

module ClassMethods
def purge_date
::Settings.container_entities.history.keep_archived_quotas.to_i_with_method.seconds.ago.utc
end

def purge_window_size
::Settings.container_entities.history.purge_window_size
end

def purge_scope(older_than)
where(arel_table[:deleted_on].lteq(older_than))
end
end
end
end
2 changes: 2 additions & 0 deletions app/models/miq_schedule_worker/jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ def archived_entities_purge_timer
queue_work(:class_name => "ContainerGroup", :method_name => "purge_timer", :zone => nil)
queue_work(:class_name => "ContainerImage", :method_name => "purge_timer", :zone => nil)
queue_work(:class_name => "ContainerProject", :method_name => "purge_timer", :zone => nil)
queue_work(:class_name => "ContainerQuota", :method_name => "purge_timer", :zone => nil)
queue_work(:class_name => "ContainerQuotaItem", :method_name => "purge_timer", :zone => nil)
end

def binary_blob_purge_timer
Expand Down
1 change: 1 addition & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@
:container_entities:
:history:
:keep_archived_entities: 6.months
:keep_archived_quotas: 6.months
:purge_window_size: 1000
:product:
:maindb: ExtManagementSystem
Expand Down
63 changes: 63 additions & 0 deletions spec/models/container_quota/purging_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
describe ContainerQuota do
context "::Purging" do
context ".purge_queue" do
before do
EvmSpecHelper.create_guid_miq_server_zone
end
let(:purge_time) { (Time.zone.now + 10).round }

it "submits to the queue" do
expect(described_class).to receive(:purge_date).and_return(purge_time)
described_class.purge_timer

q = MiqQueue.all
expect(q.length).to eq(1)
expect(q.first).to have_attributes(
:class_name => described_class.name,
:method_name => "purge_by_date",
:args => [purge_time]
)
end
end

context ".purge" do
let(:deleted_date) { 6.months.ago }

before do
@old_quota = FactoryGirl.create(:container_quota, :deleted_on => deleted_date - 1.day)
@old_quota_scope = FactoryGirl.create(:container_quota_scope, :container_quota => @old_quota)
@old_quota_old_item = FactoryGirl.create(:container_quota_item, :container_quota => @old_quota,
:deleted_on => deleted_date - 1.day)
@old_quota_active_item = FactoryGirl.create(:container_quota_item, :container_quota => @old_quota,
:deleted_on => nil)

@purge_date_quota = FactoryGirl.create(:container_quota, :deleted_on => deleted_date)

@new_quota = FactoryGirl.create(:container_quota, :deleted_on => deleted_date + 1.day)
@new_quota_scope = FactoryGirl.create(:container_quota_scope, :container_quota => @new_quota)
@new_quota_old_item = FactoryGirl.create(:container_quota_item, :container_quota => @new_quota,
:deleted_on => deleted_date - 1.day)
end

def assert_unpurged_ids(model, unpurged_ids)
expect(model.order(:id).pluck(:id)).to eq(Array(unpurged_ids).sort)
end

it "purge_date and older" do
described_class.purge(deleted_date)
assert_unpurged_ids(ContainerQuota, @new_quota.id)
assert_unpurged_ids(ContainerQuotaScope, @new_quota_scope.id)
# This quota item is itself due for purging, but not as part of ContainerQuota::Purging.
assert_unpurged_ids(ContainerQuotaItem, @new_quota_old_item.id)
end

it "with a window" do
described_class.purge(deleted_date, 1)
assert_unpurged_ids(ContainerQuota, @new_quota.id)
assert_unpurged_ids(ContainerQuotaScope, @new_quota_scope.id)
# This quota item is itself due for purging, but not as part of ContainerQuota::Purging.
assert_unpurged_ids(ContainerQuotaItem, @new_quota_old_item.id)
end
end
end
end
70 changes: 70 additions & 0 deletions spec/models/container_quota_item/purging_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
describe ContainerQuotaItem do
context "::Purging" do
context ".purge_queue" do
before do
EvmSpecHelper.create_guid_miq_server_zone
end
let(:purge_time) { (Time.zone.now + 10).round }

it "submits to the queue" do
expect(described_class).to receive(:purge_date).and_return(purge_time)
described_class.purge_timer

q = MiqQueue.all
expect(q.length).to eq(1)
expect(q.first).to have_attributes(
:class_name => described_class.name,
:method_name => "purge_by_date",
:args => [purge_time]
)
end
end

context ".purge" do
let(:deleted_date) { 6.months.ago }

before do
@old_quota = FactoryGirl.create(:container_quota, :deleted_on => deleted_date - 1.day)
@old_quota_scope = FactoryGirl.create(:container_quota_scope, :container_quota => @old_quota)
@old_quota_old_item = FactoryGirl.create(:container_quota_item, :container_quota => @old_quota,
:deleted_on => deleted_date - 1.day)
@old_quota_purge_date_item = FactoryGirl.create(:container_quota_item, :container_quota => @old_quota,
:deleted_on => deleted_date)
@old_quota_new_item = FactoryGirl.create(:container_quota_item, :container_quota => @old_quota,
:deleted_on => deleted_date + 1.day)

# Quota items may get archived as result of quota edits, while parent quota remains active.
@active_quota = FactoryGirl.create(:container_quota, :deleted_on => nil)
@active_quota_scope = FactoryGirl.create(:container_quota_scope, :container_quota => @active_quota)
@active_quota_old_item = FactoryGirl.create(:container_quota_item, :container_quota => @active_quota,
:deleted_on => deleted_date - 1.day)
@active_quota_purge_date_item = FactoryGirl.create(:container_quota_item, :container_quota => @active_quota,
:deleted_on => deleted_date)
@active_quota_new_item = FactoryGirl.create(:container_quota_item, :container_quota => @active_quota,
:deleted_on => deleted_date + 1.day)
@active_quota_active_item = FactoryGirl.create(:container_quota_item, :container_quota => @active_quota,
:deleted_on => nil)
end

def assert_unpurged_ids(model, unpurged_ids)
expect(model.order(:id).pluck(:id)).to eq(Array(unpurged_ids).sort)
end

it "purge_date and older" do
described_class.purge(deleted_date)
# @old_quota is itself due for purging, but not as part of ContainerQuotaItem::Purging.
assert_unpurged_ids(ContainerQuota, [@old_quota.id, @active_quota.id])
assert_unpurged_ids(ContainerQuotaScope, [@old_quota_scope.id, @active_quota_scope.id])
assert_unpurged_ids(ContainerQuotaItem, [@old_quota_new_item.id, @active_quota_new_item.id, @active_quota_active_item.id])
end

it "with a window" do
described_class.purge(deleted_date, 1)
# @old_quota is itself due for purging, but not as part of ContainerQuotaItem::Purging.
assert_unpurged_ids(ContainerQuota, [@old_quota.id, @active_quota.id])
assert_unpurged_ids(ContainerQuotaScope, [@old_quota_scope.id, @active_quota_scope.id])
assert_unpurged_ids(ContainerQuotaItem, [@old_quota_new_item.id, @active_quota_new_item.id, @active_quota_active_item.id])
end
end
end
end