From 2841f0742cab18e2899ee028bdc868e851a94715 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Mon, 2 Sep 2019 20:42:01 -0500
Subject: [PATCH] saved searches: fix failure when search returns no results.

* Don't try to call `sadd` when a search returns no results (`sadd`
  fails in this case).
* Add a timeout when populating the search.
* Don't offload the search to read replica. The main db is fine.
* Disable synchronous population of searches. This was too slow.
---
 app/models/application_record.rb |  2 +-
 app/models/saved_search.rb       | 20 ++++++++++----------
 test/unit/saved_search_test.rb   | 17 +++++++++++++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 28158b141..6d5ab6320 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -266,7 +266,7 @@ class ApplicationRecord < ActiveRecord::Base
         connection.execute("SET STATEMENT_TIMEOUT = #{n}") unless Rails.env == "test"
         yield
       rescue ::ActiveRecord::StatementInvalid => x
-        DanbooruLogger.log(x, expected: true)
+        DanbooruLogger.log(x, expected: false, **new_relic_params)
         return default_value
       ensure
         connection.execute("SET STATEMENT_TIMEOUT = #{CurrentUser.user.try(:statement_timeout) || 3_000}") unless Rails.env == "test"
diff --git a/app/models/saved_search.rb b/app/models/saved_search.rb
index 5d5fa3a8e..ba61f089e 100644
--- a/app/models/saved_search.rb
+++ b/app/models/saved_search.rb
@@ -19,18 +19,12 @@ class SavedSearch < ApplicationRecord
         label = normalize_label(label) if label
         queries = queries_for(user_id, label: label)
         post_ids = Set.new
-        update_count = 0
         queries.each do |query|
           redis_key = "search:#{query}"
           if redis.exists(redis_key)
             sub_ids = redis.smembers(redis_key).map(&:to_i)
             post_ids.merge(sub_ids)
             redis.expire(redis_key, REDIS_EXPIRY)
-          elsif CurrentUser.is_gold? && update_count < 5
-            SavedSearch.populate(query)
-            sub_ids = redis.smembers(redis_key).map(&:to_i)
-            post_ids.merge(sub_ids)
-            update_count += 1
           else
             PopulateSavedSearchJob.perform_later(query)
           end
@@ -104,13 +98,19 @@ class SavedSearch < ApplicationRecord
         q.apply_default_order(params)
       end
 
-      def populate(query)
+      def populate(query, timeout: 10_000)
         CurrentUser.as_system do
           redis_key = "search:#{query}"
           return if redis.exists(redis_key)
-          post_ids = Post.tag_match(query, read_only: Rails.env.production?).limit(QUERY_LIMIT).pluck(:id)
-          redis.sadd(redis_key, post_ids)
-          redis.expire(redis_key, REDIS_EXPIRY)
+
+          post_ids = Post.with_timeout(timeout, [], query: query) do
+            Post.tag_match(query).limit(QUERY_LIMIT).pluck(:id)
+          end
+
+          if post_ids.present?
+            redis.sadd(redis_key, post_ids)
+            redis.expire(redis_key, REDIS_EXPIRY)
+          end
         end
       end
     end
diff --git a/test/unit/saved_search_test.rb b/test/unit/saved_search_test.rb
index 9ba44497a..0c0dc1e2b 100644
--- a/test/unit/saved_search_test.rb
+++ b/test/unit/saved_search_test.rb
@@ -123,6 +123,23 @@ class SavedSearchTest < ActiveSupport::TestCase
     end
   end
 
+  context "Populating a saved search" do
+    setup do
+      @saved_search = create(:saved_search, query: "bkub", user: @user)
+      @post = create(:post, tag_string: "bkub")
+    end
+
+    should "work for a single tag search" do
+      SavedSearch.populate("bkub")
+      assert_equal([@post.id], SavedSearch.post_ids_for(@user.id))
+    end
+
+    should "work for a tag search returning no posts" do
+      SavedSearch.populate("does_not_exist")
+      assert_equal([], SavedSearch.post_ids_for(@user.id))
+    end
+  end
+
   context "Creating a saved search" do
     setup do
       FactoryBot.create(:tag_alias, antecedent_name: "zzz", consequent_name: "yyy", creator: @user)