Commit 17b71e6c authored by Eulyeon Ko's avatar Eulyeon Ko Committed by Steve Abrams

Fix slow events query for followed users

Changelog: performance
parent 90a57559
...@@ -46,6 +46,20 @@ class UserRecentEventsFinder ...@@ -46,6 +46,20 @@ class UserRecentEventsFinder
.order_created_desc) .order_created_desc)
end end
# rubocop: disable CodeReuse/ActiveRecord
def execute_optimized_multi(users)
Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder.new(
scope: Event.reorder(:id),
array_scope: User.select(:id).where(id: users),
array_mapping_scope: -> (author_id_expression) { Event.where(Event.arel_table[:author_id].eq(author_id_expression)) },
finder_query: -> (id_expression) { Event.where(Event.arel_table[:id].eq(id_expression)) }
)
.execute
.limit(limit)
.offset(params[:offset] || 0)
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def execute_multi def execute_multi
users = [] users = []
...@@ -55,8 +69,12 @@ class UserRecentEventsFinder ...@@ -55,8 +69,12 @@ class UserRecentEventsFinder
return Event.none if users.empty? return Event.none if users.empty?
if event_filter.filter == EventFilter::ALL
execute_optimized_multi(users)
else
event_filter.apply_filter(Event.where(author: users).limit_recent(limit, params[:offset] || 0)) event_filter.apply_filter(Event.where(author: users).limit_recent(limit, params[:offset] || 0))
end end
end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
......
# frozen_string_literal: true
class AddIndexToEventsOnAuthorIdAndActionAndId < Gitlab::Database::Migration[1.0]
INDEX_NAME = 'index_events_on_author_id_and_action_and_id'
disable_ddl_transaction!
def up
add_concurrent_index :events, [:author_id, :action, :id], name: INDEX_NAME
end
def down
remove_concurrent_index_by_name :events, INDEX_NAME
end
end
2e35347592530f2a73e1cd75871438e29d277a206f621989e2c897fc587b6f5d
\ No newline at end of file
...@@ -26024,6 +26024,8 @@ CREATE INDEX index_events_author_id_project_id_action_target_type_created_at ON ...@@ -26024,6 +26024,8 @@ CREATE INDEX index_events_author_id_project_id_action_target_type_created_at ON
CREATE INDEX index_events_on_action ON events USING btree (action); CREATE INDEX index_events_on_action ON events USING btree (action);
CREATE INDEX index_events_on_author_id_and_action_and_id ON events USING btree (author_id, action, id);
CREATE INDEX index_events_on_author_id_and_created_at ON events USING btree (author_id, created_at); CREATE INDEX index_events_on_author_id_and_created_at ON events USING btree (author_id, created_at);
CREATE INDEX index_events_on_author_id_and_created_at_merge_requests ON events USING btree (author_id, created_at) WHERE ((target_type)::text = 'MergeRequest'::text); CREATE INDEX index_events_on_author_id_and_created_at_merge_requests ON events USING btree (author_id, created_at) WHERE ((target_type)::text = 'MergeRequest'::text);
...@@ -59,14 +59,46 @@ RSpec.describe UserRecentEventsFinder do ...@@ -59,14 +59,46 @@ RSpec.describe UserRecentEventsFinder do
expect(events.size).to eq(6) expect(events.size).to eq(6)
end end
context 'selected events' do
let!(:push_event) { create(:push_event, project: public_project, author: project_owner) }
let!(:push_event_second_user) { create(:push_event, project: public_project_second_user, author: second_user) }
it 'only includes selected events (PUSH) from all users', :aggregate_failures do
event_filter = EventFilter.new(EventFilter::PUSH)
events = described_class.new(current_user, [project_owner, second_user], event_filter, params).execute
expect(events).to contain_exactly(push_event, push_event_second_user)
end
end
it 'does not include events from users with private profile', :aggregate_failures do it 'does not include events from users with private profile', :aggregate_failures do
allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(current_user, :read_user_profile, second_user).and_return(false) allow(Ability).to receive(:allowed?).with(current_user, :read_user_profile, second_user).and_return(false)
events = described_class.new(current_user, [project_owner, second_user], nil, params).execute events = described_class.new(current_user, [project_owner, second_user], nil, params).execute
expect(events).to include(private_event, internal_event, public_event) expect(events).to eq([private_event, internal_event, public_event])
expect(events.size).to eq(3) end
context 'with pagination params' do
using RSpec::Parameterized::TableSyntax
where(:limit, :offset, :ordered_expected_events) do
nil | nil | lazy { [private_event, internal_event, public_event, private_event_second_user, internal_event_second_user, public_event_second_user] }
2 | nil | lazy { [private_event, internal_event] }
nil | 4 | lazy { [internal_event_second_user, public_event_second_user] }
2 | 2 | lazy { [public_event, private_event_second_user] }
end
with_them do
let(:params) { { limit: limit, offset: offset }.compact }
it 'returns paginated events sorted by id' do
events = described_class.new(current_user, [project_owner, second_user], nil, params).execute
expect(events).to eq(ordered_expected_events)
end
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