Commit 80ebc43a authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Fix excessive Redis calls for new HSET diffs cache

We were doing multiple hmget calls when reading
the cache. This neutralizes the problem by using
a memoized cache.
parent 740dd848
......@@ -3,6 +3,8 @@
module Gitlab
module Diff
class HighlightCache
include Gitlab::Utils::StrongMemoize
EXPIRATION = 1.week
VERSION = 1
......@@ -30,12 +32,11 @@ module Gitlab
# IO generated by N+1's (1 writing for each highlighted line or file).
#
def write_if_empty
return if uncached_files.empty?
return if cacheable_files.empty?
new_cache_content = {}
uncached_files.each do |diff_file|
next unless cacheable?(diff_file)
cacheable_files.each do |diff_file|
new_cache_content[diff_file.file_path] = diff_file.highlighted_diff_lines.map(&:to_hash)
end
......@@ -49,7 +50,9 @@ module Gitlab
end
def key
@redis_key ||= ['highlighted-diff-files', diffable.cache_key, VERSION, diff_options].join(":")
strong_memoize(:redis_key) do
['highlighted-diff-files', diffable.cache_key, VERSION, diff_options].join(":")
end
end
private
......@@ -60,13 +63,17 @@ module Gitlab
# See https://gitlab.com/gitlab-org/gitlab/issues/38008
#
def deprecated_cache
@deprecated_cache ||= Gitlab::Diff::DeprecatedHighlightCache.new(@diff_collection)
strong_memoize(:deprecated_cache) do
Gitlab::Diff::DeprecatedHighlightCache.new(@diff_collection)
end
end
def uncached_files
def cacheable_files
strong_memoize(:cacheable_files) do
diff_files = @diff_collection.diff_files
diff_files.select { |file| read_cache[file.file_path].nil? }
diff_files.select { |file| cacheable?(file) && read_file(file).nil? }
end
end
# Given a hash of:
......@@ -95,13 +102,20 @@ module Gitlab
end
end
# Subsequent read_file calls would need the latest cache.
#
clear_memoization(:cached_content)
clear_memoization(:cacheable_files)
# Clean up any deprecated hash entries
#
deprecated_cache.clear
end
def file_paths
@file_paths ||= @diff_collection.diffs.collect(&:file_path)
strong_memoize(:file_paths) do
@diff_collection.diffs.collect(&:file_path)
end
end
def read_file(diff_file)
......@@ -109,7 +123,7 @@ module Gitlab
end
def cached_content
@cached_content ||= read_cache
strong_memoize(:cached_content) { read_cache }
end
def read_cache
......
......@@ -72,13 +72,22 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
describe '#write_if_empty' do
it 'filters the key/value list of entries to be caches for each invocation' do
paths = merge_request.diffs.diff_files.select(&:text?).map(&:file_path)
expect(cache).to receive(:write_to_redis_hash)
.once.with(hash_including(".gitignore")).and_call_original
expect(cache).to receive(:write_to_redis_hash).once.with({}).and_call_original
.with(hash_including(*paths))
.once
.and_call_original
2.times { cache.write_if_empty }
end
it 'reads from cache once' do
expect(cache).to receive(:read_cache).once.and_call_original
cache.write_if_empty
end
context 'different diff_collections for the same diffable' do
before do
cache.write_if_empty
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment