Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/316509

Change subject: Clean up postgres connection handling
......................................................................

Clean up postgres connection handling

* Remove non-connection magic case when no DB $user
  is given. This was removed from the base class.
* Use PGSQL_CONNECT_FORCE_NEW to let LoadBalancer
  handle connection reuse. This makes it work like
  the mysql classes.
* Make postgres connection error messages actually
  be useful by using the PHP error when possible.
  This makes it clear if the problem is authentication
  or something else and so on.

Change-Id: I3fd76c1e2db8d6008074f5347b201554579b549a
---
M includes/libs/rdbms/database/Database.php
M includes/libs/rdbms/database/DatabasePostgres.php
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
3 files changed, 19 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/09/316509/1

diff --git a/includes/libs/rdbms/database/Database.php 
b/includes/libs/rdbms/database/Database.php
index f33e244..4266912 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -651,14 +651,22 @@
                if ( $this->htmlErrors !== false ) {
                        ini_set( 'html_errors', $this->htmlErrors );
                }
+
+               return $this->getLastPHPError();
+       }
+
+       /**
+        * @return string|bool Last PHP error for this DB (typically connection 
errors)
+        */
+       protected function getLastPHPError() {
                if ( $this->mPHPError ) {
                        $error = preg_replace( '!\[<a.*</a>\]!', '', 
$this->mPHPError );
                        $error = preg_replace( '!^.*?:\s?(.*)$!', '$1', $error 
);
 
                        return $error;
-               } else {
-                       return false;
                }
+
+               return false;
        }
 
        /**
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php 
b/includes/libs/rdbms/database/DatabasePostgres.php
index 84021a0..016b9cd 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -92,10 +92,6 @@
                        );
                }
 
-               if ( !strlen( $user ) ) { # e.g. the class is being loaded
-                       return null;
-               }
-
                $this->mServer = $server;
                $this->mUser = $user;
                $this->mPassword = $password;
@@ -121,7 +117,8 @@
                $this->installErrorHandler();
 
                try {
-                       $this->mConn = pg_connect( $this->connectString );
+                       // Use new connections to let LoadBalancer/LBFactory 
handle reuse
+                       $this->mConn = pg_connect( $this->connectString, 
PGSQL_CONNECT_FORCE_NEW );
                } catch ( Exception $ex ) {
                        $this->restoreErrorHandler();
                        throw $ex;
@@ -130,10 +127,11 @@
                $phpError = $this->restoreErrorHandler();
 
                if ( !$this->mConn ) {
-                       $this->queryLogger->debug( "DB connection error\n" );
                        $this->queryLogger->debug(
+                               "DB connection error\n" .
                                "Server: $server, Database: $dbName, User: 
$user, Password: " .
-                               substr( $password, 0, 3 ) . "...\n" );
+                               substr( $password, 0, 3 ) . "...\n"
+                       );
                        $this->queryLogger->debug( $this->lastError() . "\n" );
                        throw new DBConnectionError( $this, str_replace( "\n", 
' ', $phpError ) );
                }
@@ -380,9 +378,9 @@
                        } else {
                                return pg_last_error();
                        }
-               } else {
-                       return 'No database connection';
                }
+
+               return $this->getLastPHPError() ?: 'No database connection';
        }
 
        function lastErrno() {
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php 
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 682698d..a7df9c8 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -552,7 +552,7 @@
                if ( $i == self::DB_REPLICA ) {
                        $this->mLastError = 'Unknown error'; // reset error 
string
                        # Try the general server pool if $groups are 
unavailable.
-                       $i = in_array( false, $groups, true )
+                       $i = ( $groups == [ false ] )
                                ? false // don't bother with this if that is 
what was tried above
                                : $this->getReaderIndex( false, $domain );
                        # Couldn't find a working server in getReaderIndex()?
@@ -886,7 +886,7 @@
                        // If all servers were busy, mLastError will contain 
something sensible
                        throw new DBConnectionError( null, $this->mLastError );
                } else {
-                       $context['db_server'] = $conn->getProperty( 'mServer' );
+                       $context['db_server'] = $conn->getServer();
                        $this->connLogger->warning(
                                "Connection error: {last_error} ({db_server})",
                                $context

-- 
To view, visit https://gerrit.wikimedia.org/r/316509
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3fd76c1e2db8d6008074f5347b201554579b549a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to