Skip to content

Commit d5eaef9

Browse files
author
Nick Harper
committed
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
1 parent 27f52ef commit d5eaef9

File tree

3 files changed

+44
-26
lines changed

3 files changed

+44
-26
lines changed

conf/default.conf.php

+3
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@
117117
// (e.g., db.example.com:1234).
118118
'mysql.host' => 'localhost',
119119

120+
// The number of times to try reconnecting to the MySQL database
121+
'mysql.connection-retries' => 3,
122+
120123

121124
// -- Email ----------------------------------------------------------------- //
122125

src/storage/connection/mysql/AphrontMySQLDatabaseConnection.php

+39-26
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php
22

33
/*
4-
* Copyright 2011 Facebook, Inc.
4+
* Copyright 2012 Facebook, Inc.
55
*
66
* Licensed under the Apache License, Version 2.0 (the "License");
77
* you may not use this file except in compliance with the License.
@@ -137,33 +137,43 @@ private function establishConnection() {
137137
'database' => $database,
138138
));
139139

140-
try {
141-
$conn = @mysql_connect(
142-
$host,
143-
$user,
144-
$this->getConfiguration('pass'),
145-
$new_link = true,
146-
$flags = 0);
147-
148-
if (!$conn) {
149-
$errno = mysql_errno();
150-
$error = mysql_error();
151-
throw new AphrontQueryConnectionException(
152-
"Attempt to connect to {$user}@{$host} failed with error #{$errno}: ".
153-
"{$error}.");
154-
}
140+
$retries = max(1, PhabricatorEnv::getEnvConfig('mysql.connection-retries'));
141+
while ($retries--) {
142+
try {
143+
$conn = @mysql_connect(
144+
$host,
145+
$user,
146+
$this->getConfiguration('pass'),
147+
$new_link = true,
148+
$flags = 0);
149+
150+
if (!$conn) {
151+
$errno = mysql_errno();
152+
$error = mysql_error();
153+
throw new AphrontQueryConnectionException(
154+
"Attempt to connect to {$user}@{$host} failed with error ".
155+
"#{$errno}: {$error}.", $errno);
156+
}
155157

156-
if ($database !== null) {
157-
$ret = @mysql_select_db($database, $conn);
158-
if (!$ret) {
159-
$this->throwQueryException($conn);
158+
if ($database !== null) {
159+
$ret = @mysql_select_db($database, $conn);
160+
if (!$ret) {
161+
$this->throwQueryException($conn);
162+
}
160163
}
161-
}
162164

163-
$profiler->endServiceCall($call_id, array());
164-
} catch (Exception $ex) {
165-
$profiler->endServiceCall($call_id, array());
166-
throw $ex;
165+
$profiler->endServiceCall($call_id, array());
166+
break;
167+
} catch (Exception $ex) {
168+
if ($retries && $ex->getCode() == 2003) {
169+
$class = get_class($ex);
170+
$message = $ex->getMessage();
171+
phlog("Retrying ({$retries}) after {$class}: {$message}");
172+
} else {
173+
$profiler->endServiceCall($call_id, array());
174+
throw $ex;
175+
}
176+
}
167177
}
168178

169179
self::$connectionCache[$key] = $conn;
@@ -203,7 +213,7 @@ public function selectAllResults() {
203213

204214
public function executeRawQuery($raw_query) {
205215
$this->lastResult = null;
206-
$retries = 3;
216+
$retries = max(1, PhabricatorEnv::getEnvConfig('mysql.connection-retries'));
207217
while ($retries--) {
208218
try {
209219
$this->requireConnection();
@@ -242,6 +252,9 @@ public function executeRawQuery($raw_query) {
242252
if ($this->isInsideTransaction()) {
243253
throw $ex;
244254
}
255+
$class = get_class($ex);
256+
$message = $ex->getMessage();
257+
phlog("Retrying ({$retries}) after {$class}: {$message}");
245258
$this->closeConnection();
246259
}
247260
}

src/storage/connection/mysql/__init__.php

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88

99
phutil_require_module('phabricator', 'aphront/writeguard');
10+
phutil_require_module('phabricator', 'infrastructure/env');
1011
phutil_require_module('phabricator', 'storage/connection/base');
1112
phutil_require_module('phabricator', 'storage/exception/accessdenied');
1213
phutil_require_module('phabricator', 'storage/exception/base');
@@ -15,6 +16,7 @@
1516
phutil_require_module('phabricator', 'storage/exception/duplicatekey');
1617
phutil_require_module('phabricator', 'storage/exception/recoverable');
1718

19+
phutil_require_module('phutil', 'error');
1820
phutil_require_module('phutil', 'serviceprofiler');
1921
phutil_require_module('phutil', 'utils');
2022

0 commit comments

Comments
 (0)