diff options
| -rw-r--r-- | app/models/concerns/throttled_touch.rb | 10 | ||||
| -rw-r--r-- | app/models/issue.rb | 1 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 1 | ||||
| -rw-r--r-- | app/models/note.rb | 36 | ||||
| -rw-r--r-- | changelogs/unreleased/throttle-touching-of-objects.yml | 5 | ||||
| -rw-r--r-- | spec/models/concerns/issuable_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/issue_spec.rb | 4 | ||||
| -rw-r--r-- | spec/models/merge_request_spec.rb | 4 | ||||
| -rw-r--r-- | spec/models/note_spec.rb | 2 | ||||
| -rw-r--r-- | spec/support/shared_examples/throttled_touch.rb | 20 |
10 files changed, 3 insertions, 82 deletions
diff --git a/app/models/concerns/throttled_touch.rb b/app/models/concerns/throttled_touch.rb deleted file mode 100644 index ad0ff0f20d4..00000000000 --- a/app/models/concerns/throttled_touch.rb +++ /dev/null @@ -1,10 +0,0 @@ -# ThrottledTouch can be used to throttle the number of updates triggered by -# calling "touch" on an ActiveRecord model. -module ThrottledTouch - # The amount of time to wait before "touch" can update a record again. - TOUCH_INTERVAL = 1.minute - - def touch(*args) - super if (Time.zone.now - updated_at) > TOUCH_INTERVAL - end -end diff --git a/app/models/issue.rb b/app/models/issue.rb index 33db197e612..d6ef58d150b 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -9,7 +9,6 @@ class Issue < ActiveRecord::Base include FasterCacheKeys include RelativePositioning include TimeTrackable - include ThrottledTouch DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 422f138c4ea..d7faf22fc46 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -7,7 +7,6 @@ class MergeRequest < ActiveRecord::Base include TimeTrackable include ManualInverseAssociation include EachBatch - include ThrottledTouch ignore_column :locked_at, :ref_fetched diff --git a/app/models/note.rb b/app/models/note.rb index c4c2ab8e67d..517d7985b8c 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -15,7 +15,6 @@ class Note < ActiveRecord::Base include IgnorableColumn include Editable include Gitlab::SQL::Pattern - include ThrottledTouch module SpecialRole FIRST_TIME_CONTRIBUTOR = :first_time_contributor @@ -56,7 +55,7 @@ class Note < ActiveRecord::Base participant :author belongs_to :project - belongs_to :noteable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations + belongs_to :noteable, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations belongs_to :author, class_name: "User" belongs_to :updated_by, class_name: "User" belongs_to :last_edited_by, class_name: 'User' @@ -119,7 +118,6 @@ class Note < ActiveRecord::Base before_validation :set_discussion_id, on: :create after_save :keep_around_commit, if: :for_project_noteable? after_save :expire_etag_cache - after_save :touch_noteable after_destroy :expire_etag_cache class << self @@ -371,38 +369,6 @@ class Note < ActiveRecord::Base Gitlab::EtagCaching::Store.new.touch(key) end - def touch(*args) - # We're not using an explicit transaction here because this would in all - # cases result in all future queries going to the primary, even if no writes - # are performed. - # - # We touch the noteable first so its SELECT query can run before our writes, - # ensuring it runs on a secondary (if no prior write took place). - touch_noteable - super - end - - # By default Rails will issue an "SELECT *" for the relation, which is - # overkill for just updating the timestamps. To work around this we manually - # touch the data so we can SELECT only the columns we need. - def touch_noteable - # Commits are not stored in the DB so we can't touch them. - return if for_commit? - - assoc = association(:noteable) - - noteable_object = - if assoc.loaded? - noteable - else - # If the object is not loaded (e.g. when notes are loaded async) we - # _only_ want the data we actually need. - assoc.scope.select(:id, :updated_at).take - end - - noteable_object&.touch - end - def banzai_render_context(field) super.merge(noteable: noteable) end diff --git a/changelogs/unreleased/throttle-touching-of-objects.yml b/changelogs/unreleased/throttle-touching-of-objects.yml deleted file mode 100644 index 0a57bea7c83..00000000000 --- a/changelogs/unreleased/throttle-touching-of-objects.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Throttle the number of UPDATEs triggered by touch -merge_request: -author: -type: performance diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 9df26f06a11..a53b59c4e08 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -171,7 +171,7 @@ describe Issuable do it "returns false when record has been updated" do allow(issue).to receive(:today?).and_return(true) - issue.update_attribute(:updated_at, 1.hour.ago) + issue.touch expect(issue.new?).to be_falsey end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 0ea287d007a..5f901262598 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -765,8 +765,4 @@ describe Issue do expect(described_class.public_only).to eq([public_issue]) end end - - it_behaves_like 'throttled touch' do - subject { create(:issue, updated_at: 1.hour.ago) } - end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 30a5a3bbff7..81eb62f1801 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1851,8 +1851,4 @@ describe MergeRequest do .to change { project.open_merge_requests_count }.from(1).to(0) end end - - it_behaves_like 'throttled touch' do - subject { create(:merge_request, updated_at: 1.hour.ago) } - end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index e1a0c55b6a6..6e7e8c4c570 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -5,7 +5,7 @@ describe Note do describe 'associations' do it { is_expected.to belong_to(:project) } - it { is_expected.to belong_to(:noteable).touch(false) } + it { is_expected.to belong_to(:noteable).touch(true) } it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to have_many(:todos).dependent(:destroy) } diff --git a/spec/support/shared_examples/throttled_touch.rb b/spec/support/shared_examples/throttled_touch.rb deleted file mode 100644 index 4a25bb9b750..00000000000 --- a/spec/support/shared_examples/throttled_touch.rb +++ /dev/null @@ -1,20 +0,0 @@ -shared_examples_for 'throttled touch' do - describe '#touch' do - it 'updates the updated_at timestamp' do - Timecop.freeze do - subject.touch - expect(subject.updated_at).to eq(Time.zone.now) - end - end - - it 'updates the object at most once per minute' do - first_updated_at = Time.zone.now - (ThrottledTouch::TOUCH_INTERVAL * 2) - second_updated_at = Time.zone.now - (ThrottledTouch::TOUCH_INTERVAL * 1.5) - - Timecop.freeze(first_updated_at) { subject.touch } - Timecop.freeze(second_updated_at) { subject.touch } - - expect(subject.updated_at).to eq(first_updated_at) - end - end -end |