diff --git a/app/models/concerns/cached_counting.rb b/app/models/concerns/cached_counting.rb index 7598c7be22b..1e2e84f0c25 100644 --- a/app/models/concerns/cached_counting.rb +++ b/app/models/concerns/cached_counting.rb @@ -83,9 +83,11 @@ module CachedCounting end def self.flush - @flush = true - @thread.wakeup - sleep 0.001 while @flush + if @thread + @flush = true + @thread.wakeup + sleep 0.001 while @flush + end end COUNTER_REDIS_HASH = "CounterCacheHash" @@ -163,9 +165,30 @@ module CachedCounting end class_methods do - def perform_increment!(key) - CachedCounting.ensure_thread! - CachedCounting.queue(key, self) + if Rails.env.test? + # perform increment is a risky call in test, + # it shifts stuff to background threads and leaks + # data in the DB + # Require caller is deliberate if they want that + # + # Splitting implementation to avoid any perf impact + # given this is a method that is called a lot + def perform_increment!(key, async: false) + if async + CachedCounting.ensure_thread! + CachedCounting.queue(key, self) + else + CachedCounting.queue(key, self) + CachedCounting.clear_flush_to_db_lock! + CachedCounting.flush_in_memory + CachedCounting.flush_to_db + end + end + else + def perform_increment!(key) + CachedCounting.ensure_thread! + CachedCounting.queue(key, self) + end end def write_cache!(key, count, date) diff --git a/spec/lib/concern/cached_counting_spec.rb b/spec/lib/concern/cached_counting_spec.rb index 041a4c3c24c..64ff9afc56c 100644 --- a/spec/lib/concern/cached_counting_spec.rb +++ b/spec/lib/concern/cached_counting_spec.rb @@ -80,15 +80,15 @@ RSpec.describe CachedCounting do freeze_time d1 = Time.now.utc.to_date - RailsCacheCounter.perform_increment!("a,a") - RailsCacheCounter.perform_increment!("b") - 20.times { RailsCacheCounter.perform_increment!("a,a") } + RailsCacheCounter.perform_increment!("a,a", async: true) + RailsCacheCounter.perform_increment!("b", async: true) + 20.times { RailsCacheCounter.perform_increment!("a,a", async: true) } freeze_time 2.days.from_now d2 = Time.now.utc.to_date - RailsCacheCounter.perform_increment!("a,a") - RailsCacheCounter.perform_increment!("d") + RailsCacheCounter.perform_increment!("a,a", async: true) + RailsCacheCounter.perform_increment!("d", async: true) CachedCounting.flush diff --git a/spec/lib/middleware/request_tracker_spec.rb b/spec/lib/middleware/request_tracker_spec.rb index 7c4c7720a7d..ddd8d0e147c 100644 --- a/spec/lib/middleware/request_tracker_spec.rb +++ b/spec/lib/middleware/request_tracker_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Middleware::RequestTracker do end after do + CachedCounting.reset ApplicationRequest.disable CachedCounting.disable end diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index d71e86086e4..076d5b846ca 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -1309,9 +1309,11 @@ RSpec.describe Report do end after do + CachedCounting.reset ApplicationRequest.disable CachedCounting.disable end + it "works" do 3.times { ApplicationRequest.increment!(:page_view_crawler) } 2.times { ApplicationRequest.increment!(:page_view_logged_in) } diff --git a/spec/models/web_crawler_request_spec.rb b/spec/models/web_crawler_request_spec.rb index 12e6b44cfec..afa9870d68c 100644 --- a/spec/models/web_crawler_request_spec.rb +++ b/spec/models/web_crawler_request_spec.rb @@ -6,7 +6,10 @@ RSpec.describe WebCrawlerRequest do CachedCounting.enable end - after { CachedCounting.disable } + after do + CachedCounting.reset + CachedCounting.disable + end it "can log crawler requests" do freeze_time