Commit bba7e786 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'events-performance' into 'master'

Improve performance of retrieving last update times for events

See 75195eefcca4cb3961986c90c8e837da02f30134 and
https://gitlab.com/gitlab-org/gitlab-ce/issues/12415#note_3387317
for the juicy details.

Related issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/12415

See merge request !2613
parents 614025da de7c9c7a
...@@ -36,7 +36,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -36,7 +36,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
private private
def load_events def load_events
@events = Event.in_projects(@projects.pluck(:id)) @events = Event.in_projects(@projects)
@events = @event_filter.apply_filter(@events).with_associations @events = @event_filter.apply_filter(@events).with_associations
@events = @events.limit(20).offset(params[:offset] || 0) @events = @events.limit(20).offset(params[:offset] || 0)
end end
......
...@@ -23,14 +23,14 @@ class DashboardController < Dashboard::ApplicationController ...@@ -23,14 +23,14 @@ class DashboardController < Dashboard::ApplicationController
protected protected
def load_events def load_events
project_ids = projects =
if params[:filter] == "starred" if params[:filter] == "starred"
current_user.starred_projects current_user.starred_projects
else else
current_user.authorized_projects current_user.authorized_projects
end.pluck(:id) end
@events = Event.in_projects(project_ids) @events = Event.in_projects(projects)
@events = @event_filter.apply_filter(@events).with_associations @events = @event_filter.apply_filter(@events).with_associations
@events = @events.limit(20).offset(params[:offset] || 0) @events = @events.limit(20).offset(params[:offset] || 0)
end end
......
...@@ -89,10 +89,6 @@ class GroupsController < Groups::ApplicationController ...@@ -89,10 +89,6 @@ class GroupsController < Groups::ApplicationController
@projects ||= ProjectsFinder.new.execute(current_user, group: group).sorted_by_activity.non_archived @projects ||= ProjectsFinder.new.execute(current_user, group: group).sorted_by_activity.non_archived
end end
def project_ids
@projects.pluck(:id)
end
# Dont allow unauthorized access to group # Dont allow unauthorized access to group
def authorize_read_group! def authorize_read_group!
unless @group and (@projects.present? or can?(current_user, :read_group, @group)) unless @group and (@projects.present? or can?(current_user, :read_group, @group))
...@@ -125,7 +121,7 @@ class GroupsController < Groups::ApplicationController ...@@ -125,7 +121,7 @@ class GroupsController < Groups::ApplicationController
end end
def load_events def load_events
@events = Event.in_projects(project_ids) @events = Event.in_projects(@projects)
@events = event_filter.apply_filter(@events).with_associations @events = event_filter.apply_filter(@events).with_associations
@events = @events.limit(20).offset(params[:offset] || 0) @events = @events.limit(20).offset(params[:offset] || 0)
end end
......
...@@ -47,7 +47,11 @@ class Event < ActiveRecord::Base ...@@ -47,7 +47,11 @@ class Event < ActiveRecord::Base
# Scopes # Scopes
scope :recent, -> { reorder(id: :desc) } scope :recent, -> { reorder(id: :desc) }
scope :code_push, -> { where(action: PUSHED) } scope :code_push, -> { where(action: PUSHED) }
scope :in_projects, ->(project_ids) { where(project_id: project_ids).recent }
scope :in_projects, ->(projects) do
where(project_id: projects.select(:id).reorder(nil)).recent
end
scope :with_associations, -> { includes(project: :namespace) } scope :with_associations, -> { includes(project: :namespace) }
scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) } scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) }
...@@ -64,12 +68,6 @@ class Event < ActiveRecord::Base ...@@ -64,12 +68,6 @@ class Event < ActiveRecord::Base
[Event::CREATED, Event::CLOSED, Event::MERGED]) [Event::CREATED, Event::CLOSED, Event::MERGED])
end end
def latest_update_time
row = select(:updated_at, :project_id).reorder(id: :desc).take
row ? row.updated_at : nil
end
def limit_recent(limit = 20, offset = nil) def limit_recent(limit = 20, offset = nil)
recent.limit(limit).offset(offset) recent.limit(limit).offset(offset)
end end
......
...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear ...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear
xml.link href: dashboard_projects_url(format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml" xml.link href: dashboard_projects_url(format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
xml.link href: dashboard_projects_url, rel: "alternate", type: "text/html" xml.link href: dashboard_projects_url, rel: "alternate", type: "text/html"
xml.id dashboard_projects_url xml.id dashboard_projects_url
xml.updated @events.latest_update_time.xmlschema if @events.any? xml.updated @events[0].updated_at.xmlschema if @events[0]
@events.each do |event| @events.each do |event|
event_to_atom(xml, event) event_to_atom(xml, event)
......
...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear ...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear
xml.link href: group_url(@group, format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml" xml.link href: group_url(@group, format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
xml.link href: group_url(@group), rel: "alternate", type: "text/html" xml.link href: group_url(@group), rel: "alternate", type: "text/html"
xml.id group_url(@group) xml.id group_url(@group)
xml.updated @events.latest_update_time.xmlschema if @events.any? xml.updated @events[0].updated_at.xmlschema if @events[0]
@events.each do |event| @events.each do |event|
event_to_atom(xml, event) event_to_atom(xml, event)
......
...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear ...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear
xml.link href: namespace_project_url(@project.namespace, @project, format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml" xml.link href: namespace_project_url(@project.namespace, @project, format: :atom, private_token: current_user.try(:private_token)), rel: "self", type: "application/atom+xml"
xml.link href: namespace_project_url(@project.namespace, @project), rel: "alternate", type: "text/html" xml.link href: namespace_project_url(@project.namespace, @project), rel: "alternate", type: "text/html"
xml.id namespace_project_url(@project.namespace, @project) xml.id namespace_project_url(@project.namespace, @project)
xml.updated @events.latest_update_time.xmlschema if @events.any? xml.updated @events[0].updated_at.xmlschema if @events[0?
@events.each do |event| @events.each do |event|
event_to_atom(xml, event) event_to_atom(xml, event)
......
...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear ...@@ -4,7 +4,7 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom", "xmlns:media" => "http://sear
xml.link href: user_url(@user, :atom), rel: "self", type: "application/atom+xml" xml.link href: user_url(@user, :atom), rel: "self", type: "application/atom+xml"
xml.link href: user_url(@user), rel: "alternate", type: "text/html" xml.link href: user_url(@user), rel: "alternate", type: "text/html"
xml.id user_url(@user) xml.id user_url(@user)
xml.updated @events.latest_update_time.xmlschema if @events.any? xml.updated @events[0].updated_at.xmlschema if @events[0]
@events.each do |event| @events.each do |event|
event_to_atom(xml, event) event_to_atom(xml, event)
......
...@@ -65,27 +65,6 @@ describe Event, models: true do ...@@ -65,27 +65,6 @@ describe Event, models: true do
it { expect(@event.author).to eq(@user) } it { expect(@event.author).to eq(@user) }
end end
describe '.latest_update_time' do
describe 'when events are present' do
let(:time) { Time.utc(2015, 1, 1) }
before do
create(:closed_issue_event, updated_at: time)
create(:closed_issue_event, updated_at: time + 5)
end
it 'returns the latest update time' do
expect(Event.latest_update_time).to eq(time + 5)
end
end
describe 'when no events exist' do
it 'returns nil' do
expect(Event.latest_update_time).to be_nil
end
end
end
describe '.limit_recent' do describe '.limit_recent' do
let!(:event1) { create(:closed_issue_event) } let!(:event1) { create(:closed_issue_event) }
let!(:event2) { create(:closed_issue_event) } let!(:event2) { create(:closed_issue_event) }
......
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