From 864b7b6bc86edb7332e1cc07933967809cc78962 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 11 Dec 2024 10:12:58 +0800 Subject: [PATCH] DEV: Fix flaky test (#30215) The test was flaky and failing with the following errors: ``` Failure/Error: klass .connection .select_raw(relation.arel) do |result, _| result.type_map = DB.type_map result.nfields == 1 ? result.column_values(0) : result.values end NoMethodError: undefined method `select_raw' for nil ./lib/freedom_patches/fast_pluck.rb:60:in `pluck' ./vendor/bundle/ruby/3.3.0/gems/activerecord-7.2.2.1/lib/active_record/relation/calculations.rb:354:in `pick' ./app/models/web_crawler_request.rb:27:in `request_id' ./app/models/web_crawler_request.rb:31:in `rescue in request_id' ./app/models/web_crawler_request.rb:26:in `request_id' ./app/models/web_crawler_request.rb:19:in `write_cache!' ./app/models/concerns/cached_counting.rb:135:in `block (3 levels) in flush_to_db' ./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management/null_instance.rb:49:in `with_connection' ./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management.rb:21:in `with_connection' ./app/models/concerns/cached_counting.rb:134:in `block (2 levels) in flush_to_db' ./app/models/concerns/cached_counting.rb:124:in `each' ./app/models/concerns/cached_counting.rb:124:in `block in flush_to_db' ./lib/distributed_mutex.rb:53:in `block in synchronize' ./lib/distributed_mutex.rb:49:in `synchronize' ./lib/distributed_mutex.rb:49:in `synchronize' ./lib/distributed_mutex.rb:34:in `synchronize' ./app/models/concerns/cached_counting.rb:120:in `flush_to_db' ./app/models/concerns/cached_counting.rb:187:in `perform_increment!' ./app/models/web_crawler_request.rb:15:in `increment!' ./lib/middleware/request_tracker.rb:74:in `log_request' ./lib/middleware/request_tracker.rb:409:in `block in log_later' ./lib/scheduler/defer.rb:125:in `block in do_work' ./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management/null_instance.rb:49:in `with_connection' ./vendor/bundle/ruby/3.3.0/gems/rails_multisite-6.1.0/lib/rails_multisite/connection_management.rb:21:in `with_connection' ./lib/scheduler/defer.rb:119:in `do_work' ./lib/scheduler/defer.rb:105:in `block (2 levels) in start_thread' ``` This was due to running the defer thread in an async manner which is actually no representative of the production environment. It also revealed a spot in our code base where writes are happening in a GET request which can cause requests to fail if ActiveRecord is in readonly mode. --- lib/middleware/request_tracker.rb | 6 +++++- spec/integration/activerecord_preventing_writes_spec.rb | 7 ------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb index 20ad515f98b..9f22b9c54cc 100644 --- a/lib/middleware/request_tracker.rb +++ b/lib/middleware/request_tracker.rb @@ -406,7 +406,11 @@ class Middleware::RequestTracker def log_later(data) Scheduler::Defer.later("Track view") do - self.class.log_request(data) unless Discourse.pg_readonly_mode? + begin + self.class.log_request(data) unless Discourse.pg_readonly_mode? + rescue ActiveRecord::ReadOnlyError + # Just noop if ActiveRecord is preventing writes + end end end diff --git a/spec/integration/activerecord_preventing_writes_spec.rb b/spec/integration/activerecord_preventing_writes_spec.rb index 16bddce4a3b..e9989a6c8b0 100644 --- a/spec/integration/activerecord_preventing_writes_spec.rb +++ b/spec/integration/activerecord_preventing_writes_spec.rb @@ -1,13 +1,6 @@ # frozen_string_literal: true RSpec.describe "When ActiveRecord is preventing writes" do - before do - @original_async = Scheduler::Defer.async - Scheduler::Defer.async = true - end - - after { Scheduler::Defer.async = @original_async } - it "should not result in an error response when there is a theme field that needs to be baked" do theme_field = Fabricate(