rdbms: add IDatabase::wasConnectionLoss() method

This takes the logic from wasErrorReissuable(), but puts it under
better name. The way that method was used, as well its comments,
were only about connection loss.

Make wasErrorReissuable() check if there was any error that
does not preclude the ability to retry. This matches the actual
name of the method.

Also improve some other related comments.

Change-Id: I68455d803afb2370897fecab0e79aadbb5d1a740

diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php
index acb21ed..6726aea 100644 (file)
--- a/includes/libs/rdbms/database/DBConnRef.php
+++ b/includes/libs/rdbms/database/DBConnRef.php
@@ -460,7 +460,7 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
-       public function wasErrorReissuable() {
+       public function wasConnectionLoss() {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -468,6 +468,10 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
+       public function wasErrorReissuable() {
+               return $this->__call( __FUNCTION__, func_get_args() );
+       }
+
        public function masterPosWait( DBMasterPos $pos, $timeout ) {
                return $this->__call( __FUNCTION__, func_get_args() );
        }

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 97ea266..2d90be6 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1061,7 +1061,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $ret = $this->doProfiledQuery( $sql, $commentedSql, $isNonTempWrite, $fname );
 
                # Try reconnecting if the connection was lost
-               if ( false === $ret && $this->wasErrorReissuable() ) {
+               if ( false === $ret && $this->wasConnectionLoss() ) {
                        $recoverable = $this->canRecoverFromDisconnect( $sql, $priorWritesPending );
                        # Stash the last error values before anything might clear them
                        $lastError = $this->lastError();
@@ -2888,14 +2888,22 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                return false;
        }
 
-       public function wasErrorReissuable() {
-               return false;
+       public function wasConnectionLoss() {
+               return $this->wasConnectionError( $this->lastErrno() );
        }
 
        public function wasReadOnlyError() {
                return false;
        }
 
+       public function wasErrorReissuable() {
+               return (
+                       $this->wasDeadlock() ||
+                       $this->wasLockTimeout() ||
+                       $this->wasConnectionLoss()
+               );
+       }
+
        /**
         * Do not use this method outside of Database/DBError classes
         *

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index b7778b4..7537578 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -1282,10 +1282,6 @@ abstract class DatabaseMysqlBase extends Database {
                return $this->lastErrno() == 1205;
        }
 
-       public function wasErrorReissuable() {
-               return $this->lastErrno() == 2013 || $this->lastErrno() == 2006;
-       }
-
        /**
         * Determines if the last failure was due to the database being read-only.
         *

diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index d0d62e9..d5a7489 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -716,13 +716,6 @@ class DatabaseSqlite extends Database {
                return $this->lastErrno() == 5; // SQLITE_BUSY
        }
 
-       /**
-        * @return bool
-        */
-       function wasErrorReissuable() {
-               return $this->lastErrno() == 17; // SQLITE_SCHEMA;
-       }
-
        /**
         * @return bool
         */
@@ -730,6 +723,10 @@ class DatabaseSqlite extends Database {
                return $this->lastErrno() == 8; // SQLITE_READONLY;
        }
 
+       public function wasConnectionError( $errno ) {
+               return $errno == 17; // SQLITE_SCHEMA;
+       }
+
        /**
         * @return string Wikitext of a link to the server software's web site
         */

diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index 09abaa8..e5e2076 100644 (file)
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -513,6 +513,10 @@ interface IDatabase {
         * Run an SQL query and return the result. Normally throws a DBQueryError
         * on failure. If errors are ignored, returns false instead.
         *
+        * If a connection loss is detected, then an attempt to reconnect will be made.
+        * For queries that involve no larger transactions or locks, they will be re-issued
+        * for convenience, provided the connection was re-established.
+        *
         * In new code, the query wrappers select(), insert(), update(), delete(),
         * etc. should be used where possible, since they give much better DBMS
         * independence and automatically quote or validate user input in a variety
@@ -1400,6 +1404,8 @@ interface IDatabase {
        /**
         * Determines if the last failure was due to a deadlock
         *
+        * Note that during a deadlock, the prior transaction will have been lost
+        *
         * @return bool
         */
        public function wasDeadlock();
@@ -1407,17 +1413,21 @@ interface IDatabase {
        /**
         * Determines if the last failure was due to a lock timeout
         *
+        * Note that during a lock wait timeout, the prior transaction will have been lost
+        *
         * @return bool
         */
        public function wasLockTimeout();
 
        /**
-        * Determines if the last query error was due to a dropped connection and should
-        * be dealt with by pinging the connection and reissuing the query.
+        * Determines if the last query error was due to a dropped connection
+        *
+        * Note that during a connection loss, the prior transaction will have been lost
         *
         * @return bool
+        * @since 1.31
         */
-       public function wasErrorReissuable();
+       public function wasConnectionLoss();
 
        /**
         * Determines if the last failure was due to the database being read-only.
@@ -1426,6 +1436,15 @@ interface IDatabase {
         */
        public function wasReadOnlyError();
 
+       /**
+        * Determines if the last query error was due to something outside of the query itself
+        *
+        * Note that the transaction may have been lost, discarding prior writes and results
+        *
+        * @return bool
+        */
+       public function wasErrorReissuable();
+
        /**
         * Wait for the replica DB to catch up to a given master position
         *