summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2019-08-26 14:57:59 +0100
committerSean McGivern <sean@gitlab.com>2019-08-27 14:15:25 +0100
commit7f102819a56b55607e657447b51d2eeb45b2fe94 (patch)
treecd0cbf4d2c45d7beabc2663c00d1b00f5fc83cdc
parent7671c592f826f44be5a8a7dc947fba467f5df851 (diff)
downloadgitlab-ce-7f102819a56b55607e657447b51d2eeb45b2fe94.tar.gz
Fix Peek on Puma
Peek's `Peek.request_id` method doesn't work well with a multi-threaded server and concurrent requests, because requests can 'steal' another request's ID, or unset it before it was due. The upstream change resolves this; the commit here is just to ensure that GitLab works with that upstream change, mostly by not using `Peek.request_id` any more (as the method doesn't exist).
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock16
-rw-r--r--app/controllers/concerns/with_performance_bar.rb6
-rw-r--r--app/views/peek/_bar.html.haml2
-rw-r--r--changelogs/unreleased/fix-peek-on-puma.yml5
-rw-r--r--lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb4
6 files changed, 24 insertions, 11 deletions
diff --git a/Gemfile b/Gemfile
index 595dedd4656..0611ad2110f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -295,7 +295,7 @@ gem 'gettext', '~> 3.2.2', require: false, group: :development
gem 'batch-loader', '~> 1.4.0'
# Perf bar
-gem 'peek', '~> 1.0.1'
+gem 'peek', git: 'https://github.com/smcgivern/peek', branch: 'remove-peek-request-id'
# Snowplow events tracking
gem 'snowplow-tracker', '~> 0.6.1'
diff --git a/Gemfile.lock b/Gemfile.lock
index ae0d0e78c37..347969ef4d2 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,3 +1,11 @@
+GIT
+ remote: https://github.com/smcgivern/peek
+ revision: b7164ce54b63670f5406f4dc3cb3f5594d71bfaa
+ branch: remove-peek-request-id
+ specs:
+ peek (1.0.1)
+ railties (>= 4.0.0)
+
GEM
remote: https://rubygems.org/
specs:
@@ -147,8 +155,6 @@ GEM
adamantium (~> 0.2.0)
equalizer (~> 0.0.9)
concurrent-ruby (1.1.5)
- concurrent-ruby-ext (1.1.5)
- concurrent-ruby (= 1.1.5)
connection_pool (2.2.2)
contracts (0.11.0)
crack (0.4.3)
@@ -640,10 +646,6 @@ GEM
parser (2.6.3.0)
ast (~> 2.4.0)
parslet (1.8.2)
- peek (1.0.1)
- concurrent-ruby (>= 0.9.0)
- concurrent-ruby-ext (>= 0.9.0)
- railties (>= 4.0.0)
pg (1.1.4)
po_to_json (1.0.1)
json (>= 1.6.0)
@@ -1177,7 +1179,7 @@ DEPENDENCIES
omniauth_crowd (~> 2.2.0)
omniauth_openid_connect (~> 0.3.1)
org-ruby (~> 0.9.12)
- peek (~> 1.0.1)
+ peek!
pg (~> 1.1)
premailer-rails (~> 1.9.7)
prometheus-client-mmap (~> 0.9.8)
diff --git a/app/controllers/concerns/with_performance_bar.rb b/app/controllers/concerns/with_performance_bar.rb
index 4e0ae3c59eb..b19d6eb9439 100644
--- a/app/controllers/concerns/with_performance_bar.rb
+++ b/app/controllers/concerns/with_performance_bar.rb
@@ -3,6 +3,12 @@
module WithPerformanceBar
extend ActiveSupport::Concern
+ included do
+ before_action :peek_enabled? # Warm cache
+ end
+
+ protected
+
def peek_enabled?
return false unless Gitlab::PerformanceBar.enabled?(current_user)
diff --git a/app/views/peek/_bar.html.haml b/app/views/peek/_bar.html.haml
index 5228930293c..3c2c4d30535 100644
--- a/app/views/peek/_bar.html.haml
+++ b/app/views/peek/_bar.html.haml
@@ -1,6 +1,6 @@
- return unless peek_enabled?
#js-peek{ data: { env: Peek.env,
- request_id: Peek.request_id,
+ request_id: peek_request_id,
peek_url: "#{peek_routes_path}/results" },
class: Peek.env }
diff --git a/changelogs/unreleased/fix-peek-on-puma.yml b/changelogs/unreleased/fix-peek-on-puma.yml
new file mode 100644
index 00000000000..1071607b628
--- /dev/null
+++ b/changelogs/unreleased/fix-peek-on-puma.yml
@@ -0,0 +1,5 @@
+---
+title: Fix performance bar on Puma
+merge_request: 32213
+author:
+type: fixed
diff --git a/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb b/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb
index 2d997760c46..9595ced0177 100644
--- a/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb
+++ b/lib/gitlab/performance_bar/redis_adapter_when_peek_enabled.rb
@@ -4,8 +4,8 @@
module Gitlab
module PerformanceBar
module RedisAdapterWhenPeekEnabled
- def save
- super unless ::Peek.request_id.blank?
+ def save(request_id)
+ super if ::Peek.enabled? && request_id.present?
end
end
end