Bug 124589 - support database replication

r=myk, a=justdave

9 files changed, 190 insertions, 58 deletions

diff --git a/Bug.pm b/Bug.pm
index 3dadd3cd5..11eb43af1 100755
--- a/Bug.pm
+++ b/Bug.pm
@@ -96,14 +96,6 @@ sub initBug  {
      }
   }
      
-
-  &::ConnectToDatabase();
-  &::GetVersionTable();
-
-  # this verification should already have been done by caller
-  # my $loginok = quietly_check_login();
-
-
   $self->{'whoid'} = $user_id;
 
   my $query = "
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 3dadd3cd5..11eb43af1 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -96,14 +96,6 @@ sub initBug  {
      }
   }
      
-
-  &::ConnectToDatabase();
-  &::GetVersionTable();
-
-  # this verification should already have been done by caller
-  # my $loginok = quietly_check_login();
-
-
   $self->{'whoid'} = $user_id;
 
   my $query = "
diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm
index 534367ce0..25792d476 100644
--- a/Bugzilla/Config.pm
+++ b/Bugzilla/Config.pm
@@ -138,7 +138,11 @@ sub SetParam {
     my $entry = $params{$name};
 
     # sanity check the value
-    if (exists $entry->{'checker'}) {
+
+    # XXX - This runs the checks. Which would be good, except that
+    # check_shadowdb creates the database as a sideeffect, and so the
+    # checker fails the second time arround...
+    if ($name ne 'shadowdb' && exists $entry->{'checker'}) {
         my $err = $entry->{'checker'}->($value, $entry);
         die "Param $name is not valid: $err" unless $err eq '';
     }
diff --git a/buglist.cgi b/buglist.cgi
index 50873387e..fdab9eb83 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -678,7 +678,7 @@ while (my @row = FetchSQLData()) {
 # Switch back from the shadow database to the regular database so PutFooter()
 # can determine the current user even if the "logincookies" table is corrupted
 # in the shadow database.
-SendSQL("USE $::db_name");
+ReconnectToMainDatabase();
 
 # Check for bug privacy and set $bug->{isingroups} = 1 if private 
 # to 1 or more groups
diff --git a/checksetup.pl b/checksetup.pl
index cd02538b3..f4a70c284 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -478,7 +478,11 @@ LocalVar('db_pass', '
 $db_pass = \'\';
 ');
 
-
+LocalVar('db_sock', '
+# Enter a path to the unix socket for mysql. If this is blank, then mysql\'s
+# compiled-in default will be used. You probably want that.
+$db_sock = \'\';
+');
 
 LocalVar('db_check', '
 #
@@ -619,6 +623,7 @@ my $my_db_host = ${*{$main::{'db_host'}}{SCALAR}};
 my $my_db_port = ${*{$main::{'db_port'}}{SCALAR}};
 my $my_db_name = ${*{$main::{'db_name'}}{SCALAR}};
 my $my_db_user = ${*{$main::{'db_user'}}{SCALAR}};
+my $my_db_sock = ${*{$main::{'db_sock'}}{SCALAR}};
 my $my_db_pass = ${*{$main::{'db_pass'}}{SCALAR}};
 my $my_index_html = ${*{$main::{'index_html'}}{SCALAR}};
 my $my_create_htaccess = ${*{$main::{'create_htaccess'}}{SCALAR}};
@@ -1212,10 +1217,6 @@ my $db_base = 'mysql';
 # pretty one saying they need to install it. -- justdave@syndicomm.com
 #use DBI;
 
-# get a handle to the low-level DBD driver
-my $drh = DBI->install_driver($db_base)
-    or die "Can't connect to the $db_base. Is the database installed and up and running?\n";
-
 if ($my_db_check) {
     # Do we have the database itself?
 
@@ -1226,6 +1227,9 @@ if ($my_db_check) {
 # removed the $db_name because we don't know it exists yet, and this will fail
 # if we request it here and it doesn't. - justdave@syndicomm.com 2000/09/16
     my $dsn = "DBI:$db_base:;$my_db_host;$my_db_port";
+    if ($my_db_sock ne "") {
+        $dsn .= ";mysql_socket=$my_db_sock";
+    }
     my $dbh = DBI->connect($dsn, $my_db_user, $my_db_pass)
       or die "Can't connect to the $db_base database. Is the database " .
         "installed and\nup and running?  Do you have the correct username " .
@@ -1249,7 +1253,7 @@ if ($my_db_check) {
     my @databases = $dbh->func('_ListDBs');
     unless (grep /^$my_db_name$/, @databases) {
        print "Creating database $my_db_name ...\n";
-       $drh->func('createdb', $my_db_name, "$my_db_host:$my_db_port", $my_db_user, $my_db_pass, 'admin')
+       $dbh->func('createdb', $my_db_name, 'admin')
             or die <<"EOF"
 
 The '$my_db_name' database is not accessible. This might have several reasons:
@@ -1268,6 +1272,10 @@ EOF
 
 # now get a handle to the database:
 my $connectstring = "dbi:$db_base:$my_db_name:host=$my_db_host:port=$my_db_port";
+if ($my_db_sock ne "") {
+    $connectstring .= ";mysql_socket=$my_db_sock";
+}
+
 my $dbh = DBI->connect($connectstring, $my_db_user, $my_db_pass)
     or die "Can't connect to the table '$connectstring'.\n",
            "Have you read the Bugzilla Guide in the doc directory?  Have you read the doc of '$db_base'?\n";
diff --git a/defparams.pl b/defparams.pl
index 35604294b..ba95f5fd5 100644
--- a/defparams.pl
+++ b/defparams.pl
@@ -53,7 +53,6 @@ use vars qw(@param_list);
 
 sub check_priority {
     my ($value) = (@_);
-    &::ConnectToDatabase();
     &::GetVersionTable();
     if (lsearch(\@::legal_priority, $value) < 0) {
         return "Must be a legal priority value: one of " .
@@ -68,7 +67,11 @@ sub check_shadowdb {
     if ($value eq "") {
         return "";
     }
-    &::ConnectToDatabase();
+    if (!Param("updateshadowdb")) {
+        # Can't test this, because ConnectToDatabase uses the param, but
+        # we can't set this before testing....
+        return "";
+    }
     &::SendSQL("SHOW DATABASES");
     while (&::MoreSQLData()) {
         my $n = &::FetchOneColumn();
@@ -76,11 +79,21 @@ sub check_shadowdb {
             return "The $n database already exists.  If that's really the name you want to use for the backup, please CAREFULLY make the existing database go away somehow, and then try again.";
         }
     }
+    # We trust the admin....
+    trick_taint($value);
     &::SendSQL("CREATE DATABASE $value");
     &::SendSQL("INSERT INTO shadowlog (command) VALUES ('SYNCUP')", 1);
     return "";
 }
 
+sub check_shadowdbhost {
+    my ($value) = (@_);
+    if ($value && Param("updateshadowdb")) {
+        return "Sorry, you can't have the shadowdb on a different connection to the main database if you want Bugzilla to handle the replication for you.";
+    }
+    return "";
+}
+
 sub check_urlbase {
     my ($url) = (@_);
     if ($url !~ m:^http.*/$:) {
@@ -247,28 +260,92 @@ sub check_netmask {
   },
 
   {
+   name => 'queryagainstshadowdb',
+   desc => 'If this is on, and the <tt>shadowdb</tt> parameter is set, then ' .
+           'certain queries will happen against the shadow database.',
+   type => 'b',
+   default => 0,
+  },
+
+  {
+   name => 'updateshadowdb',
+   desc => 'If this is on, and the <tt>shadowdb</tt> parameter is set, then ' .
+           'Bugzilla will use the old style of shadow database in which it ' .
+           'manually propogates changes to the shadow database. Otherwise, ' .
+           'Bugzilla will assume that the <tt>shadowdb</tt> database (if ' . 
+           'any) is being updated via replication. <b>WARNING! This ' .
+           'manual replication is deprecated and is going away soon ' .
+           '(<u>BEFORE</u> the next stable Bugzilla release).</b> It has ' .
+           'several problems with data consistency, and replication is the ' .
+           'preferred option. If this parameter is on, and you disable it, ' .
+           'make sure that the shadow database is already set up for ' .
+           'replication, or queries will return stale data.',
+   type => 'b',
+   default => 1,
+  },
+
+  # This entry must be _after_ updateshadowdb, because check_shadowdbhost uses
+  # that
+  {
+   name => 'shadowdbhost',
+   desc => 'The host the shadow database is on. If blank, then then we ' .
+           'assume it\'s on the main database host (as defined in ' .
+           'localconfig) and ingore the <tt>shadowdbport</tt> and ' .
+           '<tt>shadowdbsock</tt> parameters below, which means that this ' .
+           'parameter <em>must be filled in<em> if your shadow database is ' .
+           'on a different instance of the mysql server, even if that ' .
+           'instance runs on the same machine as the main database. Note ' .
+           'that <tt>updateshadowdb<tt> must be off if the shadow database ' .
+           'is on a difference mysql instance, since Bugzilla can\'t ' .
+           'propogate changes between instances itself, and this should be ' .
+           'left blank if the shadow database is on the same instance, ' .
+           'since Bugzilla can then reuse the same database connection for '.
+           'better performance.',
+   type => 't',
+   default => '',
+   checker => \&check_shadowdbhost,
+  },
+
+  {
+   name => 'shadowdbport',
+   desc => 'The port the shadow database is on. Ignored if ' .
+           '<tt>shadowdbhost</tt> is blank. Note: if the host is the local ' .
+           'machine, then MySQL will ignore this setting, and you must ' .
+           'specify a socket below.',
+   type => 't',
+   default => '3306',
+   checker => \&check_numeric,
+  },
+
+  {
+   name => 'shadowdbsock',
+   desc => 'The socket used to connect to the shadow database, if the host ' .
+           'is the local machine. This setting is required because MySQL ' .
+           'ignores the port specified by the client and connects using ' .
+           'its compiled-in socket path (on unix machines) when connecting ' .
+           'from a client to a local server. If you leave this blank, and ' .
+           'have the database on localhost, then the <tt>shadowdbport</tt> ' .
+           'will be ignored.',
+   type => 't',
+   default => '',
+  },
+
+  # This entry must be _after_ the shadowdb{host,port,sock} settings so that
+  # they can be used in the validation here
+  {
    name => 'shadowdb',
    desc => 'If non-empty, then this is the name of another database in ' .
            'which Bugzilla will keep a shadow read-only copy of everything. ' .
            'This is done so that long slow read-only operations can be used ' .
            'against this db, and not lock up things for everyone else. ' .
            'Turning on this parameter will create the given database ; be ' .
-           'careful not to use the name of an existing database with useful ' .
-           'data in it!',
+           'careful not to use the name of an existing database with useful ' .           'data in it!',
    type => 't',
    default => '',
    checker => \&check_shadowdb
   },
 
   {
-   name => 'queryagainstshadowdb',
-   desc => 'If this is on, and the shadowdb is set, then queries will ' .
-           'happen against the shadow database.',
-   type => 'b',
-   default => 0,
-  },
-
-  {
    name => 'useLDAP',
    desc => 'Turn this on to use an LDAP directory for user authentication ' .
            'instead of the Bugzilla database.  (User profiles will still be ' .
diff --git a/doeditparams.cgi b/doeditparams.cgi
index de40075f1..4dd4f8b52 100755
--- a/doeditparams.cgi
+++ b/doeditparams.cgi
@@ -101,9 +101,11 @@ foreach my $i (GetParamList()) {
 WriteParams();
 
 unlink "data/versioncache";
-print "<PRE>";
-system("./syncshadowdb", "-v") if (Param("shadowdb"));
-print "</PRE>";
+if (Param("updateshadowdb")) {
+    print "<PRE>";
+    system("./syncshadowdb", "-v");
+    print "</PRE>";
+}
 
 print "OK, done.<p>\n";
 print "<a href=editparams.cgi>Edit the params some more.</a><p>\n";
diff --git a/globals.pl b/globals.pl
index 3a2ff71a9..89919e5c0 100644
--- a/globals.pl
+++ b/globals.pl
@@ -20,7 +20,7 @@
 # Contributor(s): Terry Weissman <terry@mozilla.org>
 #                 Dan Mosedale <dmose@mozilla.org>
 #                 Jacob Steenhagen <jake@bugzilla.org>
-#                 Bradley Baetz <bbaetz@cs.mcgill.ca>
+#                 Bradley Baetz <bbaetz@student.usyd.edu.au>
 #                 Christopher Aillon <christopher@aillon.com>
 #                 Joel Peshkin <bugreport@peshkin.net> 
 
@@ -107,29 +107,70 @@ $::dbwritesallowed = 1;
 
 sub ConnectToDatabase {
     my ($useshadow) = (@_);
-    if (!defined $::db) {
-        my $name = $::db_name;
-        if ($useshadow && Param("shadowdb") && Param("queryagainstshadowdb")) {
-            $name = Param("shadowdb");
-            $::dbwritesallowed = 0;
+    $::dbwritesallowed = !$useshadow;
+    $useshadow = ($useshadow && Param("shadowdb") &&
+                  Param("queryagainstshadowdb"));
+    my $useshadow_dbh = ($useshadow && Param("shadowdbhost") ne "");
+    my $name = $useshadow ? Param("shadowdb") : $::db_name;
+    my $connectstring;
+
+    if ($useshadow_dbh) {
+        if (defined $::shadow_dbh) {
+            $::db = $::shadow_dbh;
+            return;
+        }
+        $connectstring="DBI:mysql:host=" . Param("shadowdbhost") .
+          ";database=$name;port=" . Param("shadowdbport");
+        if (Param("shadowdbsock") ne "") {
+            $connectstring .= ";mysql_socket=" . Param("shadowdbsock");
         }
-        $::db = DBI->connect("DBI:mysql:host=$::db_host;database=$name;port=$::db_port", $::db_user, $::db_pass)
-            || die "Bugzilla is currently broken. Please try again later. " . 
-      "If the problem persists, please contact " . Param("maintainer") .
-      ". The error you should quote is: " . $DBI::errstr;
+    } else {
+        if (defined $::main_dbh) {
+            $::db = $::main_dbh;
+            return;
+        }
+        $connectstring="DBI:mysql:host=$::db_host;database=$name;port=$::db_port";
+        if ($::db_sock ne "") {
+            $connectstring .= ";mysql_socket=$::db_sock";
+        }
+    }
+    $::db = DBI->connect($connectstring, $::db_user, $::db_pass)
+      || die "Bugzilla is currently broken. Please try again " .
+        "later. If the problem persists, please contact " .
+        Param("maintainer") . ". The error you should quote is: " .
+        $DBI::errstr;
+
+    if ($useshadow_dbh) {
+        $::shadow_dbh = $::db;
+    } else {
+        $::main_dbh = $::db;
     }
 }
 
 sub ReconnectToShadowDatabase {
+    # This will connect us to the shadowdb if we're not already connected,
+    # but if we're using the same dbh for both the main db and the shadowdb,
+    # be sure to USE the correct db
     if (Param("shadowdb") && Param("queryagainstshadowdb")) {
-        SendSQL("USE " . Param("shadowdb"));
-        $::dbwritesallowed = 0;
+        ConnectToDatabase(1);
+        if (!Param("shadowdbhost")) {
+            SendSQL("USE " . Param("shadowdb"));
+        }
+    }
+}
+
+sub ReconnectToMainDatabase {
+    if (Param("shadowdb") && Param("queryagainstshadowdb")) {
+        ConnectToDatabase();
+        if (!Param("shadowdbhost")) {
+            SendSQL("USE $::db_name");
+        }
     }
 }
 
 my $shadowchanges = 0;
 sub SyncAnyPendingShadowChanges {
-    if ($shadowchanges) {
+    if ($shadowchanges && Param("updateshadowdb")) {
         my $pid;
         FORK: {
             if ($pid = fork) { # create a fork
@@ -218,7 +259,7 @@ sub SendSQL {
 
     my $iswrite =  ($str =~ /^(INSERT|REPLACE|UPDATE|DELETE)/i);
     if ($iswrite && !$::dbwritesallowed) {
-        die "Evil code attempted to write stuff to the shadow database.";
+        die "Evil code attempted to write '$str' to the shadow database";
     }
     if ($str =~ /^LOCK TABLES/i && $str !~ /shadowlog/ && $::dbwritesallowed) {
         $str =~ s/^LOCK TABLES/LOCK TABLES shadowlog WRITE, /i;
@@ -242,7 +283,7 @@ sub SendSQL {
         die "$str: " . $errstr;
     }
     SqlLog("Done");
-    if (!$dontshadow && $iswrite && Param("shadowdb")) {
+    if (!$dontshadow && $iswrite && Param("shadowdb") && Param("updateshadowdb")) {
         my $q = SqlQuote($str);
         my $insertid;
         if ($str =~ /^(INSERT|REPLACE)/i) {
@@ -537,7 +578,7 @@ sub GetVersionTable {
     }
     if (time() - $mtime > 3600) {
         use Token;
-        Token::CleanTokenTable();
+        Token::CleanTokenTable() if $::dbwritesallowed;
         GenerateVersionTable();
     }
     require 'data/versioncache';
diff --git a/syncshadowdb b/syncshadowdb
index cb806bbe0..23d53a6f3 100755
--- a/syncshadowdb
+++ b/syncshadowdb
@@ -38,6 +38,8 @@ sub sillyness {
     open SAVEOUT,">/dev/null";
     $zz = $::db;
     $zz = $::dbwritesallowed;
+    $zz = $::db_host;
+    $zz = $::db_port;
 }
 
 my $verbose = 0;
@@ -98,6 +100,15 @@ if (!Param("shadowdb")) {
     exit;
 }
 
+if (!Param("updateshadowdb")) {
+    Verbose("This shadow database is not set to be updated by Bugzilla.\nSee the mysql replication FAQ if you want to pause the main db until the\nshadowdb catches up");
+    # I could run the commands here, but that involves keeping a connection
+    # open to the main db and the shadowdb at the same time, and our current
+    # db stuff doesn't support that. Its not sufficient to reconnect, because
+    # the lock on the main db will be dropped when the connection closes...
+    exit 1;
+}
+
 if (Param("shutdownhtml") && ! $force) {
     Verbose("Bugzilla was shutdown prior to running syncshadowdb. \n" .
             "  If you wish to sync anyway, use the -force command line option");
@@ -115,8 +126,9 @@ if ($shutdown) {
     # Now we need to wait for existing connections to this database to clear. We
     # do this by looking for connections to the main or shadow database using
     # 'mysqladmin processlist'
-    my $cmd = "$::mysqlpath/mysqladmin -u $::db_user";
-    if ($::db_pass) { $cmd .= " -p$::db_pass" }
+    my $cmd = "$::mysqlpath/mysqladmin -u $::db_user -h $::db_host -P $::db_port";
+    if ($::db_pass) { $cmd .= " -p$::db_pass"; }
+    if ($::db_sock) { $cmd .= " -S$::db_sock"; }
     $cmd .= " processlist";
     my $found_proc = 1;
     # We need to put together a nice little regular expression to use in the
@@ -240,6 +252,7 @@ if ($syncall) {
     Verbose("Dumping database to a temp file ($tempfile).");
     my @ARGS = ("-u", $::db_user);
     if ($::db_pass) { push @ARGS, "-p$::db_pass" }
+    if ($::db_sock) { push @ARGS, "-S$::db_sock" }
     push @ARGS, "-l", "-e", $::db_name, @tables;
     open SAVEOUT, ">&STDOUT";     # stash the original output stream
     open STDOUT, ">$tempfile";    # redirect to file
@@ -251,10 +264,13 @@ if ($syncall) {
     if ($::db_pass) {
         $extra .= " -p$::db_pass";
     }
+    if ($::db_sock) {
+        $extra .= " -S$::db_sock";
+    }
     if ($verbose) {
         $extra .= " -v";
     }
-    open(MYSQL, "cat $tempfile | $::mysqlpath/mysql $extra " .
+    open(MYSQL, "/bin/cat $tempfile | $::mysqlpath/mysql $extra " .
          Param("shadowdb") . "|") || die "Couldn't do db copy";
     my $count = 0;
     while (<MYSQL>) {