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
This commit is contained in:
commit
8249fdcd47
|
@ -24,6 +24,7 @@ v 8.9.0 (unreleased)
|
|||
- Reduce number of fog gem dependencies
|
||||
- Remove project notification settings associated with deleted projects
|
||||
- Fix 404 page when viewing TODOs that contain milestones or labels in different projects
|
||||
- Add a metric for the number of new Redis connections created by a transaction
|
||||
- Redesign navigation for project pages
|
||||
- Fix groups API to list only user's accessible projects
|
||||
- Redesign account and email confirmation emails
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -340,7 +340,7 @@ describe 'Git HTTP requests', lib: true do
|
|||
end
|
||||
end
|
||||
|
||||
context "when the file exists" do
|
||||
context "when the file does not exist" do
|
||||
before { get "/#{project.path_with_namespace}/blob/master/info/refs" }
|
||||
|
||||
it "returns not found" do
|
||||
|
|
Loading…
Reference in New Issue