From ec5ceae623fceff0a959c7d297970d37285532dc Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 19 Jul 2019 10:01:09 -0700
Subject: Fix Gitaly auto-detection caching

If `GitalyClient#can_use_disk?` returned `false`, it was never cached
properly and led to excessive number of Gitaly calls. Instead of using
`cached_value.present?`, we need to check `cached_value.nil?`.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64802
---
 changelogs/unreleased/sh-fix-gitaly-server-info-cache.yml |  5 +++++
 lib/gitlab/gitaly_client.rb                               |  2 +-
 spec/lib/gitlab/gitaly_client_spec.rb                     | 13 +++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/sh-fix-gitaly-server-info-cache.yml

diff --git a/changelogs/unreleased/sh-fix-gitaly-server-info-cache.yml b/changelogs/unreleased/sh-fix-gitaly-server-info-cache.yml
new file mode 100644
index 00000000000..ef8f6e25856
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-gitaly-server-info-cache.yml
@@ -0,0 +1,5 @@
+---
+title: Fix Gitaly auto-detection caching
+merge_request: 30954
+author:
+type: performance
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index e7319a7b7f0..c98de722fe1 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -392,7 +392,7 @@ module Gitlab
         @can_use_disk[storage]
       end
 
-      return cached_value if cached_value.present?
+      return cached_value unless cached_value.nil?
 
       gitaly_filesystem_id = filesystem_id(storage)
       direct_filesystem_id = filesystem_id_from_disk(storage)
diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb
index b8debee3b58..e1d24ae8977 100644
--- a/spec/lib/gitlab/gitaly_client_spec.rb
+++ b/spec/lib/gitlab/gitaly_client_spec.rb
@@ -119,6 +119,19 @@ describe Gitlab::GitalyClient do
     end
   end
 
+  describe '.can_use_disk?' do
+    it 'properly caches a false result' do
+      # spec_helper stubs this globally
+      allow(described_class).to receive(:can_use_disk?).and_call_original
+      expect(described_class).to receive(:filesystem_id).once
+      expect(described_class).to receive(:filesystem_id_from_disk).once
+
+      2.times do
+        described_class.can_use_disk?('unknown')
+      end
+    end
+  end
+
   describe '.connection_data' do
     it 'returns connection data' do
       address = 'tcp://localhost:9876'
-- 
cgit v1.2.3