Commit d132f73d authored by Mayra Cabrera's avatar Mayra Cabrera Committed by Rémy Coutable

Implements lease_release on NamespaceAggregation

Sets lease_release? to false to prevent the job to be re-executed more
often than lease timeout

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64079
parent f62aa64b
......@@ -44,4 +44,12 @@ class Namespace::AggregationSchedule < ApplicationRecord
def lease_key
"namespace:namespaces_root_statistics:#{namespace_id}"
end
# Used by ExclusiveLeaseGuard
# Overriding value as we never release the lease
# before the timeout in order to prevent multiple
# RootStatisticsWorker to start in a short span of time
def lease_release?
false
end
end
......@@ -53,10 +53,39 @@ RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state,
expect(Namespaces::RootStatisticsWorker)
.to receive(:perform_in).once
.with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id )
.with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id)
aggregation_schedule.save!
end
it 'does not release the lease' do
stub_exclusive_lease(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT)
aggregation_schedule.save!
exclusive_lease = aggregation_schedule.exclusive_lease
expect(exclusive_lease.exists?).to be_truthy
end
it 'only executes the workers once' do
# Avoid automatic deletion of Namespace::AggregationSchedule
# for testing purposes.
expect(Namespaces::RootStatisticsWorker)
.to receive(:perform_async).once
.and_return(nil)
expect(Namespaces::RootStatisticsWorker)
.to receive(:perform_in).once
.with(described_class::DEFAULT_LEASE_TIMEOUT, aggregation_schedule.namespace_id)
.and_return(nil)
# Scheduling workers for the first time
aggregation_schedule.schedule_root_storage_statistics
# Executing again, this time workers should not be scheduled
# due to the lease not been released.
aggregation_schedule.schedule_root_storage_statistics
end
end
context 'with a personalized lease timeout' do
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
describe Namespaces::ScheduleAggregationWorker, '#perform' do
describe Namespaces::ScheduleAggregationWorker, '#perform', :clean_gitlab_redis_shared_state do
let(:group) { create(:group) }
subject(:worker) { described_class.new }
......@@ -10,6 +10,8 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do
context 'when group is the root ancestor' do
context 'when aggregation schedule exists' do
it 'does not create a new one' do
stub_aggregation_schedule_statistics
Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: group.id)
expect do
......@@ -18,26 +20,25 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do
end
end
context 'when update_statistics_namespace is off' do
it 'does not create a new one' do
stub_feature_flags(update_statistics_namespace: false, namespace: group)
context 'when aggregation schedule does not exist' do
it 'creates one' do
stub_aggregation_schedule_statistics
expect do
worker.perform(group.id)
end.not_to change(Namespace::AggregationSchedule, :count)
end.to change(Namespace::AggregationSchedule, :count).by(1)
expect(group.aggregation_schedule).to be_present
end
end
context 'when aggregation schedule does not exist' do
it 'creates one' do
allow_any_instance_of(Namespace::AggregationSchedule)
.to receive(:schedule_root_storage_statistics).and_return(nil)
context 'when update_statistics_namespace is off' do
it 'does not create a new one' do
stub_feature_flags(update_statistics_namespace: false, namespace: group)
expect do
worker.perform(group.id)
end.to change(Namespace::AggregationSchedule, :count).by(1)
expect(group.aggregation_schedule).to be_present
end.not_to change(Namespace::AggregationSchedule, :count)
end
end
end
......@@ -47,8 +48,7 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do
let(:group) { create(:group, parent: parent_group) }
it 'creates an aggregation schedule for the root' do
allow_any_instance_of(Namespace::AggregationSchedule)
.to receive(:schedule_root_storage_statistics).and_return(nil)
stub_aggregation_schedule_statistics
worker.perform(group.id)
......@@ -63,4 +63,15 @@ describe Namespaces::ScheduleAggregationWorker, '#perform' do
worker.perform(12345)
end
end
def stub_aggregation_schedule_statistics
# Namespace::Aggregations are deleted by
# Namespace::AggregationSchedule::schedule_root_storage_statistics,
# which is executed async. Stubing the service so instances are not deleted
# while still running the specs.
expect_next_instance_of(Namespace::AggregationSchedule) do |aggregation_schedule|
expect(aggregation_schedule)
.to receive(:schedule_root_storage_statistics)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment