diff options
| author | Andrew Newdigate <andrew@gitlab.com> | 2019-06-17 22:20:49 +0200 |
|---|---|---|
| committer | Andrew Newdigate <andrew@gitlab.com> | 2019-06-20 23:08:36 +0200 |
| commit | 8f010583751fd354784b5ff9b4d6695e5b825197 (patch) | |
| tree | e3cd42530d9b8283026a37dafc9e5f04153b7c9e | |
| parent | 5a036d481f4d8ee131f2b858162e5937150b6569 (diff) | |
| download | gitlab-ce-an-disable-transactions-over-gitaly-calls.tar.gz | |
Disallow Gitaly calls when an active transaction is pendingan-disable-transactions-over-gitaly-calls
| -rw-r--r-- | app/models/merge_request.rb | 6 | ||||
| -rw-r--r-- | changelogs/unreleased/an-disable-transactions-over-gitaly-calls.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/shell.rb | 6 |
4 files changed, 15 insertions, 6 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index f07636e8f77..7f59d45aae8 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -667,7 +667,7 @@ class MergeRequest < ApplicationRecord fetch_ref! # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37435 - Gitlab::GitalyClient.allow_n_plus_1_calls do + Gitlab::GitalyClient.allow_n_plus_1_calls(override_transaction_safety: true) do merge_request_diffs.create! reload_merge_request_diff end @@ -880,10 +880,12 @@ class MergeRequest < ApplicationRecord return unless project.issues_enabled? return if closed? || merged? + issues_to_be_closed = closes_issues(current_user) + transaction do self.merge_requests_closing_issues.delete_all - closes_issues(current_user).each do |issue| + issues_to_be_closed.each do |issue| next if issue.is_a?(ExternalIssue) self.merge_requests_closing_issues.create!(issue: issue) diff --git a/changelogs/unreleased/an-disable-transactions-over-gitaly-calls.yml b/changelogs/unreleased/an-disable-transactions-over-gitaly-calls.yml new file mode 100644 index 00000000000..89a38316683 --- /dev/null +++ b/changelogs/unreleased/an-disable-transactions-over-gitaly-calls.yml @@ -0,0 +1,5 @@ +--- +title: Disallow Gitaly calls when an active transaction is pending +merge_request: 29774 +author: +type: other diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 47976389af6..5893739c1e1 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -285,7 +285,9 @@ module Gitlab end private_class_method :enforce_gitaly_request_limits? - def self.allow_n_plus_1_calls + def self.allow_n_plus_1_calls(override_transaction_safety: false) + raise "Allowing Gitaly n+1 calls from within an active transaction is not allowed." if !override_transaction_safety && Gitlab::Database.inside_transaction? + return yield unless Gitlab::SafeRequestStore.active? begin diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 93182607616..69fb8805e08 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -261,10 +261,10 @@ module Gitlab # add_namespace("default", "gitlab") # def add_namespace(storage, name) - # https://gitlab.com/gitlab-org/gitlab-ce/issues/58012 - Gitlab::GitalyClient.allow_n_plus_1_calls do + # # https://gitlab.com/gitlab-org/gitlab-ce/issues/58012 + # Gitlab::GitalyClient.allow_n_plus_1_calls do Gitlab::GitalyClient::NamespaceService.new(storage).add(name) - end + # end rescue GRPC::InvalidArgument => e raise ArgumentError, e.message end |