Commit 87bab3bd authored by Stan Hu's avatar Stan Hu

Merge branch 'sh-optimize-broadcast-message-redis' into 'master'

Remove unnecessary Redis deletes for broadcast messages

See merge request gitlab-org/gitlab!26541
parents 5a95a149 94305e86
...@@ -52,7 +52,9 @@ class BroadcastMessage < ApplicationRecord ...@@ -52,7 +52,9 @@ class BroadcastMessage < ApplicationRecord
end end
def cache def cache
Gitlab::JsonCache.new(cache_key_with_version: false) ::Gitlab::SafeRequestStore.fetch(:broadcast_message_json_cache) do
Gitlab::JsonCache.new(cache_key_with_version: false)
end
end end
def cache_expires_in def cache_expires_in
...@@ -68,9 +70,9 @@ class BroadcastMessage < ApplicationRecord ...@@ -68,9 +70,9 @@ class BroadcastMessage < ApplicationRecord
now_or_future = messages.select(&:now_or_future?) now_or_future = messages.select(&:now_or_future?)
# If there are cached entries but none are to be displayed we'll purge the # If there are cached entries but they don't match the ones we are
# cache so we don't keep running this code all the time. # displaying we'll refresh the cache so we don't need to keep filtering.
cache.expire(cache_key) if now_or_future.empty? cache.expire(cache_key) if now_or_future != messages
now_or_future.select(&:now?).select { |message| message.matches_current_path(current_path) } now_or_future.select(&:now?).select { |message| message.matches_current_path(current_path) }
end end
......
---
title: Remove unnecessary Redis deletes for broadcast messages
merge_request: 26541
author:
type: performance
...@@ -65,6 +65,17 @@ describe BroadcastMessage do ...@@ -65,6 +65,17 @@ describe BroadcastMessage do
end end
end end
it 'expires the value if a broadcast message has ended', :request_store do
message = create(:broadcast_message, broadcast_type: broadcast_type, ends_at: Time.now.utc + 1.day)
expect(subject.call).to match_array([message])
expect(described_class.cache).to receive(:expire).and_call_original
Timecop.travel(1.week) do
2.times { expect(subject.call).to be_empty }
end
end
it 'does not create new records' do it 'does not create new records' do
create(:broadcast_message, broadcast_type: broadcast_type) create(:broadcast_message, broadcast_type: broadcast_type)
......
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