From 7bea0f628d4a2b672eef162720338ab32b7e429f Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 25 Jun 2015 23:09:52 -0700
Subject: [PATCH] database: Make LoadBalancer not yield DB objects that
 hopelessly lost the connection

* This is useful if the first slave picked went down and others
  can be used, especially in longer running scripts.
* A possible improvement to this would be to eventually allow
  retries by removing the bad handles from the load balancer,
  since isOpen() will never change from false. This would only
  be useful for very very long running CLI scripts and is
  probably an edge case for now.

Change-Id: Iecfc4004b4b2289907a4645b431de19198790d6c
---
 includes/db/DatabaseMysqlBase.php |  3 +++
 includes/db/LoadBalancer.php      | 14 ++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/includes/db/DatabaseMysqlBase.php b/includes/db/DatabaseMysqlBase.php
index 5f27dd78cc..4085fa9c4d 100644
--- a/includes/db/DatabaseMysqlBase.php
+++ b/includes/db/DatabaseMysqlBase.php
@@ -586,9 +586,12 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
 	function ping() {
 		$ping = $this->mysqlPing();
 		if ( $ping ) {
+			// Connection was good or lost but reconnected...
+			// @note: mysqlnd (php 5.6+) does not support this (PHP bug 52561)
 			return true;
 		}
 
+		// Try a full disconnect/reconnect cycle if ping() failed
 		$this->closeConnection();
 		$this->mOpened = false;
 		$this->mConn = false;
diff --git a/includes/db/LoadBalancer.php b/includes/db/LoadBalancer.php
index 069185b796..128118781f 100644
--- a/includes/db/LoadBalancer.php
+++ b/includes/db/LoadBalancer.php
@@ -650,10 +650,7 @@ class LoadBalancer {
 	public function openConnection( $i, $wiki = false ) {
 		if ( $wiki !== false ) {
 			$conn = $this->openForeignConnection( $i, $wiki );
-
-			return $conn;
-		}
-		if ( isset( $this->mConns['local'][$i][0] ) ) {
+		} elseif ( isset( $this->mConns['local'][$i][0] ) ) {
 			$conn = $this->mConns['local'][$i][0];
 		} else {
 			$server = $this->mServers[$i];
@@ -670,6 +667,15 @@ class LoadBalancer {
 			}
 		}
 
+		if ( $conn && !$conn->isOpen() ) {
+			// Connection was made but later unrecoverably lost for some reason.
+			// Do not return a handle that will just throw exceptions on use,
+			// but let the calling code (e.g. getReaderIndex) try another server.
+			// See DatabaseMyslBase::ping() for how this can happen.
+			$this->mErrorConnection = $conn;
+			$conn = false;
+		}
+
 		return $conn;
 	}
 
-- 
2.20.1