FIX: Throttle collecting of uploads missing metrics.

FIX: Throttle collecting of uploads missing metrics.

Instead of 2 DB calls every 5 seconds, we’ll only do 2 DB calls every 60 seconds. Reduces the number of DB queries per day from 34_560 to 2880.

diff --git a/lib/internal_metric/global.rb b/lib/internal_metric/global.rb
index 98955e3..0034d17 100644
--- a/lib/internal_metric/global.rb
+++ b/lib/internal_metric/global.rb
@@ -116,6 +116,11 @@ module DiscoursePrometheus::InternalMetric
       @missing_post_uploads = missing_uploads("post")
     end
 
+    # For testing purposes
+    def reset!
+      @@missing_uploads = nil
+    end
+
     private
 
     def primary_site_readonly?
@@ -180,14 +185,21 @@ module DiscoursePrometheus::InternalMetric
       paused
     end
 
+    MISSING_UPLOADS_CHECK_SECONDS = 60
+
     def missing_uploads(type)
-      missing = {}
+      @@missing_uploads ||= {}
+      @@missing_uploads[type] ||= {}
+      @@missing_uploads[type][:stats] ||= {}
+      last_check = @@missing_uploads[type][:last_check]
 
-      if Discourse.respond_to?(:stats)
+      if Discourse.respond_to?(:stats) && (!last_check || (Time.now.to_i - last_check > MISSING_UPLOADS_CHECK_SECONDS))
         begin
           RailsMultisite::ConnectionManagement.each_connection do |db|
-            missing[{ db: db }] = Discourse.stats.get("missing_#{type}_uploads")
+            @@missing_uploads[type][:stats][{ db: db }] = Discourse.stats.get("missing_#{type}_uploads")
           end
+
+          @@missing_uploads[type][:last_check] = Time.now.to_i
         rescue => e
           if @postgres_master_available == 1
             Discourse.warn_exception(e, message: "Failed to connect to database to collect upload stats")
@@ -198,7 +210,7 @@ module DiscoursePrometheus::InternalMetric
         end
       end
 
-      missing
+      @@missing_uploads[type][:stats]
     end
   end
 end
diff --git a/spec/lib/internal_metric/global_spec.rb b/spec/lib/internal_metric/global_spec.rb
index 331c8a0..ac8485a 100644
--- a/spec/lib/internal_metric/global_spec.rb
+++ b/spec/lib/internal_metric/global_spec.rb
@@ -5,9 +5,13 @@ require 'rails_helper'
 module DiscoursePrometheus::InternalMetric
   describe Global do
     let(:db) { RailsMultisite::ConnectionManagement.current_db }
+    let(:metric) { Global.new }
+
+    after do
+      metric.reset!
+    end
 
     it "can collect global metrics" do
-      metric = Global.new
       metric.collect
 
       expect(metric.sidekiq_processes).not_to eq(nil)
@@ -19,7 +23,6 @@ module DiscoursePrometheus::InternalMetric
       Discourse.stats.set("missing_s3_uploads", 2)
       Discourse.stats.set("missing_post_uploads", 1)
 
-      metric = Global.new
       metric.collect
 
       expect(metric.missing_s3_uploads).to eq(
@@ -30,13 +33,36 @@ module DiscoursePrometheus::InternalMetric
       )
     end
 
+    it 'should throttle the collection of missing upload metrics' do
+      Discourse.stats.set("missing_s3_uploads", 2)
+
+      metric.collect
+
+      expect(metric.missing_s3_uploads).to eq(
+        { db: db } => 2
+      )
+
+      Discourse.stats.set("missing_s3_uploads", 0)
+      metric.collect
+
+      expect(metric.missing_s3_uploads).to eq(
+        { db: db } => 2
+      )
+
+      metric.reset!
+      metric.collect
+
+      expect(metric.missing_s3_uploads).to eq(
+        { db: db } => 0
+      )
+    end
+
     describe 'sidekiq paused' do
       after do
         Sidekiq.unpause_all!
       end
 
       it "should collect the right metrics" do
-        metric = Global.new
         metric.collect
 
         expect(metric.sidekiq_paused).to eq(
@@ -63,7 +89,6 @@ module DiscoursePrometheus::InternalMetric
       end
 
       it 'should collect the right metrics' do
-        metric = Global.new
         metric.collect
 
         expect(metric.postgres_master_available).to eq(1)

GitHub sha: 7323bb9f

1 Like