FIX: Prevent "uploads are missing in S3" alerts after restoring a backup

FIX: Prevent “uploads are missing in S3” alerts after restoring a backup

After restoring a backup it takes up to 48 hours for uploads stored on S3 to appear in the S3 inventory. This change prevents alerts about missing uploads by preventing the EnsureS3UploadsExistence job from running in the first 48 hours after a restore. During the restore it deletes the count of missing uploads from the PluginStore, so that an alert isn’t triggered by an old number.

diff --git a/app/jobs/scheduled/ensure_s3_uploads_existence.rb b/app/jobs/scheduled/ensure_s3_uploads_existence.rb
index 99a99ed..8a8e906 100644
--- a/app/jobs/scheduled/ensure_s3_uploads_existence.rb
+++ b/app/jobs/scheduled/ensure_s3_uploads_existence.rb
@@ -5,6 +5,8 @@ module Jobs
   class EnsureS3UploadsExistence < ::Jobs::Scheduled
     every 1.day
 
+    WAIT_AFTER_RESTORE_HOURS = 48
+
     def perform(*args)
       super
     ensure
@@ -24,7 +26,7 @@ module Jobs
     end
 
     def execute(args)
-      return unless SiteSetting.enable_s3_inventory
+      return if !executable?
       require 's3_inventory'
 
       if !@db_inventories && Rails.configuration.multisite && GlobalSetting.use_s3?
@@ -42,5 +44,15 @@ module Jobs
         S3Inventory.new(s3_helper, :upload).backfill_etags_and_list_missing
       end
     end
+
+    def executable?
+      return false if !SiteSetting.enable_s3_inventory
+
+      if last_restore_date = BackupMetadata.last_restore_date
+        return false if last_restore_date > WAIT_AFTER_RESTORE_HOURS.hours.ago
+      end
+
+      true
+    end
   end
 end
diff --git a/app/models/backup_metadata.rb b/app/models/backup_metadata.rb
index b7f3a1c..3fdcd07 100644
--- a/app/models/backup_metadata.rb
+++ b/app/models/backup_metadata.rb
@@ -6,6 +6,16 @@ class BackupMetadata < ActiveRecord::Base
   def self.value_for(name)
     where(name: name).pluck_first(:value).presence
   end
+
+  def self.last_restore_date
+    value = value_for(LAST_RESTORE_DATE)
+    value.present? ? Time.zone.parse(value) : nil
+  end
+
+  def self.update_last_restore_date(time = Time.zone.now)
+    BackupMetadata.where(name: LAST_RESTORE_DATE).delete_all
+    BackupMetadata.create!(name: LAST_RESTORE_DATE, value: time.iso8601)
+  end
 end
 
 # == Schema Information
diff --git a/lib/backup_restore/database_restorer.rb b/lib/backup_restore/database_restorer.rb
index 40c1459..2ffe09e 100644
--- a/lib/backup_restore/database_restorer.rb
+++ b/lib/backup_restore/database_restorer.rb
@@ -27,7 +27,7 @@ module BackupRestore
       migrate_database
       reconnect_database
 
-      self.class.update_last_restore_date
+      BackupMetadata.update_last_restore_date
     end
 
     def rollback
@@ -51,14 +51,6 @@ module BackupRestore
       end
     end
 
-    def self.update_last_restore_date
-      BackupMetadata.where(name: BackupMetadata::LAST_RESTORE_DATE).delete_all
-      BackupMetadata.create!(
-        name: BackupMetadata::LAST_RESTORE_DATE,
-        value: Time.zone.now.iso8601
-      )
-    end
-
     protected
 
     def restore_dump
@@ -208,14 +200,11 @@ module BackupRestore
     def self.backup_schema_dropable?
       return false unless ActiveRecord::Base.connection.schema_exists?(BACKUP_SCHEMA)
 
