From 02934ef5037a3425f677e6942be066bf645160a6 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 15 Aug 2019 10:58:54 -0400 Subject: [PATCH] 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 --- includes/libs/rdbms/database/Database.php | 33 +++---------------- .../libs/rdbms/database/DatabaseMysqli.php | 10 +----- includes/libs/rdbms/database/IDatabase.php | 8 ++--- 3 files changed, 10 insertions(+), 41 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 1b511d5ecf..6029f779e7 100644 --- 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 d32a12eb44..2f9abcf7d8 100644 --- 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 b8c1509acf..f7f87a2569 100644 --- 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 ); -- 2.20.1