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

diff --git a/includes/db/DatabaseMysqlBase.php b/includes/db/DatabaseMysqlBase.php
index 5f27dd7..4085fa9 100644 (file)
--- 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 069185b..1281187 100644 (file)
--- 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;
        }