Detect and log cross-database modifications in production
What does this MR do and why?
This adds a Rack middleware and Sidekiq middleware for detecting "Cross-database modification" violations. These occur when you query 2 different databases in the context of a single transaction. Previously we only wrapped our RSpec code with this detection logic but we found that it was often causing lots of false positives in RSpec (in test-only code) which made it hard to sort through the real issues. In addition we want extra validation by running this in production and therefore detecting code that our specs may be missing.
The middlewares use a feature flag detect_cross_database_modification
which we intend to enable for a small percentage of time.
The middlewares can also be disabled by setting the env var
DISABLE_CROSS_DATABASE_MODIFICATION_DETECTION=true
and restarting the
application. This environment variable is an extra safety net if the
middlewares are causing problems.
Screenshots or screen recordings
$ tail -f log/exceptions_json.log
{"severity":"ERROR","time":"2021-11-02T06:13:08.284Z","correlation_id":"01FKFMF9SFB8YGZ9ERYZEA1ZQ7","exception.class":"Gitlab::Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcros
sUnsupportedTablesError","exception.message":"Cross-database data modification of 'gitlab_main, gitlab_ci' were detected within a transaction modifying the 'uploads, routes, namespaces, keys, project_a
uthorizations, events, releases, ci_builds, ci_pipelines' 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.","exception.backtrace":["lib/gitlab/database/prevent_cross_database_modification.rb:116:in `prevent_cross_database_modification!'","lib/gitlab/database/prevent_cross_
database_modification.rb:24:in `block in with_cross_database_modification_prevented'","app/services/users/destroy_service.rb:68:in `execute'","ee/app/services/ee/users/destroy_service.rb:10:in `execute'","app/workers/delete_user_worker.rb:17:in `perform'","lib/gitlab/sidekiq_middleware/detect_cross_database_modification.rb:11:in `block in call'","lib/gitlab/database/prevent_cross_database_modification.rb:30:in `with_cross_database_modification_prevented'","lib/gitlab/sidekiq_middleware/detect_cross_database_modification.rb:10:in `call'","lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb:16:in `perform'","lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb:58:in `perform'","lib/gitlab/sidekiq_middleware/duplicate_jobs/server.rb:8:in `call'","lib/gitlab/sidekiq_middleware/worker_context.rb:9:in `wrap_in_optional_context'","lib/gitlab/sidekiq_middleware/worker_context/server.rb:17:in `block in call'","lib/gitlab/application_context.rb:74:in `block in use'","lib/gitlab/application_context.rb:74:in `use'","lib/gitlab/application_context.rb:27:in `with_context'","lib/gitlab/sidekiq_middleware/worker_context/server.rb:15:in `call'","lib/gitlab/sidekiq_status/server_middleware.rb:7:in `call'","lib/gitlab/sidekiq_versioning/middleware.rb:9:in `call'","lib/gitlab/sidekiq_middleware/admin_mode/server.rb:14:in `call'","lib/gitlab/sidekiq_middleware/instrumentation_logger.rb:9:in `call'","lib/gitlab/sidekiq_middleware/batch_loader.rb:7:in `call'","lib/gitlab/sidekiq_middleware/extra_done_log_metadata.rb:7:in `call'","lib/gitlab/sidekiq_middleware/request_store_middleware.rb:10:in `block in call'","lib/gitlab/with_request_store.rb:17:in `enabling_request_store'","lib/gitlab/with_request_store.rb:10:in `with_request_store'","lib/gitlab/sidekiq_middleware/request_store_middleware.rb:9:in `call'","lib/gitlab/sidekiq_middleware/server_metrics.rb:66:in `block in call'","lib/gitlab/sidekiq_middleware/server_metrics.rb:89:in `block in instrument'","lib/gitlab/metrics/background_transaction.rb:33:in `run'","lib/gitlab/sidekiq_middleware/server_metrics.rb:89:in `instrument'","lib/gitlab/sidekiq_middleware/server_metrics.rb:65:in `call'","lib/gitlab/sidekiq_middleware/monitor.rb:8:in `block in call'","lib/gitlab/sidekiq_daemon/monitor.rb:49:in `within_job'","lib/gitlab/sidekiq_middleware/monitor.rb:7:in `call'","lib/gitlab/sidekiq_middleware/size_limiter/server.rb:13:in `call'","lib/gitlab/sidekiq_logging/structured_logger.rb:19:in `call'"],"user.username":"root","tags.program":"sidekiq","tags.locale":"en","tags.feature_category":"authentication_and_authorization","tags.correlation_id":"01FKFMF9SFB8YGZ9ERYZEA1ZQ7","extra.sidekiq":{"retry":3,"queue":"default","backtrace":true,"version":0,"class":"DeleteUserWorker","args":["[FILTERED]","[FILTERED]","{\"hard_delete\"=\u003e\"true\"}"],"jid":"4d5f265653b9c51ae7884c5d","created_at":1635833587.9075613,"meta.user":"root","meta.caller_id":"Admin::UsersController#destroy","meta.remote_ip":"127.0.0.1","meta.feature_category":"authentication_and_authorization","meta.client_id":"user/1","correlation_id":"01FKFMF9SFB8YGZ9ERYZEA1ZQ7","worker_data_consistency":"always","idempotency_key":"resque:gitlab:duplicate:default:4a342bc1b7c3a9bed0dbe6366209705d403939742a0a49e59527ba126197923d","size_limiter":"validated","enqueued_at":1635833587.9133484},"extra.gitlab_schemas":["gitlab_main","gitlab_ci"],"extra.tables":["uploads","routes","namespaces","keys","project_authorizations","events","releases","ci_builds","ci_pipelines"],"extra.query":"UPDATE \"ci_pipelines\" SET \"user_id\" = NULL, \"lock_version\" = COALESCE(\"lock_version\", 0) + 1 WHERE \"ci_pipelines\".\"user_id\" = 66 /*application:sidekiq,correlation_id:01FKFMF9SFB8YGZ9ERYZEA1ZQ7,jid:4d5f265653b9c51ae7884c5d,endpoint_id:DeleteUserWorker,db_config_name:main,line:/home/dylan/.rvm/gems/ruby-2.7.4/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/"}
How to set up and validate locally
Feature.enable(:detect_cross_database_modification)
- Go to
Admin > Users
- Delete a user and contributions
- Check for an error in
log/exceptions_json.log
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #341783 (closed)