From 5f9e47be34fcf42ff7fcd5668c7555d4a38e289a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 4 Nov 2022 13:21:06 +0100 Subject: [PATCH] Add caching for payload serialization during fan-out (#19642) --- app/lib/inline_renderer.rb | 11 +++++ app/lib/status_cache_hydrator.rb | 47 +++++++++++++++++++++ app/services/fan_out_on_write_service.rb | 11 ++++- app/workers/push_update_worker.rb | 13 +++--- spec/lib/status_cache_hydrator_spec.rb | 54 ++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 app/lib/status_cache_hydrator.rb create mode 100644 spec/lib/status_cache_hydrator_spec.rb diff --git a/app/lib/inline_renderer.rb b/app/lib/inline_renderer.rb index b70814748d..4bb240b48b 100644 --- a/app/lib/inline_renderer.rb +++ b/app/lib/inline_renderer.rb @@ -11,6 +11,7 @@ class InlineRenderer case @template when :status serializer = REST::StatusSerializer + preload_associations_for_status when :notification serializer = REST::NotificationSerializer when :conversation @@ -35,6 +36,16 @@ class InlineRenderer private + def preload_associations_for_status + ActiveRecord::Associations::Preloader.new.preload(@object, { + active_mentions: :account, + + reblog: { + active_mentions: :account, + }, + }) + end + def current_user @current_account&.user end diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb new file mode 100644 index 0000000000..01e92b3855 --- /dev/null +++ b/app/lib/status_cache_hydrator.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class StatusCacheHydrator + def initialize(status) + @status = status + end + + def hydrate(account_id) + # The cache of the serialized hash is generated by the fan-out-on-write service + payload = Rails.cache.fetch("fan-out/#{@status.id}") { InlineRenderer.render(@status, nil, :status) } + + # If we're delivering to the author who disabled the display of the application used to create the + # status, we need to hydrate the application, since it was not rendered for the basic payload + payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id + + # We take advantage of the fact that some relationships can only occur with an original status, not + # the reblog that wraps it, so we can assume that some values are always false + if payload[:reblog] + payload[:favourited] = false + payload[:reblogged] = false + payload[:muted] = false + payload[:bookmarked] = false + payload[:pinned] = false + payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.reblog_of_id), @status.reblog).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json } + + # If the reblogged status is being delivered to the author who disabled the display of the application + # used to create the status, we need to hydrate it here too + payload[:reblog][:application] = ActiveModelSerializers::SerializableResource.new(@status.reblog.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id + + payload[:reblog][:favourited] = Favourite.where(account_id: account_id, status_id: @status.reblog_of_id).exists? + payload[:reblog][:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.reblog_of_id).exists? + payload[:reblog][:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.reblog.conversation_id).exists? + payload[:reblog][:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.reblog_of_id).exists? + payload[:reblog][:pinned] = StatusPin.where(account_id: account_id, status_id: @status.reblog_of_id).exists? + payload[:reblog][:filtered] = payload[:filtered] + else + payload[:favourited] = Favourite.where(account_id: account_id, status_id: @status.id).exists? + payload[:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.id).exists? + payload[:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.conversation_id).exists? + payload[:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.id).exists? + payload[:pinned] = StatusPin.where(account_id: account_id, status_id: @status.id).exists? + payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.id), @status).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json } + end + + payload + end +end diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index ce20a146e9..2554756a5d 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -14,6 +14,7 @@ class FanOutOnWriteService < BaseService @options = options check_race_condition! + warm_payload_cache! fan_out_to_local_recipients! fan_out_to_public_recipients! if broadcastable? @@ -135,13 +136,21 @@ class FanOutOnWriteService < BaseService AccountConversation.add_status(@account, @status) unless update? end + def warm_payload_cache! + Rails.cache.write("fan-out/#{@status.id}", rendered_status) + end + def anonymous_payload @anonymous_payload ||= Oj.dump( event: update? ? :'status.update' : :update, - payload: InlineRenderer.render(@status, nil, :status) + payload: rendered_status ) end + def rendered_status + @rendered_status ||= InlineRenderer.render(@status, nil, :status) + end + def update? @options[:update] end diff --git a/app/workers/push_update_worker.rb b/app/workers/push_update_worker.rb index 9f44c32b3b..72c7817495 100644 --- a/app/workers/push_update_worker.rb +++ b/app/workers/push_update_worker.rb @@ -5,11 +5,12 @@ class PushUpdateWorker include Redisable def perform(account_id, status_id, timeline_id = nil, options = {}) - @account = Account.find(account_id) - @status = Status.includes(active_mentions: :account, reblog: { active_mentions: :account }).find(status_id) - @timeline_id = timeline_id || "timeline:#{account.id}" + @status = Status.find(status_id) + @account_id = account_id + @timeline_id = timeline_id || "timeline:#{account_id}" @options = options.symbolize_keys + render_payload! publish! rescue ActiveRecord::RecordNotFound true @@ -17,14 +18,14 @@ class PushUpdateWorker private - def payload - InlineRenderer.render(@status, @account, :status) + def render_payload! + @payload = StatusCacheHydrator.new(@status).hydrate(@account_id) end def message Oj.dump( event: update? ? :'status.update' : :update, - payload: payload, + payload: @payload, queued_at: (Time.now.to_f * 1000.0).to_i ) end diff --git a/spec/lib/status_cache_hydrator_spec.rb b/spec/lib/status_cache_hydrator_spec.rb new file mode 100644 index 0000000000..ad9940a853 --- /dev/null +++ b/spec/lib/status_cache_hydrator_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe StatusCacheHydrator do + let(:status) { Fabricate(:status) } + let(:account) { Fabricate(:account) } + + describe '#hydrate' do + subject { described_class.new(status).hydrate(account.id) } + + let(:compare_to_hash) { InlineRenderer.render(status, account, :status) } + + context 'when cache is warm' do + before do + Rails.cache.write("fan-out/#{status.id}", InlineRenderer.render(status, nil, :status)) + end + + it 'renders the same attributes as a full render' do + expect(subject).to include(compare_to_hash) + end + end + + context 'when cache is cold' do + before do + Rails.cache.delete("fan-out/#{status.id}") + end + + it 'renders the same attributes as a full render' do + expect(subject).to include(compare_to_hash) + end + end + + context 'when account has favourited status' do + before do + FavouriteService.new.call(account, status) + end + + it 'renders the same attributes as a full render' do + expect(subject).to include(compare_to_hash) + end + end + + context 'when account has reblogged status' do + before do + ReblogService.new.call(account, status) + end + + it 'renders the same attributes as a full render' do + expect(subject).to include(compare_to_hash) + end + end + end +end