From 5371da341e9d7768ebab8e159b3e2cc8fad1d827 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 23 Nov 2016 14:14:04 +0100
Subject: [PATCH 1/2] Remove event caching code

Flushing the events cache worked by updating a recent number of rows in
the "events" table. This has the result that on PostgreSQL a lot of dead
tuples are produced on a regular basis. This in turn means that
PostgreSQL will spend considerable amounts of time vacuuming this table.
This in turn can lead to an increase of database load.

For GitLab.com we measured the impact of not using events caching and
found no measurable increase in response timings. Meanwhile not flushing
the events cache lead to the "events" table having no more dead tuples
as now rows are only inserted into this table.

As a result of this we are hereby removing events caching as it does not
appear to help and only increases database load.

For more information see the following comment:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6578#note_18864037
---
 .../profiles/avatars_controller.rb            |  1 -
 .../projects/avatars_controller.rb            |  1 -
 app/models/event.rb                           |  6 ------
 app/models/issue.rb                           | 12 ------------
 app/models/merge_request.rb                   | 12 ------------
 app/models/note.rb                            | 13 -------------
 app/models/project.rb                         | 17 -----------------
 app/models/user.rb                            | 14 --------------
 app/services/issuable_base_service.rb         |  2 --
 app/services/notes/delete_service.rb          |  1 -
 app/services/notes/update_service.rb          |  1 -
 app/services/projects/transfer_service.rb     |  3 ---
 app/uploaders/avatar_uploader.rb              |  6 ------
 app/views/events/_event.html.haml             | 19 +++++++++----------
 .../unreleased/events-cache-invalidation.yml  |  4 ++++
 15 files changed, 13 insertions(+), 99 deletions(-)
 create mode 100644 changelogs/unreleased/events-cache-invalidation.yml

diff --git a/app/controllers/profiles/avatars_controller.rb b/app/controllers/profiles/avatars_controller.rb
index f193adb46b4..daa51ae41df 100644
--- a/app/controllers/profiles/avatars_controller.rb
+++ b/app/controllers/profiles/avatars_controller.rb
@@ -4,7 +4,6 @@ class Profiles::AvatarsController < Profiles::ApplicationController
     @user.remove_avatar!
 
     @user.save
-    @user.reset_events_cache
 
     redirect_to profile_path
   end
diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb
index ada7db3c552..53788687076 100644
--- a/app/controllers/projects/avatars_controller.rb
+++ b/app/controllers/projects/avatars_controller.rb
@@ -20,7 +20,6 @@ class Projects::AvatarsController < Projects::ApplicationController
     @project.remove_avatar!
 
     @project.save
-    @project.reset_events_cache
 
     redirect_to edit_project_path(@project)
   end
diff --git a/app/models/event.rb b/app/models/event.rb
index 21eaca917b8..216dba46e74 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -43,12 +43,6 @@ class Event < ActiveRecord::Base
   scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) }
 
   class << self
