Merge branch 'brodock/reference-counter' into 'master'

Improve reference counter

Closes #198568

See merge request gitlab-org/gitlab!23641

lib/gitlab/reference_counter.rb

 # frozen_string_literal: true
 
 module Gitlab
+  # Reference Counter
+  #
+  # A reference counter is used as a mechanism to identify when
+  # a repository is being accessed by a writable operation.
+  #
+  # Maintenance operations would use this as a clue to when it should
+  # execute significant changes in order to avoid disrupting running traffic
   class ReferenceCounter
     REFERENCE_EXPIRE_TIME = 600
 
     attr_reader :gl_repository, :key
 
+    # Reference Counter instance
+    #
+    # @example
+    #   Gitlab::ReferenceCounter.new('project-1')
+    #
+    # @see Gitlab::GlRepository::RepoType.identifier_for_repositorable
+    # @param [String] gl_repository repository identifier
     def initialize(gl_repository)
       @gl_repository = gl_repository
       @key = "git-receive-pack-reference-counter:#{gl_repository}"
     end
 
+    # Return the actual counter value
+    #
+    # @return [Integer] value
     def value
-      Gitlab::Redis::SharedState.with { |redis| (redis.get(key) || 0).to_i }
+      Gitlab::Redis::SharedState.with do |redis|
+        (redis.get(key) || 0).to_i
+      end
     end
 
+    # Increase the counter
+    #
+    # @return [Boolean] whether operation was a success
     def increase
       redis_cmd do |redis|
         redis.incr(key)
@@ -22,26 +44,51 @@ module Gitlab
       end
     end
 
-    # rubocop:disable Gitlab/RailsLogger
+    # Decrease the counter
+    #
+    # @return [Boolean] whether operation was a success
     def decrease
       redis_cmd do |redis|
         current_value = redis.decr(key)
         if current_value < 0
+          # rubocop:disable Gitlab/RailsLogger
           Rails.logger.warn("Reference counter for #{gl_repository} decreased" \
-            " when its value was less than 1. Reseting the counter.")
+            " when its value was less than 1. Resetting the counter.")
+          # rubocop:enable Gitlab/RailsLogger
           redis.del(key)
         end
       end
     end
-    # rubocop:enable Gitlab/RailsLogger
+
+    # Reset the reference counter
+    #
+    # @private Used internally by SRE and debugging purpose
+    # @return [Boolean] whether reset was a success
+    def reset!
+      redis_cmd do |redis|
+        redis.del(key)
+      end
+    end
+
+    # When the reference counter would expire
+    #
+    # @api private Used internally by SRE and debugging purpose
+    # @return [Integer] Number in seconds until expiration or false if never
+    def expires_in
+      Gitlab::Redis::SharedState.with do |redis|
+        redis.ttl(key)
+      end
+    end
 
     private
 
     def redis_cmd
       Gitlab::Redis::SharedState.with { |redis| yield(redis) }
+
       true
     rescue => e
       Rails.logger.warn("GitLab: An unexpected error occurred in writing to Redis: #{e}") # rubocop:disable Gitlab/RailsLogger
+
       false
     end
   end

spec/lib/gitlab/reference_counter_spec.rb

@@ -2,38 +2,54 @@
 
 require 'spec_helper'
 
-describe Gitlab::ReferenceCounter do
-  let(:redis) { double('redis') }
-  let(:reference_counter_key) { "git-receive-pack-reference-counter:project-1" }
+describe Gitlab::ReferenceCounter, :clean_gitlab_redis_shared_state do
   let(:reference_counter) { described_class.new('project-1') }
 
-  before do
-    allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis)
+  describe '#increase' do
+    it 'increases and sets the expire time of a reference count for a path' do
+      expect { reference_counter.increase }.to change { reference_counter.value }.by(1)
+      expect(reference_counter.expires_in).to be_positive
+      expect(reference_counter.increase).to be(true)
+    end
   end
 
-  it 'increases and set the expire time of a reference count for a path' do
-    expect(redis).to receive(:incr).with(reference_counter_key)
-    expect(redis).to receive(:expire).with(reference_counter_key,
-        described_class::REFERENCE_EXPIRE_TIME)
-    expect(reference_counter.increase).to be(true)
+  describe '#decrease' do
+    it 'decreases the reference count for a path' do
+      reference_counter.increase
+
+      expect { reference_counter.decrease }.to change { reference_counter.value }.by(-1)
+    end
+
+    it 'warns if attempting to decrease a counter with a value of zero or less, and resets the counter' do
+      expect(Rails.logger).to receive(:warn).with("Reference counter for project-1" \
+        " decreased when its value was less than 1. Resetting the counter.")
+      expect { reference_counter.decrease }.not_to change { reference_counter.value }
+    end
   end
 
-  it 'decreases the reference count for a path' do
-    allow(redis).to receive(:decr).and_return(0)
-    expect(redis).to receive(:decr).with(reference_counter_key)
-    expect(reference_counter.decrease).to be(true)
+  describe '#value' do
+    it 'get the reference count for a path' do
+      expect(reference_counter.value).to eq(0)
+
+      reference_counter.increase
+
+      expect(reference_counter.value).to eq(1)
+    end
   end
 
-  it 'warns if attempting to decrease a counter with a value of one or less, and resets the counter' do
-    expect(redis).to receive(:decr).and_return(-1)
-    expect(redis).to receive(:del)
-    expect(Rails.logger).to receive(:warn).with("Reference counter for project-1" \
-        " decreased when its value was less than 1. Reseting the counter.")
-    expect(reference_counter.decrease).to be(true)
+  describe '#reset!' do
+    it 'resets reference count down to zero' do
+      3.times { reference_counter.increase }
+
+      expect { reference_counter.reset! }.to change { reference_counter.value}.from(3).to(0)
+    end
   end
 
-  it 'get the reference count for a path' do
-    allow(redis).to receive(:get).and_return(1)
-    expect(reference_counter.value).to be(1)
+  describe '#expires_in' do
+    it 'displays the expiration time in seconds' do
+      reference_counter.increase
+
+      expect(reference_counter.expires_in).to be_between(500, 600)
+    end
   end
 end