-      last_restore_date = BackupMetadata.value_for(BackupMetadata::LAST_RESTORE_DATE)
-
-      if last_restore_date.present?
-        last_restore_date = Time.zone.parse(last_restore_date)
+      if last_restore_date = BackupMetadata.last_restore_date
         return last_restore_date + DROP_BACKUP_SCHEMA_AFTER_DAYS.days < Time.zone.now
       end
 
-      update_last_restore_date
+      BackupMetadata.update_last_restore_date
       false
     end
     private_class_method :backup_schema_dropable?
diff --git a/lib/backup_restore/restorer.rb b/lib/backup_restore/restorer.rb
index 30f4c09..a6aba0b 100644
--- a/lib/backup_restore/restorer.rb
+++ b/lib/backup_restore/restorer.rb
@@ -55,6 +55,7 @@ module BackupRestore
       clear_category_cache
       clear_emoji_cache
       clear_theme_cache
+      clear_stats
       reload_translations
 
       @uploads_restorer.restore(@tmp_directory)
@@ -170,6 +171,10 @@ module BackupRestore
       Stylesheet::Manager.cache.clear
     end
 
+    def clear_stats
+      Discourse.stats.remove("missing_s3_uploads")
+    end
+
     def after_restore_hook
       log "Executing the after_restore_hook..."
       DiscourseEvent.trigger(:restore_complete)
diff --git a/spec/jobs/ensure_s3_uploads_existence_spec.rb b/spec/jobs/ensure_s3_uploads_existence_spec.rb
new file mode 100644
index 0000000..baaf1a4
--- /dev/null
+++ b/spec/jobs/ensure_s3_uploads_existence_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Jobs::EnsureS3UploadsExistence do
+  context "S3 inventory enabled" do
+    before do
+      SiteSetting.enable_s3_uploads = true
+      SiteSetting.s3_access_key_id = "abc"
+      SiteSetting.s3_secret_access_key = "def"
+      SiteSetting.enable_s3_inventory = true
+    end
+
+    it "works" do
+      S3Inventory.any_instance.expects(:backfill_etags_and_list_missing).once
+      subject.execute({})
+    end
+
+    it "doesn't execute when the site was restored within the last 48 hours" do
+      S3Inventory.any_instance.expects(:backfill_etags_and_list_missing).never
+      BackupMetadata.update_last_restore_date(47.hours.ago)
+
+      subject.execute({})
+    end
+
+    it "executes when the site was restored more than 48 hours ago" do
+      S3Inventory.any_instance.expects(:backfill_etags_and_list_missing).once
+      BackupMetadata.update_last_restore_date(49.hours.ago)
+
+      subject.execute({})
+    end
+  end
+
+  context "S3 inventory disabled" do
+    before { SiteSetting.enable_s3_inventory = false }
+
+    it "doesn't execute" do
+      S3Inventory.any_instance.expects(:backfill_etags_and_list_missing).never
+      subject.execute({})
+    end
+  end
+end
diff --git a/spec/lib/backup_restore/database_restorer_spec.rb b/spec/lib/backup_restore/database_restorer_spec.rb
index 09c2038..a11c8cc 100644
--- a/spec/lib/backup_restore/database_restorer_spec.rb
+++ b/spec/lib/backup_restore/database_restorer_spec.rb
@@ -249,20 +249,14 @@ describe BackupRestore::DatabaseRestorer do
 
       it "drops the schema when the last restore was long ago" do
         ActiveRecord::Base.connection.expects(:drop_schema).with("backup")
-
-        freeze_time(8.days.ago) do
-          subject.update_last_restore_date
-        end
+        BackupMetadata.update_last_restore_date(8.days.ago)
 
         subject.drop_backup_schema
       end
 
       it "doesn't drop the schema when the last restore was recently" do
         ActiveRecord::Base.connection.expects(:drop_schema).with("backup").never
-
-        freeze_time(6.days.ago) do
-          subject.update_last_restore_date
-        end
+        BackupMetadata.update_last_restore_date(6.days.ago)
 
         subject.drop_backup_schema
       end

GitHub sha: ac70c48b