Resolve cross DB issues in ee/app/services/vulnerabilities/base_service.rb

Summary

The modification of vulnerabilities is usually done with a system note, this results in a cross join interaction between the Sec and Main DB's that we will need to remediate.

Further details

See temporary_ignore_tables_in_transaction in Vulnerabilities::BaseService#update_vulnerability_with:

    def update_vulnerability_with(params)
      Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction(
        %w[
          notes
          system_note_metadata
          vulnerability_user_mentions
          vulnerability_state_transitions
          vulnerability_feedback
          vulnerabilities
        ], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/482672'
      ) do
        @vulnerability.transaction do
          yield if block_given?

          update_with_note(params)
        end

        update_statistics
      end
    end

This is the base class for the following services:

ee/app/services/vulnerabilities/confirm_service.rb
ee/app/services/vulnerabilities/dismiss_service.rb
ee/app/services/vulnerabilities/resolve_service.rb
ee/app/services/vulnerabilities/revert_to_detected_service.rb

Command to run the corresponding specs:

bundle exec rspec ee/spec/services/vulnerabilities/{confirm,dismiss,resolve,revert_to_detected}_service_spec.rb

Right now these specs fail with a CrossDatabaseModificationAcrossUnsupportedTablesError when the temporary_ignore_tables_in_transaction is removed.

Proposal

The issue is caused by the notes (main DB) and vulnerabilities (sec DB) table being accessed in the same transaction.

Use run_after_commit_or_now to separate the queries from the Vulnerability transaction.

Refactor to perform update_with_note and update_statistics in run_after_commit_or_now:

vulnerability.run_after_commit_or_now do
  SystemNoteService.change_vulnerability_state(vulnerability.clone, user)
  Vulnerabilities::Statistics::UpdateService.update_for(vulnerability) if vulnerability.previous_changes.present?
end

There are test cases that assert if the above two statements were executed in the right order, so this proposed change preserves that order.

Edited by Arpit Gogia