Commit c1b23b10 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch...

Merge branch '218745-single-issue-api-performance-regression-degraded-by-8000ms-after-upgrade-or-fresh-installs' into 'master'

Fix API performance regression in issues API

See merge request gitlab-org/gitlab!33235
parents f862e3e7 0ef53254
...@@ -102,7 +102,7 @@ module CacheMarkdownField ...@@ -102,7 +102,7 @@ module CacheMarkdownField
def updated_cached_html_for(markdown_field) def updated_cached_html_for(markdown_field)
return unless cached_markdown_fields.markdown_fields.include?(markdown_field) return unless cached_markdown_fields.markdown_fields.include?(markdown_field)
refresh_markdown_cache if attribute_invalidated?(cached_markdown_fields.html_field(markdown_field)) refresh_markdown_cache! if attribute_invalidated?(cached_markdown_fields.html_field(markdown_field))
cached_html_for(markdown_field) cached_html_for(markdown_field)
end end
......
---
title: Fix issue and MR API performance regression when Markdown cache is stale
merge_request: 33235
author:
type: fixed
...@@ -209,8 +209,8 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do ...@@ -209,8 +209,8 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do
thing.cached_markdown_version += 1 thing.cached_markdown_version += 1
end end
it 'calls #refresh_markdown_cache' do it 'calls #refresh_markdown_cache!' do
expect(thing).to receive(:refresh_markdown_cache) expect(thing).to receive(:refresh_markdown_cache!)
expect(thing.updated_cached_html_for(:description)).to eq(html) expect(thing.updated_cached_html_for(:description)).to eq(html)
end end
...@@ -227,8 +227,8 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do ...@@ -227,8 +227,8 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do
thing.try(:save) thing.try(:save)
end end
it 'does not call #refresh_markdown_cache' do it 'does not call #refresh_markdown_cache!' do
expect(thing).not_to receive(:refresh_markdown_cache) expect(thing).not_to receive(:refresh_markdown_cache!)
expect(thing.updated_cached_html_for(:description)).to eq(html) expect(thing.updated_cached_html_for(:description)).to eq(html)
end end
......
...@@ -80,13 +80,11 @@ RSpec.shared_examples 'a mentionable' do ...@@ -80,13 +80,11 @@ RSpec.shared_examples 'a mentionable' do
context 'when there are cached markdown fields' do context 'when there are cached markdown fields' do
before do before do
if subject.is_a?(CacheMarkdownField) skip unless subject.is_a?(CacheMarkdownField)
subject.refresh_markdown_cache
end
end end
it 'sends in cached markdown fields when appropriate' do it 'sends in cached markdown fields when appropriate' do
if subject.is_a?(CacheMarkdownField) && subject.extractors[author].blank? subject.extractors[author] = nil
expect_next_instance_of(Gitlab::ReferenceExtractor) do |ext| expect_next_instance_of(Gitlab::ReferenceExtractor) do |ext|
attrs = subject.class.mentionable_attrs.collect(&:first) & subject.cached_markdown_fields.markdown_fields attrs = subject.class.mentionable_attrs.collect(&:first) & subject.cached_markdown_fields.markdown_fields
attrs.each do |field| attrs.each do |field|
...@@ -94,13 +92,11 @@ RSpec.shared_examples 'a mentionable' do ...@@ -94,13 +92,11 @@ RSpec.shared_examples 'a mentionable' do
end end
end end
expect(subject).not_to receive(:refresh_markdown_cache)
expect(subject).to receive(:cached_markdown_fields).at_least(:once).and_call_original expect(subject).to receive(:cached_markdown_fields).at_least(:once).and_call_original
subject.all_references(author) subject.all_references(author)
end end
end end
end
it 'creates cross-reference notes' do it 'creates cross-reference notes' do
mentioned_objects = [mentioned_issue, mentioned_mr, mentioned_commit, mentioned_objects = [mentioned_issue, mentioned_mr, mentioned_commit,
...@@ -126,28 +122,42 @@ RSpec.shared_examples 'an editable mentionable' do ...@@ -126,28 +122,42 @@ RSpec.shared_examples 'an editable mentionable' do
context 'when there are cached markdown fields' do context 'when there are cached markdown fields' do
before do before do
if subject.is_a?(CacheMarkdownField) skip unless subject.is_a?(CacheMarkdownField)
subject.refresh_markdown_cache
end
end
it 'refreshes markdown cache if necessary' do
subject.save! subject.save!
end
it 'refreshes markdown cache if necessary' do
set_mentionable_text.call('This is a text') set_mentionable_text.call('This is a text')
if subject.is_a?(CacheMarkdownField) && subject.extractors[author].blank? subject.extractors[author] = nil
expect_next_instance_of(Gitlab::ReferenceExtractor) do |ext| expect_next_instance_of(Gitlab::ReferenceExtractor) do |ext|
subject.cached_markdown_fields.markdown_fields.each do |field| subject.cached_markdown_fields.markdown_fields.each do |field|
expect(ext).to receive(:analyze).with(subject.send(field), hash_including(rendered: anything)) expect(ext).to receive(:analyze).with(subject.send(field), hash_including(rendered: anything))
end end
end end
expect(subject).to receive(:refresh_markdown_cache) expect(subject).to receive(:refresh_markdown_cache).and_call_original
expect(subject).to receive(:cached_markdown_fields).at_least(:once).and_call_original expect(subject).to receive(:cached_markdown_fields).at_least(:once).and_call_original
subject.all_references(author) subject.all_references(author)
end end
context 'when the markdown cache is stale' do
before do
expect(subject).to receive(:latest_cached_markdown_version).at_least(:once) do
(Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION + 1) << 16
end
end
it 'persists the refreshed cache so that it does not have to be refreshed every time' do
expect(subject).to receive(:refresh_markdown_cache).once.and_call_original
subject.all_references(author)
subject.reload
subject.all_references(author)
end
end end
end end
......
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