Tekin Suleyman
2010-Sep-02 16:54 UTC
Observers no longer fire outside of ActiveRecord transactions
I''ve just encountered a strange bug in an app where a newly created model was not visible in the database to a process was kicked off by an observer. Looking through the logs and then the rails source, it looks like Observer methods are now getting fired as standard AR callbacks, meaning they fire in the AR transaction. This wasn''t the case in 2.3 as far as I can tell. In my mind, observer callbacks should not be fired inside the AR transaction to avoid race conditions when observers kick of processes that try and access the new model before all the callbacks have completed and the transaction is committed. Any thoughts? Tekin Suleyman http://tekin.co.uk -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Michael Koziarski
2010-Sep-03 01:39 UTC
Re: Observers no longer fire outside of ActiveRecord transactions
> In my mind, observer callbacks should not be fired inside the AR transaction > to avoid race conditions when observers kick of processes that try and > access the new model before all the callbacks have completed and the > transaction is committed.You could make the same argument about regular callbacks though, after_create could be used to kick off jobs etc. I can see arguments on both sides, observers are a different object so having a slightly detached lifecycle makes sense. But at the same time it''s really just an organisation method, so why is there a difference.> Any thoughts?Is it fair to say that the best fix would be another specific hook which fires after the transaction is successfully committed?> > Tekin Suleyman > http://tekin.co.uk > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to > rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. >-- Cheers Koz -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Yehuda Katz
2010-Sep-03 01:56 UTC
Re: Observers no longer fire outside of ActiveRecord transactions
That''s why we have after_commit in Rails 3. ;) Sent from my iPhone On Sep 2, 2010, at 6:39 PM, Michael Koziarski <michael@koziarski.com> wrote:>> In my mind, observer callbacks should not be fired inside the AR transaction >> to avoid race conditions when observers kick of processes that try and >> access the new model before all the callbacks have completed and the >> transaction is committed. > > You could make the same argument about regular callbacks though, > after_create could be used to kick off jobs etc. I can see arguments > on both sides, observers are a different object so having a slightly > detached lifecycle makes sense. But at the same time it''s really just > an organisation method, so why is there a difference. > >> Any thoughts? > > Is it fair to say that the best fix would be another specific hook > which fires after the transaction is successfully committed? > >> >> Tekin Suleyman >> http://tekin.co.uk >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To post to this group, send email to rubyonrails-core@googlegroups.com. >> To unsubscribe from this group, send email to >> rubyonrails-core+unsubscribe@googlegroups.com. >> For more options, visit this group at >> http://groups.google.com/group/rubyonrails-core?hl=en. >> > > > > -- > Cheers > > Koz > > -- > You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Michael Koziarski
2010-Sep-03 02:01 UTC
Re: Observers no longer fire outside of ActiveRecord transactions
> That''s why we have after_commit in Rails 3. ;)Dude, you stole my punch line :)> Sent from my iPhone > > On Sep 2, 2010, at 6:39 PM, Michael Koziarski <michael@koziarski.com> wrote: > >>> In my mind, observer callbacks should not be fired inside the AR transaction >>> to avoid race conditions when observers kick of processes that try and >>> access the new model before all the callbacks have completed and the >>> transaction is committed. >> >> You could make the same argument about regular callbacks though, >> after_create could be used to kick off jobs etc. I can see arguments >> on both sides, observers are a different object so having a slightly >> detached lifecycle makes sense. But at the same time it''s really just >> an organisation method, so why is there a difference. >> >>> Any thoughts? >> >> Is it fair to say that the best fix would be another specific hook >> which fires after the transaction is successfully committed? >> >>> >>> Tekin Suleyman >>> http://tekin.co.uk >>> >>> -- >>> You received this message because you are subscribed to the Google Groups >>> "Ruby on Rails: Core" group. >>> To post to this group, send email to rubyonrails-core@googlegroups.com. >>> To unsubscribe from this group, send email to >>> rubyonrails-core+unsubscribe@googlegroups.com. >>> For more options, visit this group at >>> http://groups.google.com/group/rubyonrails-core?hl=en. >>> >> >> >> >> -- >> Cheers >> >> Koz >> >> -- >> You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. >> To post to this group, send email to rubyonrails-core@googlegroups.com. >> To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. >> For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. >> > > -- > You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > >-- Cheers Koz -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Tekin Suleyman
2010-Sep-07 09:56 UTC
Re: Observers no longer fire outside of ActiveRecord transactions
> That''s why we have after_commit in Rails 3. ;)OK, I''ll use after_commit to get around this particular issue. This does raise a few questions though. For example, now that Observer methods are effectively being rolled into the normal callback chain of the models they observe, what exactly is their point? Wouldn''t it now be just as effective, and arguably easier to grok, to simply put peripheral callbacks in a module and include them directly in the model? Also, doesn''t this new refactoring mean that observer callbacks can now halt the execution chain where they couldn''t before? Finally, using after_commit in the observer is not quite the same thing as the previous behaviour as you can''t distinguish between after_commit on create and update without doing it in the model directly, right? I can''t say I feel particularly strongly either way, just interested in stimulating some debate on what are some subtle but still potentially significant behavioural changes. At the very least, the docs/guides should probably be updated to document the new behaviour. Tekin On 3 Sep 2010, at 03:01, Michael Koziarski wrote:>> That''s why we have after_commit in Rails 3. ;) > > Dude, you stole my punch line :) > >> Sent from my iPhone >> >> On Sep 2, 2010, at 6:39 PM, Michael Koziarski <michael@koziarski.com> wrote: >> >>>> In my mind, observer callbacks should not be fired inside the AR transaction >>>> to avoid race conditions when observers kick of processes that try and >>>> access the new model before all the callbacks have completed and the >>>> transaction is committed. >>> >>> You could make the same argument about regular callbacks though, >>> after_create could be used to kick off jobs etc. I can see arguments >>> on both sides, observers are a different object so having a slightly >>> detached lifecycle makes sense. But at the same time it''s really just >>> an organisation method, so why is there a difference. >>> >>>> Any thoughts? >>> >>> Is it fair to say that the best fix would be another specific hook >>> which fires after the transaction is successfully committed? >>> >>>> >>>> Tekin Suleyman >>>> http://tekin.co.uk >>>> >>>> -- >>>> You received this message because you are subscribed to the Google Groups >>>> "Ruby on Rails: Core" group. >>>> To post to this group, send email to rubyonrails-core@googlegroups.com. >>>> To unsubscribe from this group, send email to >>>> rubyonrails-core+unsubscribe@googlegroups.com. >>>> For more options, visit this group at >>>> http://groups.google.com/group/rubyonrails-core?hl=en. >>>> >>> >>> >>> >>> -- >>> Cheers >>> >>> Koz >>> >>> -- >>> You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. >>> To post to this group, send email to rubyonrails-core@googlegroups.com. >>> To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. >>> For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. >>> >> >> -- >> You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. >> To post to this group, send email to rubyonrails-core@googlegroups.com. >> To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. >> For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. >> >> > > > > -- > Cheers > > Koz > > -- > You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Michael Koziarski
2010-Sep-08 04:11 UTC
Re: Observers no longer fire outside of ActiveRecord transactions
> For example, now that Observer methods are effectively being rolled into the > normal callback chain of the models they observe, what exactly is their > point? Wouldn''t it now be just as effective, and arguably easier to grok, to > simply put peripheral callbacks in a module and include them directly in the > model?I''m not so sure that it''s actually easier to understand. There''s definitely something nice about having a single class which has a particular aspect of object lifecycle. That class can be tested and refined in isolation and can be turned on and off independently of the model in question. In my own code I typically use model callbacks for the bulk of the callback behaviour, observers are typically only for truly tangential things like cache expiry.> Also, doesn''t this new refactoring mean that observer callbacks can now halt > the execution chain where they couldn''t before?Good question, if they can I''m not sure that it would be the end of the world.> Finally, using after_commit in the observer is not quite the same thing as > the previous behaviour as you can''t distinguish between after_commit on > create and update without doing it in the model directly, right?after_commit fires on destroy too, so it''s even slightly more confusing. There''s after_commit :your_thing, :on=>:create for model level callbacks, but I don''t believe that''s available if you''re using observers. There probably should be.> I can''t say I feel particularly strongly either way, just interested in > stimulating some debate on what are some subtle but still potentially > significant behavioural changes. At the very least, the docs/guides should > probably be updated to document the new behaviour.I definitely agree here, the change isn''t particularly earth shattering or confusing, so long as it''s well documented with a few examples. The current callbacks documentation is actually quite good, but perhaps could do with some refinement.> Tekin > > > On 3 Sep 2010, at 03:01, Michael Koziarski wrote: > > That''s why we have after_commit in Rails 3. ;) > > Dude, you stole my punch line :) > > Sent from my iPhone > > On Sep 2, 2010, at 6:39 PM, Michael Koziarski <michael@koziarski.com> wrote: > > In my mind, observer callbacks should not be fired inside the AR transaction > > to avoid race conditions when observers kick of processes that try and > > access the new model before all the callbacks have completed and the > > transaction is committed. > > You could make the same argument about regular callbacks though, > > after_create could be used to kick off jobs etc. I can see arguments > > on both sides, observers are a different object so having a slightly > > detached lifecycle makes sense. But at the same time it''s really just > > an organisation method, so why is there a difference. > > Any thoughts? > > Is it fair to say that the best fix would be another specific hook > > which fires after the transaction is successfully committed? > > > Tekin Suleyman > > http://tekin.co.uk > > -- > > You received this message because you are subscribed to the Google Groups > > "Ruby on Rails: Core" group. > > To post to this group, send email to rubyonrails-core@googlegroups.com. > > To unsubscribe from this group, send email to > > rubyonrails-core+unsubscribe@googlegroups.com. > > For more options, visit this group at > > http://groups.google.com/group/rubyonrails-core?hl=en. > > > > > -- > > Cheers > > Koz > > -- > > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > > To post to this group, send email to rubyonrails-core@googlegroups.com. > > To unsubscribe from this group, send email to > rubyonrails-core+unsubscribe@googlegroups.com. > > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. > > > -- > > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > > To post to this group, send email to rubyonrails-core@googlegroups.com. > > To unsubscribe from this group, send email to > rubyonrails-core+unsubscribe@googlegroups.com. > > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. > > > > > > -- > Cheers > > Koz > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to > rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to > rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. >-- Cheers Koz -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.