From d5eaef956709e94e9848ab3265b68e4dc92f1acd Mon Sep 17 00:00:00 2001
From: Nick Harper <nharper@fb.com>
Date: Thu, 19 Jan 2012 15:04:38 -0800
Subject: [PATCH] Add retry loop when trying to establish db connection, log
 retries

Summary:
We retried if a db connection was lost when executing a query, but not when
establishing a connection. I've seen a lot of failures establishing connections
in our install (they go away when retrying), so this diff retries when
establishing connections, and logs when we retry.

Test Plan:
- Loaded phabricator in a sandbox
- Temporarily added a check in the try block to throw if there were still
  retries (to test logging, retry logic)

Reviewers: epriestley, blair

Reviewed By: epriestley

CC: aran, btrahan

Differential Revision: https://secure.phabricator.com/D1460
---
 conf/default.conf.php                         |  3 +
 .../mysql/AphrontMySQLDatabaseConnection.php  | 63 +++++++++++--------
 src/storage/connection/mysql/__init__.php     |  2 +
 3 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/conf/default.conf.php b/conf/default.conf.php
index 7067ab735d..51566c1714 100644
--- a/conf/default.conf.php
+++ b/conf/default.conf.php
@@ -117,6 +117,9 @@ return array(
   // (e.g., db.example.com:1234).
   'mysql.host' => 'localhost',
 
+  // The number of times to try reconnecting to the MySQL database
+  'mysql.connection-retries' => 3,
+
 
 // -- Email ----------------------------------------------------------------- //
 
diff --git a/src/storage/connection/mysql/AphrontMySQLDatabaseConnection.php b/src/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
index 6a9b7639c9..6d20d45114 100644
--- a/src/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
+++ b/src/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -137,33 +137,43 @@ class AphrontMySQLDatabaseConnection extends AphrontDatabaseConnection {
         'database'  => $database,
       ));
 
-    try {
-      $conn = @mysql_connect(
-        $host,
-        $user,
-        $this->getConfiguration('pass'),
-        $new_link = true,
-        $flags = 0);
+    $retries = max(1, PhabricatorEnv::getEnvConfig('mysql.connection-retries'));
+    while ($retries--) {
+      try {
+        $conn = @mysql_connect(
+          $host,
+          $user,
+          $this->getConfiguration('pass'),
+          $new_link = true,
+          $flags = 0);
 
-      if (!$conn) {
-        $errno = mysql_errno();
-        $error = mysql_error();
-        throw new AphrontQueryConnectionException(
-          "Attempt to connect to {$user}@{$host} failed with error #{$errno}: ".
-          "{$error}.");
-      }
+        if (!$conn) {
+          $errno = mysql_errno();
+          $error = mysql_error();
+          throw new AphrontQueryConnectionException(
+            "Attempt to connect to {$user}@{$host} failed with error ".
+            "#{$errno}: {$error}.", $errno);
+        }
 
-      if ($database !== null) {
-        $ret = @mysql_select_db($database, $conn);
-        if (!$ret) {
-          $this->throwQueryException($conn);
+        if ($database !== null) {
+          $ret = @mysql_select_db($database, $conn);
+          if (!$ret) {
+            $this->throwQueryException($conn);
+          }
+        }
+
+        $profiler->endServiceCall($call_id, array());
+        break;
+      } catch (Exception $ex) {
+        if ($retries && $ex->getCode() == 2003) {
+          $class = get_class($ex);
+          $message = $ex->getMessage();
+          phlog("Retrying ({$retries}) after {$class}: {$message}");
+        } else {
+          $profiler->endServiceCall($call_id, array());
+          throw $ex;
         }
       }
-
-      $profiler->endServiceCall($call_id, array());
-    } catch (Exception $ex) {
-      $profiler->endServiceCall($call_id, array());
-      throw $ex;
     }
 
     self::$connectionCache[$key] = $conn;
@@ -203,7 +213,7 @@ class AphrontMySQLDatabaseConnection extends AphrontDatabaseConnection {
 
   public function executeRawQuery($raw_query) {
     $this->lastResult = null;
-    $retries = 3;
+    $retries = max(1, PhabricatorEnv::getEnvConfig('mysql.connection-retries'));
     while ($retries--) {
       try {
         $this->requireConnection();
@@ -242,6 +252,9 @@ class AphrontMySQLDatabaseConnection extends AphrontDatabaseConnection {
         if ($this->isInsideTransaction()) {
           throw $ex;
         }
+        $class = get_class($ex);
+        $message = $ex->getMessage();
+        phlog("Retrying ({$retries}) after {$class}: {$message}");
         $this->closeConnection();
       }
     }
diff --git a/src/storage/connection/mysql/__init__.php b/src/storage/connection/mysql/__init__.php
index 7b181925c5..4fbd82949e 100644
--- a/src/storage/connection/mysql/__init__.php
+++ b/src/storage/connection/mysql/__init__.php
@@ -7,6 +7,7 @@
 
 
 phutil_require_module('phabricator', 'aphront/writeguard');
+phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'storage/connection/base');
 phutil_require_module('phabricator', 'storage/exception/accessdenied');
 phutil_require_module('phabricator', 'storage/exception/base');
@@ -15,6 +16,7 @@ phutil_require_module('phabricator', 'storage/exception/connectionlost');
 phutil_require_module('phabricator', 'storage/exception/duplicatekey');
 phutil_require_module('phabricator', 'storage/exception/recoverable');
 
+phutil_require_module('phutil', 'error');
 phutil_require_module('phutil', 'serviceprofiler');
 phutil_require_module('phutil', 'utils');