rdbms: ignore DBO_NOBUFFER flag in IDatabase

Callers should use query batching instead. Without buffering, either
the RAM burden is shifted onto the RDBMS server (with MVCC vacuuming
also being stalled) or the RDBMS server has to keep adding locks as
the cursor advances. Also, if a caller does not read all the results
(possibly due to an exception), then the SQL commands sent/read get
out of sync, which is too fragile.

There are no DBO_NOBUFFER callers in WMF gerrit repos.

Change-Id: I3712149633d0f01bb6990e324e53dd58abba9cfd

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 1b511d5..6029f77 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -250,8 +250,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                $this->flags |= self::DBO_TRX;
                        }
                }
-               // Disregard deprecated DBO_IGNORE flag (T189999)
-               $this->flags &= ~self::DBO_IGNORE;
 
                $this->connectionVariables = $params['variables'];
 
@@ -516,35 +514,14 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        /**
-        * Turns buffering of SQL result sets on (true) or off (false). Default is "on".
+        * Backwards-compatibility no-op method for disabling query buffering
         *
-        * Unbuffered queries are very troublesome in MySQL:
-        *
-        *   - If another query is executed while the first query is being read
-        *     out, the first query is killed. This means you can't call normal
-        *     Database functions while you are reading an unbuffered query result
-        *     from a normal Database connection.
-        *
-        *   - Unbuffered queries cause the MySQL server to use large amounts of
-        *     memory and to hold broad locks which block other queries.
-        *
-        * If you want to limit client-side memory, it's almost always better to
-        * split up queries into batches using a LIMIT clause than to switch off
-        * buffering.
-        *
-        * @param null|bool $buffer
-        * @return bool The previous value of the flag
-        * @deprecated Since 1.34 Use query batching
+        * @param null|bool $buffer Whether to buffer queries (ignored)
+        * @return bool Whether buffering was already enabled (always true)
+        * @deprecated Since 1.34 Use query batching; this no longer does anything
         */
        public function bufferResults( $buffer = null ) {
-               $res = !$this->getFlag( self::DBO_NOBUFFER );
-               if ( $buffer !== null ) {
-                       $buffer
-                               ? $this->clearFlag( self::DBO_NOBUFFER )
-                               : $this->setFlag( self::DBO_NOBUFFER );
-               }
-
-               return $res;
+               return true;
        }
 
        final public function trxLevel() {

diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php b/includes/libs/rdbms/database/DatabaseMysqli.php
index d32a12e..2f9abcf 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqli.php
+++ b/includes/libs/rdbms/database/DatabaseMysqli.php
@@ -40,15 +40,7 @@ class DatabaseMysqli extends DatabaseMysqlBase {
         * @return mysqli_result|bool
         */
        protected function doQuery( $sql ) {
-               $conn = $this->getBindingHandle();
-
-               if ( $this->getFlag( self::DBO_NOBUFFER ) ) {
-                       $ret = $conn->query( $sql, MYSQLI_USE_RESULT );
-               } else {
-                       $ret = $conn->query( $sql );
-               }
-
-               return $ret;
+               return $this->getBindingHandle()->query( $sql );
        }
 
        /**

diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index b8c1509..f7f87a2 100644 (file)
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -89,9 +89,9 @@ interface IDatabase {
 
        /** @var int Enable debug logging of all SQL queries */
        const DBO_DEBUG = 1;
-       /** @var int Disable query buffering (only one result set can be iterated at a time) */
+       /** @var int Unused since 1.34 */
        const DBO_NOBUFFER = 2;
-       /** @var int Ignore query errors (internal use only!) */
+       /** @var int Unused since 1.31 */
        const DBO_IGNORE = 4;
        /** @var int Automatically start a transaction before running a query if none is active */
        const DBO_TRX = 8;
@@ -297,7 +297,7 @@ interface IDatabase {
        /**
         * Set a flag for this connection
         *
-        * @param int $flag IDatabase::DBO_DEBUG, IDatabase::DBO_NOBUFFER, or IDatabase::DBO_TRX
+        * @param int $flag One of (IDatabase::DBO_DEBUG, IDatabase::DBO_TRX)
         * @param string $remember IDatabase::REMEMBER_* constant [default: REMEMBER_NOTHING]
         */
        public function setFlag( $flag, $remember = self::REMEMBER_NOTHING );
@@ -305,7 +305,7 @@ interface IDatabase {
        /**
         * Clear a flag for this connection
         *
-        * @param int $flag IDatabase::DBO_DEBUG, IDatabase::DBO_NOBUFFER, or IDatabase::DBO_TRX
+        * @param int $flag One of (IDatabase::DBO_DEBUG, IDatabase::DBO_TRX)
         * @param string $remember IDatabase::REMEMBER_* constant [default: REMEMBER_NOTHING]
         */
        public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING );