[OSM-dev] [PATCH] Persistant database connection

[Christopher Schmidt]

On Sun, Jul 09, 2006 at 08:20:00PM +0100, James Mastros wrote:
> Currently, the core of OSM is a bit silly: Every time it wants to execute
> some SQL, it connects to the database, runs the sql, gets the entire result
> set, and then disconnects from the database.  There's a number of ways in
> which that's silly, but the simplest one is that there's no reason to keep
> connecting and disconnecting -- connect when you first need the connection,
> and disconnect when you're all done.
> 
> The attached patch touches only a very small amount of code, and will do
> just that.  There's still plenty of sillyness about, but this will at least
> pick this piece of low-hanging fruit.

I've attached a patch that fixes the get_local_connection aspects as
well. It was not obvious that get_local_connection is, for the time
being, using the same database handle as the get_connection is.

I've tested that:
 
 * Before applying the patch, each query opens a new mysql connection
 * After applying the patch, the queries all execute in the same
   connection
 * After applying the patch, the connection closes as soon as the test
   script finishes.
 * call_local_sql works in the same way as above.
 
However, what I have not tested is whether the connection closes when
the dao instance goes out of scope -- I don't know how. This could be a
problem: We can't go through all the code and tell it to call a destroy
function.

With SteveC's help, the test script is now:

#!/usr/bin/ruby -w

require 'cgi'
require 'osm/dao.rb'

dao = OSM::Dao.instance
dao.create_account("crschmidt at crschmidt.net", "pass")
dao.set_timeout("crschmidt at crschmidt.net")
dao = nil
GC.start
sleep(100)

This properly cleans up after the dao mysql connection immediately (not
after the ruby script closes, after the sleep(100)), which indicates
that garbage collection will properly close the database handle. I'm not
sure when garbage collection runs under mod_ruby: presumably, it cleans
up after each request.

Assuming this is the case, I can heartily approve of the patch attached,
which grew out of theorbtwo's patch. MySQL query log display before and
after:

060709 17:27:35 1408776 Connect     root at localhost on osm
                1408776 Query       select id from users where email = 'crschmidt at crschmidt.net' and active = true
                1408776 Query       insert into users(email, timeout,token, active, pass_crypt, creation_time) values ('crschmidt at crschmidt.net', NOW(), '4ifIbBKHGGrHS5oJwmXkmrYd3ja5fq',false, md5('pass'), NOW())                
                1408776 Query       update users set timeout = NOW() + INTERVAL 1 DAY where email = 'crschmidt at crschmidt.net' and active = true
                1408776 Quit

Before:

060709 17:29:22 1408798 Connect     root at localhost on osm
                1408798 Query       select id from users where email = 'crschmidt at crschmidt.net' and active = true
                1408798 Quit       
                1408799 Connect     root at localhost on osm
                1408799 Query       insert into users(email, timeout,token, active, pass_crypt, creation_time) values ('crschmidt at crschmidt.net', NOW(), 'SA8zrSieoLpg5CYVuYo0rpy2t10jxS',false, md5('pass'), NOW())                
                1408799 Quit
                1408800 Connect     root at localhost on osm                
                1408800 Query update users set timeout = NOW() + INTERVAL 1 DAY where email = 'crschmidt at crschmidt.net' and active = true                
                1408800 Quit      

Again, patch attached, feedback welcome.

-- 
Christopher Schmidt
Web Developer
-------------- next part --------------
Index: osm/dao.rb
===================================================================
--- osm/dao.rb	(revision 1155)
+++ osm/dao.rb	(working copy)
@@ -163,7 +163,8 @@
 
     def get_connection
       begin
-        return Mysql.real_connect($DBSERVER, $USERNAME, $PASSWORD, $DATABASE)
+        @connection ||= Mysql.real_connect($DBSERVER, $USERNAME, $PASSWORD, $DATABASE)
+        return @connection
       rescue MysqlError => e
         mysql_error(e)
       end
@@ -174,7 +175,8 @@
       
       #whilst local db's are down, just talk to the main server
       #begin
-      #  return Mysql.real_connect('localhost', $USERNAME, $PASSWORD, $DATABASE)
+      #  @local_connection || = Mysql.real_connect('localhost', $USERNAME, $PASSWORD, $DATABASE)
+      #  return @local_connection
       #rescue MysqlError => e
       #  mysql_error(e)
       #end
@@ -228,8 +230,6 @@
         if res.nil? then return true else return res end
       rescue MysqlError =>ex
         mysql_error(ex)
-      ensure
-        dbh.close unless dbh.nil?
       end
       nil
     end
@@ -245,8 +245,6 @@
         if res.nil? then return true else return res end
       rescue MysqlError =>ex
         mysql_error(ex)
-      ensure
-        dbh.close unless dbh.nil?
       end
       nil
     end