Changeset 1345

Timestamp:

11/14/09 10:31:25 (4 months ago)

Author:

dormando

Message:

Revert "Allow regeneration of DB connections if we have given out too many"

This was done in a dangerous spot. Potentially breaking transactions/etc.

I'll redo it.

Location:

trunk/server

Files:

• CHANGES (modified) (1 diff)
• lib/MogileFS/Config.pm (modified) (3 diffs)
• lib/MogileFS/Store.pm (modified) (5 diffs)
• mogdbsetup (modified) (2 diffs)

trunk/server/CHANGES

@@ -1 @@
-        * Add 'max_handles' config option to restart a DB connection if there
-          are too many handles to it (victori).
-
         * Close possibly invalid DB connections as we go out of scope
           (victori).

trunk/server/lib/MogileFS/Config.pm

@@ -58 +58 @@
     $reaper_jobs,
     $monitor_jobs,
-    $max_handles,
     $min_free_space,
     $max_disk_age,
@@ -94 +93 @@
                              'default_mindevcount=i' => \$cmdline{default_mindevcount},
                              'node_timeout=i' => \$cmdline{node_timeout},
-                             'max_handles=i'  => \$cmdline{max_handles},
                              'pidfile=s'      => \$cmdline{pidfile},
                              'no_schema_check' => \$cmdline{no_schema_check},
@@ -155 +153 @@
     $min_free_space = choose_value( 'min_free_space', 100 );
     $max_disk_age   = choose_value( 'max_disk_age', 5 );
-    $max_handles    = choose_value( 'max_handles', 0 );
     $DEBUG          = choose_value( 'debug', $ENV{DEBUG} || 0 );
     $pidfile        = choose_value( 'pidfile', "" );

trunk/server/lib/MogileFS/Store.pm

@@ -21 +21 @@
 sub new {
     my ($class) = @_;
-    return $class->new_from_dsn_user_pass(map { MogileFS->config($_) } qw(db_dsn db_user db_pass max_handles));
+    return $class->new_from_dsn_user_pass(map { MogileFS->config($_) } qw(db_dsn db_user db_pass));
 }
 
 sub new_from_dsn_user_pass {
-    my ($class, $dsn, $user, $pass, $max_handles) = @_;
+    my ($class, $dsn, $user, $pass) = @_;
     my $subclass;
     if ($dsn =~ /^DBI:mysql:/i) {
@@ -45 +45 @@
         user   => $user,
         pass   => $pass,
-        max_handles => $max_handles, # Max number of handles to allow
         raise_errors => $subclass->want_raise_errors,
         slave_list_cachetime => 0,
@@ -51 +50 @@
         recheck_req_gen  => 0,  # incremented generation, of recheck of dbh being requested
         recheck_done_gen => 0,  # once recheck is done, copy of what the request generation was
-        handles_given => 0,     # amount of handles given out.
         server_setting_cache => {}, # value-agnostic db setting cache.
     }, $subclass;
@@ -252 +250 @@
 sub dbh {
     my $self = shift;
-    
     if ($self->{dbh}) {
         if ($self->{recheck_done_gen} != $self->{recheck_req_gen}) {
@@ -258 +255 @@
             $self->{recheck_done_gen} = $self->{recheck_req_gen};
         }
-        
-        # If we have exceeded the number of handles/requests-for-dbh, then it
-        # is time to reset for a new connection. Reported to give a large
-        # performance boost on Solaris with Postgresql.
-        if ($self->{dbh} &&
-                $self->{max_handles} &&
-                $self->{max_handles} > 0 && 
-                $self->{handles_given} > $self->{max_handles}) {
-                        # TODO: Not sure about this disconnect. We probably want existing
-                        # users of the connection to continue to do so.
-                        $self->{dbh}->disconnect();
-            $self->{dbh} = undef;
-            $self->{handles_given} = 0;
-        }
-    }
-
-    unless($self->{dbh}) {
-        $self->{dbh} = DBI->connect($self->{dsn}, $self->{user}, $self->{pass}, {
-            PrintError => 0,
-            AutoCommit => 1,
-            InactiveDestroy => 1,
-            # FUTURE: will default to on (have to validate all callers first):
-            RaiseError => ($self->{raise_errors} || 0),
-        }) or
-            die "Failed to connect to database: " . DBI->errstr;
-        $self->post_dbi_connect;
-    }
-    $self->{handles_given} += 1;
+        return $self->{dbh} if $self->{dbh};
+    }
+
+    $self->{dbh} = DBI->connect($self->{dsn}, $self->{user}, $self->{pass}, {
+        PrintError => 0,
+        AutoCommit => 1,
+        InactiveDestroy => 1,
+        # FUTURE: will default to on (have to validate all callers first):
+        RaiseError => ($self->{raise_errors} || 0),
+    }) or
+        die "Failed to connect to database: " . DBI->errstr;
+    $self->post_dbi_connect;
     return $self->{dbh};
 }

trunk/server/mogdbsetup

@@ -4 +4 @@
 use lib 'lib';
 use MogileFS::Store;
-use MogileFS::Config;
 
 # Rename binary in process list to make init scripts saner
@@ -109 +108 @@
 $sclass->on_confirm(\&confirm);
 
-MogileFS::Config->load_config;
-
 my $sto = $sclass->new_from_mogdbsetup(
                                        map { $_ => $args{$_} }