comments: fewer db round trips on creation
Writes a vote directly to avoid vote_thusly doing round trips to check if one exists, etc. Removes redundant transactions from controllers from #899. Rails already creates a transaction for the .save. Unifies Story cache updating. Previously recalculate_hotness! was called twice on comment creation. Moves comment counting into the db. Shorter transaction should reduce the frequence of lobsters/lobsters-ansible/issues/39 but seems unlikely to eliminate it as the create + upvote transactions for stories + comments still read/write from stories, comments, and votes.
This commit is contained in:
parent
d806f7a60d
commit
d13c6c4676
|
@ -51,7 +51,7 @@ class CommentsController < ApplicationController
|
|||
content_type: "text/html", locals: {comment: comment}
|
||||
end
|
||||
|
||||
if comment.valid? && params[:preview].blank? && ActiveRecord::Base.transaction { comment.save }
|
||||
if comment.valid? && params[:preview].blank? && comment.save
|
||||
comment.current_vote = {vote: 1}
|
||||
render_created_comment(comment)
|
||||
else
|
||||
|
|
|
@ -20,7 +20,7 @@ class StoriesController < ApplicationController
|
|||
@story.attributes = story_params
|
||||
|
||||
if @story.valid? && !(@story.already_posted_recently? && !@story.seen_previous)
|
||||
if ActiveRecord::Base.transaction { @story.save }
|
||||
if @story.save
|
||||
ReadRibbon.where(user: @user, story: @story).first_or_create
|
||||
return redirect_to @story.comments_path
|
||||
end
|
||||
|
|
|
@ -29,10 +29,6 @@ class Comment < ApplicationRecord
|
|||
end
|
||||
after_create :record_initial_upvote, :mark_submitter, :deliver_reply_notifications,
|
||||
:deliver_mention_notifications, :log_hat_use
|
||||
after_create do
|
||||
# fire this once after record_initial_upvote
|
||||
update_score_and_recalculate! 0, 0
|
||||
end
|
||||
after_destroy :unassign_votes
|
||||
|
||||
scope :deleted, -> { where(is_deleted: true) }
|
||||
|
@ -259,7 +255,7 @@ class Comment < ApplicationRecord
|
|||
save(validate: false)
|
||||
Comment.record_timestamps = true
|
||||
|
||||
story.update_comments_count!
|
||||
story.update_cached_columns
|
||||
self.user.refresh_counts!
|
||||
end
|
||||
|
||||
|
@ -371,7 +367,7 @@ class Comment < ApplicationRecord
|
|||
confidence_order = concat(lpad(char(65536 - floor(((confidence - -0.2) * 65535) / 1.2) using binary), 2, '0'), char(id & 0xff using binary))
|
||||
WHERE id = #{id.to_i}
|
||||
SQL
|
||||
story.recalculate_hotness!
|
||||
story.update_cached_columns
|
||||
end
|
||||
|
||||
def gone_text
|
||||
|
@ -504,11 +500,11 @@ class Comment < ApplicationRecord
|
|||
end
|
||||
|
||||
def record_initial_upvote
|
||||
Vote.vote_thusly_on_story_or_comment_for_user_because(
|
||||
1, story_id, id, user_id, nil, false
|
||||
)
|
||||
# not calling vote_thusly to save round-trips of validations
|
||||
Vote.create! story: story, comment: self, user: user, vote: 1
|
||||
update_score_and_recalculate! 0, 0 # trigger db calculation
|
||||
|
||||
story.update_comments_count!
|
||||
story.update_cached_columns
|
||||
end
|
||||
|
||||
def score_for_user(u)
|
||||
|
@ -541,7 +537,7 @@ class Comment < ApplicationRecord
|
|||
end
|
||||
|
||||
def unassign_votes
|
||||
story.update_comments_count!
|
||||
story.update_cached_columns
|
||||
end
|
||||
|
||||
def url
|
||||
|
@ -592,7 +588,7 @@ class Comment < ApplicationRecord
|
|||
save(validate: false)
|
||||
Comment.record_timestamps = true
|
||||
|
||||
story.update_comments_count!
|
||||
story.update_cached_columns
|
||||
self.user.refresh_counts!
|
||||
end
|
||||
|
||||
|
|
|
@ -157,7 +157,7 @@ class Story < ApplicationRecord
|
|||
before_save :log_moderation
|
||||
before_save :fix_bogus_chars
|
||||
after_create :mark_submitter, :record_initial_upvote
|
||||
after_save :update_merged_into_story_comments, :recalculate_hotness!, :update_story_text
|
||||
after_save :update_cached_columns, :update_story_text
|
||||
|
||||
validate do
|
||||
if url.present?
|
||||
|
@ -270,8 +270,8 @@ class Story < ApplicationRecord
|
|||
|
||||
def self.recalculate_all_hotnesses!
|
||||
# do the front page first, since find_each can't take an order
|
||||
Story.order("id DESC").limit(100).each(&:recalculate_hotness!)
|
||||
Story.find_each(&:recalculate_hotness!)
|
||||
Story.order("id DESC").limit(100).each(&:update_cached_columns)
|
||||
Story.find_each(&:update_cached_columns)
|
||||
true
|
||||
end
|
||||
|
||||
|
@ -620,10 +620,6 @@ class Story < ApplicationRecord
|
|||
merged_story_id ? merged_into_story.try(:short_id) : nil
|
||||
end
|
||||
|
||||
def recalculate_hotness!
|
||||
update_column :hotness, calculated_hotness
|
||||
end
|
||||
|
||||
def record_initial_upvote
|
||||
Vote.vote_thusly_on_story_or_comment_for_user_because(1, id, nil, user_id, nil, false)
|
||||
end
|
||||
|
@ -812,17 +808,11 @@ class Story < ApplicationRecord
|
|||
end
|
||||
end
|
||||
|
||||
def update_comments_count!
|
||||
comments = merged_comments
|
||||
def update_cached_columns
|
||||
update_column :comments_count, merged_comments.active.count
|
||||
merged_into_story&.update_cached_columns
|
||||
|
||||
# calculate count after removing deleted comments and threads
|
||||
update_column :comments_count, (comments.count { |c| !c.is_gone? })
|
||||
update_merged_into_story_comments
|
||||
recalculate_hotness!
|
||||
end
|
||||
|
||||
def update_merged_into_story_comments
|
||||
merged_into_story&.update_comments_count!
|
||||
update_column :hotness, calculated_hotness
|
||||
end
|
||||
|
||||
def update_story_text
|
||||
|
|
|
@ -405,7 +405,7 @@ describe Story do
|
|||
end
|
||||
end
|
||||
|
||||
describe "#update_comments_count!" do
|
||||
describe "#update_cached_columns" do
|
||||
context "with a merged_into_story" do
|
||||
let(:merged_into_story) { create(:story) }
|
||||
let(:story) { create(:story, merged_into_story: merged_into_story) }
|
||||
|
@ -414,7 +414,7 @@ describe Story do
|
|||
expect(story.comments_count).to eq 0
|
||||
expect(merged_into_story.comments_count).to eq 0
|
||||
create(:comment, story: story)
|
||||
story.update_comments_count!
|
||||
story.update_cached_columns
|
||||
expect(story.comments_count).to eq 1
|
||||
expect(merged_into_story.comments_count).to eq 1
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue