edshot99/eBooru
1
0
Fork 0
forked from e621ng/e621ng
Code Issues 11 Releases 1 Activity

[Posts] Relax voting isolation level and remove lock

Browse Source 
"The serialization of the vote and favorite actions was likely excessive
 even in the production configuration, and it has been relaxed to use
 repeatable_read in this implementation. The locks on rows were also
 removed as they do not have strict locking requirements with this transaction
 level to my knowledge."
This commit is contained in:
Earlopain 2023-01-28 15:37:20 +01:00
parent b50bc1fdd5
commit 902e35673f
No known key found for this signature in database
GPG Key ID: 6CFB948E15246897
3 changed files with 62 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
app/logical/favorite_manager.rb
View File

@ -1,9 +1,10 @@
class FavoriteManager
def self.add!(user:, post:, force: false, isolation: true)
ISOLATION = Rails.env.test? ? {} : { isolation: :repeatable_read }
def self.add!(user:, post:, force: false)
retries = 5
begin
target_isolation = Rails.env.test? || !isolation ? {} : {isolation: :serializable}
Favorite.transaction(**target_isolation) do
Favorite.transaction(**ISOLATION) do
unless force
if user.favorite_count >= user.favorite_limit
raise Favorite::Error, "You can only keep up to #{user.favorite_limit} favorites."
@ -24,13 +25,12 @@ class FavoriteManager
end
end
def self.remove!(user:, post:, post_id: nil, isolation: true)
def self.remove!(user:, post:, post_id: nil)
post_id = post ? post.id : post_id
raise Favorite::Error, "Must specify a post or post_id to remove favorite" unless post_id
retries = 5
begin
target_isolation = Rails.env.test? || !isolation ? {} : {isolation: :serializable}
Favorite.transaction(**target_isolation) do
Favorite.transaction(**ISOLATION) do
unless Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post_id).exists?
return
end
@ -47,15 +47,15 @@ class FavoriteManager
end
end
def self.give_to_parent!(post, isolation: true)
def self.give_to_parent!(post)
# TODO Much better and more intelligent logic can exist for this
parent = post.parent
return false unless parent
post.favorites.each do |fav|
tries = 5
begin
FavoriteManager.remove!(user: fav.user, post: post, isolation: isolation)
FavoriteManager.add!(user: fav.user, post: parent, force: true, isolation: isolation)
FavoriteManager.remove!(user: fav.user, post: post)
FavoriteManager.add!(user: fav.user, post: parent, force: true)
rescue ActiveRecord::SerializationFailure
tries -= 1
retry if tries > 0

114
app/logical/vote_manager.rb
View File