-    def reset_event_cache_for(target)
-      Event.where(target_id: target.id, target_type: target.class.to_s).
-        order('id DESC').limit(100).
-        update_all(updated_at: Time.now)
-    end
-
     # Update Gitlab::ContributionsCalendar#activity_dates if this changes
     def contributions
       where("action = ? OR (target_type in (?) AND action in (?))",
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 6e8f5d3c422..544f830cc69 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -182,18 +182,6 @@ class Issue < ActiveRecord::Base
     branches_with_iid - branches_with_merge_request
   end
 
-  # Reset issue events cache
-  #
-  # Since we do cache @event we need to reset cache in special cases:
-  # * when an issue is updated
-  # Events cache stored like  events/23-20130109142513.
-  # The cache key includes updated_at timestamp.
-  # Thus it will automatically generate a new fragment
-  # when the event is updated because the key changes.
-  def reset_events_cache
-    Event.reset_event_cache_for(self)
-  end
-
   # To allow polymorphism with MergeRequest.
   def source_project
     project
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6c3c093d084..bf9edb0b823 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -601,18 +601,6 @@ class MergeRequest < ActiveRecord::Base
     self.target_project.repository.branch_names.include?(self.target_branch)
   end
 
-  # Reset merge request events cache
-  #
-  # Since we do cache @event we need to reset cache in special cases:
-  # * when a merge request is updated
-  # Events cache stored like  events/23-20130109142513.
-  # The cache key includes updated_at timestamp.
-  # Thus it will automatically generate a new fragment
-  # when the event is updated because the key changes.
-  def reset_events_cache
-    Event.reset_event_cache_for(self)
-  end
-
   def merge_commit_message
     message = "Merge branch '#{source_branch}' into '#{target_branch}'\n\n"
     message << "#{title}\n\n"
diff --git a/app/models/note.rb b/app/models/note.rb
index 9ff5e308ed2..9881506edc8 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -198,19 +198,6 @@ class Note < ActiveRecord::Base
     super(noteable_type.to_s.classify.constantize.base_class.to_s)
   end
 
-  # Reset notes events cache
-  #
-  # Since we do cache @event we need to reset cache in special cases:
-  # * when a note is updated
-  # * when a note is removed
-  # Events cache stored like  events/23-20130109142513.
-  # The cache key includes updated_at timestamp.
-  # Thus it will automatically generate a new fragment
-  # when the event is updated because the key changes.
-  def reset_events_cache
-    Event.reset_event_cache_for(self)
-  end
-
   def editable?
     !system?
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 76c1fc4945d..21d9ed0f51f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -973,7 +973,6 @@ class Project < ActiveRecord::Base
       begin
         gitlab_shell.mv_repository(repository_storage_path, "#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki")
         send_move_instructions(old_path_with_namespace)
-        reset_events_cache
 
         @old_path_with_namespace = old_path_with_namespace
 
@@ -1040,22 +1039,6 @@ class Project < ActiveRecord::Base
     attrs
   end
 
-  # Reset events cache related to this project
-  #
-  # Since we do cache @event we need to reset cache in special cases:
-  # * when project was moved
-  # * when project was renamed
-  # * when the project avatar changes
-  # Events cache stored like  events/23-20130109142513.
-  # The cache key includes updated_at timestamp.
-  # Thus it will automatically generate a new fragment
-  # when the event is updated because the key changes.
-  def reset_events_cache
-    Event.where(project_id: self.id).
-      order('id DESC').limit(100).
-      update_all(updated_at: Time.now)
-  end
-
   def project_member(user)
     project_members.find_by(user_id: user)
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 29fb849940a..8a67aa94e79 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -704,20 +704,6 @@ class User < ActiveRecord::Base
       project.project_member(self)
   end
 
-  # Reset project events cache related to this user
-  #
-  # Since we do cache @event we need to reset cache in special cases:
-  # * when the user changes their avatar
-  # Events cache stored like  events/23-20130109142513.
-  # The cache key includes updated_at timestamp.
-  # Thus it will automatically generate a new fragment
-  # when the event is updated because the key changes.
-  def reset_events_cache
-    Event.where(author_id: id).
-      order('id DESC').limit(1000).
-      update_all(updated_at: Time.now)
-  end
-
   def full_website_url
     return "http://#{website_url}" if website_url !~ /\Ahttps?:\/\//
 
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 575795788de..d698b295e6d 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -184,8 +184,6 @@ class IssuableBaseService < BaseService
     params[:label_ids] = process_label_ids(params, existing_label_ids: issuable.label_ids)
 
     if params.present? && update_issuable(issuable, params)
-      issuable.reset_events_cache
-
       # We do not touch as it will affect a update on updated_at field
       ActiveRecord::Base.no_touching do
         handle_common_system_notes(issuable, old_labels: old_labels)
diff --git a/app/services/notes/delete_service.rb b/app/services/notes/delete_service.rb
index 7f1b30ec84e..a673e8e9dde 100644
--- a/app/services/notes/delete_service.rb
+++ b/app/services/notes/delete_service.rb
@@ -2,7 +2,6 @@ module Notes
   class DeleteService < BaseService
     def execute(note)
       note.destroy
-      note.reset_events_cache
     end
   end
 end
diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb
index 1361b1e0300..75a4b3ed826 100644
--- a/app/services/notes/update_service.rb
+++ b/app/services/notes/update_service.rb
@@ -5,7 +5,6 @@ module Notes
 
       note.update_attributes(params.merge(updated_by: current_user))
       note.create_new_cross_references!(current_user)
-      note.reset_events_cache
 
       if note.previous_changes.include?('note')
         TodoService.new.update_note(note, current_user)
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 28470f59807..34ec575e808 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -61,9 +61,6 @@ module Projects
         # Move missing group labels to project
         Labels::TransferService.new(current_user, old_group, project).execute
 
-        # clear project cached events
-        project.reset_events_cache
-
         # Move uploads
         Gitlab::UploadsTransfer.new.move_project(project.path, old_namespace.path, new_namespace.path)
 
diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb
index 71ff14a3f20..38683fdf6d7 100644
--- a/app/uploaders/avatar_uploader.rb
+++ b/app/uploaders/avatar_uploader.rb
@@ -3,16 +3,10 @@ class AvatarUploader < CarrierWave::Uploader::Base
 
   storage :file
 
-  after :store, :reset_events_cache
-
   def store_dir
     "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
   end
 
-  def reset_events_cache(file)
-    model.reset_events_cache if model.is_a?(User)
-  end
-
   def exists?
     model.avatar.file && model.avatar.file.exists?
   end
diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml
index 5c318cd3b8b..a0bd14df209 100644
--- a/app/views/events/_event.html.haml
+++ b/app/views/events/_event.html.haml
@@ -3,14 +3,13 @@
     .event-item-timestamp
       #{time_ago_with_tooltip(event.created_at)}
 
-    = cache [event, current_application_settings, "v2.2"] do
-      = author_avatar(event, size: 40)
+    = author_avatar(event, size: 40)
 
-      - if event.created_project?
-        = render "events/event/created_project", event: event
-      - elsif event.push?
-        = render "events/event/push", event: event
-      - elsif event.commented?
-        = render "events/event/note", event: event
-      - else
-        = render "events/event/common", event: event
+    - if event.created_project?
+      = render "events/event/created_project", event: event
+    - elsif event.push?
+      = render "events/event/push", event: event
+    - elsif event.commented?
+      = render "events/event/note", event: event
+    - else
+      = render "events/event/common", event: event
diff --git a/changelogs/unreleased/events-cache-invalidation.yml b/changelogs/unreleased/events-cache-invalidation.yml
new file mode 100644
index 00000000000..2b30f4dcbce
--- /dev/null
+++ b/changelogs/unreleased/events-cache-invalidation.yml
@@ -0,0 +1,4 @@
+---
+title: Remove caching of events data
+merge_request: 6578
+author: 

From 0ba03d7eb1d80e019b9b8266f0e14356d32e7d69 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 24 Nov 2016 11:25:23 +0100
Subject: [PATCH 2/2] Removed data-user-is view code

With events no longer being cached this is no longer needed.
---
 app/models/event.rb                        |  4 ++++
 app/views/events/event/_push.html.haml     |  6 +++---
 app/views/layouts/_head.html.haml          |  2 --
 app/views/layouts/_user_styles.html.haml   | 24 ----------------------
 spec/models/event_spec.rb                  | 18 ++++++++++++++++
 spec/views/layouts/_head.html.haml_spec.rb |  4 ----
 6 files changed, 25 insertions(+), 33 deletions(-)
 delete mode 100644 app/views/layouts/_user_styles.html.haml

diff --git a/app/models/event.rb b/app/models/event.rb
index 216dba46e74..2662f170765 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -347,6 +347,10 @@ class Event < ActiveRecord::Base
       update_all(last_activity_at: created_at)
   end
 
+  def authored_by?(user)
+    user ? author_id == user.id : false
+  end
+
   private
 
   def recent_update?
diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml
index 44fff49d99c..64ca3c32e01 100644
--- a/app/views/events/event/_push.html.haml
+++ b/app/views/events/event/_push.html.haml
@@ -18,7 +18,7 @@
       - few_commits.each do |commit|
         = render "events/commit", commit: commit, project: project, event: event
 
-      - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project)
+      - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user)
       - if event.commits_count > 1
         %li.commits-stat
           - if event.commits_count > 2
