From 45a4d79a0f3268cb1cc49fc37dd63af1073e6858 Mon Sep 17 00:00:00 2001 From: Dave Riddle Date: Mon, 20 Jan 2025 11:13:50 -0600 Subject: [PATCH 1/8] Make cutoff_age_in_days a named parameter Better aligns with style used throughout codebase. Done in anticipation of adding more parameters to the `initialize` method. --- app/jobs/runtime/events_cleanup.rb | 2 +- app/repositories/app_usage_event_repository.rb | 2 +- app/repositories/service_usage_event_repository.rb | 2 +- lib/database/old_record_cleanup.rb | 4 ++-- spec/unit/lib/database/old_record_cleanup_spec.rb | 8 ++++---- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/jobs/runtime/events_cleanup.rb b/app/jobs/runtime/events_cleanup.rb index a1841576395..873c6a7f319 100644 --- a/app/jobs/runtime/events_cleanup.rb +++ b/app/jobs/runtime/events_cleanup.rb @@ -9,7 +9,7 @@ def initialize(cutoff_age_in_days) end def perform - Database::OldRecordCleanup.new(Event, cutoff_age_in_days).delete + Database::OldRecordCleanup.new(Event, cutoff_age_in_days:).delete end def job_name_in_configuration diff --git a/app/repositories/app_usage_event_repository.rb b/app/repositories/app_usage_event_repository.rb index 51d751a8cd1..02e05662d84 100644 --- a/app/repositories/app_usage_event_repository.rb +++ b/app/repositories/app_usage_event_repository.rb @@ -152,7 +152,7 @@ def purge_and_reseed_started_apps! end def delete_events_older_than(cutoff_age_in_days) - Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days, keep_at_least_one_record: true).delete + Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true).delete end private diff --git a/app/repositories/service_usage_event_repository.rb b/app/repositories/service_usage_event_repository.rb index 35fbef5a1f0..63e5645e02e 100644 --- a/app/repositories/service_usage_event_repository.rb +++ b/app/repositories/service_usage_event_repository.rb @@ -92,7 +92,7 @@ def purge_and_reseed_service_instances! end def delete_events_older_than(cutoff_age_in_days) - Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days, keep_at_least_one_record: true).delete + Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true).delete end end end diff --git a/lib/database/old_record_cleanup.rb b/lib/database/old_record_cleanup.rb index 44227d84507..0e1d148dfb1 100644 --- a/lib/database/old_record_cleanup.rb +++ b/lib/database/old_record_cleanup.rb @@ -5,9 +5,9 @@ class OldRecordCleanup class NoCurrentTimestampError < StandardError; end attr_reader :model, :days_ago, :keep_at_least_one_record - def initialize(model, days_ago, keep_at_least_one_record: false) + def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false) @model = model - @days_ago = days_ago + @days_ago = cutoff_age_in_days @keep_at_least_one_record = keep_at_least_one_record end diff --git a/spec/unit/lib/database/old_record_cleanup_spec.rb b/spec/unit/lib/database/old_record_cleanup_spec.rb index 7b2258bbaec..4e0511ad7cf 100644 --- a/spec/unit/lib/database/old_record_cleanup_spec.rb +++ b/spec/unit/lib/database/old_record_cleanup_spec.rb @@ -9,7 +9,7 @@ let!(:fresh_event) { VCAP::CloudController::Event.make(created_at: 1.day.ago + 1.minute) } it 'deletes records older than specified days' do - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, 1) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 1) expect do record_cleanup.delete @@ -22,7 +22,7 @@ context "when there are no records at all but you're trying to keep at least one" do it "doesn't keep one because there aren't any to keep" do - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, 1, keep_at_least_one_record: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: true) expect { record_cleanup.delete }.not_to raise_error expect(VCAP::CloudController::ServiceUsageEvent.count).to eq(0) @@ -31,12 +31,12 @@ it 'only retrieves the current timestamp from the database once' do expect(VCAP::CloudController::Event.db).to receive(:fetch).with('SELECT CURRENT_TIMESTAMP as now').once.and_call_original - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, 1) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 1) record_cleanup.delete end it 'keeps the last row when :keep_at_least_one_record is true even if it is older than the cutoff date' do - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, 0, keep_at_least_one_record: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 0, keep_at_least_one_record: true) expect do record_cleanup.delete From 46135a8b6e4ef290eca36cbfb13bd9debec2f5a4 Mon Sep 17 00:00:00 2001 From: Dave Riddle Date: Mon, 20 Jan 2025 12:13:53 -0600 Subject: [PATCH 2/8] Keep usage event records of running apps and services AppUsageEvent and ServiceUsageEvent records can be used to track the lifecycle of apps and services in a foundation. However, if the start event is pruned before end event, the foundation no longer maintains an accurate state of its running apps and services. By holding on to the start events, consumers of these records are able to get an accurate picture of the current state whenever they choose to begin consuming usage event records. --- .../app_usage_event_repository.rb | 2 +- .../service_usage_event_repository.rb | 2 +- lib/database/old_record_cleanup.rb | 57 +++++++++++++- .../runtime/app_usage_events_cleanup_spec.rb | 2 +- .../service_usage_events_cleanup_spec.rb | 2 +- .../lib/database/old_record_cleanup_spec.rb | 76 +++++++++++++++++-- 6 files changed, 128 insertions(+), 13 deletions(-) diff --git a/app/repositories/app_usage_event_repository.rb b/app/repositories/app_usage_event_repository.rb index 02e05662d84..8955c326b19 100644 --- a/app/repositories/app_usage_event_repository.rb +++ b/app/repositories/app_usage_event_repository.rb @@ -152,7 +152,7 @@ def purge_and_reseed_started_apps! end def delete_events_older_than(cutoff_age_in_days) - Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true).delete + Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true).delete end private diff --git a/app/repositories/service_usage_event_repository.rb b/app/repositories/service_usage_event_repository.rb index 63e5645e02e..1f55434c621 100644 --- a/app/repositories/service_usage_event_repository.rb +++ b/app/repositories/service_usage_event_repository.rb @@ -92,7 +92,7 @@ def purge_and_reseed_service_instances! end def delete_events_older_than(cutoff_age_in_days) - Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true).delete + Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true).delete end end end diff --git a/lib/database/old_record_cleanup.rb b/lib/database/old_record_cleanup.rb index 0e1d148dfb1..b865a859d8e 100644 --- a/lib/database/old_record_cleanup.rb +++ b/lib/database/old_record_cleanup.rb @@ -3,12 +3,13 @@ module Database class OldRecordCleanup class NoCurrentTimestampError < StandardError; end - attr_reader :model, :days_ago, :keep_at_least_one_record + attr_reader :model, :days_ago, :keep_at_least_one_record, :keep_running_records - def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false) + def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false, keep_running_records: false) @model = model @days_ago = cutoff_age_in_days @keep_at_least_one_record = keep_at_least_one_record + @keep_running_records = keep_running_records end def delete @@ -21,6 +22,8 @@ def delete end logger.info("Cleaning up #{old_records.count} #{model.table_name} table rows") + old_records = exclude_running_records(old_records) if keep_running_records + Database::BatchDelete.new(old_records, 1000).delete end @@ -35,5 +38,55 @@ def current_timestamp_from_database def logger @logger ||= Steno.logger('cc.old_record_cleanup') end + + def exclude_running_records(old_records) + return old_records unless has_duration?(model) + + beginning_string = beginning_string(model) + ending_string = ending_string(model) + guid_symbol = guid_symbol(model) + + raise "Invalid duration model: #{model}" if beginning_string.nil? || ending_string.nil? || guid_symbol.nil? + + initial_records = old_records.where(state: beginning_string).from_self(alias: :initial_records) + final_records = old_records.where(state: ending_string).from_self(alias: :final_records) + + exists_condition = final_records.where(Sequel[:final_records][guid_symbol] => Sequel[:initial_records][guid_symbol]).where do + Sequel[:final_records][:id] > Sequel[:initial_records][:id] + end.select(1).exists + + prunable_initial_records = initial_records.where(exists_condition) + other_records = old_records.exclude(state: [beginning_string, ending_string]) + + prunable_initial_records.union(final_records, all: true).union(other_records, all: true) + end + + def has_duration?(model) + return true if model == VCAP::CloudController::AppUsageEvent + return true if model == VCAP::CloudController::ServiceUsageEvent + + false + end + + def beginning_string(model) + return VCAP::CloudController::ProcessModel::STARTED if model == VCAP::CloudController::AppUsageEvent + return VCAP::CloudController::Repositories::ServiceUsageEventRepository::CREATED_EVENT_STATE if model == VCAP::CloudController::ServiceUsageEvent + + nil + end + + def ending_string(model) + return VCAP::CloudController::ProcessModel::STOPPED if model == VCAP::CloudController::AppUsageEvent + return VCAP::CloudController::Repositories::ServiceUsageEventRepository::DELETED_EVENT_STATE if model == VCAP::CloudController::ServiceUsageEvent + + nil + end + + def guid_symbol(model) + return :app_guid if model == VCAP::CloudController::AppUsageEvent + return :service_instance_guid if model == VCAP::CloudController::ServiceUsageEvent + + nil + end end end diff --git a/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb b/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb index 8018ee0ee69..ce4f1df9956 100644 --- a/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb +++ b/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb @@ -5,7 +5,7 @@ module Jobs::Runtime RSpec.describe AppUsageEventsCleanup, job_context: :worker do let(:cutoff_age_in_days) { 30 } let(:logger) { double(Steno::Logger, info: nil) } - let!(:event_before_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago) } + let!(:event_before_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago, state: 'STOPPED') } let!(:event_after_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days - 1).days.ago) } subject(:job) do diff --git a/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb b/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb index 181ad89e18d..da11beb39b5 100644 --- a/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb +++ b/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb @@ -5,7 +5,7 @@ module Jobs::Services RSpec.describe ServiceUsageEventsCleanup, job_context: :worker do let(:cutoff_age_in_days) { 30 } let(:logger) { double(Steno::Logger, info: nil) } - let!(:event_before_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago) } + let!(:event_before_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago, state: 'DELETED') } let!(:event_after_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days - 1).days.ago) } subject(:job) do diff --git a/spec/unit/lib/database/old_record_cleanup_spec.rb b/spec/unit/lib/database/old_record_cleanup_spec.rb index 4e0511ad7cf..9d7318b3a98 100644 --- a/spec/unit/lib/database/old_record_cleanup_spec.rb +++ b/spec/unit/lib/database/old_record_cleanup_spec.rb @@ -3,12 +3,12 @@ RSpec.describe Database::OldRecordCleanup do describe '#delete' do - let!(:stale_event1) { VCAP::CloudController::Event.make(created_at: 1.day.ago - 1.minute) } - let!(:stale_event2) { VCAP::CloudController::Event.make(created_at: 2.days.ago) } + it 'deletes records older than specified days' do + stale_event1 = VCAP::CloudController::Event.make(created_at: 1.day.ago - 1.minute) + stale_event2 = VCAP::CloudController::Event.make(created_at: 2.days.ago) - let!(:fresh_event) { VCAP::CloudController::Event.make(created_at: 1.day.ago + 1.minute) } + fresh_event = VCAP::CloudController::Event.make(created_at: 1.day.ago + 1.minute) - it 'deletes records older than specified days' do record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 1) expect do @@ -22,10 +22,10 @@ context "when there are no records at all but you're trying to keep at least one" do it "doesn't keep one because there aren't any to keep" do - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppEvent, cutoff_age_in_days: 1, keep_at_least_one_record: true, keep_running_records: true) expect { record_cleanup.delete }.not_to raise_error - expect(VCAP::CloudController::ServiceUsageEvent.count).to eq(0) + expect(VCAP::CloudController::AppEvent.count).to eq(0) end end @@ -36,7 +36,12 @@ end it 'keeps the last row when :keep_at_least_one_record is true even if it is older than the cutoff date' do - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 0, keep_at_least_one_record: true) + stale_event1 = VCAP::CloudController::Event.make(created_at: 1.day.ago - 1.minute) + stale_event2 = VCAP::CloudController::Event.make(created_at: 2.days.ago) + + fresh_event = VCAP::CloudController::Event.make(created_at: 1.day.ago + 1.minute) + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 0, keep_at_least_one_record: true, keep_running_records: true) expect do record_cleanup.delete @@ -46,5 +51,62 @@ expect { stale_event1.reload }.to raise_error(Sequel::NoExistingObject) expect { stale_event2.reload }.to raise_error(Sequel::NoExistingObject) end + + # Testing keep_running_records feature + it 'keeps AppUsageEvent start record when there is no corresponding stop record' do + stale_app_usage_event_start = VCAP::CloudController::AppUsageEvent.make(created_at: 2.days.ago, state: 'STARTED', app_guid: 'guid1') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup.delete + expect(stale_app_usage_event_start.reload).to be_present + end + + it 'keeps AppUsageEvent start record when stop record is fresh' do + stale_app_usage_event_start = VCAP::CloudController::AppUsageEvent.make(created_at: 2.days.ago, state: 'STARTED', app_guid: 'guid1') + fresh_app_usage_event_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.day.ago + 1.minute, state: 'STOPPED', app_guid: 'guid1') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup.delete + expect(stale_app_usage_event_start.reload).to be_present + expect(fresh_app_usage_event_stop.reload).to be_present + end + + it 'keeps AppUsageEvent start record when stop record is newer' do + stale_app_usage_event_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 3.days.ago, state: 'STOPPED', app_guid: 'guid1') + stale_app_usage_event_start = VCAP::CloudController::AppUsageEvent.make(created_at: 2.days.ago, state: 'STARTED', app_guid: 'guid1') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup.delete + expect(stale_app_usage_event_start.reload).to be_present + expect { stale_app_usage_event_stop.reload }.to raise_error(Sequel::NoExistingObject) + end + + it 'keeps ServiceUsageEvent create record when there is no corresponding delete record' do + stale_service_usage_event_create = VCAP::CloudController::ServiceUsageEvent.make(created_at: 2.days.ago, state: 'CREATED', service_instance_guid: 'guid1') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup.delete + expect(stale_service_usage_event_create.reload).to be_present + end + + it 'keeps ServiceUsageEvent create record when delete record is fresh' do + stale_service_usage_event_create = VCAP::CloudController::ServiceUsageEvent.make(created_at: 2.days.ago, state: 'CREATED', service_instance_guid: 'guid1') + fresh_service_usage_event_delete = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.day.ago + 1.minute, state: 'DELETED', service_instance_guid: 'guid1') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup.delete + expect(stale_service_usage_event_create.reload).to be_present + expect(fresh_service_usage_event_delete.reload).to be_present + end + + it 'keeps ServiceUsageEvent create record when delete record is newer' do + stale_service_usage_event_delete = VCAP::CloudController::ServiceUsageEvent.make(created_at: 3.days.ago, state: 'DELETED', service_instance_guid: 'guid1') + stale_service_usage_event_create = VCAP::CloudController::ServiceUsageEvent.make(created_at: 2.days.ago, state: 'CREATED', service_instance_guid: 'guid1') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup.delete + expect(stale_service_usage_event_create.reload).to be_present + expect { stale_service_usage_event_delete.reload }.to raise_error(Sequel::NoExistingObject) + end end end From 80f2cc2eb130116c8d0eac8e8b610670432a7335 Mon Sep 17 00:00:00 2001 From: Dave Riddle Date: Mon, 20 Jan 2025 17:05:50 -0600 Subject: [PATCH 3/8] Maintain un-processed usage event records Allow consumer_guid to be passed into request for AppUsageEvent and ServiceUsageEvent records. This establishes a consumer of these events. Un-processed consumer records will be maintained by cloud controller. Registered consumers are automatically deleted if associated event is removed. --- app/access/app_usage_consumer_access.rb | 79 +++++++++++++ app/access/service_usage_consumer_access.rb | 79 +++++++++++++ .../v3/app_usage_events_controller.rb | 14 +++ .../v3/service_usage_events_controller.rb | 14 +++ app/messages/app_usage_events_list_message.rb | 7 +- .../service_usage_events_list_message.rb | 3 +- app/models.rb | 2 + app/models/runtime/app_usage_consumer.rb | 26 +++++ app/models/runtime/app_usage_event.rb | 7 ++ app/models/services/service_usage_consumer.rb | 26 +++++ app/models/services/service_usage_event.rb | 7 ++ .../app_usage_event_repository.rb | 3 +- .../service_usage_event_repository.rb | 3 +- ...241205162359_create_app_usage_consumers.rb | 9 ++ ...18113511_create_service_usage_consumers.rb | 9 ++ lib/database/old_record_cleanup.rb | 34 +++++- spec/support/fakes/blueprints.rb | 10 ++ .../lib/database/old_record_cleanup_spec.rb | 105 ++++++++++++++++-- 18 files changed, 420 insertions(+), 17 deletions(-) create mode 100644 app/access/app_usage_consumer_access.rb create mode 100644 app/access/service_usage_consumer_access.rb create mode 100644 app/models/runtime/app_usage_consumer.rb create mode 100644 app/models/services/service_usage_consumer.rb create mode 100644 db/migrations/20241205162359_create_app_usage_consumers.rb create mode 100644 db/migrations/20241218113511_create_service_usage_consumers.rb diff --git a/app/access/app_usage_consumer_access.rb b/app/access/app_usage_consumer_access.rb new file mode 100644 index 00000000000..47594feada9 --- /dev/null +++ b/app/access/app_usage_consumer_access.rb @@ -0,0 +1,79 @@ +module VCAP::CloudController + class AppUsageConsumerAccess < BaseAccess + def create?(_object, _params=nil) + admin_user? + end + + def read?(object) + return @ok_read if instance_variable_defined?(:@ok_read) + + @ok_read = admin_user? || admin_read_only_user? || global_auditor? || object_is_visible_to_user?(object, context.user) + end + + def read_for_update?(_object, _params=nil) + admin_user? + end + + def can_remove_related_object?(object, params=nil) + read_for_update?(object, params) + end + + def read_related_object_for_update?(object, params=nil) + read_for_update?(object, params) + end + + def update?(_object, _params=nil) + admin_user? + end + + def delete?(_object) + admin_user? + end + + # These methods should be called first to determine if the user's token has the appropriate scope for the operation + + def read_with_token?(_) + admin_user? || admin_read_only_user? || has_read_scope? || global_auditor? + end + + def create_with_token?(_) + admin_user? || has_write_scope? + end + + def read_for_update_with_token?(_) + admin_user? || has_write_scope? + end + + def can_remove_related_object_with_token?(*) + read_for_update_with_token?(*) + end + + def read_related_object_for_update_with_token?(*) + read_for_update_with_token?(*) + end + + def update_with_token?(_) + admin_user? || has_write_scope? + end + + def delete_with_token?(_) + admin_user? || has_write_scope? + end + + def index_with_token?(_) + admin_user? || admin_read_only_user? + end + + def index?(_object_class, _params=nil) + admin_user? || admin_read_only_user? + end + + def reset?(_object_class) + admin_user? + end + + def reset_with_token?(_object_class) + admin_user? + end + end +end diff --git a/app/access/service_usage_consumer_access.rb b/app/access/service_usage_consumer_access.rb new file mode 100644 index 00000000000..16dbcd33ef3 --- /dev/null +++ b/app/access/service_usage_consumer_access.rb @@ -0,0 +1,79 @@ +module VCAP::CloudController + class ServiceUsageConsumerAccess < BaseAccess + def create?(_object, _params=nil) + admin_user? + end + + def read?(object) + return @ok_read if instance_variable_defined?(:@ok_read) + + @ok_read = admin_user? || admin_read_only_user? || global_auditor? || object_is_visible_to_user?(object, context.user) + end + + def read_for_update?(_object, _params=nil) + admin_user? + end + + def can_remove_related_object?(object, params=nil) + read_for_update?(object, params) + end + + def read_related_object_for_update?(object, params=nil) + read_for_update?(object, params) + end + + def update?(_object, _params=nil) + admin_user? + end + + def delete?(_object) + admin_user? + end + + # These methods should be called first to determine if the user's token has the appropriate scope for the operation + + def read_with_token?(_) + admin_user? || admin_read_only_user? || has_read_scope? || global_auditor? + end + + def create_with_token?(_) + admin_user? || has_write_scope? + end + + def read_for_update_with_token?(_) + admin_user? || has_write_scope? + end + + def can_remove_related_object_with_token?(*) + read_for_update_with_token?(*) + end + + def read_related_object_for_update_with_token?(*) + read_for_update_with_token?(*) + end + + def update_with_token?(_) + admin_user? || has_write_scope? + end + + def delete_with_token?(_) + admin_user? || has_write_scope? + end + + def index_with_token?(_) + admin_user? || admin_read_only_user? + end + + def index?(_object_class, _params=nil) + admin_user? || admin_read_only_user? + end + + def reset?(_object_class) + admin_user? + end + + def reset_with_token?(_object_class) + admin_user? + end + end +end diff --git a/app/controllers/v3/app_usage_events_controller.rb b/app/controllers/v3/app_usage_events_controller.rb index 7dca1a627f0..ad50766a8a2 100644 --- a/app/controllers/v3/app_usage_events_controller.rb +++ b/app/controllers/v3/app_usage_events_controller.rb @@ -11,6 +11,20 @@ def index app_usage_events = AppUsageEventListFetcher.fetch_all(message, AppUsageEvent.dataset) if permission_queryer.can_read_globally? + if message.consumer_guid && message.after_guid&.first && permission_queryer.can_write_globally? + begin + consumer = AppUsageConsumer.find_or_create(consumer_guid: message.consumer_guid) do |c| + c.last_processed_guid = message.after_guid.first + end + + consumer.update(last_processed_guid: message.after_guid.first) if !consumer.new? && consumer.last_processed_guid != message.after_guid.first + rescue Sequel::ValidationFailed => e + unprocessable!(e.message) + rescue Sequel::Error + error!('Failed to update consumer tracking', 500) + end + end + render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( presenter: Presenters::V3::AppUsageEventPresenter, paginated_result: SequelPaginator.new.get_page(app_usage_events, message.try(:pagination_options)), diff --git a/app/controllers/v3/service_usage_events_controller.rb b/app/controllers/v3/service_usage_events_controller.rb index bfbe65e3080..b6a1de856e1 100644 --- a/app/controllers/v3/service_usage_events_controller.rb +++ b/app/controllers/v3/service_usage_events_controller.rb @@ -11,6 +11,20 @@ def index service_usage_events = ServiceUsageEventListFetcher.fetch_all(message, ServiceUsageEvent.dataset) if permission_queryer.can_read_globally? + if message.consumer_guid && message.after_guid&.first && permission_queryer.can_write_globally? + begin + consumer = ServiceUsageConsumer.find_or_create(consumer_guid: message.consumer_guid) do |c| + c.last_processed_guid = message.after_guid.first + end + + consumer.update(last_processed_guid: message.after_guid.first) if !consumer.new? && consumer.last_processed_guid != message.after_guid.first + rescue Sequel::ValidationFailed => e + unprocessable!(e.message) + rescue Sequel::Error + error!('Failed to update consumer tracking', 500) + end + end + render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( presenter: Presenters::V3::ServiceUsageEventPresenter, paginated_result: SequelPaginator.new.get_page(service_usage_events, message.try(:pagination_options)), diff --git a/app/messages/app_usage_events_list_message.rb b/app/messages/app_usage_events_list_message.rb index 2656808a79f..b63b8158a47 100644 --- a/app/messages/app_usage_events_list_message.rb +++ b/app/messages/app_usage_events_list_message.rb @@ -2,8 +2,9 @@ module VCAP::CloudController class AppUsageEventsListMessage < ListMessage - register_allowed_keys [ - :after_guid + register_allowed_keys %i[ + after_guid + consumer_guid ] validates_with NoAdditionalParamsValidator @@ -19,7 +20,7 @@ def valid_order_by_values end def self.from_params(params) - super(params, %w[after_guid]) + super(params, %w[after_guid consumer_guid]) end end end diff --git a/app/messages/service_usage_events_list_message.rb b/app/messages/service_usage_events_list_message.rb index 08f8033578b..2252798e20c 100644 --- a/app/messages/service_usage_events_list_message.rb +++ b/app/messages/service_usage_events_list_message.rb @@ -6,6 +6,7 @@ class ServiceUsageEventsListMessage < ListMessage after_guid service_instance_types service_offering_guids + consumer_guid ] validates_with NoAdditionalParamsValidator @@ -24,7 +25,7 @@ def valid_order_by_values end def self.from_params(params) - super(params, %w[after_guid service_instance_types service_offering_guids]) + super(params, %w[after_guid service_instance_types service_offering_guids consumer_guid]) end end end diff --git a/app/models.rb b/app/models.rb index 2c6840fc8e4..3ea0f431fbd 100644 --- a/app/models.rb +++ b/app/models.rb @@ -18,6 +18,7 @@ require 'models/runtime/security_group' require 'models/runtime/security_groups_space' require 'models/runtime/staging_security_groups_space' +require 'models/runtime/app_usage_consumer' require 'models/runtime/app_usage_event' require 'models/runtime/auto_detection_buildpack' require 'models/runtime/app_event' @@ -140,6 +141,7 @@ require 'models/services/service_plan_visibility' require 'models/services/service_plan_annotation_model' require 'models/services/service_plan_label_model' +require 'models/services/service_usage_consumer' require 'models/services/service_usage_event' require 'models/services/service_key' require 'models/services/service_key_label_model' diff --git a/app/models/runtime/app_usage_consumer.rb b/app/models/runtime/app_usage_consumer.rb new file mode 100644 index 00000000000..9338d1691e9 --- /dev/null +++ b/app/models/runtime/app_usage_consumer.rb @@ -0,0 +1,26 @@ +module VCAP::CloudController + class AppUsageConsumer < Sequel::Model + plugin :validation_helpers + + many_to_one :last_processed_event, + class: 'VCAP::CloudController::AppUsageEvent', + key: :last_processed_guid, + primary_key: :guid + + def validate + validates_presence %i[consumer_guid last_processed_guid] + validates_unique :consumer_guid + validates_max_length 255, :consumer_guid, message: 'must be less than 255 characters' + end + + def last_processed_guid=(guid) + self[:last_processed_guid] = guid + end + + def last_processed_guid + self[:last_processed_guid] + end + + export_attributes :consumer_guid, :last_processed_guid + end +end diff --git a/app/models/runtime/app_usage_event.rb b/app/models/runtime/app_usage_event.rb index 32913f8cdda..03bebfc67cc 100644 --- a/app/models/runtime/app_usage_event.rb +++ b/app/models/runtime/app_usage_event.rb @@ -2,6 +2,13 @@ module VCAP::CloudController class AppUsageEvent < Sequel::Model plugin :serialization + one_to_many :consumers, + class: 'VCAP::CloudController::AppUsageConsumer', + key: :last_processed_guid, + primary_key: :guid + + add_association_dependencies consumers: :destroy + export_attributes :state, :previous_state, :memory_in_mb_per_instance, :previous_memory_in_mb_per_instance, :instance_count, :previous_instance_count, diff --git a/app/models/services/service_usage_consumer.rb b/app/models/services/service_usage_consumer.rb new file mode 100644 index 00000000000..2a4eb7011f0 --- /dev/null +++ b/app/models/services/service_usage_consumer.rb @@ -0,0 +1,26 @@ +module VCAP::CloudController + class ServiceUsageConsumer < Sequel::Model + plugin :validation_helpers + + many_to_one :last_processed_event, + class: 'VCAP::CloudController::ServiceUsageEvent', + key: :last_processed_guid, + primary_key: :guid + + def validate + validates_presence %i[consumer_guid last_processed_guid] + validates_unique :consumer_guid + validates_max_length 255, :consumer_guid, message: 'must be less than 255 characters' + end + + def last_processed_guid=(guid) + self[:last_processed_guid] = guid + end + + def last_processed_guid + self[:last_processed_guid] + end + + export_attributes :consumer_guid, :last_processed_guid + end +end diff --git a/app/models/services/service_usage_event.rb b/app/models/services/service_usage_event.rb index f5e694169f9..1090ff3d25b 100644 --- a/app/models/services/service_usage_event.rb +++ b/app/models/services/service_usage_event.rb @@ -2,6 +2,13 @@ module VCAP::CloudController class ServiceUsageEvent < Sequel::Model plugin :serialization + one_to_many :consumers, + class: 'VCAP::CloudController::ServiceUsageConsumer', + key: :last_processed_guid, + primary_key: :guid + + add_association_dependencies consumers: :destroy + export_attributes :state, :org_guid, :space_guid, :space_name, :service_instance_guid, :service_instance_name, :service_instance_type, :service_plan_guid, :service_plan_name, diff --git a/app/repositories/app_usage_event_repository.rb b/app/repositories/app_usage_event_repository.rb index 8955c326b19..5b3d2497f23 100644 --- a/app/repositories/app_usage_event_repository.rb +++ b/app/repositories/app_usage_event_repository.rb @@ -152,7 +152,8 @@ def purge_and_reseed_started_apps! end def delete_events_older_than(cutoff_age_in_days) - Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true).delete + Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true, + keep_unprocessed_records: true).delete end private diff --git a/app/repositories/service_usage_event_repository.rb b/app/repositories/service_usage_event_repository.rb index 1f55434c621..7537e6ed33a 100644 --- a/app/repositories/service_usage_event_repository.rb +++ b/app/repositories/service_usage_event_repository.rb @@ -92,7 +92,8 @@ def purge_and_reseed_service_instances! end def delete_events_older_than(cutoff_age_in_days) - Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true).delete + Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true, + keep_unprocessed_records: true).delete end end end diff --git a/db/migrations/20241205162359_create_app_usage_consumers.rb b/db/migrations/20241205162359_create_app_usage_consumers.rb new file mode 100644 index 00000000000..622e83a66d9 --- /dev/null +++ b/db/migrations/20241205162359_create_app_usage_consumers.rb @@ -0,0 +1,9 @@ +Sequel.migration do + change do + create_table :app_usage_consumers do |_t| + VCAP::Migration.common(self) + String :consumer_guid, null: false, size: 255 + String :last_processed_guid, null: false, size: 255 + end + end +end diff --git a/db/migrations/20241218113511_create_service_usage_consumers.rb b/db/migrations/20241218113511_create_service_usage_consumers.rb new file mode 100644 index 00000000000..1337db767a9 --- /dev/null +++ b/db/migrations/20241218113511_create_service_usage_consumers.rb @@ -0,0 +1,9 @@ +Sequel.migration do + change do + create_table :service_usage_consumers do |_t| + VCAP::Migration.common(self) + String :consumer_guid, null: false, size: 255 + String :last_processed_guid, null: false, size: 255 + end + end +end diff --git a/lib/database/old_record_cleanup.rb b/lib/database/old_record_cleanup.rb index b865a859d8e..604f791a059 100644 --- a/lib/database/old_record_cleanup.rb +++ b/lib/database/old_record_cleanup.rb @@ -3,13 +3,14 @@ module Database class OldRecordCleanup class NoCurrentTimestampError < StandardError; end - attr_reader :model, :days_ago, :keep_at_least_one_record, :keep_running_records + attr_reader :model, :days_ago, :keep_at_least_one_record, :keep_running_records, :keep_unprocessed_records - def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false, keep_running_records: false) + def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false, keep_running_records: false, keep_unprocessed_records: false) @model = model @days_ago = cutoff_age_in_days @keep_at_least_one_record = keep_at_least_one_record @keep_running_records = keep_running_records + @keep_unprocessed_records = keep_unprocessed_records end def delete @@ -20,9 +21,11 @@ def delete last_record = model.order(:id).last old_records = old_records.where(Sequel.lit('id < ?', last_record.id)) if last_record end - logger.info("Cleaning up #{old_records.count} #{model.table_name} table rows") old_records = exclude_running_records(old_records) if keep_running_records + old_records = exclude_unprocessed_records(old_records) if keep_unprocessed_records + + logger.info("Cleaning up #{old_records.count} #{model.table_name} table rows") Database::BatchDelete.new(old_records, 1000).delete end @@ -88,5 +91,30 @@ def guid_symbol(model) nil end + + def exclude_unprocessed_records(old_records) + consumer_model = registered_consumer_model(model) + + return old_records unless consumer_model + + # Find the usage event record with the lowest ID + # of any that are referenced by a consumer + referenced_event = model. + join(consumer_model.table_name.to_sym, last_processed_guid: :guid). + select(Sequel.function(:min, Sequel.qualify(model.table_name, :id)).as(:min_id)). + first + + return old_records if referenced_event[:min_id].nil? + + old_records.where { id < referenced_event[:min_id] } + end + + def registered_consumer_model(model) + return VCAP::CloudController::AppUsageConsumer if model == VCAP::CloudController::AppUsageEvent && VCAP::CloudController::AppUsageConsumer.present? + + return VCAP::CloudController::ServiceUsageConsumer if model == VCAP::CloudController::ServiceUsageEvent && VCAP::CloudController::ServiceUsageConsumer.present? + + false + end end end diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb index 33937670fd6..18b049861f7 100644 --- a/spec/support/fakes/blueprints.rb +++ b/spec/support/fakes/blueprints.rb @@ -732,6 +732,16 @@ module VCAP::CloudController process_type { 'web' } end + AppUsageConsumer.blueprint do + consumer_guid { SecureRandom.uuid } + last_processed_guid { VCAP::CloudController::AppUsageEvent.make.guid } + end + + ServiceUsageConsumer.blueprint do + consumer_guid { SecureRandom.uuid } + last_processed_guid { VCAP::CloudController::ServiceUsageEvent.make.guid } + end + ServiceUsageEvent.blueprint do state { 'CREATED' } org_guid { Sham.guid } diff --git a/spec/unit/lib/database/old_record_cleanup_spec.rb b/spec/unit/lib/database/old_record_cleanup_spec.rb index 9d7318b3a98..9fd3f7b6b89 100644 --- a/spec/unit/lib/database/old_record_cleanup_spec.rb +++ b/spec/unit/lib/database/old_record_cleanup_spec.rb @@ -22,7 +22,8 @@ context "when there are no records at all but you're trying to keep at least one" do it "doesn't keep one because there aren't any to keep" do - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppEvent, cutoff_age_in_days: 1, keep_at_least_one_record: true, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppEvent, cutoff_age_in_days: 1, keep_at_least_one_record: true, keep_running_records: true, + keep_unprocessed_records: false) expect { record_cleanup.delete }.not_to raise_error expect(VCAP::CloudController::AppEvent.count).to eq(0) @@ -41,7 +42,8 @@ fresh_event = VCAP::CloudController::Event.make(created_at: 1.day.ago + 1.minute) - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 0, keep_at_least_one_record: true, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::Event, cutoff_age_in_days: 0, keep_at_least_one_record: true, keep_running_records: true, + keep_unprocessed_records: false) expect do record_cleanup.delete @@ -56,7 +58,8 @@ it 'keeps AppUsageEvent start record when there is no corresponding stop record' do stale_app_usage_event_start = VCAP::CloudController::AppUsageEvent.make(created_at: 2.days.ago, state: 'STARTED', app_guid: 'guid1') - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true, + keep_unprocessed_records: false) record_cleanup.delete expect(stale_app_usage_event_start.reload).to be_present end @@ -65,7 +68,8 @@ stale_app_usage_event_start = VCAP::CloudController::AppUsageEvent.make(created_at: 2.days.ago, state: 'STARTED', app_guid: 'guid1') fresh_app_usage_event_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.day.ago + 1.minute, state: 'STOPPED', app_guid: 'guid1') - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true, + keep_unprocessed_records: false) record_cleanup.delete expect(stale_app_usage_event_start.reload).to be_present expect(fresh_app_usage_event_stop.reload).to be_present @@ -75,7 +79,8 @@ stale_app_usage_event_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 3.days.ago, state: 'STOPPED', app_guid: 'guid1') stale_app_usage_event_start = VCAP::CloudController::AppUsageEvent.make(created_at: 2.days.ago, state: 'STARTED', app_guid: 'guid1') - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true, + keep_unprocessed_records: false) record_cleanup.delete expect(stale_app_usage_event_start.reload).to be_present expect { stale_app_usage_event_stop.reload }.to raise_error(Sequel::NoExistingObject) @@ -84,7 +89,8 @@ it 'keeps ServiceUsageEvent create record when there is no corresponding delete record' do stale_service_usage_event_create = VCAP::CloudController::ServiceUsageEvent.make(created_at: 2.days.ago, state: 'CREATED', service_instance_guid: 'guid1') - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true, + keep_unprocessed_records: false) record_cleanup.delete expect(stale_service_usage_event_create.reload).to be_present end @@ -93,7 +99,8 @@ stale_service_usage_event_create = VCAP::CloudController::ServiceUsageEvent.make(created_at: 2.days.ago, state: 'CREATED', service_instance_guid: 'guid1') fresh_service_usage_event_delete = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.day.ago + 1.minute, state: 'DELETED', service_instance_guid: 'guid1') - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true, + keep_unprocessed_records: false) record_cleanup.delete expect(stale_service_usage_event_create.reload).to be_present expect(fresh_service_usage_event_delete.reload).to be_present @@ -103,10 +110,92 @@ stale_service_usage_event_delete = VCAP::CloudController::ServiceUsageEvent.make(created_at: 3.days.ago, state: 'DELETED', service_instance_guid: 'guid1') stale_service_usage_event_create = VCAP::CloudController::ServiceUsageEvent.make(created_at: 2.days.ago, state: 'CREATED', service_instance_guid: 'guid1') - record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true) + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: true, + keep_unprocessed_records: false) record_cleanup.delete expect(stale_service_usage_event_create.reload).to be_present expect { stale_service_usage_event_delete.reload }.to raise_error(Sequel::NoExistingObject) end + + # Testing keep_unprocessed_records feature + it 'keeps unprocessed AppUsageEvent records older than the cutoff date' do + stale_app_usage_event_1_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid1') + stale_app_usage_event_2_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid2') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: stale_app_usage_event_2_stop.guid) + stale_app_usage_event_3_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid3') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, + keep_unprocessed_records: true) + record_cleanup.delete + expect { stale_app_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) + expect(stale_app_usage_event_2_stop.reload).to be_present + expect(stale_app_usage_event_3_stop.reload).to be_present + end + + it 'keeps unprocessed ServiceUsageEvent records older than the cutoff date' do + stale_service_usage_event_1_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid1') + stale_service_usage_event_2_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid2') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: stale_service_usage_event_2_stop.guid) + stale_service_usage_event_3_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid3') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, + keep_unprocessed_records: true) + record_cleanup.delete + expect { stale_service_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) + expect(stale_service_usage_event_2_stop.reload).to be_present + expect(stale_service_usage_event_3_stop.reload).to be_present + end + + it 'deletes all stale AppUsageEvent records when all registered consumers reference non-existant guids' do + stale_app_usage_event_1_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid1') + stale_app_usage_event_2_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid2') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: 'fake-guid-1') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: 'fake-guid-2') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, + keep_unprocessed_records: true) + record_cleanup.delete + expect { stale_app_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) + expect { stale_app_usage_event_2_stop.reload }.to raise_error(Sequel::NoExistingObject) + end + + it 'deletes all stale ServiceUsageEvent records when all registered consumers reference non-existant guids' do + stale_service_usage_event_1_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid1') + stale_service_usage_event_2_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid2') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: 'fake-guid-1') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: 'fake-guid-2') + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, + keep_unprocessed_records: true) + record_cleanup.delete + expect { stale_service_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) + expect { stale_service_usage_event_2_stop.reload }.to raise_error(Sequel::NoExistingObject) + end + + it 'deletes stale AppUsageEvent records even if 1 consumer references a non-existant guid' do + stale_app_usage_event_1_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid1') + stale_app_usage_event_2_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid2') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: 'fake-guid-1') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: stale_app_usage_event_2_stop.guid) + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, + keep_unprocessed_records: true) + record_cleanup.delete + expect { stale_app_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) + expect(stale_app_usage_event_2_stop.reload).to be_present + end + + it 'deletes stale ServiceUsageEvent records even if 1 consumer references a non-existant guid' do + stale_service_usage_event_1_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid1') + stale_service_usage_event_2_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid2') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: 'fake-guid-1') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: stale_service_usage_event_2_stop.guid) + + record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, + keep_unprocessed_records: true) + record_cleanup.delete + expect { stale_service_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) + expect(stale_service_usage_event_2_stop.reload).to be_present + end end end From 29454af9a262f3f0908674304b0e6f5ab1736ad7 Mon Sep 17 00:00:00 2001 From: Dave Riddle Date: Mon, 20 Jan 2025 21:53:00 -0600 Subject: [PATCH 4/8] Set threshold for keeping un-processed records Only keep un-processed records if usage event table size is below threshold. This is a safeguard to ensure row count does not grow unbounded in the event of a zombie consumer. --- app/jobs/runtime/app_usage_events_cleanup.rb | 7 +- .../services/service_usage_events_cleanup.rb | 7 +- .../app_usage_event_repository.rb | 4 +- .../service_usage_event_repository.rb | 5 +- config/bosh-lite.yml | 22 +++-- config/cloud_controller.yml | 2 + lib/cloud_controller/clock/scheduler.rb | 14 ++- .../config_schemas/base/clock_schema.rb | 8 +- lib/database/old_record_cleanup.rb | 54 ++++++++--- spec/fixtures/config/port_8181_config.yml | 1 + .../runtime/app_usage_events_cleanup_spec.rb | 3 +- .../service_usage_events_cleanup_spec.rb | 3 +- .../cloud_controller/clock/scheduler_spec.rb | 14 ++- .../lib/database/old_record_cleanup_spec.rb | 93 +++++++++++++++++-- .../app_usage_event_repository_spec.rb | 5 +- .../service_usage_event_repository_spec.rb | 5 +- 16 files changed, 194 insertions(+), 53 deletions(-) diff --git a/app/jobs/runtime/app_usage_events_cleanup.rb b/app/jobs/runtime/app_usage_events_cleanup.rb index e10fcae21b2..9a5d1b520cd 100644 --- a/app/jobs/runtime/app_usage_events_cleanup.rb +++ b/app/jobs/runtime/app_usage_events_cleanup.rb @@ -4,10 +4,11 @@ module VCAP::CloudController module Jobs module Runtime class AppUsageEventsCleanup < VCAP::CloudController::Jobs::CCJob - attr_accessor :cutoff_age_in_days + attr_accessor :cutoff_age_in_days, :threshold_for_keeping_unprocessed_records - def initialize(cutoff_age_in_days) + def initialize(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) @cutoff_age_in_days = cutoff_age_in_days + @threshold_for_keeping_unprocessed_records = threshold_for_keeping_unprocessed_records end def perform @@ -15,7 +16,7 @@ def perform logger.info('Cleaning up old AppUsageEvent rows') repository = Repositories::AppUsageEventRepository.new - repository.delete_events_older_than(cutoff_age_in_days) + repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end def job_name_in_configuration diff --git a/app/jobs/v2/services/service_usage_events_cleanup.rb b/app/jobs/v2/services/service_usage_events_cleanup.rb index 5a0173cdeb5..16e01476a83 100644 --- a/app/jobs/v2/services/service_usage_events_cleanup.rb +++ b/app/jobs/v2/services/service_usage_events_cleanup.rb @@ -4,10 +4,11 @@ module VCAP::CloudController module Jobs module Services class ServiceUsageEventsCleanup < VCAP::CloudController::Jobs::CCJob - attr_accessor :cutoff_age_in_days + attr_accessor :cutoff_age_in_days, :threshold_for_keeping_unprocessed_records - def initialize(cutoff_age_in_days) + def initialize(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) @cutoff_age_in_days = cutoff_age_in_days + @threshold_for_keeping_unprocessed_records = threshold_for_keeping_unprocessed_records end def perform @@ -15,7 +16,7 @@ def perform logger.info('Cleaning up old ServiceUsageEvent rows') repository = Repositories::ServiceUsageEventRepository.new - repository.delete_events_older_than(cutoff_age_in_days) + repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end def job_name_in_configuration diff --git a/app/repositories/app_usage_event_repository.rb b/app/repositories/app_usage_event_repository.rb index 5b3d2497f23..d1a6886237d 100644 --- a/app/repositories/app_usage_event_repository.rb +++ b/app/repositories/app_usage_event_repository.rb @@ -151,9 +151,9 @@ def purge_and_reseed_started_apps! AppUsageEvent.insert(column_map.keys, usage_query) end - def delete_events_older_than(cutoff_age_in_days) + def delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true, - keep_unprocessed_records: true).delete + keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: threshold_for_keeping_unprocessed_records).delete end private diff --git a/app/repositories/service_usage_event_repository.rb b/app/repositories/service_usage_event_repository.rb index 7537e6ed33a..bf50e36957b 100644 --- a/app/repositories/service_usage_event_repository.rb +++ b/app/repositories/service_usage_event_repository.rb @@ -91,9 +91,10 @@ def purge_and_reseed_service_instances! ServiceUsageEvent.insert(column_map.keys, usage_query) end - def delete_events_older_than(cutoff_age_in_days) + def delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true, - keep_unprocessed_records: true).delete + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: threshold_for_keeping_unprocessed_records).delete end end end diff --git a/config/bosh-lite.yml b/config/bosh-lite.yml index f4a7fd2f19d..d79c60ad69e 100644 --- a/config/bosh-lite.yml +++ b/config/bosh-lite.yml @@ -10,7 +10,7 @@ newrelic_enabled: false external_protocol: https external_domain: api.bosh-lite.com -system_domain_organization: +system_domain_organization: system_domain: bosh-lite.com app_domains: - name: bosh-lite.com @@ -21,9 +21,11 @@ jobs: app_usage_events: cutoff_age_in_days: 31 + threshold_for_keeping_unprocessed_records: 5000000 service_usage_events: cutoff_age_in_days: 31 + threshold_for_keeping_unprocessed_records: 5000000 audit_events: cutoff_age_in_days: 31 @@ -133,8 +135,8 @@ resource_pool: maximum_size: 536870912 resource_directory_key: bosh-lite.com-cc-resources cdn: - uri: - key_pair_id: + uri: + key_pair_id: private_key: "" fog_connection: {} @@ -150,8 +152,8 @@ packages: max_valid_packages_stored: 5 max_package_size: 1073741824 cdn: - uri: - key_pair_id: + uri: + key_pair_id: private_key: "" fog_connection: {} @@ -165,8 +167,8 @@ droplets: password: blobstore-password droplet_directory_key: bosh-lite.com-cc-droplets cdn: - uri: - key_pair_id: + uri: + key_pair_id: private_key: "" fog_connection: {} max_staged_droplets_stored: 5 @@ -181,8 +183,8 @@ buildpacks: password: blobstore-password buildpack_directory_key: bosh-lite.com-cc-buildpacks cdn: - uri: - key_pair_id: + uri: + key_pair_id: private_key: "" fog_connection: {} @@ -211,7 +213,7 @@ default_app_ssh_access: true skip_cert_verify: true -install_buildpacks: +install_buildpacks: - name: staticfile_buildpack package: staticfile-buildpack - name: java_buildpack diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index f0cbb85e6c2..d5f33f4f728 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -38,9 +38,11 @@ jobs: app_usage_events: cutoff_age_in_days: 31 + threshold_for_keeping_unprocessed_records: 5000000 service_usage_events: cutoff_age_in_days: 31 + threshold_for_keeping_unprocessed_records: 5000000 audit_events: cutoff_age_in_days: 31 diff --git a/lib/cloud_controller/clock/scheduler.rb b/lib/cloud_controller/clock/scheduler.rb index cc2dbb853b6..460b9b8f6a5 100644 --- a/lib/cloud_controller/clock/scheduler.rb +++ b/lib/cloud_controller/clock/scheduler.rb @@ -5,9 +5,11 @@ module VCAP::CloudController class Scheduler CLEANUPS = [ - { name: 'app_usage_events', class: Jobs::Runtime::AppUsageEventsCleanup, time: '18:00', arg_from_config: %i[app_usage_events cutoff_age_in_days] }, + { name: 'app_usage_events', class: Jobs::Runtime::AppUsageEventsCleanup, time: '18:00', + arg_from_config: [%i[app_usage_events cutoff_age_in_days], %i[app_usage_events threshold_for_keeping_unprocessed_records]] }, { name: 'audit_events', class: Jobs::Runtime::EventsCleanup, time: '20:00', arg_from_config: %i[audit_events cutoff_age_in_days] }, - { name: 'service_usage_events', class: Jobs::Services::ServiceUsageEventsCleanup, time: '22:00', arg_from_config: %i[service_usage_events cutoff_age_in_days] }, + { name: 'service_usage_events', class: Jobs::Services::ServiceUsageEventsCleanup, time: '22:00', + arg_from_config: [%i[service_usage_events cutoff_age_in_days], %i[service_usage_events threshold_for_keeping_unprocessed_records]] }, { name: 'completed_tasks', class: Jobs::Runtime::PruneCompletedTasks, time: '23:00', arg_from_config: %i[completed_tasks cutoff_age_in_days] }, { name: 'expired_blob_cleanup', class: Jobs::Runtime::ExpiredBlobCleanup, time: '00:00' }, { name: 'expired_resource_cleanup', class: Jobs::Runtime::ExpiredResourceCleanup, time: '00:30' }, @@ -87,7 +89,13 @@ def start_daily_jobs klass = cleanup_config[:class] if cleanup_config[:arg_from_config] - klass.new(@config.get(*cleanup_config[:arg_from_config])) + args = cleanup_config[:arg_from_config] + if args.first.is_a?(Array) + args = args.map { |arg| @config.get(*arg) } + klass.new(*args) + else + klass.new(@config.get(*args)) + end else klass.new end diff --git a/lib/cloud_controller/config_schemas/base/clock_schema.rb b/lib/cloud_controller/config_schemas/base/clock_schema.rb index 85574a71986..d844bab4d91 100644 --- a/lib/cloud_controller/config_schemas/base/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/base/clock_schema.rb @@ -16,7 +16,8 @@ class ClockSchema < VCAP::Config clock: Integer }, app_usage_events: { - cutoff_age_in_days: Integer + cutoff_age_in_days: Integer, + threshold_for_keeping_unprocessed_records: Integer }, audit_events: { cutoff_age_in_days: Integer @@ -169,7 +170,10 @@ class ClockSchema < VCAP::Config frequency_in_seconds: Integer }, - service_usage_events: { cutoff_age_in_days: Integer }, + service_usage_events: { + cutoff_age_in_days: Integer, + threshold_for_keeping_unprocessed_records: Integer + }, default_app_ssh_access: bool, allow_app_ssh_access: bool, jobs: { diff --git a/lib/database/old_record_cleanup.rb b/lib/database/old_record_cleanup.rb index 604f791a059..6a877a24fcc 100644 --- a/lib/database/old_record_cleanup.rb +++ b/lib/database/old_record_cleanup.rb @@ -3,14 +3,16 @@ module Database class OldRecordCleanup class NoCurrentTimestampError < StandardError; end - attr_reader :model, :days_ago, :keep_at_least_one_record, :keep_running_records, :keep_unprocessed_records + attr_reader :model, :days_ago, :keep_at_least_one_record, :keep_running_records, :keep_unprocessed_records, :threshold_for_keeping_unprocessed_records - def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false, keep_running_records: false, keep_unprocessed_records: false) + def initialize(model, cutoff_age_in_days:, keep_at_least_one_record: false, keep_running_records: false, keep_unprocessed_records: false, + threshold_for_keeping_unprocessed_records: nil) @model = model @days_ago = cutoff_age_in_days @keep_at_least_one_record = keep_at_least_one_record @keep_running_records = keep_running_records @keep_unprocessed_records = keep_unprocessed_records + @threshold_for_keeping_unprocessed_records = threshold_for_keeping_unprocessed_records end def delete @@ -97,16 +99,22 @@ def exclude_unprocessed_records(old_records) return old_records unless consumer_model - # Find the usage event record with the lowest ID - # of any that are referenced by a consumer - referenced_event = model. - join(consumer_model.table_name.to_sym, last_processed_guid: :guid). - select(Sequel.function(:min, Sequel.qualify(model.table_name, :id)).as(:min_id)). - first - - return old_records if referenced_event[:min_id].nil? - - old_records.where { id < referenced_event[:min_id] } + if approximate_row_count(model) < threshold_for_keeping_unprocessed_records.to_i + # Find the usage event record with the lowest ID + # of any that are referenced by a consumer + referenced_event = model. + join(consumer_model.table_name.to_sym, last_processed_guid: :guid). + select(Sequel.function(:min, Sequel.qualify(model.table_name, :id)).as(:min_id)). + first + + return old_records if referenced_event[:min_id].nil? + + old_records.where { id < referenced_event[:min_id] } + else + # When above threshold, we don't exclude any records + # Associated consumers will be automatically deleted by Sequel + old_records + end end def registered_consumer_model(model) @@ -116,5 +124,27 @@ def registered_consumer_model(model) false end + + def approximate_row_count(model) + case model.db.database_type + when :postgres + result = model.db[<<-SQL.squish + SELECT reltuples::bigint AS estimate + FROM pg_class + WHERE relname = '#{model.table_name}' + SQL + ].first + result[:estimate].to_i + when :mysql, :mysql2 + result = model.db[<<-SQL.squish + SELECT table_rows + FROM information_schema.tables + WHERE table_schema = DATABASE() + AND table_name = '#{model.table_name}' + SQL + ].first + result[:table_rows].to_i + end + end end end diff --git a/spec/fixtures/config/port_8181_config.yml b/spec/fixtures/config/port_8181_config.yml index 9800796ddd9..1ee9578b6f9 100644 --- a/spec/fixtures/config/port_8181_config.yml +++ b/spec/fixtures/config/port_8181_config.yml @@ -30,6 +30,7 @@ jobs: app_usage_events: cutoff_age_in_days: 31 + threshold_for_keeping_unprocessed_records: 5000000 audit_events: cutoff_age_in_days: 31 diff --git a/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb b/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb index ce4f1df9956..c9afaeb0d07 100644 --- a/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb +++ b/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb @@ -4,12 +4,13 @@ module VCAP::CloudController module Jobs::Runtime RSpec.describe AppUsageEventsCleanup, job_context: :worker do let(:cutoff_age_in_days) { 30 } + let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } let(:logger) { double(Steno::Logger, info: nil) } let!(:event_before_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago, state: 'STOPPED') } let!(:event_after_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days - 1).days.ago) } subject(:job) do - AppUsageEventsCleanup.new(cutoff_age_in_days) + AppUsageEventsCleanup.new(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end before do diff --git a/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb b/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb index da11beb39b5..908cbef4008 100644 --- a/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb +++ b/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb @@ -4,12 +4,13 @@ module VCAP::CloudController module Jobs::Services RSpec.describe ServiceUsageEventsCleanup, job_context: :worker do let(:cutoff_age_in_days) { 30 } + let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } let(:logger) { double(Steno::Logger, info: nil) } let!(:event_before_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago, state: 'DELETED') } let!(:event_after_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days - 1).days.ago) } subject(:job) do - ServiceUsageEventsCleanup.new(cutoff_age_in_days) + ServiceUsageEventsCleanup.new(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end before do diff --git a/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb b/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb index 4a44cd3c305..0d4dd00670a 100644 --- a/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb +++ b/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb @@ -16,12 +16,18 @@ module VCAP::CloudController jobs: { global: { timeout_in_seconds: global_timeout } }, - app_usage_events: { cutoff_age_in_days: 1 }, + app_usage_events: { + cutoff_age_in_days: 1, + threshold_for_keeping_unprocessed_records: 5_000_000 + }, audit_events: { cutoff_age_in_days: 3 }, failed_jobs: { frequency_in_seconds: 400, cutoff_age_in_days: 4, max_number_of_failed_delayed_jobs: 10 }, pollable_jobs: { cutoff_age_in_days: 2 }, service_operations_initial_cleanup: { frequency_in_seconds: 600 }, - service_usage_events: { cutoff_age_in_days: 5 }, + service_usage_events: { + cutoff_age_in_days: 5, + threshold_for_keeping_unprocessed_records: 5_000_000 + }, completed_tasks: { cutoff_age_in_days: 6 }, pending_droplets: { frequency_in_seconds: 300, expiration_in_seconds: 600 }, pending_builds: { frequency_in_seconds: 400, expiration_in_seconds: 700 }, @@ -61,7 +67,7 @@ module VCAP::CloudController expect(clock).to receive(:schedule_daily_job) do |args, &block| expect(args).to eql(name: 'app_usage_events', at: '18:00', priority: 0) - expect(Jobs::Runtime::AppUsageEventsCleanup).to receive(:new).with(1).and_call_original + expect(Jobs::Runtime::AppUsageEventsCleanup).to receive(:new).with(1, 5_000_000).and_call_original expect(block.call).to be_instance_of(Jobs::Runtime::AppUsageEventsCleanup) end @@ -73,7 +79,7 @@ module VCAP::CloudController expect(clock).to receive(:schedule_daily_job) do |args, &block| expect(args).to eql(name: 'service_usage_events', at: '22:00', priority: 0) - expect(Jobs::Services::ServiceUsageEventsCleanup).to receive(:new).with(5).and_call_original + expect(Jobs::Services::ServiceUsageEventsCleanup).to receive(:new).with(5, 5_000_000).and_call_original expect(block.call).to be_instance_of(Jobs::Services::ServiceUsageEventsCleanup) end diff --git a/spec/unit/lib/database/old_record_cleanup_spec.rb b/spec/unit/lib/database/old_record_cleanup_spec.rb index 9fd3f7b6b89..496916f3ba7 100644 --- a/spec/unit/lib/database/old_record_cleanup_spec.rb +++ b/spec/unit/lib/database/old_record_cleanup_spec.rb @@ -125,7 +125,7 @@ stale_app_usage_event_3_stop = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', app_guid: 'guid3') record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, - keep_unprocessed_records: true) + keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: 5_000_000) record_cleanup.delete expect { stale_app_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) expect(stale_app_usage_event_2_stop.reload).to be_present @@ -139,7 +139,8 @@ stale_service_usage_event_3_stop = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED', service_instance_guid: 'guid3') record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, - keep_unprocessed_records: true) + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000) record_cleanup.delete expect { stale_service_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) expect(stale_service_usage_event_2_stop.reload).to be_present @@ -153,7 +154,7 @@ VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: 'fake-guid-2') record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, - keep_unprocessed_records: true) + keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: 5_000_000) record_cleanup.delete expect { stale_app_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) expect { stale_app_usage_event_2_stop.reload }.to raise_error(Sequel::NoExistingObject) @@ -166,7 +167,8 @@ VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: 'fake-guid-2') record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, - keep_unprocessed_records: true) + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000) record_cleanup.delete expect { stale_service_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) expect { stale_service_usage_event_2_stop.reload }.to raise_error(Sequel::NoExistingObject) @@ -179,7 +181,7 @@ VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: stale_app_usage_event_2_stop.guid) record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::AppUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, - keep_unprocessed_records: true) + keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: 5_000_000) record_cleanup.delete expect { stale_app_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) expect(stale_app_usage_event_2_stop.reload).to be_present @@ -192,10 +194,89 @@ VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid2', last_processed_guid: stale_service_usage_event_2_stop.guid) record_cleanup = Database::OldRecordCleanup.new(VCAP::CloudController::ServiceUsageEvent, cutoff_age_in_days: 1, keep_at_least_one_record: false, keep_running_records: false, - keep_unprocessed_records: true) + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000) record_cleanup.delete expect { stale_service_usage_event_1_stop.reload }.to raise_error(Sequel::NoExistingObject) expect(stale_service_usage_event_2_stop.reload).to be_present end + + describe 'threshold_for_keeping_unprocessed_records behavior' do + context 'when row count is below threshold' do + it 'keeps unprocessed app usage events that are referenced by consumers' do + stale_app_usage_event = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: stale_app_usage_event.guid) + + record_cleanup = Database::OldRecordCleanup.new( + VCAP::CloudController::AppUsageEvent, + cutoff_age_in_days: 1, + keep_at_least_one_record: false, + keep_running_records: false, + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000 + ) + + record_cleanup.delete + expect(stale_app_usage_event.reload).to be_present + end + + it 'keeps unprocessed service usage events that are referenced by consumers' do + stale_service_usage_event = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: stale_service_usage_event.guid) + + record_cleanup = Database::OldRecordCleanup.new( + VCAP::CloudController::ServiceUsageEvent, + cutoff_age_in_days: 1, + keep_at_least_one_record: false, + keep_running_records: false, + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000 + ) + + record_cleanup.delete + expect(stale_service_usage_event.reload).to be_present + end + end + + context 'when row count is above threshold' do + before do + allow_any_instance_of(Database::OldRecordCleanup).to receive(:approximate_row_count).and_return(6_000_000) + end + + it 'deletes app usage events even if referenced by consumers' do + stale_app_usage_event = VCAP::CloudController::AppUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED') + VCAP::CloudController::AppUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: stale_app_usage_event.guid) + + record_cleanup = Database::OldRecordCleanup.new( + VCAP::CloudController::AppUsageEvent, + cutoff_age_in_days: 1, + keep_at_least_one_record: false, + keep_running_records: false, + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000 + ) + + record_cleanup.delete + expect { stale_app_usage_event.reload }.to raise_error(Sequel::NoExistingObject) + end + + it 'deletes service usage events even if referenced by consumers' do + stale_service_usage_event = VCAP::CloudController::ServiceUsageEvent.make(created_at: 1.year.ago, state: 'STOPPED') + VCAP::CloudController::ServiceUsageConsumer.make(consumer_guid: 'guid1', last_processed_guid: stale_service_usage_event.guid) + + record_cleanup = Database::OldRecordCleanup.new( + VCAP::CloudController::ServiceUsageEvent, + cutoff_age_in_days: 1, + keep_at_least_one_record: false, + keep_running_records: false, + keep_unprocessed_records: true, + threshold_for_keeping_unprocessed_records: 5_000_000 + ) + + record_cleanup.delete + expect { stale_service_usage_event.reload }.to raise_error(Sequel::NoExistingObject) + end + end + end end end diff --git a/spec/unit/repositories/app_usage_event_repository_spec.rb b/spec/unit/repositories/app_usage_event_repository_spec.rb index 02a3307a626..3f4e27c3854 100644 --- a/spec/unit/repositories/app_usage_event_repository_spec.rb +++ b/spec/unit/repositories/app_usage_event_repository_spec.rb @@ -680,6 +680,7 @@ module Repositories describe '#delete_events_older_than' do let(:cutoff_age_in_days) { 1 } + let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } before do AppUsageEvent.dataset.delete @@ -698,7 +699,7 @@ module Repositories repository.create_from_process(process) expect do - repository.delete_events_older_than(cutoff_age_in_days) + repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end.to change(AppUsageEvent, :count).to(1) expect(AppUsageEvent.last).to match_app(process) @@ -706,7 +707,7 @@ module Repositories it 'keeps the last record even if before the cutoff age' do expect do - repository.delete_events_older_than(cutoff_age_in_days) + repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end.to change(AppUsageEvent, :count).to(1) expect(AppUsageEvent.last.created_at).to be < cutoff_age_in_days.days.ago diff --git a/spec/unit/repositories/service_usage_event_repository_spec.rb b/spec/unit/repositories/service_usage_event_repository_spec.rb index 6dd8398e038..bb471999b68 100644 --- a/spec/unit/repositories/service_usage_event_repository_spec.rb +++ b/spec/unit/repositories/service_usage_event_repository_spec.rb @@ -157,6 +157,7 @@ module Repositories describe '#delete_events_older_than' do let!(:service_instance) { ManagedServiceInstance.make } let(:cutoff_age_in_days) { 1 } + let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } before do ServiceUsageEvent.dataset.delete @@ -174,7 +175,7 @@ module Repositories new_event = repository.create_from_service_instance(service_instance, 'SOME-STATE') expect do - repository.delete_events_older_than(cutoff_age_in_days) + repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end.to change(ServiceUsageEvent, :count).to(1) expect(ServiceUsageEvent.last).to eq(new_event.reload) @@ -182,7 +183,7 @@ module Repositories it 'keeps the last record even if before the cutoff age' do expect do - repository.delete_events_older_than(cutoff_age_in_days) + repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) end.to change(ServiceUsageEvent, :count).to(1) expect(ServiceUsageEvent.last.created_at).to be < cutoff_age_in_days.days.ago From b07c209e451f4522ba72e709eeb295aa60645146 Mon Sep 17 00:00:00 2001 From: Dave Riddle Date: Mon, 20 Jan 2025 21:57:42 -0600 Subject: [PATCH 5/8] Provide index, show, and destroy endpoints for usage consumers Allows for usage consumers to de-register themselves with the cloud controller. Index and show methods allow consumers to make informed decisions before initiating a purge of usage events. Usage event endpoint re-ordered to align with convention of other resources. --- app/actions/app_usage_consumer_delete.rb | 9 + app/actions/service_usage_consumer_delete.rb | 9 + .../v3/app_usage_consumers_controller.rb | 41 +++++ .../v3/service_usage_consumers_controller.rb | 41 +++++ .../app_usage_consumer_list_fetcher.rb | 17 ++ .../service_usage_consumer_list_fetcher.rb | 17 ++ .../app_usage_consumers_list_message.rb | 14 ++ .../service_usage_consumers_list_message.rb | 14 ++ .../v3/app_usage_consumer_presenter.rb | 29 ++++ .../v3/service_usage_consumer_presenter.rb | 29 ++++ config/routes.rb | 14 +- .../v3/app_usage_consumers_controller_spec.rb | 156 ++++++++++++++++++ ...service_usage_consumers_controller_spec.rb | 156 ++++++++++++++++++ 13 files changed, 544 insertions(+), 2 deletions(-) create mode 100644 app/actions/app_usage_consumer_delete.rb create mode 100644 app/actions/service_usage_consumer_delete.rb create mode 100644 app/controllers/v3/app_usage_consumers_controller.rb create mode 100644 app/controllers/v3/service_usage_consumers_controller.rb create mode 100644 app/fetchers/app_usage_consumer_list_fetcher.rb create mode 100644 app/fetchers/service_usage_consumer_list_fetcher.rb create mode 100644 app/messages/app_usage_consumers_list_message.rb create mode 100644 app/messages/service_usage_consumers_list_message.rb create mode 100644 app/presenters/v3/app_usage_consumer_presenter.rb create mode 100644 app/presenters/v3/service_usage_consumer_presenter.rb create mode 100644 spec/unit/controllers/v3/app_usage_consumers_controller_spec.rb create mode 100644 spec/unit/controllers/v3/service_usage_consumers_controller_spec.rb diff --git a/app/actions/app_usage_consumer_delete.rb b/app/actions/app_usage_consumer_delete.rb new file mode 100644 index 00000000000..f47567e1c13 --- /dev/null +++ b/app/actions/app_usage_consumer_delete.rb @@ -0,0 +1,9 @@ +module VCAP::CloudController + class AppUsageConsumerDelete + def delete(app_usage_consumer) + app_usage_consumer.destroy + rescue Sequel::Error => e + raise CloudController::Errors::ApiError.new_from_details('AppUsageConsumerDeleteError', e.message) + end + end +end diff --git a/app/actions/service_usage_consumer_delete.rb b/app/actions/service_usage_consumer_delete.rb new file mode 100644 index 00000000000..f0fdfe5c268 --- /dev/null +++ b/app/actions/service_usage_consumer_delete.rb @@ -0,0 +1,9 @@ +module VCAP::CloudController + class ServiceUsageConsumerDelete + def delete(service_usage_consumer) + service_usage_consumer.destroy + rescue Sequel::Error => e + raise CloudController::Errors::ApiError.new_from_details('ServiceUsageConsumerDeleteError', e.message) + end + end +end diff --git a/app/controllers/v3/app_usage_consumers_controller.rb b/app/controllers/v3/app_usage_consumers_controller.rb new file mode 100644 index 00000000000..96b29db8eb7 --- /dev/null +++ b/app/controllers/v3/app_usage_consumers_controller.rb @@ -0,0 +1,41 @@ +require 'presenters/v3/app_usage_consumer_presenter' +require 'messages/app_usage_consumers_list_message' +require 'fetchers/app_usage_consumer_list_fetcher' +require 'actions/app_usage_consumer_delete' + +class AppUsageConsumersController < ApplicationController + def index + message = AppUsageConsumersListMessage.from_params(query_params) + invalid_param!(message.errors.full_messages) unless message.valid? + + app_usage_consumers = AppUsageConsumer.where(guid: []) + + app_usage_consumers = AppUsageConsumerListFetcher.fetch_all(message, AppUsageConsumer.dataset) if permission_queryer.can_read_globally? + + render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( + presenter: Presenters::V3::AppUsageConsumerPresenter, + paginated_result: SequelPaginator.new.get_page(app_usage_consumers, message.try(:pagination_options)), + path: '/v3/app_usage_consumers', + message: message, + extra_presenter_args: {} + ) + end + + def show + app_usage_consumer = AppUsageConsumer.where(consumer_guid: params[:guid]).first + resource_not_found!(:app_usage_consumer) unless app_usage_consumer && permission_queryer.can_read_globally? + + render status: :ok, json: Presenters::V3::AppUsageConsumerPresenter.new(app_usage_consumer) + end + + def destroy + unauthorized! unless permission_queryer.can_write_globally? + + app_usage_consumer = AppUsageConsumer.where(consumer_guid: params[:guid]).first + resource_not_found!(:app_usage_consumer) unless app_usage_consumer + + AppUsageConsumerDelete.new.delete(app_usage_consumer) + + head :no_content + end +end diff --git a/app/controllers/v3/service_usage_consumers_controller.rb b/app/controllers/v3/service_usage_consumers_controller.rb new file mode 100644 index 00000000000..daa39d8f88e --- /dev/null +++ b/app/controllers/v3/service_usage_consumers_controller.rb @@ -0,0 +1,41 @@ +require 'presenters/v3/service_usage_consumer_presenter' +require 'messages/service_usage_consumers_list_message' +require 'fetchers/service_usage_consumer_list_fetcher' +require 'actions/service_usage_consumer_delete' + +class ServiceUsageConsumersController < ApplicationController + def index + message = ServiceUsageConsumersListMessage.from_params(query_params) + invalid_param!(message.errors.full_messages) unless message.valid? + + service_usage_consumers = ServiceUsageConsumer.where(guid: []) + + service_usage_consumers = ServiceUsageConsumerListFetcher.fetch_all(message, ServiceUsageConsumer.dataset) if permission_queryer.can_read_globally? + + render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( + presenter: Presenters::V3::ServiceUsageConsumerPresenter, + paginated_result: SequelPaginator.new.get_page(service_usage_consumers, message.try(:pagination_options)), + path: '/v3/service_usage_consumers', + message: message, + extra_presenter_args: {} + ) + end + + def show + service_usage_consumer = ServiceUsageConsumer.where(consumer_guid: params[:guid]).first + resource_not_found!(:service_usage_consumer) unless service_usage_consumer && permission_queryer.can_read_globally? + + render status: :ok, json: Presenters::V3::ServiceUsageConsumerPresenter.new(service_usage_consumer) + end + + def destroy + unauthorized! unless permission_queryer.can_write_globally? + + service_usage_consumer = ServiceUsageConsumer.where(consumer_guid: params[:guid]).first + resource_not_found!(:service_usage_consumer) unless service_usage_consumer + + ServiceUsageConsumerDelete.new.delete(service_usage_consumer) + + head :no_content + end +end diff --git a/app/fetchers/app_usage_consumer_list_fetcher.rb b/app/fetchers/app_usage_consumer_list_fetcher.rb new file mode 100644 index 00000000000..7f03c7165fb --- /dev/null +++ b/app/fetchers/app_usage_consumer_list_fetcher.rb @@ -0,0 +1,17 @@ +require 'fetchers/base_list_fetcher' + +module VCAP::CloudController + class AppUsageConsumerListFetcher < BaseListFetcher + class << self + def fetch_all(message, dataset) + dataset = filter(message, dataset, AppUsageConsumer) + + dataset = dataset.where(consumer_guid: message.consumer_guids) if message.requested?(:consumer_guids) + + dataset = dataset.where(last_processed_guid: message.last_processed_guids) if message.requested?(:last_processed_guids) + + dataset + end + end + end +end diff --git a/app/fetchers/service_usage_consumer_list_fetcher.rb b/app/fetchers/service_usage_consumer_list_fetcher.rb new file mode 100644 index 00000000000..19bde66e47d --- /dev/null +++ b/app/fetchers/service_usage_consumer_list_fetcher.rb @@ -0,0 +1,17 @@ +require 'fetchers/base_list_fetcher' + +module VCAP::CloudController + class ServiceUsageConsumerListFetcher < BaseListFetcher + class << self + def fetch_all(message, dataset) + dataset = filter(message, dataset, ServiceUsageConsumer) + + dataset = dataset.where(consumer_guid: message.consumer_guids) if message.requested?(:consumer_guids) + + dataset = dataset.where(last_processed_guid: message.last_processed_guids) if message.requested?(:last_processed_guids) + + dataset + end + end + end +end diff --git a/app/messages/app_usage_consumers_list_message.rb b/app/messages/app_usage_consumers_list_message.rb new file mode 100644 index 00000000000..643b39e9411 --- /dev/null +++ b/app/messages/app_usage_consumers_list_message.rb @@ -0,0 +1,14 @@ +require 'messages/list_message' + +module VCAP::CloudController + class AppUsageConsumersListMessage < ListMessage + register_allowed_keys %i[consumer_guids last_processed_guids] + + validates :consumer_guids, array: true, allow_nil: true + validates :last_processed_guids, array: true, allow_nil: true + + def self.from_params(params) + super(params, %w[consumer_guids last_processed_guids]) + end + end +end diff --git a/app/messages/service_usage_consumers_list_message.rb b/app/messages/service_usage_consumers_list_message.rb new file mode 100644 index 00000000000..a865fc92d1a --- /dev/null +++ b/app/messages/service_usage_consumers_list_message.rb @@ -0,0 +1,14 @@ +require 'messages/list_message' + +module VCAP::CloudController + class ServiceUsageConsumersListMessage < ListMessage + register_allowed_keys %i[consumer_guids last_processed_guids] + + validates :consumer_guids, array: true, allow_nil: true + validates :last_processed_guids, array: true, allow_nil: true + + def self.from_params(params) + super(params, %w[consumer_guids last_processed_guids]) + end + end +end diff --git a/app/presenters/v3/app_usage_consumer_presenter.rb b/app/presenters/v3/app_usage_consumer_presenter.rb new file mode 100644 index 00000000000..4aab165c2eb --- /dev/null +++ b/app/presenters/v3/app_usage_consumer_presenter.rb @@ -0,0 +1,29 @@ +require 'presenters/v3/base_presenter' + +module VCAP::CloudController::Presenters::V3 + class AppUsageConsumerPresenter < BasePresenter + def to_hash + { + guid: app_usage_consumer.consumer_guid, + last_processed_guid: app_usage_consumer.last_processed_guid, + created_at: app_usage_consumer.created_at, + updated_at: app_usage_consumer.updated_at, + links: build_links + } + end + + private + + def app_usage_consumer + @resource + end + + def build_links + { + self: { + href: url_builder.build_url(path: "/v3/app_usage_consumers/#{app_usage_consumer.consumer_guid}") + } + } + end + end +end diff --git a/app/presenters/v3/service_usage_consumer_presenter.rb b/app/presenters/v3/service_usage_consumer_presenter.rb new file mode 100644 index 00000000000..274db392fc1 --- /dev/null +++ b/app/presenters/v3/service_usage_consumer_presenter.rb @@ -0,0 +1,29 @@ +require 'presenters/v3/base_presenter' + +module VCAP::CloudController::Presenters::V3 + class ServiceUsageConsumerPresenter < BasePresenter + def to_hash + { + guid: service_usage_consumer.consumer_guid, + last_processed_guid: service_usage_consumer.last_processed_guid, + created_at: service_usage_consumer.created_at, + updated_at: service_usage_consumer.updated_at, + links: build_links + } + end + + private + + def service_usage_consumer + @resource + end + + def build_links + { + self: { + href: url_builder.build_url(path: "/v3/service_usage_consumers/#{service_usage_consumer.consumer_guid}") + } + } + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 5994216e73b..b63ede64964 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -323,15 +323,25 @@ get '/audit_events/:guid', to: 'events#show' # app usage events - get '/app_usage_events/:guid', to: 'app_usage_events#show' get '/app_usage_events', to: 'app_usage_events#index' + get '/app_usage_events/:guid', to: 'app_usage_events#show' post '/app_usage_events/actions/destructively_purge_all_and_reseed', to: 'app_usage_events#destructively_purge_all_and_reseed' # service usage events - get '/service_usage_events/:guid', to: 'service_usage_events#show' get '/service_usage_events', to: 'service_usage_events#index' + get '/service_usage_events/:guid', to: 'service_usage_events#show' post '/service_usage_events/actions/destructively_purge_all_and_reseed', to: 'service_usage_events#destructively_purge_all_and_reseed' + # app usage consumers + get '/app_usage_consumers', to: 'app_usage_consumers#index' + get '/app_usage_consumers/:guid', to: 'app_usage_consumers#show' + delete '/app_usage_consumers/:guid', to: 'app_usage_consumers#destroy' + + # service usage consumers + get '/service_usage_consumers', to: 'service_usage_consumers#index' + get '/service_usage_consumers/:guid', to: 'service_usage_consumers#show' + delete '/service_usage_consumers/:guid', to: 'service_usage_consumers#destroy' + # environment variable groups get '/environment_variable_groups/:name', to: 'environment_variable_groups#show' patch '/environment_variable_groups/:name', to: 'environment_variable_groups#update' diff --git a/spec/unit/controllers/v3/app_usage_consumers_controller_spec.rb b/spec/unit/controllers/v3/app_usage_consumers_controller_spec.rb new file mode 100644 index 00000000000..eecbfce7887 --- /dev/null +++ b/spec/unit/controllers/v3/app_usage_consumers_controller_spec.rb @@ -0,0 +1,156 @@ +require 'rails_helper' +require 'permissions_spec_helper' + +RSpec.describe AppUsageConsumersController, type: :controller do + describe '#index' do + let(:user) { VCAP::CloudController::User.make } + let!(:app_usage_consumer_1) do + event = VCAP::CloudController::AppUsageEvent.make + VCAP::CloudController::AppUsageConsumer.make( + consumer_guid: 'consumer-1', + last_processed_guid: event.guid + ) + end + let!(:app_usage_consumer_2) do + event = VCAP::CloudController::AppUsageEvent.make + VCAP::CloudController::AppUsageConsumer.make( + consumer_guid: 'consumer-2', + last_processed_guid: event.guid + ) + end + + before do + set_current_user(user) + end + + context 'when the user is not an admin' do + it 'returns an empty list' do + get :index + + expect(response.status).to eq(200) + expect(parsed_body['resources']).to be_empty + end + end + + context 'when the user is an admin' do + before do + set_current_user_as_admin(user:) + end + + it 'returns all app usage consumers' do + get :index + + expect(response.status).to eq(200) + expect(parsed_body['resources'].length).to eq(2) + expect(parsed_body['resources'][0]['guid']).to eq('consumer-1') + expect(parsed_body['resources'][1]['guid']).to eq('consumer-2') + end + + context 'when filtering by consumer_guids' do + it 'returns filtered consumers' do + get :index, params: { consumer_guids: 'consumer-1' } + + expect(response.status).to eq(200) + expect(parsed_body['resources'].length).to eq(1) + expect(parsed_body['resources'][0]['guid']).to eq('consumer-1') + end + end + + context 'when filtering by last_processed_guids' do + it 'returns filtered consumers' do + get :index, params: { last_processed_guids: app_usage_consumer_1.last_processed_guid } + + expect(response.status).to eq(200) + expect(parsed_body['resources'].length).to eq(1) + expect(parsed_body['resources'][0]['guid']).to eq('consumer-1') + end + end + end + end + + describe '#show' do + let(:user) { VCAP::CloudController::User.make } + let!(:app_usage_consumer) do + event = VCAP::CloudController::AppUsageEvent.make + VCAP::CloudController::AppUsageConsumer.make( + consumer_guid: 'consumer-1', + last_processed_guid: event.guid + ) + end + + before do + set_current_user(user) + end + + context 'when the user is not an admin' do + it 'returns 404' do + get :show, params: { guid: app_usage_consumer.consumer_guid } + + expect(response.status).to eq(404) + end + end + + context 'when the user is an admin' do + before do + set_current_user_as_admin(user:) + end + + it 'returns the requested app usage consumer' do + get :show, params: { guid: app_usage_consumer.consumer_guid } + + expect(response.status).to eq(200) + expect(parsed_body['guid']).to eq(app_usage_consumer.consumer_guid) + expect(parsed_body['last_processed_guid']).to eq(app_usage_consumer.last_processed_guid) + end + + it 'returns 404 when the consumer does not exist' do + get :show, params: { guid: 'nonexistent-guid' } + + expect(response.status).to eq(404) + end + end + end + + describe '#destroy' do + let(:user) { VCAP::CloudController::User.make } + let!(:app_usage_consumer) do + event = VCAP::CloudController::AppUsageEvent.make + VCAP::CloudController::AppUsageConsumer.make( + consumer_guid: 'consumer-1', + last_processed_guid: event.guid + ) + end + + before do + set_current_user(user) + end + + context 'when the user is not an admin' do + it 'returns 403' do + delete :destroy, params: { guid: app_usage_consumer.consumer_guid } + + expect(response.status).to eq(403) + end + end + + context 'when the user is an admin' do + before do + set_current_user_as_admin(user:) + end + + it 'deletes the app usage consumer' do + expect do + delete :destroy, params: { guid: app_usage_consumer.consumer_guid } + end.to change(VCAP::CloudController::AppUsageConsumer, :count).by(-1) + + expect(response.status).to eq(204) + end + + it 'returns 404 when the consumer does not exist' do + delete :destroy, params: { guid: 'nonexistent-guid' } + + expect(response.status).to eq(404) + end + end + end +end diff --git a/spec/unit/controllers/v3/service_usage_consumers_controller_spec.rb b/spec/unit/controllers/v3/service_usage_consumers_controller_spec.rb new file mode 100644 index 00000000000..842e1992bdd --- /dev/null +++ b/spec/unit/controllers/v3/service_usage_consumers_controller_spec.rb @@ -0,0 +1,156 @@ +require 'rails_helper' +require 'permissions_spec_helper' + +RSpec.describe ServiceUsageConsumersController, type: :controller do + describe '#index' do + let(:user) { VCAP::CloudController::User.make } + let!(:service_usage_consumer_1) do + event = VCAP::CloudController::ServiceUsageEvent.make + VCAP::CloudController::ServiceUsageConsumer.make( + consumer_guid: 'consumer-1', + last_processed_guid: event.guid + ) + end + let!(:service_usage_consumer_2) do + event = VCAP::CloudController::ServiceUsageEvent.make + VCAP::CloudController::ServiceUsageConsumer.make( + consumer_guid: 'consumer-2', + last_processed_guid: event.guid + ) + end + + before do + set_current_user(user) + end + + context 'when the user is not an admin' do + it 'returns an empty list' do + get :index + + expect(response.status).to eq(200) + expect(parsed_body['resources']).to be_empty + end + end + + context 'when the user is an admin' do + before do + set_current_user_as_admin(user:) + end + + it 'returns all service usage consumers' do + get :index + + expect(response.status).to eq(200) + expect(parsed_body['resources'].length).to eq(2) + expect(parsed_body['resources'][0]['guid']).to eq('consumer-1') + expect(parsed_body['resources'][1]['guid']).to eq('consumer-2') + end + + context 'when filtering by consumer_guids' do + it 'returns filtered consumers' do + get :index, params: { consumer_guids: 'consumer-1' } + + expect(response.status).to eq(200) + expect(parsed_body['resources'].length).to eq(1) + expect(parsed_body['resources'][0]['guid']).to eq('consumer-1') + end + end + + context 'when filtering by last_processed_guids' do + it 'returns filtered consumers' do + get :index, params: { last_processed_guids: service_usage_consumer_1.last_processed_guid } + + expect(response.status).to eq(200) + expect(parsed_body['resources'].length).to eq(1) + expect(parsed_body['resources'][0]['guid']).to eq('consumer-1') + end + end + end + end + + describe '#show' do + let(:user) { VCAP::CloudController::User.make } + let!(:service_usage_consumer) do + event = VCAP::CloudController::ServiceUsageEvent.make + VCAP::CloudController::ServiceUsageConsumer.make( + consumer_guid: 'consumer-1', + last_processed_guid: event.guid + ) + end + + before do + set_current_user(user) + end + + context 'when the user is not an admin' do + it 'returns 404' do + get :show, params: { guid: service_usage_consumer.consumer_guid } + + expect(response.status).to eq(404) + end + end + + context 'when the user is an admin' do + before do + set_current_user_as_admin(user:) + end + + it 'returns the requested service usage consumer' do + get :show, params: { guid: service_usage_consumer.consumer_guid } + + expect(response.status).to eq(200) + expect(parsed_body['guid']).to eq(service_usage_consumer.consumer_guid) + expect(parsed_body['last_processed_guid']).to eq(service_usage_consumer.last_processed_guid) + end + + it 'returns 404 when the consumer does not exist' do + get :show, params: { guid: 'nonexistent-guid' } + + expect(response.status).to eq(404) + end + end + end + + describe '#destroy' do + let(:user) { VCAP::CloudController::User.make } + let!(:service_usage_consumer) do + event = VCAP::CloudController::ServiceUsageEvent.make + VCAP::CloudController::ServiceUsageConsumer.make( + consumer_guid: 'consumer-1', + last_processed_guid: event.guid + ) + end + + before do + set_current_user(user) + end + + context 'when the user is not an admin' do + it 'returns 403' do + delete :destroy, params: { guid: service_usage_consumer.consumer_guid } + + expect(response.status).to eq(403) + end + end + + context 'when the user is an admin' do + before do + set_current_user_as_admin(user:) + end + + it 'deletes the service usage consumer' do + expect do + delete :destroy, params: { guid: service_usage_consumer.consumer_guid } + end.to change(VCAP::CloudController::ServiceUsageConsumer, :count).by(-1) + + expect(response.status).to eq(204) + end + + it 'returns 404 when the consumer does not exist' do + delete :destroy, params: { guid: 'nonexistent-guid' } + + expect(response.status).to eq(404) + end + end + end +end From 6b6e4dd782ca31601e798766f812b0b4cd2134d2 Mon Sep 17 00:00:00 2001 From: David Riddle Date: Mon, 3 Feb 2025 06:49:48 -0600 Subject: [PATCH 6/8] typo --- lib/tasks/db.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 8cbdb229c9b..4af8f77e22f 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -38,7 +38,7 @@ namespace :db do migrate end - desc 'Make up to 5 attempts to connect to the database. Succeed it one is successful, and fail otherwise.' + desc 'Make up to 5 attempts to connect to the database. Succeed if one is successful, and fail otherwise.' task connect: :environment do RakeConfig.context = :migrate From 9cb58c31a94e61bab2f47404f4feb7b5369a1486 Mon Sep 17 00:00:00 2001 From: David Riddle Date: Mon, 3 Feb 2025 12:32:28 -0600 Subject: [PATCH 7/8] Seed Usage Event records Useful for integration testing. May only be needed while validating Usage Consumer feature. --- db/seeds/usage_events.rb | 197 ++++++++++++++++++++++++++++++++++++ lib/tasks/usage_events.rake | 16 +++ 2 files changed, 213 insertions(+) create mode 100644 db/seeds/usage_events.rb create mode 100644 lib/tasks/usage_events.rake diff --git a/db/seeds/usage_events.rb b/db/seeds/usage_events.rb new file mode 100644 index 00000000000..f9c5683959a --- /dev/null +++ b/db/seeds/usage_events.rb @@ -0,0 +1,197 @@ +require_relative '../../spec/support/fakes/blueprints' + +module VCAP::CloudController + FIXED_ORGS = [ + { name: 'sales-org', guid: Sham.guid }, + { name: 'engineering-org', guid: Sham.guid }, + { name: 'marketing-org', guid: Sham.guid } + ].freeze + + FIXED_SPACES = FIXED_ORGS.map do |org| + [ + { name: "#{org[:name]}-dev", guid: Sham.guid, org: org }, + { name: "#{org[:name]}-staging", guid: Sham.guid, org: org }, + { name: "#{org[:name]}-prod", guid: Sham.guid, org: org } + ] + end.flatten + + BUILDPACKS = { + 'ruby_buildpack' => Sham.guid, + 'nodejs_buildpack' => Sham.guid, + 'go_buildpack' => Sham.guid + }.freeze + + SERVICE_PLANS = { + 'small' => Sham.guid, + 'medium' => Sham.guid, + 'large' => Sham.guid, + 'premium' => Sham.guid + }.freeze + + SERVICE_BROKERS = { + 'aws-service-broker' => Sham.guid, + 'gcp-service-broker' => Sham.guid, + 'azure-service-broker' => Sham.guid + }.freeze + + SERVICES = { + 'postgresql' => Sham.guid, + 'redis' => Sham.guid, + 'rabbitmq' => Sham.guid, + 'mongodb' => Sham.guid + }.freeze + + SERVICE_INSTANCE_TYPES = %w[ + managed_service_instance + user_provided_service_instance + ].freeze + + SERVICE_INSTANCE_NAMES = { + 'postgresql' => %w[users-db orders-db inventory-db], + 'redis' => %w[session-cache api-cache queue-cache], + 'rabbitmq' => %w[event-queue worker-queue notification-queue], + 'mongodb' => %w[analytics-store metrics-store logs-store] + }.freeze + + APP_NAMES = { + 'frontend-ui' => Sham.guid, + 'backend-api' => Sham.guid, + 'auth-service' => Sham.guid, + 'payment-processor' => Sham.guid, + 'notification-service' => Sham.guid, + 'user-service' => Sham.guid, + 'order-service' => Sham.guid, + 'inventory-service' => Sham.guid, + 'search-service' => Sham.guid, + 'analytics-service' => Sham.guid + }.freeze + + CURRENT_TIME = Time.new(2025, 1, 10, 9, 0, 0) + THREE_DAYS_AGO = CURRENT_TIME - 3.days + TWO_YEARS_AGO = CURRENT_TIME - (2 * 365 * 24 * 3600) + + def self.generate_stop_time(start_time) + max_possible_hours = ((CURRENT_TIME - start_time) / 3600).floor + min_hours = 20 + max_hours = [600, max_possible_hours].min + + hours_to_add = if max_possible_hours < min_hours + max_possible_hours + else + rand(min_hours..max_hours) + end + + start_time + hours_to_add.hours + end + + app_events_to_create = [] + + 100.times do + space = FIXED_SPACES.sample + buildpack_name = BUILDPACKS.keys.sample + app_name = APP_NAMES.keys.sample + app_guid = Sham.guid + + common_app_attrs = { + memory_in_mb_per_instance: [128, 256, 512, 1024].sample, + previous_memory_in_mb_per_instance: nil, + instance_count: rand(1..10), + previous_instance_count: nil, + process_type: 'web', + parent_app_guid: APP_NAMES[app_name], + parent_app_name: app_name, + app_guid: app_guid, + app_name: app_name, + space_name: space[:name], + space_guid: space[:guid], + org_guid: space[:org][:guid], + buildpack_guid: BUILDPACKS[buildpack_name], + buildpack_name: buildpack_name, + package_state: 'STAGED', + previous_package_state: nil, + task_guid: nil, + task_name: nil, + previous_state: nil + } + + start_time = Time.at(rand(TWO_YEARS_AGO.to_i..THREE_DAYS_AGO.to_i)) + stop_time = generate_stop_time(start_time) + + started_event = { + attributes: common_app_attrs.merge( + state: 'STARTED', + created_at: start_time + ), + created_at: start_time + } + + stopped_event = { + attributes: common_app_attrs.merge( + state: 'STOPPED', + previous_state: 'STARTED', + created_at: stop_time + ), + created_at: stop_time + } + + app_events_to_create.push(started_event, stopped_event) + end + + service_events_to_create = [] + + 100.times do + space = FIXED_SPACES.sample + service_plan_name = SERVICE_PLANS.keys.sample + service_broker_name = SERVICE_BROKERS.keys.sample + service_label = SERVICES.keys.sample + service_instance_type = SERVICE_INSTANCE_TYPES.sample + service_instance_name = SERVICE_INSTANCE_NAMES[service_label].sample + service_instance_guid = Sham.guid + + common_service_attrs = { + space_name: space[:name], + space_guid: space[:guid], + org_guid: space[:org][:guid], + service_instance_guid: service_instance_guid, + service_instance_name: service_instance_name, + service_instance_type: service_instance_type, + service_plan_guid: SERVICE_PLANS[service_plan_name], + service_plan_name: service_plan_name, + service_guid: SERVICES[service_label], + service_label: service_label, + service_broker_guid: SERVICE_BROKERS[service_broker_name], + service_broker_name: service_broker_name + } + + creation_time = Time.at(rand(TWO_YEARS_AGO.to_i..THREE_DAYS_AGO.to_i)) + deletion_time = generate_stop_time(creation_time) + + created_event = { + attributes: common_service_attrs.merge( + state: 'CREATED', + created_at: creation_time + ), + created_at: creation_time + } + + deleted_event = { + attributes: common_service_attrs.merge( + state: 'DELETED', + created_at: deletion_time + ), + created_at: deletion_time + } + + service_events_to_create.push(created_event, deleted_event) + end + + events = (app_events_to_create + service_events_to_create).sort_by { |e| e[:created_at] } + + events.each do |event| + if event[:attributes][:service_instance_guid] + ServiceUsageEvent.make(**event[:attributes]) + else + AppUsageEvent.make(**event[:attributes]) + end + end +end diff --git a/lib/tasks/usage_events.rake b/lib/tasks/usage_events.rake new file mode 100644 index 00000000000..b7c01176bfd --- /dev/null +++ b/lib/tasks/usage_events.rake @@ -0,0 +1,16 @@ +namespace :db do + desc 'Seed usage events' + task seed_usage_events: :environment do + $LOAD_PATH.unshift(File.expand_path('../../spec', __dir__)) + + require 'machinist/sequel' + require 'machinist/object' + require 'support/bootstrap/spec_bootstrap' + + # Initialize the test environment + VCAP::CloudController::SpecBootstrap.init + + require File.expand_path('../../db/seeds/usage_events', __dir__) + puts 'Created seed usage events' + end +end From 136cf8ac99296b60b6a9d1cf5e76e9bc8951bebe Mon Sep 17 00:00:00 2001 From: David Riddle Date: Tue, 27 May 2025 12:12:49 -0500 Subject: [PATCH 8/8] Make keep_unprocessed_records feature configurable Keep it off by default --- app/jobs/runtime/app_usage_events_cleanup.rb | 7 ++++--- app/jobs/v2/services/service_usage_events_cleanup.rb | 7 ++++--- app/repositories/app_usage_event_repository.rb | 5 +++-- app/repositories/service_usage_event_repository.rb | 4 ++-- config/bosh-lite.yml | 2 ++ config/cloud_controller.yml | 2 ++ lib/cloud_controller/clock/scheduler.rb | 6 ++++-- lib/cloud_controller/config_schemas/base/clock_schema.rb | 2 ++ spec/fixtures/config/port_8181_config.yml | 1 + spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb | 3 ++- .../jobs/services/service_usage_events_cleanup_spec.rb | 3 ++- spec/unit/lib/cloud_controller/clock/scheduler_spec.rb | 6 ++++-- spec/unit/repositories/app_usage_event_repository_spec.rb | 5 +++-- .../repositories/service_usage_event_repository_spec.rb | 5 +++-- 14 files changed, 38 insertions(+), 20 deletions(-) diff --git a/app/jobs/runtime/app_usage_events_cleanup.rb b/app/jobs/runtime/app_usage_events_cleanup.rb index 9a5d1b520cd..62af2e8090f 100644 --- a/app/jobs/runtime/app_usage_events_cleanup.rb +++ b/app/jobs/runtime/app_usage_events_cleanup.rb @@ -4,10 +4,11 @@ module VCAP::CloudController module Jobs module Runtime class AppUsageEventsCleanup < VCAP::CloudController::Jobs::CCJob - attr_accessor :cutoff_age_in_days, :threshold_for_keeping_unprocessed_records + attr_accessor :cutoff_age_in_days, :keep_unprocessed_records, :threshold_for_keeping_unprocessed_records - def initialize(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + def initialize(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) @cutoff_age_in_days = cutoff_age_in_days + @keep_unprocessed_records = keep_unprocessed_records @threshold_for_keeping_unprocessed_records = threshold_for_keeping_unprocessed_records end @@ -16,7 +17,7 @@ def perform logger.info('Cleaning up old AppUsageEvent rows') repository = Repositories::AppUsageEventRepository.new - repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + repository.delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end def job_name_in_configuration diff --git a/app/jobs/v2/services/service_usage_events_cleanup.rb b/app/jobs/v2/services/service_usage_events_cleanup.rb index 16e01476a83..4a42b3663f8 100644 --- a/app/jobs/v2/services/service_usage_events_cleanup.rb +++ b/app/jobs/v2/services/service_usage_events_cleanup.rb @@ -4,10 +4,11 @@ module VCAP::CloudController module Jobs module Services class ServiceUsageEventsCleanup < VCAP::CloudController::Jobs::CCJob - attr_accessor :cutoff_age_in_days, :threshold_for_keeping_unprocessed_records + attr_accessor :cutoff_age_in_days, :keep_unprocessed_records, :threshold_for_keeping_unprocessed_records - def initialize(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + def initialize(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) @cutoff_age_in_days = cutoff_age_in_days + @keep_unprocessed_records = keep_unprocessed_records @threshold_for_keeping_unprocessed_records = threshold_for_keeping_unprocessed_records end @@ -16,7 +17,7 @@ def perform logger.info('Cleaning up old ServiceUsageEvent rows') repository = Repositories::ServiceUsageEventRepository.new - repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + repository.delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end def job_name_in_configuration diff --git a/app/repositories/app_usage_event_repository.rb b/app/repositories/app_usage_event_repository.rb index d1a6886237d..931c01bf23f 100644 --- a/app/repositories/app_usage_event_repository.rb +++ b/app/repositories/app_usage_event_repository.rb @@ -151,9 +151,10 @@ def purge_and_reseed_started_apps! AppUsageEvent.insert(column_map.keys, usage_query) end - def delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + def delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) Database::OldRecordCleanup.new(AppUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true, - keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: threshold_for_keeping_unprocessed_records).delete + keep_unprocessed_records: keep_unprocessed_records, + threshold_for_keeping_unprocessed_records: threshold_for_keeping_unprocessed_records).delete end private diff --git a/app/repositories/service_usage_event_repository.rb b/app/repositories/service_usage_event_repository.rb index bf50e36957b..c9c85612ab5 100644 --- a/app/repositories/service_usage_event_repository.rb +++ b/app/repositories/service_usage_event_repository.rb @@ -91,9 +91,9 @@ def purge_and_reseed_service_instances! ServiceUsageEvent.insert(column_map.keys, usage_query) end - def delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + def delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) Database::OldRecordCleanup.new(ServiceUsageEvent, cutoff_age_in_days: cutoff_age_in_days, keep_at_least_one_record: true, keep_running_records: true, - keep_unprocessed_records: true, + keep_unprocessed_records: keep_unprocessed_records, threshold_for_keeping_unprocessed_records: threshold_for_keeping_unprocessed_records).delete end end diff --git a/config/bosh-lite.yml b/config/bosh-lite.yml index d79c60ad69e..d42db1c2e15 100644 --- a/config/bosh-lite.yml +++ b/config/bosh-lite.yml @@ -21,10 +21,12 @@ jobs: app_usage_events: cutoff_age_in_days: 31 + keep_unprocessed_records: false threshold_for_keeping_unprocessed_records: 5000000 service_usage_events: cutoff_age_in_days: 31 + keep_unprocessed_records: false threshold_for_keeping_unprocessed_records: 5000000 audit_events: diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index d5f33f4f728..d517fcf3e22 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -38,10 +38,12 @@ jobs: app_usage_events: cutoff_age_in_days: 31 + keep_unprocessed_records: false threshold_for_keeping_unprocessed_records: 5000000 service_usage_events: cutoff_age_in_days: 31 + keep_unprocessed_records: false threshold_for_keeping_unprocessed_records: 5000000 audit_events: diff --git a/lib/cloud_controller/clock/scheduler.rb b/lib/cloud_controller/clock/scheduler.rb index 460b9b8f6a5..2c539d20048 100644 --- a/lib/cloud_controller/clock/scheduler.rb +++ b/lib/cloud_controller/clock/scheduler.rb @@ -6,10 +6,12 @@ module VCAP::CloudController class Scheduler CLEANUPS = [ { name: 'app_usage_events', class: Jobs::Runtime::AppUsageEventsCleanup, time: '18:00', - arg_from_config: [%i[app_usage_events cutoff_age_in_days], %i[app_usage_events threshold_for_keeping_unprocessed_records]] }, + arg_from_config: [%i[app_usage_events cutoff_age_in_days], %i[app_usage_events keep_unprocessed_records], + %i[app_usage_events threshold_for_keeping_unprocessed_records]] }, { name: 'audit_events', class: Jobs::Runtime::EventsCleanup, time: '20:00', arg_from_config: %i[audit_events cutoff_age_in_days] }, { name: 'service_usage_events', class: Jobs::Services::ServiceUsageEventsCleanup, time: '22:00', - arg_from_config: [%i[service_usage_events cutoff_age_in_days], %i[service_usage_events threshold_for_keeping_unprocessed_records]] }, + arg_from_config: [%i[service_usage_events cutoff_age_in_days], %i[service_usage_events keep_unprocessed_records], + %i[service_usage_events threshold_for_keeping_unprocessed_records]] }, { name: 'completed_tasks', class: Jobs::Runtime::PruneCompletedTasks, time: '23:00', arg_from_config: %i[completed_tasks cutoff_age_in_days] }, { name: 'expired_blob_cleanup', class: Jobs::Runtime::ExpiredBlobCleanup, time: '00:00' }, { name: 'expired_resource_cleanup', class: Jobs::Runtime::ExpiredResourceCleanup, time: '00:30' }, diff --git a/lib/cloud_controller/config_schemas/base/clock_schema.rb b/lib/cloud_controller/config_schemas/base/clock_schema.rb index d844bab4d91..db5218df040 100644 --- a/lib/cloud_controller/config_schemas/base/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/base/clock_schema.rb @@ -17,6 +17,7 @@ class ClockSchema < VCAP::Config }, app_usage_events: { cutoff_age_in_days: Integer, + keep_unprocessed_records: bool, threshold_for_keeping_unprocessed_records: Integer }, audit_events: { @@ -172,6 +173,7 @@ class ClockSchema < VCAP::Config service_usage_events: { cutoff_age_in_days: Integer, + keep_unprocessed_records: bool, threshold_for_keeping_unprocessed_records: Integer }, default_app_ssh_access: bool, diff --git a/spec/fixtures/config/port_8181_config.yml b/spec/fixtures/config/port_8181_config.yml index 1ee9578b6f9..7da2ecb0ab3 100644 --- a/spec/fixtures/config/port_8181_config.yml +++ b/spec/fixtures/config/port_8181_config.yml @@ -30,6 +30,7 @@ jobs: app_usage_events: cutoff_age_in_days: 31 + keep_unprocessed_records: true threshold_for_keeping_unprocessed_records: 5000000 audit_events: diff --git a/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb b/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb index c9afaeb0d07..51c068df26e 100644 --- a/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb +++ b/spec/unit/jobs/runtime/app_usage_events_cleanup_spec.rb @@ -4,13 +4,14 @@ module VCAP::CloudController module Jobs::Runtime RSpec.describe AppUsageEventsCleanup, job_context: :worker do let(:cutoff_age_in_days) { 30 } + let(:keep_unprocessed_records) { true } let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } let(:logger) { double(Steno::Logger, info: nil) } let!(:event_before_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago, state: 'STOPPED') } let!(:event_after_threshold) { AppUsageEvent.make(created_at: (cutoff_age_in_days - 1).days.ago) } subject(:job) do - AppUsageEventsCleanup.new(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + AppUsageEventsCleanup.new(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end before do diff --git a/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb b/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb index 908cbef4008..8edc5c6d879 100644 --- a/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb +++ b/spec/unit/jobs/services/service_usage_events_cleanup_spec.rb @@ -4,13 +4,14 @@ module VCAP::CloudController module Jobs::Services RSpec.describe ServiceUsageEventsCleanup, job_context: :worker do let(:cutoff_age_in_days) { 30 } + let(:keep_unprocessed_records) { true } let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } let(:logger) { double(Steno::Logger, info: nil) } let!(:event_before_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days + 1).days.ago, state: 'DELETED') } let!(:event_after_threshold) { ServiceUsageEvent.make(created_at: (cutoff_age_in_days - 1).days.ago) } subject(:job) do - ServiceUsageEventsCleanup.new(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + ServiceUsageEventsCleanup.new(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end before do diff --git a/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb b/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb index 0d4dd00670a..513e0342eb7 100644 --- a/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb +++ b/spec/unit/lib/cloud_controller/clock/scheduler_spec.rb @@ -18,6 +18,7 @@ module VCAP::CloudController }, app_usage_events: { cutoff_age_in_days: 1, + keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: 5_000_000 }, audit_events: { cutoff_age_in_days: 3 }, @@ -26,6 +27,7 @@ module VCAP::CloudController service_operations_initial_cleanup: { frequency_in_seconds: 600 }, service_usage_events: { cutoff_age_in_days: 5, + keep_unprocessed_records: true, threshold_for_keeping_unprocessed_records: 5_000_000 }, completed_tasks: { cutoff_age_in_days: 6 }, @@ -67,7 +69,7 @@ module VCAP::CloudController expect(clock).to receive(:schedule_daily_job) do |args, &block| expect(args).to eql(name: 'app_usage_events', at: '18:00', priority: 0) - expect(Jobs::Runtime::AppUsageEventsCleanup).to receive(:new).with(1, 5_000_000).and_call_original + expect(Jobs::Runtime::AppUsageEventsCleanup).to receive(:new).with(1, true, 5_000_000).and_call_original expect(block.call).to be_instance_of(Jobs::Runtime::AppUsageEventsCleanup) end @@ -79,7 +81,7 @@ module VCAP::CloudController expect(clock).to receive(:schedule_daily_job) do |args, &block| expect(args).to eql(name: 'service_usage_events', at: '22:00', priority: 0) - expect(Jobs::Services::ServiceUsageEventsCleanup).to receive(:new).with(5, 5_000_000).and_call_original + expect(Jobs::Services::ServiceUsageEventsCleanup).to receive(:new).with(5, true, 5_000_000).and_call_original expect(block.call).to be_instance_of(Jobs::Services::ServiceUsageEventsCleanup) end diff --git a/spec/unit/repositories/app_usage_event_repository_spec.rb b/spec/unit/repositories/app_usage_event_repository_spec.rb index 3f4e27c3854..b114a6d70e6 100644 --- a/spec/unit/repositories/app_usage_event_repository_spec.rb +++ b/spec/unit/repositories/app_usage_event_repository_spec.rb @@ -680,6 +680,7 @@ module Repositories describe '#delete_events_older_than' do let(:cutoff_age_in_days) { 1 } + let(:keep_unprocessed_records) { true } let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } before do @@ -699,7 +700,7 @@ module Repositories repository.create_from_process(process) expect do - repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + repository.delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end.to change(AppUsageEvent, :count).to(1) expect(AppUsageEvent.last).to match_app(process) @@ -707,7 +708,7 @@ module Repositories it 'keeps the last record even if before the cutoff age' do expect do - repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + repository.delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end.to change(AppUsageEvent, :count).to(1) expect(AppUsageEvent.last.created_at).to be < cutoff_age_in_days.days.ago diff --git a/spec/unit/repositories/service_usage_event_repository_spec.rb b/spec/unit/repositories/service_usage_event_repository_spec.rb index bb471999b68..6a99c2c536c 100644 --- a/spec/unit/repositories/service_usage_event_repository_spec.rb +++ b/spec/unit/repositories/service_usage_event_repository_spec.rb @@ -157,6 +157,7 @@ module Repositories describe '#delete_events_older_than' do let!(:service_instance) { ManagedServiceInstance.make } let(:cutoff_age_in_days) { 1 } + let(:keep_unprocessed_records) { true } let(:threshold_for_keeping_unprocessed_records) { 5_000_000 } before do @@ -175,7 +176,7 @@ module Repositories new_event = repository.create_from_service_instance(service_instance, 'SOME-STATE') expect do - repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + repository.delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end.to change(ServiceUsageEvent, :count).to(1) expect(ServiceUsageEvent.last).to eq(new_event.reload) @@ -183,7 +184,7 @@ module Repositories it 'keeps the last record even if before the cutoff age' do expect do - repository.delete_events_older_than(cutoff_age_in_days, threshold_for_keeping_unprocessed_records) + repository.delete_events_older_than(cutoff_age_in_days, keep_unprocessed_records, threshold_for_keeping_unprocessed_records) end.to change(ServiceUsageEvent, :count).to(1) expect(ServiceUsageEvent.last.created_at).to be < cutoff_age_in_days.days.ago