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

Loading