From 51e28dc3221509e084d51e5c948d6b76ee9688f5 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 17 Jan 2022 10:47:05 -0800 Subject: [PATCH] Remove child event tracking from AS::Subscriber Previously ActiveSupport::Subscriber would manage its own events, keep them in a thread-local stack, and track "child" events of the same subscriber. I don't think this was particularly useful, since it only considered notifications delivered to the same subscriber, and I can't find evidence of this being used in the wild. I think this was intended to support tracing, but any uch tool would want to do this itself (at minimum in order to support multiple namespaces). Additionally, this has caused a few users to OOM in production environments, notably when queueing many jobs from another job. --- .../lib/active_support/log_subscriber.rb | 8 ++----- .../lib/active_support/subscriber.rb | 24 ++----------------- activesupport/test/log_subscriber_test.rb | 4 +++- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/activesupport/lib/active_support/log_subscriber.rb b/activesupport/lib/active_support/log_subscriber.rb index 29a6941c04..e36a99f739 100644 --- a/activesupport/lib/active_support/log_subscriber.rb +++ b/activesupport/lib/active_support/log_subscriber.rb @@ -107,14 +107,10 @@ module ActiveSupport LogSubscriber.logger end - def start(name, id, payload) - super if logger - end - - def finish(name, id, payload) + def call(event) super if logger rescue => e - log_exception(name, e) + log_exception(event.name, e) end def publish_event(event) diff --git a/activesupport/lib/active_support/subscriber.rb b/activesupport/lib/active_support/subscriber.rb index 0f71443e42..078cd40cbc 100644 --- a/activesupport/lib/active_support/subscriber.rb +++ b/activesupport/lib/active_support/subscriber.rb @@ -126,26 +126,12 @@ module ActiveSupport attr_reader :patterns # :nodoc: def initialize - @queue_key = [self.class.name, object_id].join "-" @patterns = {} super end - def start(name, id, payload) - event = ActiveSupport::Notifications::Event.new(name, nil, nil, id, payload) - event.start! - parent = event_stack.last - parent << event if parent - - event_stack.push event - end - - def finish(name, id, payload) - event = event_stack.pop - event.finish! - event.payload.merge!(payload) - - method = name.split(".").first + def call(event) + method = event.name.split(".").first send(method, event) end @@ -153,11 +139,5 @@ module ActiveSupport method = event.name.split(".").first send(method, event) end - - private - def event_stack - registry = ActiveSupport::IsolatedExecutionState[:active_support_subscriber_queue_registry] ||= {} - registry[@queue_key] ||= [] - end end end diff --git a/activesupport/test/log_subscriber_test.rb b/activesupport/test/log_subscriber_test.rb index 0bf8e2a801..f16b28f516 100644 --- a/activesupport/test/log_subscriber_test.rb +++ b/activesupport/test/log_subscriber_test.rb @@ -77,7 +77,9 @@ class SyncLogSubscriberTest < ActiveSupport::TestCase def test_event_attributes ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber - instrument "some_event.my_log_subscriber" + instrument "some_event.my_log_subscriber" do + [] # Make an allocation + end wait event = @log_subscriber.event if defined?(JRUBY_VERSION)