@ -1,4 +1,6 @@
class VoteManager
ISOLATION = Rails.env.test? ? {} : { isolation: :repeatable_read }
def self.vote!(user:, post:, score:)
@vote = nil
retries = 5
@ -6,36 +8,33 @@ class VoteManager
begin
raise UserVote::Error.new("Invalid vote") unless [1, -1].include?(score)
raise UserVote::Error.new("You do not have permission to vote") unless user.is_voter?
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
PostVote.transaction(**target_isolation) do
PostVote.transaction(**ISOLATION) do
PostVote.uncached do
post.with_lock do
score_modifier = score
old_vote = PostVote.where(user_id: user.id, post_id: post.id).first
if old_vote
raise UserVote::Error.new("Vote is locked") if old_vote.score == 0
if old_vote.score == score
return :need_unvote
else
score_modifier *= 2
end
old_vote.destroy
end
@vote = vote = PostVote.create!(user: user, score: score, post: post)
vote_cols = "score = score + #{score_modifier}"
if vote.score > 0
vote_cols += ", up_score = up_score + #{vote.score}"
vote_cols += ", down_score = down_score - #{old_vote.score}" if old_vote
score_modifier = score
old_vote = PostVote.where(user_id: user.id, post_id: post.id).first
if old_vote
raise UserVote::Error.new("Vote is locked") if old_vote.score == 0
if old_vote.score == score
return :need_unvote
else
vote_cols += ", down_score = down_score + #{vote.score}"
vote_cols += ", up_score = up_score - #{old_vote.score}" if old_vote
score_modifier *= 2
end
Post.where(id: post.id).update_all(vote_cols)
post.reload
old_vote.destroy
end
@vote = vote = PostVote.create!(user: user, score: score, post: post)
vote_cols = "score = score + #{score_modifier}"
if vote.score > 0
vote_cols += ", up_score = up_score + #{vote.score}"
vote_cols += ", down_score = down_score - #{old_vote.score}" if old_vote
else
vote_cols += ", down_score = down_score + #{vote.score}"
vote_cols += ", up_score = up_score - #{old_vote.score}" if old_vote
end
Post.where(id: post.id).update_all(vote_cols)
post.reload
end
end
rescue ActiveRecord::SerializationFailure
rescue ActiveRecord::SerializationFailure => e
retries -= 1
retry if retries > 0
raise UserVote::Error.new("Failed to vote, please try again later")
@ -49,17 +48,14 @@ class VoteManager
def self.unvote!(user:, post:, force: false)
retries = 5
begin
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
PostVote.transaction(**target_isolation) do
PostVote.transaction(**ISOLATION) do
PostVote.uncached do
post.with_lock do
vote = PostVote.where(user_id: user.id, post_id: post.id).first
return unless vote
raise UserVote::Error.new "You can't remove locked votes" if vote.score == 0 && !force
post.votes.where(user: user).delete_all
subtract_vote(post, vote)
post.reload
end
vote = PostVote.where(user_id: user.id, post_id: post.id).first
return unless vote
raise UserVote::Error.new "You can't remove locked votes" if vote.score == 0 && !force
post.votes.where(user: user).delete_all
subtract_vote(post, vote)
post.reload
end
end
rescue ActiveRecord::SerializationFailure
@ -72,15 +68,14 @@ class VoteManager
def self.lock!(id)
post = nil
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
PostVote.transaction(**target_isolation) do
PostVote.transaction(**ISOLATION) do
vote = PostVote.find_by(id: id)
return unless vote
post = vote.post
subtract_vote(post, vote)
vote.update_column(:score, 0)
end
post.update_index if post
post&.update_index
end
def self.admin_unvote!(id)
@ -95,24 +90,21 @@ class VoteManager
begin
raise UserVote::Error.new("Invalid vote") unless [1, -1].include?(score)
raise UserVote::Error.new("You do not have permission to vote") unless user.is_voter?
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
CommentVote.transaction(**target_isolation) do
CommentVote.transaction(**ISOLATION) do
CommentVote.uncached do
comment.with_lock do
score_modifier = score
old_vote = CommentVote.where(user_id: user.id, comment_id: comment.id).first
if old_vote
raise UserVote::Error.new("Vote is locked") if old_vote.score == 0
if old_vote.score == score
return :need_unvote
else
score_modifier *= 2
end
old_vote.destroy
score_modifier = score
old_vote = CommentVote.where(user_id: user.id, comment_id: comment.id).first
if old_vote
raise UserVote::Error.new("Vote is locked") if old_vote.score == 0
if old_vote.score == score
return :need_unvote
else
score_modifier *= 2
end
@vote = CommentVote.create!(user_id: user.id, score: score, comment_id: comment.id)
Comment.where(id: comment.id).update_all("score = score + #{score_modifier}")
old_vote.destroy
end
@vote = CommentVote.create!(user_id: user.id, score: score, comment_id: comment.id)
Comment.where(id: comment.id).update_all("score = score + #{score_modifier}")
end
end
rescue ActiveRecord::SerializationFailure
@ -126,23 +118,19 @@ class VoteManager
end
def self.comment_unvote!(user:, comment:, force: false)
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
CommentVote.transaction(**target_isolation) do
CommentVote.transaction(**ISOLATION) do
CommentVote.uncached do
comment.with_lock do
vote = CommentVote.where(user_id: user.id, comment_id: comment.id).first
return unless vote
raise UserVote::Error.new("You can't remove locked votes") if vote.score == 0 && !force
CommentVote.where(user_id: user.id, comment_id: comment.id).delete_all
Comment.where(id: comment.id).update_all("score = score - #{vote.score}")
end
vote = CommentVote.where(user_id: user.id, comment_id: comment.id).first
return unless vote
raise UserVote::Error.new("You can't remove locked votes") if vote.score == 0 && !force
CommentVote.where(user_id: user.id, comment_id: comment.id).delete_all
Comment.where(id: comment.id).update_all("score = score - #{vote.score}")
end
end
end
def self.comment_lock!(id)
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
CommentVote.transaction(**target_isolation) do
CommentVote.transaction(**ISOLATION) do
vote = CommentVote.find_by(id: id)
return unless vote
comment = vote.comment

4
test/unit/post_test.rb
View File

@ -23,7 +23,7 @@ class PostTest < ActiveSupport::TestCase
Sidekiq::Testing.fake!
@upload = UploadService.new(attributes_for(:jpg_upload)).start!
@post = @upload.post
FavoriteManager.add!(user: @post.uploader, post: @post, isolation: false)
FavoriteManager.add!(user: @post.uploader, post: @post)
end
should "delete the files" do
@ -1570,7 +1570,7 @@ class PostTest < ActiveSupport::TestCase
posts = users.map do |u|
as(u) do
post = create(:post, tag_string: "abc")
FavoriteManager.add!(user: u, post: post, isolation: false)
FavoriteManager.add!(user: u, post: post)
post
end
end