diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-15 09:56:42 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-06-15 09:56:42 +0000 |
commit | 8249fdcd47b33c71e5f9908c0ac877487070e39d (patch) | |
tree | feb1ce2b2c6dbb23126f47c042014ef69e9c73e4 /config/initializers | |
parent | 870470730c92593d8904c7c3621902fa853e5d7c (diff) | |
parent | d07426ac198af72538663e7acc36f8c8bfe8de28 (diff) | |
download | gitlab-ce-8249fdcd47b33c71e5f9908c0ac877487070e39d.tar.gz |
Merge branch '18451-track-new-redis-connections' into 'master'
Resolve "Track the number of new Redis connections per transaction"
## What does this MR do?
Add a new metric counter, `new_redis_connections`, that contains the number of calls to `Redis::Client#connect` in the current transaction.
## Are there points in the code the reviewer needs to double check?
Not sure. I tested this in kind of a brute-force way:
1. Add a debugger in the monkey-patched `connect` method.
2. With metrics enabled, start the app and load a page.
3. The first Redis connection is created by `Rack::Attack` and isn't in a transaction, but still works fine.
4. The second Redis connection is within a transaction (the page load), and increments the counter.
5. If I reload the page, neither debugger is hit.
6. If I use a Redis client and do `CLIENT KILL` on my two existing clients, then reload the page, I get 3 and 4 again.
7. If I disable metrics collection, the debugger never gets hit.
## Why was this MR needed?
We may have a Redis connection leak somewhere, so adding metrics will let us track this.
## What are the relevant issue numbers?
Closes #18451.
## Screenshots (if relevant)
Hahaha nope, not relevant.
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- [ ] Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
cc @yorickpeterse
See merge request !4649
Diffstat (limited to 'config/initializers')
-rw-r--r-- | config/initializers/metrics.rb | 16 |
1 files changed, 16 insertions, 0 deletions
diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index 989404c6a61..d159f4eded2 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -133,4 +133,20 @@ if Gitlab::Metrics.enabled? GC::Profiler.enable Gitlab::Metrics::Sampler.new.start + + module TrackNewRedisConnections + def connect(*args) + val = super + + if current_transaction = Gitlab::Metrics::Transaction.current + current_transaction.increment(:new_redis_connections, 1) + end + + val + end + end + + class ::Redis::Client + prepend TrackNewRedisConnections + end end |