summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-01-15 16:21:04 +0100
committerYorick Peterse <yorickpeterse@gmail.com>2018-02-01 17:00:46 +0100
commitcca61980d5ad9c4db65b9498fe49d936657bc0e2 (patch)
tree1a844c4121c0530ea35f29d035e65a16be6e491a /lib
parent5b73e0eb35f5b9b78c228a4867ef78538ef05653 (diff)
downloadgitlab-ce-cca61980d5ad9c4db65b9498fe49d936657bc0e2.tar.gz
Track and act upon the number of executed queriesquery-counts
This ensures that we have more visibility in the number of SQL queries that are executed in web requests. The current threshold is hardcoded to 100 as we will rarely (maybe once or twice) change it. In production and development we use Sentry if enabled, in the test environment we raise an error. This feature is also only enabled in production/staging when running on GitLab.com as it's not very useful to other users.
Diffstat (limited to 'lib')
-rw-r--r--lib/api/branches.rb2
-rw-r--r--lib/api/issues.rb6
-rw-r--r--lib/api/merge_requests.rb6
-rw-r--r--lib/api/pipelines.rb2
-rw-r--r--lib/api/projects.rb2
-rw-r--r--lib/api/triggers.rb2
-rw-r--r--lib/api/users.rb2
-rw-r--r--lib/api/v3/branches.rb2
-rw-r--r--lib/api/v3/issues.rb6
-rw-r--r--lib/api/v3/merge_requests.rb4
-rw-r--r--lib/api/v3/pipelines.rb2
-rw-r--r--lib/api/v3/triggers.rb2
-rw-r--r--lib/gitlab/query_limiting.rb36
-rw-r--r--lib/gitlab/query_limiting/active_support_subscriber.rb11
-rw-r--r--lib/gitlab/query_limiting/middleware.rb55
-rw-r--r--lib/gitlab/query_limiting/transaction.rb83
16 files changed, 223 insertions, 0 deletions
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index 0791a110c39..1794207e29b 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -29,6 +29,8 @@ module API
use :pagination
end
get ':id/repository/branches' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42329')
+
repository = user_project.repository
branches = ::Kaminari.paginate_array(repository.branches.sort_by(&:name))
merged_branch_names = repository.merged_branch_names(branches.map(&:name))
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index c99fe3ab5b3..b6c278c89d0 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -161,6 +161,8 @@ module API
use :issue_params
end
post ':id/issues' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42320')
+
authorize! :create_issue, user_project
# Setting created_at time only allowed for admins and project owners
@@ -201,6 +203,8 @@ module API
:labels, :created_at, :due_date, :confidential, :state_event
end
put ':id/issues/:issue_iid' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42322')
+
issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
authorize! :update_issue, issue
@@ -234,6 +238,8 @@ module API
requires :to_project_id, type: Integer, desc: 'The ID of the new project'
end
post ':id/issues/:issue_iid/move' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42323')
+
issue = user_project.issues.find_by(iid: params[:issue_iid])
not_found!('Issue') unless issue
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 420aaf1c964..719afa09295 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -152,6 +152,8 @@ module API
use :optional_params
end
post ":id/merge_requests" do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42316')
+
authorize! :create_merge_request, user_project
mr_params = declared_params(include_missing: false)
@@ -256,6 +258,8 @@ module API
at_least_one_of(*at_least_one_of_ce)
end
put ':id/merge_requests/:merge_request_iid' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42318')
+
merge_request = find_merge_request_with_access(params.delete(:merge_request_iid), :update_merge_request)
mr_params = declared_params(include_missing: false)
@@ -283,6 +287,8 @@ module API
optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
end
put ':id/merge_requests/:merge_request_iid/merge' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42317')
+
merge_request = find_project_merge_request(params[:merge_request_iid])
merge_when_pipeline_succeeds = to_boolean(params[:merge_when_pipeline_succeeds])
diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb
index 675c963bae2..d2b8b832e4e 100644
--- a/lib/api/pipelines.rb
+++ b/lib/api/pipelines.rb
@@ -42,6 +42,8 @@ module API
requires :ref, type: String, desc: 'Reference'
end
post ':id/pipeline' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42124')
+
authorize! :create_pipeline, user_project
new_pipeline = Ci::CreatePipelineService.new(user_project,
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 8b5e4f8edcc..5b481121a10 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -210,6 +210,8 @@ module API
optional :namespace, type: String, desc: 'The ID or name of the namespace that the project will be forked into'
end
post ':id/fork' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42284')
+
fork_params = declared_params(include_missing: false)
namespace_id = fork_params[:namespace]
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index dd6801664b1..b3709455bc3 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -15,6 +15,8 @@ module API
optional :variables, type: Hash, desc: 'The list of variables to be injected into build'
end
post ":id/(ref/:ref/)trigger/pipeline", requirements: { ref: /.+/ } do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42283')
+
# validate variables
params[:variables] = params[:variables].to_h
unless params[:variables].all? { |key, value| key.is_a?(String) && value.is_a?(String) }
diff --git a/lib/api/users.rb b/lib/api/users.rb
index e5de31ad51b..c7c2aa280d5 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -383,6 +383,8 @@ module API
optional :hard_delete, type: Boolean, desc: "Whether to remove a user's contributions"
end
delete ":id" do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42279')
+
authenticated_as_admin!
user = User.find_by(id: params[:id])
diff --git a/lib/api/v3/branches.rb b/lib/api/v3/branches.rb
index b201bf77667..25176c5b38e 100644
--- a/lib/api/v3/branches.rb
+++ b/lib/api/v3/branches.rb
@@ -14,6 +14,8 @@ module API
success ::API::Entities::Branch
end
get ":id/repository/branches" do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42276')
+
repository = user_project.repository
branches = repository.branches.sort_by(&:name)
merged_branch_names = repository.merged_branch_names(branches.map(&:name))
diff --git a/lib/api/v3/issues.rb b/lib/api/v3/issues.rb
index cb371fdbab8..b59947d81d9 100644
--- a/lib/api/v3/issues.rb
+++ b/lib/api/v3/issues.rb
@@ -134,6 +134,8 @@ module API
use :issue_params
end
post ':id/issues' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42131')
+
# Setting created_at time only allowed for admins and project owners
unless current_user.admin? || user_project.owner == current_user
params.delete(:created_at)
@@ -169,6 +171,8 @@ module API
:labels, :created_at, :due_date, :confidential, :state_event
end
put ':id/issues/:issue_id' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42132')
+
issue = user_project.issues.find(params.delete(:issue_id))
authorize! :update_issue, issue
@@ -201,6 +205,8 @@ module API
requires :to_project_id, type: Integer, desc: 'The ID of the new project'
end
post ':id/issues/:issue_id/move' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42133')
+
issue = user_project.issues.find_by(id: params[:issue_id])
not_found!('Issue') unless issue
diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb
index 0a24fea52a3..ce216497996 100644
--- a/lib/api/v3/merge_requests.rb
+++ b/lib/api/v3/merge_requests.rb
@@ -91,6 +91,8 @@ module API
use :optional_params
end
post ":id/merge_requests" do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42126')
+
authorize! :create_merge_request, user_project
mr_params = declared_params(include_missing: false)
@@ -167,6 +169,8 @@ module API
:remove_source_branch
end
put path do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42127')
+
merge_request = find_merge_request_with_access(params.delete(:merge_request_id), :update_merge_request)
mr_params = declared_params(include_missing: false)
diff --git a/lib/api/v3/pipelines.rb b/lib/api/v3/pipelines.rb
index c48cbd2b765..6d31c12f572 100644
--- a/lib/api/v3/pipelines.rb
+++ b/lib/api/v3/pipelines.rb
@@ -19,6 +19,8 @@ module API
desc: 'Either running, branches, or tags'
end
get ':id/pipelines' do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42123')
+
authorize! :read_pipeline, user_project
pipelines = PipelinesFinder.new(user_project, scope: params[:scope]).execute
diff --git a/lib/api/v3/triggers.rb b/lib/api/v3/triggers.rb
index 534911fde5c..34f07dfb486 100644
--- a/lib/api/v3/triggers.rb
+++ b/lib/api/v3/triggers.rb
@@ -16,6 +16,8 @@ module API
optional :variables, type: Hash, desc: 'The list of variables to be injected into build'
end
post ":id/(ref/:ref/)trigger/builds", requirements: { ref: /.+/ } do
+ Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42121')
+
# validate variables
params[:variables] = params[:variables].to_h
unless params[:variables].all? { |key, value| key.is_a?(String) && value.is_a?(String) }
diff --git a/lib/gitlab/query_limiting.rb b/lib/gitlab/query_limiting.rb
new file mode 100644
index 00000000000..f64f1757144
--- /dev/null
+++ b/lib/gitlab/query_limiting.rb
@@ -0,0 +1,36 @@
+module Gitlab
+ module QueryLimiting
+ # Returns true if we should enable tracking of query counts.
+ #
+ # This is only enabled in production/staging if we're running on GitLab.com.
+ # This ensures we don't produce any errors that users can't do anything
+ # about themselves.
+ def self.enable?
+ Gitlab.com? || Rails.env.development? || Rails.env.test?
+ end
+
+ # Allows the current request to execute any number of SQL queries.
+ #
+ # This method should _only_ be used when there's a corresponding issue to
+ # reduce the number of queries.
+ #
+ # The issue URL is only meant to push developers into creating an issue
+ # instead of blindly whitelisting offending blocks of code.
+ def self.whitelist(issue_url)
+ return unless enable_whitelist?
+
+ unless issue_url.start_with?('https://')
+ raise(
+ ArgumentError,
+ 'You must provide a valid issue URL in order to whitelist a block of code'
+ )
+ end
+
+ Transaction&.current&.whitelisted = true
+ end
+
+ def self.enable_whitelist?
+ Rails.env.development? || Rails.env.test?
+ end
+ end
+end
diff --git a/lib/gitlab/query_limiting/active_support_subscriber.rb b/lib/gitlab/query_limiting/active_support_subscriber.rb
new file mode 100644
index 00000000000..66049c94ec6
--- /dev/null
+++ b/lib/gitlab/query_limiting/active_support_subscriber.rb
@@ -0,0 +1,11 @@
+module Gitlab
+ module QueryLimiting
+ class ActiveSupportSubscriber < ActiveSupport::Subscriber
+ attach_to :active_record
+
+ def sql(*)
+ Transaction.current&.increment
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/query_limiting/middleware.rb b/lib/gitlab/query_limiting/middleware.rb
new file mode 100644
index 00000000000..949ae79a047
--- /dev/null
+++ b/lib/gitlab/query_limiting/middleware.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module QueryLimiting
+ # Middleware for reporting (or raising) when a request performs more than a
+ # certain amount of database queries.
+ class Middleware
+ CONTROLLER_KEY = 'action_controller.instance'.freeze
+ ENDPOINT_KEY = 'api.endpoint'.freeze
+
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ transaction, retval = Transaction.run do
+ @app.call(env)
+ end
+
+ transaction.action = action_name(env)
+ transaction.act_upon_results
+
+ retval
+ end
+
+ def action_name(env)
+ if env[CONTROLLER_KEY]
+ action_for_rails(env)
+ elsif env[ENDPOINT_KEY]
+ action_for_grape(env)
+ end
+ end
+
+ private
+
+ def action_for_rails(env)
+ controller = env[CONTROLLER_KEY]
+ action = "#{controller.class.name}##{controller.action_name}"
+
+ if controller.content_type == 'text/html'
+ action
+ else
+ "#{action} (#{controller.content_type})"
+ end
+ end
+
+ def action_for_grape(env)
+ endpoint = env[ENDPOINT_KEY]
+ route = endpoint.route rescue nil
+
+ "#{route.request_method} #{route.path}" if route
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/query_limiting/transaction.rb b/lib/gitlab/query_limiting/transaction.rb
new file mode 100644
index 00000000000..3cbafadb0d0
--- /dev/null
+++ b/lib/gitlab/query_limiting/transaction.rb
@@ -0,0 +1,83 @@
+module Gitlab
+ module QueryLimiting
+ class Transaction
+ THREAD_KEY = :__gitlab_query_counts_transaction
+
+ attr_accessor :count, :whitelisted
+
+ # The name of the action (e.g. `UsersController#show`) that is being
+ # executed.
+ attr_accessor :action
+
+ # The maximum number of SQL queries that can be executed in a request. For
+ # the sake of keeping things simple we hardcode this value here, it's not
+ # supposed to be changed very often anyway.
+ THRESHOLD = 100
+
+ # Error that is raised whenever exceeding the maximum number of queries.
+ ThresholdExceededError = Class.new(StandardError)
+
+ def self.current
+ Thread.current[THREAD_KEY]
+ end
+
+ # Starts a new transaction and returns it and the blocks' return value.
+ #
+ # Example:
+ #
+ # transaction, retval = Transaction.run do
+ # 10
+ # end
+ #
+ # retval # => 10
+ def self.run
+ transaction = new
+ Thread.current[THREAD_KEY] = transaction
+
+ [transaction, yield]
+ ensure
+ Thread.current[THREAD_KEY] = nil
+ end
+
+ def initialize
+ @action = nil
+ @count = 0
+ @whitelisted = false
+ end
+
+ # Sends a notification based on the number of executed SQL queries.
+ def act_upon_results
+ return unless threshold_exceeded?
+
+ error = ThresholdExceededError.new(error_message)
+
+ if raise_error?
+ raise(error)
+ else
+ # Raven automatically logs to the Rails log if disabled, thus we don't
+ # need to manually log anything in case Sentry support is not enabled.
+ Raven.capture_exception(error)
+ end
+ end
+
+ def increment
+ @count += 1 unless whitelisted
+ end
+
+ def raise_error?
+ Rails.env.test?
+ end
+
+ def threshold_exceeded?
+ count > THRESHOLD
+ end
+
+ def error_message
+ header = 'Too many SQL queries were executed'
+ header += " in #{action}" if action
+
+ "#{header}: a maximum of #{THRESHOLD} is allowed but #{count} SQL queries were executed"
+ end
+ end
+ end
+end