Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Slow app boot when trying to observe ActiveRecord::Base #35

Open
joakimk opened this issue Jun 22, 2015 · 7 comments
Open

Slow app boot when trying to observe ActiveRecord::Base #35

joakimk opened this issue Jun 22, 2015 · 7 comments

Comments

@joakimk
Copy link

joakimk commented Jun 22, 2015

If you observe ActiveRecord::Base in order to do something if any model is changed there is some bug in how the observers lists are created causing the app boot to take a long time when eager loading code.

In our case we had an observer that removed comments if a model was destroyed and it had associated comments.

I've reproduced the issue here: https://github.com/barsoom/reproduce_observer_error

We don't use observers in any apps anymore, but thought I'd post this in case someone else had the same issue.
@henrik
Copy link

henrik commented Jun 22, 2015

Just as Google food: in our case, we (I'm on the same team as @joakimk) noticed this as Resque workers taking a long time to boot (several minutes) after upgrading from Rails 3 to Rails 4 and switching to rails-observers.

@anthonyjsmith
Copy link

The list of observed models seems to grow exponentially, containing each model many times (duplicates). It is apparently solved by putting models.uniq! in Observing.observe(*models). I'll try to put a fix together...

@henrik
Copy link

henrik commented Aug 26, 2015

👍

anthonyjsmith added a commit to anthonyjsmith/rails-observers that referenced this issue Aug 26, 2015
@anthonyjsmith
Do not observe the same model more than once. Fixes rails#35.
ce9e6a6 
* This was happening when a subclass of an observed class was created.
* The subclass was being observed twice.
* I've changed one of the tests to catch this.
anthonyjsmith added a commit to anthonyjsmith/rails-observers that referenced this issue Aug 27, 2015
@anthonyjsmith
Add subclasses to observed_classes when observer is instantiated. For r…
8080eea 
…ails#35.

* Subclasses were being observed (via add_observer) but not added to
  observed_classes
* Added call to observe() in initialize to update observed_classes
* Moved searching of descendants to initialize, so the comment "Start observing
  the declared classes and their subclasses." will be correct for all subclasses
  of ActiveModel::Observer (not just ActiveRecord::Observer)
* Removed ActiveRecord::Observer#observed_classes, as descendants are now
  searched on initialize (this was the main cause of rails#35)
* No need for models.uniq! in #observe (undoing previous commit)
* Improved tests to better cover uniqueness of observed_classes
* Tests don't work with minitest 5.8.0; require < 5
@henrik
Copy link

henrik commented May 21, 2018

@anthonyjsmith Do you consider this issue solved? Just wondering if we can remove https://github.com/barsoom/reproduce_observer_error.

@anthonyjsmith
Copy link

It's been a few years, and I'm not working with Rails any more, so my memory is a bit hazy, but yes, I consider it solved.

@anthonyjsmith
Copy link

... although the pull request seems to still be open, so maybe it isn't fully solved yet...? #36

@cjbottaro
Copy link

Definitely not resolved yet, it takes us like 20 seconds to load each constant. Stack trace is always in this code:

 0: /Users/cjbottaro/.gem/ruby/2.3.3/gems/activesupport-5.1.6/lib/active_support/core_ext/regexp.rb:7:in `match?'
 1: /Users/cjbottaro/.gem/ruby/2.3.3/gems/activerecord-5.1.6/lib/active_record/dynamic_matchers.rb:33:in `block in match'
 2: /Users/cjbottaro/.gem/ruby/2.3.3/gems/activerecord-5.1.6/lib/active_record/dynamic_matchers.rb:33:in `each'
 3: /Users/cjbottaro/.gem/ruby/2.3.3/gems/activerecord-5.1.6/lib/active_record/dynamic_matchers.rb:33:in `find'
 4: /Users/cjbottaro/.gem/ruby/2.3.3/gems/activerecord-5.1.6/lib/active_record/dynamic_matchers.rb:33:in `match'
 5: /Users/cjbottaro/.gem/ruby/2.3.3/gems/activerecord-5.1.6/lib/active_record/dynamic_matchers.rb:8:in `respond_to_missing?'
 6: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:309:in `respond_to?'
 7: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:309:in `block in observe'
 8: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:309:in `collect!'
 9: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:309:in `observe'
10: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:358:in `observed_class_inherited'
11: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:352:in `update'
12: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:147:in `block in notify_observers'
13: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:147:in `each'
14: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:147:in `notify_observers'
15: /Users/cjbottaro/.gem/ruby/2.3.3/gems/rails-observers-0.1.5/lib/rails/observers/active_model/observing.rb:199:in `inherited'

dramanat added a commit to AcademicWorks/rails-observers that referenced this issue Sep 7, 2018
@dramanat
Fixes Issue rails#35
f8a2964 
Do not observe the same model more than once.

copied from https://github.com/rails/rails-observers/pull/36/files
dramanat added a commit to AcademicWorks/rails-observers that referenced this issue Sep 10, 2018
@dramanat
Fixes Issue rails#35
a3d4c0e 
Do not observe the same model more than once.

copied from https://github.com/rails/rails-observers/pull/36/files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@henrik @cjbottaro @joakimk @anthonyjsmith