Skip to content

Remove defaulted connections for BBM

Patrick Bair requested to merge pb-remove-default-bbm-connections into master

What does this MR do and why?

Based on top of !83644 (merged), related to #357033 (closed)

Remove the defaults for connection: parameters at entry points through the batched background migration code. We used to set these defaults to ApplicationRecord.connection as part of the conversion to work with multiple databases, but we should now only use explicit connections.

As most of the callers of these methods appear to already pass the correct connection, so this should have no impact on behavior. The only place we weren't correctly passing the connection was in a development rake task, which I've fixed in this MR.

How to set up and validate locally

There's not a lot to validate since the callsites pass the correct connection information, but we can verify the workers properly run:

  1. Verify the main worker can talk to the database without a default connection: 
    pry(main)> Database::BatchedBackgroundMigrationWorker.new.perform
  Gitlab::Database::BackgroundMigration::BatchedMigration Load (0.3ms)  SELECT "batched_background_migrations".* FROM "batched_background_migrations" WHERE "batched_background_migrations"."status" = 1 ORDER BY "batched_background_migrations"."id" ASC LIMIT 1 /*application:console,db_config_name:main_replica,line:/lib/gitlab/database/background_migration/batched_migration.rb:45:in `active_migration'*/
=> nil
  2. Verify the ci worker can talk to the database without a default connection: 
    Feature.enable(:execute_batched_migrations_on_schedule_ci_database)
pry(main)> Database::BatchedBackgroundMigration::CiDatabaseWorker.new.perform
  Gitlab::Database::BackgroundMigration::BatchedMigration Load (1.1ms)  SELECT "batched_background_migrations".* FROM "batched_background_migrations" WHERE "batched_background_migrations"."status" = 1 ORDER BY "batched_background_migrations"."id" ASC LIMIT 1 /*application:console,db_config_name:main,line:/lib/gitlab/database/background_migration/batched_migration.rb:45:in `active_migration'*/
=> nil

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 Patrick Bair

Merge request reports