From 73f257ec638e3c12d13c1218e3f89ae4fd5c612e Mon Sep 17 00:00:00 2001 From: r888888888 Date: Thu, 16 Nov 2017 13:32:20 -0800 Subject: [PATCH] disable manual post count expiration, rely solely on timed expiries (fixes #3376) --- app/logical/tag_correction.rb | 1 - app/models/post.rb | 98 ++++++++++----------------- app/models/tag.rb | 2 - test/unit/post_test.rb | 122 +++++++++++++--------------------- 4 files changed, 84 insertions(+), 139 deletions(-) diff --git a/app/logical/tag_correction.rb b/app/logical/tag_correction.rb index 292dfecd5..9094ea904 100644 --- a/app/logical/tag_correction.rb +++ b/app/logical/tag_correction.rb @@ -42,6 +42,5 @@ class TagCorrection def fix! tag.delay(:queue => "default").fix_post_count tag.update_category_cache_for_all - Post.expire_cache_for_all([tag.name]) end end diff --git a/app/models/post.rb b/app/models/post.rb index b1a20a5ae..50c0d6323 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -613,7 +613,6 @@ class Post < ApplicationRecord if decrement_tags.any? Tag.decrement_post_counts(decrement_tags) end - Post.expire_cache_for_all([""]) if new_record? || id <= 100_000 end def set_tag_count(category,tagcount) @@ -1137,50 +1136,10 @@ class Post < ApplicationRecord end module CountMethods - def fix_post_counts(post) - post.set_tag_counts(false) - if post.changed? - args = Hash[TagCategory.categories.map {|x| ["tag_count_#{x}",post.send("tag_count_#{x}")]}].update(:tag_count => post.tag_count) - post.update_columns(args) - end - end - - def get_count_from_cache(tags) - count = Cache.get(count_cache_key(tags)) - - if count.nil? && !CurrentUser.safe_mode? && !CurrentUser.hide_deleted_posts? - count = select_value_sql("SELECT post_count FROM tags WHERE name = ?", tags.to_s) - end - - count - end - - def set_count_in_cache(tags, count, expiry = nil) - if expiry.nil? - if count < 100 - expiry = 1.minute - else - expiry = (count * 4).minutes - end - end - - Cache.put(count_cache_key(tags), count, expiry) - end - - def count_cache_key(tags) - if CurrentUser.safe_mode? - tags = "#{tags} rating:s".strip - end - - if CurrentUser.user && CurrentUser.hide_deleted_posts? && tags !~ /(?:^|\s)(?:-)?status:.+/ - tags = "#{tags} -status:deleted".strip - end - - "pfc:#{Cache.hash(tags)}" - end - def fast_count(tags = "", options = {}) - tags = tags.to_s.strip + tags += " rating:s" if CurrentUser.safe_mode? + tags += " -status:deleted" if CurrentUser.hide_deleted_posts? && tags !~ /(?:^|\s)(?:-)?status:.+/ + tags = Tag.normalize_query(tags) # optimize some cases. these are just estimates but at these # quantities being off by a few hundred doesn't matter much @@ -1205,11 +1164,11 @@ class Post < ApplicationRecord count = get_count_from_cache(tags) - if count.to_i == 0 + if count.nil? count = fast_count_search(tags, options) end - count.to_i + count rescue SearchError 0 end @@ -1219,17 +1178,18 @@ class Post < ApplicationRecord PostReadOnly.tag_match(tags).count end - if count == nil && tags !~ / / + if count.nil? count = fast_count_search_batched(tags, options) end - if count - set_count_in_cache(tags, count) - else + if count.nil? + # give up count = Danbooru.config.blank_tag_search_fast_count + else + set_count_in_cache(tags, count) end - count + count ? count.to_i : nil end def fast_count_search_batched(tags, options) @@ -1248,20 +1208,36 @@ class Post < ApplicationRecord end sum end - end - module CacheMethods - def expire_cache_for_all(tag_names) - expire_cache(tag_names) - Danbooru.config.other_server_hosts.each do |host| - delay(:queue => host).expire_cache(tag_names) + def fix_post_counts(post) + post.set_tag_counts(false) + if post.changed? + args = Hash[TagCategory.categories.map {|x| ["tag_count_#{x}",post.send("tag_count_#{x}")]}].update(:tag_count => post.tag_count) + post.update_columns(args) end end - def expire_cache(tag_names) - tag_names.each do |tag_name| - Cache.delete(Post.count_cache_key(tag_name)) + def get_count_from_cache(tags) + count = Cache.get(count_cache_key(tags)) # this will only have a value for multi-tag searches + + if count.nil? && !tags.include?(" ") + count = select_value_sql("SELECT post_count FROM tags WHERE name = ?", tags.to_s) + # set_count_in_cache(tags, count.to_i) if count end + + count ? count.to_i : nil + end + + def set_count_in_cache(tags, count, expiry = nil) + if expiry.nil? + [count.seconds, 20.hours].min + end + + Cache.put(count_cache_key(tags), count, expiry) + end + + def count_cache_key(tags) + "pfc:#{Cache.hash(tags)}" end end @@ -1430,7 +1406,6 @@ class Post < ApplicationRecord self.approver_id = CurrentUser.id flags.each {|x| x.resolve!} save - Post.expire_cache_for_all(tag_array) ModAction.log("undeleted post ##{id}") end @@ -1746,7 +1721,6 @@ class Post < ApplicationRecord include PoolMethods include VoteMethods extend CountMethods - extend CacheMethods include ParentMethods include DeletionMethods include VersionMethods diff --git a/app/models/tag.rb b/app/models/tag.rb index 18d2f67a2..a71ec7807 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -59,12 +59,10 @@ class Tag < ApplicationRecord def increment_post_counts(tag_names) Tag.where(:name => tag_names).update_all("post_count = post_count + 1") - Post.expire_cache_for_all(tag_names) end def decrement_post_counts(tag_names) Tag.where(:name => tag_names).update_all("post_count = post_count - 1") - Post.expire_cache_for_all(tag_names) end def clean_up_negative_post_counts! diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 1b69fc95c..c56bfc51a 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -2378,92 +2378,66 @@ class PostTest < ActiveSupport::TestCase setup do Danbooru.config.stubs(:blank_tag_search_fast_count).returns(nil) Danbooru.config.stubs(:estimate_post_counts).returns(false) + FactoryGirl.create(:tag_alias, :antecedent_name => "alias", :consequent_name => "aaa") + FactoryGirl.create(:post, :tag_string => "aaa") end - context "with a primed cache" do - setup do - Cache.put("pfc:aaa", 0) - Cache.put("pfc:alias", 0) - Cache.put("pfc:width:50", 0) - Danbooru.config.stubs(:blank_tag_search_fast_count).returns(1_000_000) - FactoryGirl.create(:tag_alias, :antecedent_name => "alias", :consequent_name => "aaa") - FactoryGirl.create(:post, :tag_string => "aaa") + context "a blank search" do + should "should execute a search" do + Cache.delete(Post.count_cache_key('')) + Post.expects(:select_value_sql).with(kind_of(String), "").once.returns(nil) + Post.expects(:fast_count_search).with("", kind_of(Hash)).once.returns(1) + assert_equal(1, Post.fast_count("")) end - should "be counted correctly in fast_count" do - assert_equal(1, Post.count) - assert_equal(1, Post.fast_count("")) - assert_equal(1, Post.fast_count("aaa")) + should "set the value in cache" do + Post.expects(:set_count_in_cache).with("", kind_of(Integer)).once + Post.fast_count("") + end + + context "with a primed cache" do + setup do + Cache.put(Post.count_cache_key(''), "100") + end + + should "fetch the value from the cache" do + assert_equal(100, Post.fast_count("")) + end + end + + should "translate an alias" do assert_equal(1, Post.fast_count("alias")) + end + + should "return 0 for a nonexisting tag" do assert_equal(0, Post.fast_count("bbb")) end - end - should "increment the post count" do - assert_equal(0, Post.fast_count("")) - post = FactoryGirl.create(:post, :tag_string => "aaa bbb") - assert_equal(1, Post.fast_count("")) - assert_equal(1, Post.fast_count("aaa")) - assert_equal(1, Post.fast_count("bbb")) - assert_equal(0, Post.fast_count("ccc")) + context "in safe mode" do + setup do + CurrentUser.stubs(:safe_mode?).returns(true) + end - post.tag_string = "ccc" - post.save + should "execute a search" do + Post.expects(:select_value_sql).once.with(kind_of(String), "rating:s").returns(nil) + Post.expects(:fast_count_search).once.with("rating:s", kind_of(Hash)).returns(1) + assert_equal(1, Post.fast_count("")) + end - assert_equal(1, Post.fast_count("")) - assert_equal(0, Post.fast_count("aaa")) - assert_equal(0, Post.fast_count("bbb")) - assert_equal(1, Post.fast_count("ccc")) - end - end + should "set the value in cache" do + Post.expects(:set_count_in_cache).with("rating:s", kind_of(Integer)).once + Post.fast_count("") + end - context "The cache" do - context "when shared between users on danbooru/safebooru" do - setup do - Danbooru.config.stubs(:blank_tag_search_fast_count).returns(nil) - FactoryGirl.create(:post, :tag_string => "aaa bbb", :rating => "q") - FactoryGirl.create(:post, :tag_string => "aaa bbb", :rating => "s") - FactoryGirl.create(:post, :tag_string => "aaa bbb", :rating => "s") - CurrentUser.stubs(:safe_mode?).returns(true) - Post.fast_count("aaa") - CurrentUser.stubs(:safe_mode?).returns(false) - Post.fast_count("bbb") - end + context "with a primed cache" do + setup do + Cache.put(Post.count_cache_key('rating:s'), "100") + end - should "be accurate on danbooru" do - CurrentUser.stubs(:safe_mode?).returns(false) - assert_equal(3, Post.fast_count("aaa")) - assert_equal(3, Post.fast_count("bbb")) - end - - should "be accurate on safebooru" do - CurrentUser.stubs(:safe_mode?).returns(true) - assert_equal(2, Post.fast_count("aaa")) - assert_equal(2, Post.fast_count("bbb")) - end - end - - context "when shared between users with the deleted post filter on/off" do - setup do - FactoryGirl.create(:post, :tag_string => "aaa bbb", :is_deleted => true) - FactoryGirl.create(:post, :tag_string => "aaa bbb", :is_deleted => false) - FactoryGirl.create(:post, :tag_string => "aaa bbb", :is_deleted => false) - CurrentUser.user.stubs(:hide_deleted_posts?).returns(true) - Post.fast_count("aaa") - CurrentUser.user.stubs(:hide_deleted_posts?).returns(false) - Post.fast_count("bbb") - end - - should "be accurate with the deleted post filter on" do - CurrentUser.user.stubs(:hide_deleted_posts?).returns(true) - assert_equal(2, Post.fast_count("aaa")) - assert_equal(2, Post.fast_count("bbb")) - end - - should "be accurate with the deleted post filter off" do - CurrentUser.user.stubs(:hide_deleted_posts?).returns(false) - assert_equal(3, Post.fast_count("aaa")) - assert_equal(3, Post.fast_count("bbb")) + should "fetch the value from the cache" do + assert_equal(100, Post.fast_count("")) + end + end end end end