Skip to content

Update MultipleDatabase cop to allow some methods

Patrick Bair requested to merge pb-fix-multiple-database-false-positives into master

What does this MR do and why?

Related issue: #350191 (closed)

Update the Database/MultipleDatabases cop to have an allowlist of methods that are safe to call on ActiveRecord::Base. For now there is only one method added: no_touching, which is safe to call even with multiple databases configured.

Also remove offending files from the rubocop todo list based on their usage of no_touching being considered safe.

How to set up and validate locally

We can verify that ActiveRecord::Base.no_touching is safe to use with multiple databases.

  1. Setup multiple databases according to https://docs.gitlab.com/ee/development/database/multiple_databases.html#development-setup
  2. Start a rails console with separate connections for main and ci:
    GITLAB_USE_MODEL_LOAD_BALANCING=true rails c
  3. First, we can try the behavior without no_touching. If you run the below, you'll see update queries for each object to set the updated_at:
    Project.first.touch; Ci::Pipeline.first.touch
  4. Now try with no_touching, and it will load each object but no longer run the update queries:
    ActiveRecord::Base.no_touching { Project.first.touch; Ci::Pipeline.first.touch }
  5. We can also try with no_touching only on a specific base model. The following will only apply no_touching to the pipeline model:
    Ci::ApplicationRecord.no_touching { Project.first.touch; Ci::Pipeline.first.touch }

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