NoBrainer::Document.all Inconsistent in Rails dev #78

Closed
ajselvig opened this Issue Aug 4, 2014 · 7 comments

Comments

Projects
None yet
2 participants
@ajselvig
Contributor

ajselvig commented Aug 4, 2014

When using NoBrainer in a Rails app in development mode, NoBrainer::Document.all only works correctly occasionally. Often it will return no classes, and sometimes it returns the classes several times. I haven't figured out a pattern quite yet, but it's definitely not right.

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Aug 4, 2014

Owner

That's weird. I've specifically made sure all the models are loaded when calling Document.all with https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/document/core.rb#L8

Do you have models that are defined outside of app/models ?

Owner

nviennot commented Aug 4, 2014

That's weird. I've specifically made sure all the models are loaded when calling Document.all with https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/document/core.rb#L8

Do you have models that are defined outside of app/models ?

@ajselvig

This comment has been minimized.

Show comment
Hide comment
@ajselvig

ajselvig Aug 4, 2014

Contributor

No, all models are in app/models.

I really don't know much about how Rails does the eager loading, but
perhaps it's a problem that Rails.application.eager_load! is deprecated?

http://apidock.com/rails/Rails/Application/eager_load!

On Sun, Aug 3, 2014 at 10:46 PM, Nicolas Viennot notifications@github.com
wrote:

That's weird. I've specifically made sure all the models are loaded when
calling Document.all with
https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/document/core.rb#L8

Do you have models that are defined outside of app/models ?


Reply to this email directly or view it on GitHub
#78 (comment).

Contributor

ajselvig commented Aug 4, 2014

No, all models are in app/models.

I really don't know much about how Rails does the eager loading, but
perhaps it's a problem that Rails.application.eager_load! is deprecated?

http://apidock.com/rails/Rails/Application/eager_load!

On Sun, Aug 3, 2014 at 10:46 PM, Nicolas Viennot notifications@github.com
wrote:

That's weird. I've specifically made sure all the models are loaded when
calling Document.all with
https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/document/core.rb#L8

Do you have models that are defined outside of app/models ?


Reply to this email directly or view it on GitHub
#78 (comment).

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Aug 4, 2014

Owner

I don't see it deprecated. Also I can't reproduce the bug on your project.

https://github.com/rails/rails/blob/master/railties/lib/rails/application.rb#L53
https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L56
https://github.com/rails/rails/blob/master/railties/lib/rails/engine.rb#L346
https://github.com/rails/rails/blob/master/railties/lib/rails/engine/configuration.rb#L46

Running Rails.application.config.eager_load_namespaces shows what libraries needs to be eager loaded, and running: Rails.application.config.eager_load_paths in a rails console shows the paths that are eager loaded in the application.

Owner

nviennot commented Aug 4, 2014

I don't see it deprecated. Also I can't reproduce the bug on your project.

https://github.com/rails/rails/blob/master/railties/lib/rails/application.rb#L53
https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L56
https://github.com/rails/rails/blob/master/railties/lib/rails/engine.rb#L346
https://github.com/rails/rails/blob/master/railties/lib/rails/engine/configuration.rb#L46

Running Rails.application.config.eager_load_namespaces shows what libraries needs to be eager loaded, and running: Rails.application.config.eager_load_paths in a rails console shows the paths that are eager loaded in the application.

@ajselvig

This comment has been minimized.

Show comment
Hide comment
@ajselvig

ajselvig Aug 4, 2014

Contributor

I should've been more specific about how the error manifests. Immediately after (re)starting the web server (in my case, pow), NoBrainer::Document.all works correctly. However, after repeated web requests, erroneous results are returned. It can be temporarily fixed by restarting the server.

I'll verify that the same behavior exists with other servers.

Contributor

ajselvig commented Aug 4, 2014

I should've been more specific about how the error manifests. Immediately after (re)starting the web server (in my case, pow), NoBrainer::Document.all works correctly. However, after repeated web requests, erroneous results are returned. It can be temporarily fixed by restarting the server.

I'll verify that the same behavior exists with other servers.

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Aug 4, 2014

Owner

This seem to be a reloading problem. NoBrainer supports rails reloading properly (or so I think):

https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/railtie.rb#L41-L43
https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/loader.rb#L3

try unicorn, puma, thin, or webrick.

Owner

nviennot commented Aug 4, 2014

This seem to be a reloading problem. NoBrainer supports rails reloading properly (or so I think):

https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/railtie.rb#L41-L43
https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/loader.rb#L3

try unicorn, puma, thin, or webrick.

@ajselvig

This comment has been minimized.

Show comment
Hide comment
@ajselvig

ajselvig Aug 4, 2014

Contributor

Okay, verified the same behavior on thin, so it's definitely not a pow
problem. I also have a more specific set of steps/results:

  1. Start server
  2. Load a page/resource that uses NoBrainer::Document.all
    • all documents will be return correctly
  3. Modify a model class
  4. Reload the resource
    • each document appears in the list twice
  5. Reload the resource again
    • no documents appear, this will be the case until the server restarts

I looked through the relevant NoBrainer code briefly and nothing stands
out. It seems like it's probably due to a subtle, undocumented behavior in
the rails eager loading code. Fun!

On Mon, Aug 4, 2014 at 12:47 AM, Nicolas Viennot notifications@github.com
wrote:

This seem to be a reloading problem. NoBrainer supports rails reloading
properly (or so I think):

https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/railtie.rb#L41-L43

https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/loader.rb#L3

try unicorn, puma, thin, or webrick.


Reply to this email directly or view it on GitHub
#78 (comment).

Contributor

ajselvig commented Aug 4, 2014

Okay, verified the same behavior on thin, so it's definitely not a pow
problem. I also have a more specific set of steps/results:

  1. Start server
  2. Load a page/resource that uses NoBrainer::Document.all
    • all documents will be return correctly
  3. Modify a model class
  4. Reload the resource
    • each document appears in the list twice
  5. Reload the resource again
    • no documents appear, this will be the case until the server restarts

I looked through the relevant NoBrainer code briefly and nothing stands
out. It seems like it's probably due to a subtle, undocumented behavior in
the rails eager loading code. Fun!

On Mon, Aug 4, 2014 at 12:47 AM, Nicolas Viennot notifications@github.com
wrote:

This seem to be a reloading problem. NoBrainer supports rails reloading
properly (or so I think):

https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/railtie.rb#L41-L43

https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/loader.rb#L3

try unicorn, puma, thin, or webrick.


Reply to this email directly or view it on GitHub
#78 (comment).

@nviennot

This comment has been minimized.

Show comment
Hide comment
@nviennot

nviennot Aug 4, 2014

Owner

install http://tmate.io/ and share your term with me

Owner

nviennot commented Aug 4, 2014

install http://tmate.io/ and share your term with me

@nviennot nviennot closed this in 5ec66e5 Aug 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment