jenkins-bot has submitted this change and it was merged.
Change subject: Postgres installation fixes
......................................................................
Postgres installation fixes
* Make isTransactableQuery() exclude CREATE/ALTER.
Starting transactions for schema changes like this can cause
errors as it is not supported for MySQL and some Postgres
operations. Note that temporary tables are session-level,
so they are not effected by this change.
* Clean up the transaction logic in determineCoreSchema()
so a transaction is not left dangling.
* Fix broken getSchemaPath() call in PostgresInstaller.
* Avoid warnings in DatabasePostgres::closeConnection() if
mConn is already unset.
* Commit master changes in doMaintenance.php before running
deferred updates, just as MediaWiki.php does.
* Change E_WARNING to E_USER_WARNING to avoid notices in the
default /rdbms error handlers.
* Also avoid trying to rollback in MWExceptionHandler if the
LBFactory service is disabled, which just results in an error.
Bug: T147599
Change-Id: I64ccab7f9b74f60309ba0c9a8ce68337c42ffb0f
---
M includes/exception/MWExceptionHandler.php
M includes/installer/PostgresInstaller.php
M includes/libs/rdbms/database/Database.php
M includes/libs/rdbms/database/DatabaseMysqlBase.php
M includes/libs/rdbms/database/DatabaseMysqli.php
M includes/libs/rdbms/database/DatabasePostgres.php
M includes/libs/rdbms/database/DatabaseSqlite.php
M includes/libs/rdbms/lbfactory/LBFactory.php
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
M maintenance/doMaintenance.php
10 files changed, 82 insertions(+), 61 deletions(-)
Approvals:
Gergő Tisza: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/exception/MWExceptionHandler.php
b/includes/exception/MWExceptionHandler.php
index 4a1f190..736cb06 100644
--- a/includes/exception/MWExceptionHandler.php
+++ b/includes/exception/MWExceptionHandler.php
@@ -87,7 +87,12 @@
* @param Exception|Throwable $e
*/
public static function rollbackMasterChangesAndLog( $e ) {
- $lbFactory =
MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+ $services = MediaWikiServices::getInstance();
+ if ( $services->isServiceDisabled( 'DBLoadBalancerFactory' ) ) {
+ return; // T147599
+ }
+
+ $lbFactory = $services->getDBLoadBalancerFactory();
if ( $lbFactory->hasMasterChanges() ) {
$logger = LoggerFactory::getInstance( 'Bug56269' );
$logger->warning(
diff --git a/includes/installer/PostgresInstaller.php
b/includes/installer/PostgresInstaller.php
index 33e1a1f..6dfa28b 100644
--- a/includes/installer/PostgresInstaller.php
+++ b/includes/installer/PostgresInstaller.php
@@ -587,9 +587,7 @@
return $status;
}
- /**
- * @var $conn Database
- */
+ /** @var $conn DatabasePostgres */
$conn = $status->value;
if ( $conn->tableExists( 'archive' ) ) {
@@ -606,7 +604,7 @@
return $status;
}
- $error = $conn->sourceFile( $conn->getSchemaPath() );
+ $error = $conn->sourceFile( $this->getSchemaPath( $conn ) );
if ( $error !== true ) {
$conn->reportQueryError( $error, 0, '', __METHOD__ );
$conn->rollback( __METHOD__ );
diff --git a/includes/libs/rdbms/database/Database.php
b/includes/libs/rdbms/database/Database.php
index a5a170b..f33e244 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -78,7 +78,7 @@
/** @var callback Error logging callback */
protected $errorLogger;
- /** @var resource Database connection */
+ /** @var resource|null Database connection */
protected $mConn = null;
/** @var bool */
protected $mOpened = false;
@@ -382,7 +382,7 @@
}
if ( !isset( $p['errorLogger'] ) ) {
$p['errorLogger'] = function ( Exception $e ) {
- trigger_error( get_class( $e ) . ': ' .
$e->getMessage(), E_WARNING );
+ trigger_error( get_class( $e ) . ': ' .
$e->getMessage(), E_USER_WARNING );
};
}
@@ -773,8 +773,11 @@
* @return bool
*/
protected function isTransactableQuery( $sql ) {
- $verb = $this->getQueryVerb( $sql );
- return !in_array( $verb, [ 'BEGIN', 'COMMIT', 'ROLLBACK',
'SHOW', 'SET' ], true );
+ return !in_array(
+ $this->getQueryVerb( $sql ),
+ [ 'BEGIN', 'COMMIT', 'ROLLBACK', 'SHOW', 'SET',
'CREATE', 'ALTER' ],
+ true
+ );
}
/**
@@ -2828,7 +2831,7 @@
$fnames = implode( ', ',
$this->pendingWriteAndCallbackCallers() );
throw new DBUnexpectedError(
$this,
- "$fname: Cannot COMMIT to clear snapshot
because writes are pending ($fnames)."
+ "$fname: Cannot flush snapshot because writes
are pending ($fnames)."
);
}
@@ -3249,7 +3252,7 @@
$fnames = implode( ', ',
$this->pendingWriteAndCallbackCallers() );
throw new DBUnexpectedError(
$this,
- "$fname: Cannot COMMIT to clear snapshot
because writes are pending ($fnames)."
+ "$fname: Cannot flush pre-lock snapshot because
writes are pending ($fnames)."
);
}
@@ -3369,6 +3372,28 @@
}
/**
+ * Get the underlying binding handle, mConn
+ *
+ * Makes sure that mConn is set (disconnects and ping() failure can
unset it).
+ * This catches broken callers than catch and ignore disconnection
exceptions.
+ * Unlike checking isOpen(), this is safe to call inside of open().
+ *
+ * @return resource|object
+ * @throws DBUnexpectedError
+ * @since 1.26
+ */
+ protected function getBindingHandle() {
+ if ( !$this->mConn ) {
+ throw new DBUnexpectedError(
+ $this,
+ 'DB connection was already closed or the
connection dropped.'
+ );
+ }
+
+ return $this->mConn;
+ }
+
+ /**
* @since 1.19
* @return string
*/
@@ -3422,8 +3447,11 @@
}
if ( $this->mConn ) {
- // Avoid connection leaks for sanity
+ // Avoid connection leaks for sanity. Normally,
resources close at script completion.
+ // The connection might already be closed in zend/hhvm
by now, so suppress warnings.
+ \MediaWiki\suppressWarnings();
$this->closeConnection();
+ \MediaWiki\restoreWarnings();
$this->mConn = false;
$this->mOpened = false;
}
diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php
b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 9662a5b..3b7681e 100644
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -1205,28 +1205,6 @@
}
/**
- * Get the underlying binding handle, mConn
- *
- * Makes sure that mConn is set (disconnects and ping() failure can
unset it).
- * This catches broken callers than catch and ignore disconnection
exceptions.
- * Unlike checking isOpen(), this is safe to call inside of open().
- *
- * @return resource|object
- * @throws DBUnexpectedError
- * @since 1.26
- */
- protected function getBindingHandle() {
- if ( !$this->mConn ) {
- throw new DBUnexpectedError(
- $this,
- 'DB connection was already closed or the
connection dropped.'
- );
- }
-
- return $this->mConn;
- }
-
- /**
* @param string $oldName
* @param string $newName
* @param bool $temporary
diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php
b/includes/libs/rdbms/database/DatabaseMysqli.php
index c34f901..2f27ff9 100644
--- a/includes/libs/rdbms/database/DatabaseMysqli.php
+++ b/includes/libs/rdbms/database/DatabaseMysqli.php
@@ -29,8 +29,7 @@
* @see Database
*/
class DatabaseMysqli extends DatabaseMysqlBase {
- /** @var mysqli */
- protected $mConn;
+ /** @var $mConn mysqli */
/**
* @param string $sql
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php
b/includes/libs/rdbms/database/DatabasePostgres.php
index f82d76d..84021a0 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -61,10 +61,12 @@
}
function hasConstraint( $name ) {
+ $conn = $this->getBindingHandle();
+
$sql = "SELECT 1 FROM pg_catalog.pg_constraint c,
pg_catalog.pg_namespace n " .
"WHERE c.connamespace = n.oid AND conname = '" .
- pg_escape_string( $this->mConn, $name ) . "' AND
n.nspname = '" .
- pg_escape_string( $this->mConn, $this->getCoreSchema()
) . "'";
+ pg_escape_string( $conn, $name ) . "' AND n.nspname =
'" .
+ pg_escape_string( $conn, $this->getCoreSchema() ) . "'";
$res = $this->doQuery( $sql );
return $this->numRows( $res );
@@ -185,19 +187,21 @@
* @return bool
*/
protected function closeConnection() {
- return pg_close( $this->mConn );
+ return $this->mConn ? pg_close( $this->mConn ) : true;
}
public function doQuery( $sql ) {
+ $conn = $this->getBindingHandle();
+
$sql = mb_convert_encoding( $sql, 'UTF-8' );
// Clear previously left over PQresult
- while ( $res = pg_get_result( $this->mConn ) ) {
+ while ( $res = pg_get_result( $conn ) ) {
pg_free_result( $res );
}
- if ( pg_send_query( $this->mConn, $sql ) === false ) {
+ if ( pg_send_query( $conn, $sql ) === false ) {
throw new DBUnexpectedError( $this, "Unable to post new
query to PostgreSQL\n" );
}
- $this->mLastResult = pg_get_result( $this->mConn );
+ $this->mLastResult = pg_get_result( $conn );
$this->mAffectedRows = null;
if ( pg_result_error( $this->mLastResult ) ) {
return false;
@@ -281,10 +285,11 @@
# @todo hashar: not sure if the following test really trigger
if the object
# fetching failed.
- if ( pg_last_error( $this->mConn ) ) {
+ $conn = $this->getBindingHandle();
+ if ( pg_last_error( $conn ) ) {
throw new DBUnexpectedError(
$this,
- 'SQL error: ' . htmlspecialchars(
pg_last_error( $this->mConn ) )
+ 'SQL error: ' . htmlspecialchars(
pg_last_error( $conn ) )
);
}
@@ -298,10 +303,12 @@
MediaWiki\suppressWarnings();
$row = pg_fetch_array( $res );
MediaWiki\restoreWarnings();
- if ( pg_last_error( $this->mConn ) ) {
+
+ $conn = $this->getBindingHandle();
+ if ( pg_last_error( $conn ) ) {
throw new DBUnexpectedError(
$this,
- 'SQL error: ' . htmlspecialchars(
pg_last_error( $this->mConn ) )
+ 'SQL error: ' . htmlspecialchars(
pg_last_error( $conn ) )
);
}
@@ -315,10 +322,12 @@
MediaWiki\suppressWarnings();
$n = pg_num_rows( $res );
MediaWiki\restoreWarnings();
- if ( pg_last_error( $this->mConn ) ) {
+
+ $conn = $this->getBindingHandle();
+ if ( pg_last_error( $conn ) ) {
throw new DBUnexpectedError(
$this,
- 'SQL error: ' . htmlspecialchars(
pg_last_error( $this->mConn ) )
+ 'SQL error: ' . htmlspecialchars(
pg_last_error( $conn ) )
);
}
@@ -1033,7 +1042,7 @@
$this->mCoreSchema . "\"\n" );
}
/* Commit SET otherwise it will be rollbacked on error or
IGNORE SELECT */
- $this->commit( __METHOD__ );
+ $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
}
/**
@@ -1051,7 +1060,8 @@
*/
function getServerVersion() {
if ( !isset( $this->numericVersion ) ) {
- $versionInfo = pg_version( $this->mConn );
+ $conn = $this->getBindingHandle();
+ $versionInfo = pg_version( $conn );
if ( version_compare( $versionInfo['client'], '7.4.0',
'lt' ) ) {
// Old client, abort install
$this->numericVersion = '7.3 or earlier';
@@ -1060,7 +1070,7 @@
$this->numericVersion = $versionInfo['server'];
} else {
// Bug 16937: broken pgsql extension from
PHP<5.3
- $this->numericVersion = pg_parameter_status(
$this->mConn, 'server_version' );
+ $this->numericVersion = pg_parameter_status(
$conn, 'server_version' );
}
}
@@ -1229,7 +1239,7 @@
function strencode( $s ) {
// Should not be called by us
- return pg_escape_string( $this->mConn, $s );
+ return pg_escape_string( $this->getBindingHandle(), $s );
}
/**
@@ -1237,6 +1247,8 @@
* @return string|int
*/
function addQuotes( $s ) {
+ $conn = $this->getBindingHandle();
+
if ( is_null( $s ) ) {
return 'NULL';
} elseif ( is_bool( $s ) ) {
@@ -1245,12 +1257,12 @@
if ( $s instanceof PostgresBlob ) {
$s = $s->fetch();
} else {
- $s = pg_escape_bytea( $this->mConn, $s->fetch()
);
+ $s = pg_escape_bytea( $conn, $s->fetch() );
}
return "'$s'";
}
- return "'" . pg_escape_string( $this->mConn, $s ) . "'";
+ return "'" . pg_escape_string( $conn, $s ) . "'";
}
/**
diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php
b/includes/libs/rdbms/database/DatabaseSqlite.php
index 3ccf3f0..31bb26b 100644
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -44,8 +44,7 @@
/** @var resource */
protected $mLastResult;
- /** @var PDO */
- protected $mConn;
+ /** @var $mConn PDO */
/** @var FSLockManager (hopefully on the same server as the DB) */
protected $lockMgr;
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php
b/includes/libs/rdbms/lbfactory/LBFactory.php
index 929bd4d..d107e10 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -96,7 +96,7 @@
$this->errorLogger = isset( $conf['errorLogger'] )
? $conf['errorLogger']
: function ( Exception $e ) {
- trigger_error( E_WARNING, get_class( $e ) . ':
' . $e->getMessage() );
+ trigger_error( E_USER_WARNING, get_class( $e )
. ': ' . $e->getMessage() );
};
$this->profiler = isset( $params['profiler'] ) ?
$params['profiler'] : null;
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 32df19d..682698d 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -189,7 +189,7 @@
$this->errorLogger = isset( $params['errorLogger'] )
? $params['errorLogger']
: function ( Exception $e ) {
- trigger_error( get_class( $e ) . ': ' .
$e->getMessage(), E_WARNING );
+ trigger_error( get_class( $e ) . ': ' .
$e->getMessage(), E_USER_WARNING );
};
foreach ( [ 'replLogger', 'connLogger', 'queryLogger',
'perfLogger' ] as $key ) {
diff --git a/maintenance/doMaintenance.php b/maintenance/doMaintenance.php
index 60b24a2..f3561b5 100644
--- a/maintenance/doMaintenance.php
+++ b/maintenance/doMaintenance.php
@@ -25,6 +25,7 @@
* @file
* @ingroup Maintenance
*/
+use MediaWiki\MediaWikiServices;
if ( !defined( 'RUN_MAINTENANCE_IF_MAIN' ) ) {
echo "This file must be included after Maintenance.php\n";
@@ -113,12 +114,13 @@
$maintenance->globals();
// Perform deferred updates.
+$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+$lbFactory->commitMasterChanges( $maintClass );
DeferredUpdates::doUpdates();
// log profiling info
wfLogProfilingData();
// Commit and close up!
-$factory = wfGetLBFactory();
-$factory->commitMasterChanges( 'doMaintenance' );
-$factory->shutdown( $factory::SHUTDOWN_NO_CHRONPROT );
+$lbFactory->commitMasterChanges( 'doMaintenance' );
+$lbFactory->shutdown( $lbFactory::SHUTDOWN_NO_CHRONPROT );
--
To view, visit https://gerrit.wikimedia.org/r/316399
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I64ccab7f9b74f60309ba0c9a8ce68337c42ffb0f
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Paladox <thomasmulhall...@yahoo.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
