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.