Commit abf5d151 authored by Marc Shaw's avatar Marc Shaw

Merge branch '350340-optimize-projects-activity' into 'master'

Fix slow query for "All" tabs in "Your projects" activity page

See merge request gitlab-org/gitlab!82296
parents fded4c30 c12eb806
......@@ -44,31 +44,31 @@ class EventCollection
private
def project_events
relation_with_join_lateral('project_id', projects)
in_operator_optimized_relation('project_id', projects)
end
def project_and_group_events
group_events = relation_with_join_lateral('group_id', groups)
def group_events
in_operator_optimized_relation('group_id', groups)
end
def project_and_group_events
Event.from_union([project_events, group_events]).recent
end
# This relation is built using JOIN LATERAL, producing faster queries than a
# regular LIMIT + OFFSET approach.
def relation_with_join_lateral(parent_column, parents)
parents_for_lateral = parents.select(:id).to_sql
lateral = filtered_events
# Applying the limit here (before we filter (permissions) means we may get less than limit)
.limit(limit_for_join_lateral)
.where("events.#{parent_column} = parents_for_lateral.id") # rubocop:disable GitlabSecurity/SqlInjection
.to_sql
# The outer query does not need to re-apply the filters since the JOIN
# LATERAL body already takes care of this.
base_relation
.from("(#{parents_for_lateral}) parents_for_lateral")
.joins("JOIN LATERAL (#{lateral}) AS #{Event.table_name} ON true")
def in_operator_optimized_relation(parent_column, parents)
scope = filtered_events
array_scope = parents.select(:id)
array_mapping_scope = -> (parent_id_expression) { Event.where(Event.arel_table[parent_column].eq(parent_id_expression)).reorder(id: :desc) }
finder_query = -> (id_expression) { Event.where(Event.arel_table[:id].eq(id_expression)) }
Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder
.new(
scope: scope,
array_scope: array_scope,
array_mapping_scope: array_mapping_scope,
finder_query: finder_query
)
.execute
end
def filtered_events
......@@ -85,16 +85,6 @@ class EventCollection
Event.unscoped.recent
end
def limit_for_join_lateral
# Applying the OFFSET on the inside of a JOIN LATERAL leads to incorrect
# results. To work around this we need to increase the inner limit for every
# page.
#
# This means that on page 1 we use LIMIT 20, and an outer OFFSET of 0. On
# page 2 we use LIMIT 40 and an outer OFFSET of 20.
@limit + @offset
end
def current_page
(@offset / @limit) + 1
end
......
......@@ -71,9 +71,9 @@ RSpec.describe EventCollection do
end
it 'can paginate through events' do
events = described_class.new(projects, offset: 20).to_a
events = described_class.new(projects, limit: 5, offset: 15).to_a
expect(events.length).to eq(2)
expect(events.length).to eq(5)
end
it 'returns an empty Array when crossing the maximum page number' do
......@@ -124,6 +124,19 @@ RSpec.describe EventCollection do
expect(subject).to eq([event1])
end
context 'pagination through events' do
let_it_be(:project_events) { create_list(:event, 10, project: project) }
let_it_be(:group_events) { create_list(:event, 10, group: group, author: user) }
let(:subject) { described_class.new(projects, limit: 10, offset: 5, groups: groups).to_a }
it 'returns recent groups and projects events' do
recent_events_with_offset = (project_events[5..] + group_events[..4]).reverse
expect(subject).to eq(recent_events_with_offset)
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