Commit fc0d2286 authored by Patrick Bajao's avatar Patrick Bajao

Support bulk reading markdown cache via pipeline

When rendering a collection with markdown cache in Redis, it can
only be done by making a request per object. That can result to
an N+1 requests to Redis cache.
parent 3dbea317
...@@ -22,16 +22,32 @@ module Gitlab ...@@ -22,16 +22,32 @@ module Gitlab
end end
end end
private prepended do
def self.preload_markdown_cache!(objects)
fields = Gitlab::MarkdownCache::Redis::Store.bulk_read(objects)
def save_markdown(updates) objects.each do |object|
markdown_store.save(updates) fields[object.cache_key].value.each do |field_name, value|
object.write_markdown_field(field_name, value)
end
end
end
end end
def write_markdown_field(field_name, value) def write_markdown_field(field_name, value)
# The value read from redis is a string, so we're converting it back
# to an int.
value = value.to_i if field_name == :cached_markdown_version
instance_variable_set("@#{field_name}", value) instance_variable_set("@#{field_name}", value)
end end
private
def save_markdown(updates)
markdown_store.save(updates)
end
def markdown_field_changed?(field_name) def markdown_field_changed?(field_name)
false false
end end
......
...@@ -6,6 +6,20 @@ module Gitlab ...@@ -6,6 +6,20 @@ module Gitlab
class Store class Store
EXPIRES_IN = 1.day EXPIRES_IN = 1.day
def self.bulk_read(subjects)
results = {}
Gitlab::Redis::Cache.with do |r|
r.pipelined do
subjects.each do |subject|
results[subject.cache_key] = new(subject).read
end
end
end
results
end
def initialize(subject) def initialize(subject)
@subject = subject @subject = subject
@loaded = false @loaded = false
...@@ -23,13 +37,9 @@ module Gitlab ...@@ -23,13 +37,9 @@ module Gitlab
def read def read
@loaded = true @loaded = true
results = Gitlab::Redis::Cache.with do |r| Gitlab::Redis::Cache.with do |r|
r.mapped_hmget(markdown_cache_key, *fields) r.mapped_hmget(markdown_cache_key, *fields)
end end
# The value read from redis is a string, so we're converting it back
# to an int.
results[:cached_markdown_version] = results[:cached_markdown_version].to_i
results
end end
def loaded? def loaded?
......
...@@ -49,6 +49,31 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Extension, :clean_gitlab_redis_cach ...@@ -49,6 +49,31 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Extension, :clean_gitlab_redis_cach
expect(thing.cached_markdown_version).to eq(cache_version) expect(thing.cached_markdown_version).to eq(cache_version)
end end
describe '.preload_markdown_cache!' do
before do
Gitlab::Redis::Cache.with do |r|
r.mapped_hmset(expected_cache_key,
title_html: 'hello',
description_html: 'world',
cached_markdown_version: cache_version)
end
end
it 'does not preload the markdown twice' do
expect(Gitlab::MarkdownCache::Redis::Store).to receive(:bulk_read).and_call_original
expect(Gitlab::Redis::Cache).to receive(:with).twice.and_call_original
klass.preload_markdown_cache!([thing])
aggregate_failures do
expect(Gitlab::Redis::Cache).not_to receive(:with)
expect(thing.title_html).to eq('hello')
expect(thing.description_html).to eq('world')
expect(thing.cached_markdown_version).to eq(cache_version)
end
end
end
describe "#refresh_markdown_cache!" do describe "#refresh_markdown_cache!" do
it "stores the value in redis" do it "stores the value in redis" do
expected_results = { "title_html" => "`Hello`", expected_results = { "title_html" => "`Hello`",
......
...@@ -37,6 +37,23 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Store, :clean_gitlab_redis_cache do ...@@ -37,6 +37,23 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Store, :clean_gitlab_redis_cache do
end end
end end
describe '.bulk_read' do
before do
store.save(field_1_html: "hello", field_2_html: "world", cached_markdown_version: 1)
end
it 'returns a hash of values from store' do
Gitlab::Redis::Cache.with do |redis|
expect(redis).to receive(:pipelined).and_call_original
end
results = described_class.bulk_read([storable])
expect(results[storable.cache_key].value.symbolize_keys)
.to eq(field_1_html: "hello", field_2_html: "world", cached_markdown_version: "1")
end
end
describe '#save' do describe '#save' do
it 'stores updates to html fields and version' do it 'stores updates to html fields and version' do
values_to_store = { field_1_html: "hello", field_2_html: "world", cached_markdown_version: 1 } values_to_store = { field_1_html: "hello", field_2_html: "world", cached_markdown_version: 1 }
...@@ -44,7 +61,7 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Store, :clean_gitlab_redis_cache do ...@@ -44,7 +61,7 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Store, :clean_gitlab_redis_cache do
store.save(values_to_store) store.save(values_to_store)
expect(read_values) expect(read_values)
.to eq({ field_1_html: "hello", field_2_html: "world", cached_markdown_version: "1" }) .to eq(field_1_html: "hello", field_2_html: "world", cached_markdown_version: "1")
end end
end end
...@@ -54,7 +71,8 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Store, :clean_gitlab_redis_cache do ...@@ -54,7 +71,8 @@ RSpec.describe Gitlab::MarkdownCache::Redis::Store, :clean_gitlab_redis_cache do
store_values(stored_values) store_values(stored_values)
expect(store.read.symbolize_keys).to eq(stored_values) expect(store.read.symbolize_keys)
.to eq(field_1_html: "hello", field_2_html: "world", cached_markdown_version: "1")
end end
it 'is mared loaded after reading' do it 'is mared loaded after reading' do
......
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