@@ -35,12 +35,12 @@
             Compare #{from_label}...#{truncate_sha(event.commit_to)}
 
           - if create_mr
-            %span{"data-user-is" => event.author_id, "data-display" => "inline"}
+            %span
               or
               = link_to create_mr_path(project.default_branch, event.ref_name, project) do
                 create a merge request
       - elsif create_mr
-        %li.commits-stat{"data-user-is" => event.author_id}
+        %li.commits-stat
           = link_to create_mr_path(project.default_branch, event.ref_name, project) do
             Create Merge Request
 - elsif event.rm_ref?
diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml
index 757de92d6d4..3e488cf73b9 100644
--- a/app/views/layouts/_head.html.haml
+++ b/app/views/layouts/_head.html.haml
@@ -56,5 +56,3 @@
   = render 'layouts/google_analytics' if extra_config.has_key?('google_analytics_id')
   = render 'layouts/piwik' if extra_config.has_key?('piwik_url') && extra_config.has_key?('piwik_site_id')
   = render 'layouts/bootlint' if Rails.env.development?
-
-  = render 'layouts/user_styles'
diff --git a/app/views/layouts/_user_styles.html.haml b/app/views/layouts/_user_styles.html.haml
deleted file mode 100644
index b76b3cb5510..00000000000
--- a/app/views/layouts/_user_styles.html.haml
+++ /dev/null
@@ -1,24 +0,0 @@
-:css
-  [data-user-is] {
-    display: none !important;
-  }
-
-  [data-user-is="#{current_user.try(:id)}"] {
-    display: block !important;
-  }
-
-  [data-user-is="#{current_user.try(:id)}"][data-display="inline"] {
-    display: inline !important;
-  }
-
-  [data-user-is-not] {
-    display: block !important;
-  }
-
-  [data-user-is-not][data-display="inline"] {
-    display: inline !important;
-  }
-
-  [data-user-is-not="#{current_user.try(:id)}"] {
-    display: none !important;
-  }
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index b684053cd02..f8660da031d 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -260,6 +260,24 @@ describe Event, models: true do
     end
   end
 
+  describe '#authored_by?' do
+    let(:event) { build(:event) }
+
+    it 'returns true when the event author and user are the same' do
+      expect(event.authored_by?(event.author)).to eq(true)
+    end
+
+    it 'returns false when passing nil as an argument' do
+      expect(event.authored_by?(nil)).to eq(false)
+    end
+
+    it 'returns false when the given user is not the author of the event' do
+      user = double(:user, id: -1)
+
+      expect(event.authored_by?(user)).to eq(false)
+    end
+  end
+
   def create_event(project, user, attrs = {})
     data = {
       before: Gitlab::Git::BLANK_SHA,
diff --git a/spec/views/layouts/_head.html.haml_spec.rb b/spec/views/layouts/_head.html.haml_spec.rb
index 3fddfb3b62f..8020faa1f9c 100644
--- a/spec/views/layouts/_head.html.haml_spec.rb
+++ b/spec/views/layouts/_head.html.haml_spec.rb
@@ -1,10 +1,6 @@
 require 'spec_helper'
 
 describe 'layouts/_head' do
-  before do
-    stub_template 'layouts/_user_styles.html.haml' => ''
-  end
-
   it 'escapes HTML-safe strings in page_title' do
     stub_helper_with_safe_string(:page_title)