tdfowler@pacbell.net
2007-Jan-11 16:04 UTC
Rails not properly handling Oracle db connections/sesions in dev mode
We are running edge rails with oracle. After a few hundred requests all available sesions are used up. It seems prior connections are being left open. When this happens no one using the installation of Oracle can create a new session until you kill your mongrel/webrick server. Patch #6928 addresses this problem, and i applied it to my vendor rails and it worked. I think, if possible, this should go into 1.2....it really is a major pain, and for people like me who are introducing rails into enterprisey situations it doesn''t help the effort... cheers,tom --~--~---------~--~----~------------~-------~--~----~ 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 A. Schoen
2007-Jan-11 17:40 UTC
Re: Rails not properly handling Oracle db connections/sesions in dev mode
tdfowler@pacbell.net wrote:> We are running edge rails with oracle. After a few hundred requests > all available sesions are used up. It seems prior connections are > being left open. When this happens no one using the installation of > Oracle can create a new session until you kill your mongrel/webrick > server.I''m not seeing this behavior. Reconnecting to Oracle on every request would suck regardless, even in dev mode. Anyone else seeing this behavior? --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
tdfowler@pacbell.net
2007-Jan-11 22:09 UTC
Re: Rails not properly handling Oracle db connections/sesions in dev mode
Michael, I have done some further investigating on this issue. I can reproduce this problem without fail. The root problem I believe is in clear_reloadable_connections! where it deletes the connection name from active_connections. Patch 6928 stops sessions from building up, however, you then have the needless overhead of setting up a new oracle connection/session on each request. I believe clear_reloadable_connections! should be as follows: # Clears the cache which maps classes def clear_reloadable_connections! @@active_connections.each do |name, conn| if conn.supports_reloading? conn.disconnect! @@active_connections.delete(name) end end end Here is my complete analysis/trace of this through the code: After each request dispatcher.rb calls reset_after_dispatch if you are running in development (Dependencies.mechanism == :load) then reset_application! is called, which in turn calls: "ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord)" (connection_specification.rb:) # Clears the cache which maps classes 1: def clear_reloadable_connections! 2: @@active_connections.each do |name, conn| 3: conn.disconnect! if conn.supports_reloading? 4: @@active_connections.delete(name) 5: end 6: end notice line 4 above deletes the connection name from active_connections. Now when the next request comes in eventually ActiveRecord::Base.connection gets called. Which in turn ends up calling retrieve_connection: 1: def self.retrieve_connection #:nodoc: 2: # Name is nil if establish_connection hasn''t been called for 3: # some class along the inheritance chain up to AR::Base yet. 4: if name = active_connection_name 5: if conn = active_connections[name] 6: # Verify the connection. 7: conn.verify!(@@verification_timeout) 8: elsif spec = @@defined_connections[name] 9: # Activate this connection specification. 10: klass = name.constantize 11: klass.connection = spec 12: conn = active_connections[name] 13: end 14: end 15: 16: conn or raise ConnectionNotEstablished 17: end In this method you end up executing lines 9-12. The call to connection= on line 11 takes you to connection_specification.rb: # Set the connection for the class. 1: def self.connection=(spec) #:nodoc: 2: if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter) 3: active_connections[name] = spec 4: elsif spec.kind_of?(ConnectionSpecification) 5: config = spec.config.reverse_merge(:allow_concurrency => @@allow_concurrency) 7: self.connection = self.send(spec.adapter_method, config) 8: elsif spec.nil? 9: raise ConnectionNotEstablished 10: else 11: establish_connection spec 12: end 13: end where lines 5,6 get executed. line 7 calls oracle_connection in oracle_adapter.rb. At this point you have 2 sessions open and will get an additional one on each request. --~--~---------~--~----~------------~-------~--~----~ 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 A. Schoen
2007-Jan-12 05:10 UTC
Re: Rails not properly handling Oracle db connections/sesions in dev mode
tdfowler@pacbell.net wrote:> I believe clear_reloadable_connections! should be as follows: > > # Clears the cache which maps classes > def clear_reloadable_connections! > @@active_connections.each do |name, conn| > if conn.supports_reloading? > conn.disconnect! > @@active_connections.delete(name) > end > end > endAgreed. I don''t fully understand the intent of the reloadable bit, from the comments on the initial checkin it seems related to a sqlite issue. Can someone from core comment? And agree that this seems like a blocker on 1.2. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
tdfowler@pacbell.net
2007-Jan-12 05:52 UTC
Re: Rails not properly handling Oracle db connections/sesions in dev mode
Thanks for the quick reply Michael. For what''s worth, the above results were @work (RHEL 4, Oracle 10g) Curious, I also tested under mysql 5 - with rails on one server, db on another. Each request sets up a new connection (monitored with mysqladmin processlist) Interesting thing is that it leaves 3 connections open, on the 4th request the 3 connections are cleared and new one is established. The sequence repeats. (the app had one controller and one model) I also tested this out at home on windoze, with Oracle XE and saw the same issue. To see the sessions building up on oracle I look at v$session table. Thanks Again, Tom --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
<resending w/ a more enticing subject for core folks> tdfowler@pacbell.net wrote:> I believe clear_reloadable_connections! should be as follows: > > # Clears the cache which maps classes > def clear_reloadable_connections! > @@active_connections.each do |name, conn| > if conn.supports_reloading? > conn.disconnect! > @@active_connections.delete(name) > end > end > endAgreed. I don''t fully understand the intent of the reloadable bit, from the comments on the initial checkin it seems related to a sqlite issue. I don''t understand why the current code would _not_ disconnect the connection (if it doesn''t support reloading), but would still delete it from the cache. Can someone from core comment? And agree that this seems like a blocker on 1.2. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
<bump> (and w/ more comments below) Michael A. Schoen wrote:> <resending w/ a more enticing subject for core folks> > tdfowler@pacbell.net wrote: >> I believe clear_reloadable_connections! should be as follows: >> >> # Clears the cache which maps classes >> def clear_reloadable_connections! >> @@active_connections.each do |name, conn| >> if conn.supports_reloading? >> conn.disconnect! >> @@active_connections.delete(name) >> end >> end >> end > > Agreed. I don''t fully understand the intent of the reloadable bit, from > the comments on the initial checkin it seems related to a sqlite issue. > I don''t understand why the current code would _not_ disconnect the > connection (if it doesn''t support reloading), but would still delete it > from the cache. > > Can someone from core comment? > > And agree that this seems like a blocker on 1.2.Looking at this history of this, seems this was added to deal w/ a sqlite problem, in that sqlite requires a new connection in order to see schema changes. Presuming that''s the only db that requires a new connection on every request, I''d suggest the following: 1) change #supports_reloading? to #requires_reloading? (or somesuch), make it clear that it''s an issue of "requiring" a new connection (for sqlite only). 2) make the change above, such that both the cache is cleared and the connection dropped ONLY for sqlite. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> 1) change #supports_reloading? to #requires_reloading? (or somesuch), > make it clear that it''s an issue of "requiring" a new connection (for > sqlite only). > > 2) make the change above, such that both the cache is cleared and the > connection dropped ONLY for sqlite.That should already be the case. The abstract implementation is: # Returns true if its safe to reload the connection between requests for development mode. # This is not the case for Ruby/MySQL and it''s not necessary for any adapters except SQLite. def supports_reloading? false end And only SQLite overwrites it. Agree that it''s a blocker. Please do investigate, if you have the time. Otherwise I''ll do when I get around to it. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
DHH wrote:> >> 1) change #supports_reloading? to #requires_reloading? (or somesuch), >> make it clear that it''s an issue of "requiring" a new connection (for >> sqlite only). >> >> 2) make the change above, such that both the cache is cleared and the >> connection dropped ONLY for sqlite. > > That should already be the case. The abstract implementation is: > > # Returns true if its safe to reload the connection between > requests for development mode. > # This is not the case for Ruby/MySQL and it''s not necessary for > any adapters except SQLite. > def supports_reloading? > false > end > > And only SQLite overwrites it. > > Agree that it''s a blocker. Please do investigate, if you have the time. > Otherwise I''ll do when I get around to it.The problem is that clear_reloadable_connections! is currently defined as: # Clears the cache which maps classes def clear_reloadable_connections! @@active_connections.each do |name, conn| conn.disconnect! if conn.supports_reloading? @@active_connections.delete(name) end end So it only disconnects for sqlite, BUT removes the connection from the cache always. I can provide a patch, just want to make sure I understood what was intended here. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> So it only disconnects for sqlite, BUT removes the connection from the > cache always. > > I can provide a patch, just want to make sure I understood what was > intended here.You''re right. That was a bug. Could ya''all try now after http://dev.rubyonrails.org/changeset/5955 has been made? I believe this is the last issue before 1.2 leaves the station. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
DHH wrote:> > So it only disconnects for sqlite, BUT removes the connection from the > > cache always. > > > > I can provide a patch, just want to make sure I understood what was > > intended here. > > You''re right. That was a bug. > > Could ya''all try now after http://dev.rubyonrails.org/changeset/5955 > has been made? > > I believe this is the last issue before 1.2 leaves the station.I just updated, and it works for me. tom --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
DHH wrote:> I believe this is the last issue before 1.2 leaves the station.I''ve still got 2 Oracle-related tickets open, the first one in particular is a long-standing and silly bug (not possible to insert the string "null" into the db). Both are patched and ready-to-go... http://dev.rubyonrails.org/ticket/6997 http://dev.rubyonrails.org/ticket/6780 Would it be possible to get this applied as well? Thanks. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Both are patched and ready-to-go... > > http://dev.rubyonrails.org/ticket/6997 > > http://dev.rubyonrails.org/ticket/6780 > > Would it be possible to get this applied as well?done and done. -- 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 -~----------~----~----~----~------~----~------~--~---