Skip to content

Fix loose_foreign_keys/cleanup_worker_spec with fully decomposed

What does this MR do and why?

To reproduce:

  1. Configure multiple databases following https://docs.gitlab.com/ee/development/database/multiple_databases.html#development-setup

  2. Configure fully decomposed with:

    export GITLAB_USE_MODEL_LOAD_BALANCING=true
  3. Run the spec:

    date; bin/rspec spec/workers/loose_foreign_keys/cleanup_worker_spec.rb

On master we will see the following failures.

  1. 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>'

  1. Failure because there's no loose_foreign_keys_deleted_records on the ci: 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.

Edited by Thong Kuah

Merge request reports