Rails core - Jan 2007 - 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
-~----------~----~----~----~------~----~------~--~---

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
-~----------~----~----~----~------~----~------~--~---

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
-~----------~----~----~----~------~----~------~--~---

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.

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
-~----------~----~----~----~------~----~------~--~---

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
> 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.



--~--~---------~--~----~------------~-------~--~----~
 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
-~----------~----~----~----~------~----~------~--~---