Changeset 1339

Timestamp:

11/03/09 05:04:29 (5 months ago)

Author:

robbat2

Message:

Allow regeneration of DB connections if we have given out too many
handles to the same connection.

Thanks to <victoriggy@…> for the idea and the original
implementation. Refactored extensively and max_requests renamed to
max_handles to better represent the functionality.

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

@@ -59 +59 @@
     $reaper_jobs,
     $monitor_jobs,
+    $max_handles,
     $min_free_space,
     $max_disk_age,
@@ -94 +95 @@
                              '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},
@@ -153 +155 @@
     $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));
+    return $class->new_from_dsn_user_pass(map { MogileFS->config($_) } qw(db_dsn db_user db_pass max_handles));
 }
 
 sub new_from_dsn_user_pass {
-    my ($class, $dsn, $user, $pass) = @_;
+    my ($class, $dsn, $user, $pass, $max_handles) = @_;
     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,
@@ -50 +51 @@
         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;
@@ -250 +252 @@
 sub dbh {
     my $self = shift;
+    
     if ($self->{dbh}) {
         if ($self->{recheck_done_gen} != $self->{recheck_req_gen}) {
@@ -255 +258 @@
             $self->{recheck_done_gen} = $self->{recheck_req_gen};
         }
-        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;
+        
+        # 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};
 }

trunk/server/mogdbsetup

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