Remove unnecessary Redis deletes for broadcast messages

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/21038 removed an
optimization that prevented a Redis DEL call from being issued if no
broadcast messages were present. Now, two DEL calls are issued for every
request, which led to a significant performance regression.

The Redis delete optimization was intended to prevent filtering stale
messages over and over again. However, the optimization only worked when
there were no other broadcast messages for the future. For example, if
there were 1 current message and 1 message scheduled for 5 days later,
the filtering would still need to be done.

To improve this optimization, we check to see whether the filtered set
is different from the set we retrieved from the cache. If there is a
difference, we know it's time to flush the cache.

This came out of an investigation in
https://gitlab.com/gitlab-com/gl-infra/production/issues/1722.

app/models/broadcast_message.rb

@@ -52,7 +52,9 @@ class BroadcastMessage < ApplicationRecord
     end
 
     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
 
     def cache_expires_in
@@ -68,9 +70,9 @@ class BroadcastMessage < ApplicationRecord
 
       now_or_future = messages.select(&:now_or_future?)
 
-      # If there are cached entries but none are to be displayed we'll purge the
-      # cache so we don't keep running this code all the time.
-      cache.expire(cache_key) if now_or_future.empty?
+      # If there are cached entries but they don't match the ones we are
+      # displaying we'll refresh the cache so we don't need to keep filtering.
+      cache.expire(cache_key) if now_or_future != messages
 
       now_or_future.select(&:now?).select { |message| message.matches_current_path(current_path) }
     end

changelogs/unreleased/sh-optimize-broadcast-message-redis.yml

+---
+title: Remove unnecessary Redis deletes for broadcast messages
+merge_request: 26541
+author:
+type: performance

spec/models/broadcast_message_spec.rb

@@ -65,6 +65,17 @@ describe BroadcastMessage do
       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
       create(:broadcast_message, broadcast_type: broadcast_type)