Resolve cross DB issues in ee/app/services/system_notes/vulnerabilities_service.rb
Summary
Creation of a vulnerability system note in this service relies on a single transaction which attempts to create both system notes and vulnerability_user_mentions that will be separated due to the decomposition and will need to be remediated.
Further details
module SystemNotes
class VulnerabilitiesService < ::SystemNotes::BaseService
# Called when state is changed for 'vulnerability'
# Message is established based on the logic relating to the
# vulnerability state enum and the current state.
# If no state transition is present, we assume the vulnerability
# is newly detected.
def change_vulnerability_state(body = nil)
Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.temporary_ignore_tables_in_transaction(
%w[
notes
system_note_metadata
vulnerability_user_mentions
], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/482743'
) do
body ||= state_change_body
create_note(NoteSummary.new(noteable, project, author, body, action: "vulnerability_#{to_state}"))
end
endSQL queries in transaction, from first failing spec:
INSERT INTO "notes" ("note", "noteable_type", "author_id", "created_at", "updated_at", "project_id", "noteable_id", "system", "discussion_id", "note_html", "cached_markdown_version", "namespace_id") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING "id" /*application:test,correlation_id:5e9e2947c3e048558bd49332110f8c65,db_config_database:gitlabhq_test,db_config_name:main,line:/app/services/system_notes/base_service.rb:22:in `create_note'*/
INSERT INTO "system_note_metadata" ("action", "created_at", "updated_at", "note_id") VALUES ($1, $2, $3, $4) RETURNING "id" /*application:test,correlation_id:5e9e2947c3e048558bd49332110f8c65,db_config_database:gitlabhq_test,db_config_name:main,line:/app/services/system_notes/base_service.rb:22:in `create_note'*/
DELETE FROM "vulnerability_user_mentions" WHERE "vulnerability_user_mentions"."vulnerability_id" = $1 AND "vulnerability_user_mentions"."note_id" = $2 /*application:test,correlation_id:5e9e2947c3e048558bd49332110f8c65,db_config_database:gitlabhq_test_sec,db_config_name:sec,line:/app/models/concerns/cache_markdown_field.rb:168:in `store_mentions!'*/vulnerability_user_mentions records are deleted by CacheMarkdownField#store_mentions!
if references.compact.any?
user_mention_class.upsert(references.merge(identifier), unique_by: identifier.compact.keys)
else
user_mention_class.delete_by(identifier)
endIt seems that upsert is never called when running the specs, but delete_by is called.
Spec failure
The corresponding rspec file fails when temporary_ignore_tables_in_transaction is removed.
rspec output
$ bundle exec rspec ee/spec/services/system_notes/vulnerabilities_service_spec.rb
Test environment set up in 1.254166209 seconds
F.F.F.F.F.F.FF.
Failures:
1) SystemNotes::VulnerabilitiesService#change_vulnerability_state when no state transition is present creates the note text correctly
Failure/Error: raise CrossDatabaseModificationAcrossUnsupportedTablesError, messages.join("\n\n")
Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError:
Cross-database data modification of 'gitlab_sec, gitlab_main_cell, gitlab_main' were detected within a transaction modifying the 'vulnerability_user_mentions, notes, system_note_metadata' tables.
Please refer to https://docs.gitlab.com/ee/development/database/multiple_databases.html#removing-cross-database-transactions for details on how to resolve this exception.
0: INSERT INTO "notes" ("note", "noteable_type", "author_id", "created_at", "updated_at", "project_id", "noteable_id", "system", "discussion_id", "note_html", "cached_markdown_version", "namespace_id") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING "id" /*application:test,correlation_id:5e9e2947c3e048558bd49332110f8c65,db_config_database:gitlabhq_test,db_config_name:main,line:/app/services/system_notes/base_service.rb:22:in `create_note'*/
1: INSERT INTO "system_note_metadata" ("action", "created_at", "updated_at", "note_id") VALUES ($1, $2, $3, $4) RETURNING "id" /*application:test,correlation_id:5e9e2947c3e048558bd49332110f8c65,db_config_database:gitlabhq_test,db_config_name:main,line:/app/services/system_notes/base_service.rb:22:in `create_note'*/
2: DELETE FROM "vulnerability_user_mentions" WHERE "vulnerability_user_mentions"."vulnerability_id" = $1 AND "vulnerability_user_mentions"."note_id" = $2 /*application:test,correlation_id:5e9e2947c3e048558bd49332110f8c65,db_config_database:gitlabhq_test_sec,db_config_name:sec,line:/app/models/concerns/cache_markdown_field.rb:168:in `store_mentions!'*/
# ./lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb:127:in `analyze'
# ./lib/gitlab/database/query_analyzer.rb:134:in `block in process_sql'
# ./lib/gitlab/database/query_analyzer.rb:131:in `each'
# ./lib/gitlab/database/query_analyzer.rb:131:in `process_sql'
# ./lib/gitlab/database/query_analyzer.rb:74:in `block (2 levels) in hook!'
# ./lib/gitlab/database/query_analyzer.rb:146:in `with_ignored_recursive_calls'
# ./lib/gitlab/database/query_analyzer.rb:73:in `block in hook!'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `public_send'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `block in write_using_load_balancer'
# ./lib/gitlab/database/load_balancing/load_balancer.rb:141:in `block in read_write'
# ./lib/gitlab/database/load_balancing/load_balancer.rb:228:in `retry_with_backoff'
# ./lib/gitlab/database/load_balancing/load_balancer.rb:130:in `read_write'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:126:in `write_using_load_balancer'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:61:in `block (2 levels) in <class:ConnectionProxy>'
# ./app/models/concerns/cache_markdown_field.rb:168:in `store_mentions!'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `public_send'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `block in write_using_load_balancer'
# ./lib/gitlab/database/load_balancing/load_balancer.rb:141:in `block in read_write'
# ./lib/gitlab/database/load_balancing/load_balancer.rb:228:in `retry_with_backoff'
# ./lib/gitlab/database/load_balancing/load_balancer.rb:130:in `read_write'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:126:in `write_using_load_balancer'
# ./lib/gitlab/database/load_balancing/connection_proxy.rb:78:in `transaction'
# ./app/services/system_notes/base_service.rb:22:in `create_note'
# ./ee/app/services/system_notes/vulnerabilities_service.rb:13:in `change_vulnerability_state'
# ./ee/spec/services/system_notes/vulnerabilities_service_spec.rb:17:in `block (4 levels) in <top (required)>'
# ./ee/spec/services/system_notes/vulnerabilities_service_spec.rb:24:in `block (4 levels) in <top (required)>'
# ./spec/spec_helper.rb:474:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
# ./spec/spec_helper.rb:473:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:468:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:459:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:455:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:94:in `with_raw_context'
# ./spec/spec_helper.rb:455:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:426:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/ci/config/feature_flags.rb:38:in `ensure_correct_usage'
# ./spec/spec_helper.rb:425:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:275:in `block (2 levels) in <top (required)>'
# ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
# ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
# ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>Proposal
Remove temporary_ignore_tables_in_transaction after doing Resolve cross DB issue in CacheMarkdownField (v... (#507439 - closed).
Edited by Fabien Catteau