From 1670ffe82dbfa46f33df94312a8649ab8117d19b Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 10 Dec 2024 06:29:46 +0800 Subject: [PATCH] DEV: Remove unnecessary thread in `Jobs::Base::JobInstrumenter` (#30179) In `Jobs::Base::JobInstrumenter.raw_log`, we were creating an instance of `Queue` and then pushing messages to the queue before popping it off the queue in a thread. However, this complexity is not necessary when we can just write directly to the logger without much overhead. This is how all logging is done in other parts of the app as well. --- app/jobs/base.rb | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/app/jobs/base.rb b/app/jobs/base.rb index bf5ad4f64bc..1480aedd103 100644 --- a/app/jobs/base.rb +++ b/app/jobs/base.rb @@ -44,6 +44,7 @@ module Jobs class JobInstrumenter def initialize(job_class:, opts:, db:, jid:) return unless enabled? + self.class.mutex.synchronize do @data = {} @@ -75,6 +76,7 @@ module Jobs def stop(exception:) return unless enabled? + self.class.mutex.synchronize do profile = MethodProfiler.stop @@ -102,30 +104,15 @@ module Jobs end def self.raw_log(message) - @@logger ||= - begin - f = File.open "#{Rails.root}/log/sidekiq.log", "a" - f.sync = true - Logger.new f - end - - @@log_queue ||= Queue.new - - if !defined?(@@log_thread) || !@@log_thread.alive? - @@log_thread = - Thread.new do - loop do - @@logger << @@log_queue.pop - rescue Exception => e - Discourse.warn_exception( - e, - message: "Exception encountered while logging Sidekiq job", - ) - end - end + begin + logger << message + rescue => e + Discourse.warn_exception(e, message: "Exception encountered while logging Sidekiq job") end + end - @@log_queue.push(message) + def self.logger + @@logger ||= Logger.new("#{Rails.root}/log/sidekiq.log") end def current_duration @@ -259,6 +246,7 @@ module Jobs requeued = true return end + parent_thread = Thread.current cluster_concurrency_redis_key = self.class.cluster_concurrency_redis_key @@ -343,6 +331,7 @@ module Jobs keepalive_thread.join self.class.clear_cluster_concurrency_lock! end + ActiveRecord::Base.connection_handler.clear_active_connections! end end