Fix loose_foreign_keys/cleanup_worker_spec with fully decomposed
What does this MR do and why?
To reproduce:
-
Configure multiple databases following https://docs.gitlab.com/ee/development/database/multiple_databases.html#development-setup
-
Configure fully decomposed with:
export GITLAB_USE_MODEL_LOAD_BALANCING=true
-
Run the spec:
date; bin/rspec spec/workers/loose_foreign_keys/cleanup_worker_spec.rb
On master
we will see the following failures.
- Time-based failures. If the spec runs on an odd minute, it will attempt to cleanup for the
ci:
database instead:
Failures:
1) LooseForeignKeys::CleanupWorker cleans up all rows
Failure/Error: expect(loose_fk_child_table_1_1.count).to eq(0)
expected: 0
got: 3
(compared using ==)
# ./spec/workers/loose_foreign_keys/cleanup_worker_spec.rb:108:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:412:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:399:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:45:in `with_raw_context'
# ./spec/spec_helper.rb:399:in `block (2 levels) in <top (required)>'
# ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'
2) LooseForeignKeys::CleanupWorker when deleting in batches cleans up all rows
Failure/Error: expect(loose_fk_child_table_1_1.count).to eq(0)
expected: 0
got: 3
(compared using ==)
# ./spec/workers/loose_foreign_keys/cleanup_worker_spec.rb:123:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:412:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:399:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:45:in `with_raw_context'
# ./spec/spec_helper.rb:399:in `block (2 levels) in <top (required)>'
# ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'
3) LooseForeignKeys::CleanupWorker when the deleted rows count limit have been reached cleans up 2 rows
Failure/Error: expect { described_class.new.perform }.to change { count_deletable_rows }.by(-2)
expected `count_deletable_rows` to have changed by -2, but was changed by 0
# ./spec/workers/loose_foreign_keys/cleanup_worker_spec.rb:140:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:412:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:399:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:45:in `with_raw_context'
# ./spec/spec_helper.rb:399:in `block (2 levels) in <top (required)>'
# ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'
- Failure because there's no
loose_foreign_keys_deleted_records
on theci:
database:
4) LooseForeignKeys::CleanupWorker multi-database support current_minute: 3, configured_base_models: {:main=>ApplicationRecord(abstract), :ci=>Ci::ApplicationRecord(abstract)}, expected_connection: #<Gitlab::Database::LoadBalancing::ConnectionProxy:0x00000001057acb40 @load_balancer=#<Gitlab::Database::LoadBalancing::LoadBalancer:0x00000001057ade00 @configuration=#<Gitlab::Database::LoadBalancing::Configuration:0x00000001057ae788 @max_replication_difference=8388608, @max_replication_lag_time=60.0, @replica_check_interval=60.0, @model=Ci::ApplicationRecord(abstract), @hosts=[], @service_discovery={:nameserver=>"localhost", :port=>8600, :record=>nil, :record_type=>"A", :interval=>60, :disconnect_timeout=>120, :use_tcp=>false}, @primary_model=nil>, @primary_only=true, @host_list=#<Gitlab::Database::LoadBalancing::HostList:0x00000001057add88 @hosts=[#<Gitlab::Database::LoadBalancing::PrimaryHost:0x00000001057addd8 @load_balancer=#<Gitlab::Database::LoadBalancing::LoadBalancer:0x00000001057ade00 ...>>], @index=0, @mutex=#<Thread::Mutex:0x00000001057add38>, @hosts_gauge=#<Prometheus::Client::Gauge:0x00000001057be2c8 @mutex=#<Thread::Mutex:0x00000001057be2a0>, @validator=#<Prometheus::Client::LabelSetValidator:0x00000001057be278 @reserved_labels=[:job, :instance], @validated={82852882984023558=>{}}>, @values={{}=>#<Prometheus::Client::MmapedValue:0x00000001057bddc8 @file_prefix="gauge_all", @metric_name=:db_load_balancing_hosts, @name=:db_load_balancing_hosts, @labels={}, @pid=93793, @mutex=#<Thread::Mutex:0x00000001057bdc60>, @file=#<Prometheus::Client::MmapedDict:0x78230>, @key="[\"db_load_balancing_hosts\",\"db_load_balancing_hosts\",[],[]]", @value=1>}, @name=:db_load_balancing_hosts, @docstring="Current number of load balancing hosts", @base_labels={}, @multiprocess_mode=:all>>>> uses the correct connection
Failure/Error:
target.to receive(:allocate).and_wrap_original do |method|
method.call.tap do |allocation|
# ActiveRecord::Core.allocate returns a frozen object:
# https://github.com/rails/rails/blob/291a3d2ef29a3842d1156ada7526f4ee60dd2b59/activerecord/lib/active_record/core.rb#L620
# It's unexpected behavior and probably a bug in Rails
# Let's work it around by setting the attributes to default to unfreeze the object for now
allocation.instance_variable_set(:@attributes, klass._default_attributes)
yield(allocation)
end
(LooseForeignKeys::DeletedRecord(id: integer, primary_key_value: integer, status: integer, created_at: datetime_with_timezone, fully_qualified_table_name: text, consume_after: datetime_with_timezone, cleanup_attempts: integer) (class)).allocate(*(any args))
expected: 1 time with any arguments
received: 0 times with any arguments
# ./spec/support/helpers/next_found_instance_of.rb:29:in `stub_allocate'
# ./spec/spec_helper.rb:410:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:401:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:397:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:45:in `with_raw_context'
# ./spec/spec_helper.rb:397:in `block (2 levels) in <top (required)>'
# ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'
Finished in 10.23 seconds (files took 9.41 seconds to load)
7 examples, 4 failures
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Thong Kuah