rdbms: clean up use of ATTACH queries in DatabaseSqlite

Defer the queries until a connection exists. Only issue issue the
them for databases that are different than the currently opened file.
Also, make handleSessionLossPreconnect() aware of attached databases.

In LoadBalancer::reallyOpenConnection(), avoid having the "catch" block
appear like it returns a half-constructed Database.

Change-Id: I9f676bb72a1ab06f0eac5820dce28231741c283d

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index b5ec652..6aa3f6f 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1487,6 +1487,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxAtomicCounter = 0;
                $this->trxIdleCallbacks = []; // T67263; transaction already lost
                $this->trxPreCommitCallbacks = []; // T67263; transaction already lost
+               // Clear additional subclass fields
+               $this->doHandleSessionLossPreconnect();
                // @note: leave trxRecurringCallbacks in place
                if ( $this->trxDoneWrites ) {
                        $this->trxProfiler->transactionWritingOut(
@@ -1499,6 +1501,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
        }
 
+       /**
+        * Reset any additional subclass trx* and session* fields
+        */
+       protected function doHandleSessionLossPreconnect() {
+               // no-op
+       }
+
        /**
         * Clean things up after session (and thus transaction) loss after reconnect
         */

diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index 2977291..657d308 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -55,7 +55,7 @@ class DatabaseSqlite extends Database {
        protected $lockMgr;
 
        /** @var array List of shared database already attached to this connection */
-       private $alreadyAttached = [];
+       private $sessionAttachedDbs = [];
 
        /** @var string[] See https://www.sqlite.org/lang_transaction.html */
        private static $VALID_TRX_MODES = [ '', 'DEFERRED', 'IMMEDIATE', 'EXCLUSIVE' ];
@@ -182,6 +182,7 @@ class DatabaseSqlite extends Database {
                        if ( in_array( $sync, [ 'EXTRA', 'FULL', 'NORMAL', 'OFF' ], true ) ) {
                                $this->query( "PRAGMA synchronous = $sync", __METHOD__, $flags );
                        }
+                       $this->attachDatabasesFromTableAliases();
                } catch ( Exception $e ) {
                        throw $this->newExceptionAfterConnectError( $e->getMessage() );
                }
@@ -1124,12 +1125,23 @@ class DatabaseSqlite extends Database {
 
        public function setTableAliases( array $aliases ) {
                parent::setTableAliases( $aliases );
+               if ( $this->isOpen() ) {
+                       $this->attachDatabasesFromTableAliases();
+               }
+       }
+
+       /**
+        * Issue ATTATCH statements for all unattached foreign DBs in table aliases
+        */
+       private function attachDatabasesFromTableAliases() {
                foreach ( $this->tableAliases as $params ) {
-                       if ( isset( $this->alreadyAttached[$params['dbname']] ) ) {
-                               continue;
+                       if (
+                               $params['dbname'] !== $this->getDBname() &&
+                               !isset( $this->sessionAttachedDbs[$params['dbname']] )
+                       ) {
+                               $this->attachDatabase( $params['dbname'] );
+                               $this->sessionAttachedDbs[$params['dbname']] = true;
                        }
-                       $this->attachDatabase( $params['dbname'] );
-                       $this->alreadyAttached[$params['dbname']] = true;
                }
        }
 
@@ -1147,6 +1159,10 @@ class DatabaseSqlite extends Database {
                return true;
        }
 
+       protected function doHandleSessionLossPreconnect() {
+               $this->sessionAttachedDbs = [];
+       }
+
        /**
         * @return PDO
         */

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 771700c..f60e8db 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -1303,23 +1303,7 @@ class LoadBalancer implements ILoadBalancer {
                $server['flags'] = $server['flags'] ?? IDatabase::DBO_DEFAULT;
 
                // Create a live connection object
-               try {
-                       $conn = Database::factory( $server['type'], $server );
-                       // Log when many connection are made on requests
-                       ++$this->connectionCounter;
-                       $currentConnCount = $this->getCurrentConnectionCount();
-                       if ( $currentConnCount >= self::CONN_HELD_WARN_THRESHOLD ) {
-                               $this->perfLogger->warning(
-                                       __METHOD__ . ": {connections}+ connections made (master={masterdb})",
-                                       [ 'connections' => $currentConnCount, 'masterdb' => $masterName ]
-                               );
-                       }
-               } catch ( DBConnectionError $e ) {
-                       // FIXME: This is probably the ugliest thing I have ever done to
-                       // PHP. I'm half-expecting it to segfault, just out of disgust. -- TS
-                       $conn = $e->db;
-               }
-
+               $conn = Database::factory( $server['type'], $server, Database::NEW_UNCONNECTED );
                $conn->setLBInfo( $server );
                $conn->setLazyMasterHandle(
                        $this->getLazyConnectionRef( self::DB_MASTER, [], $conn->getDomainID() )
@@ -1327,6 +1311,13 @@ class LoadBalancer implements ILoadBalancer {
                $conn->setTableAliases( $this->tableAliases );
                $conn->setIndexAliases( $this->indexAliases );
 
+               try {
+                       $conn->initConnection();
+                       ++$this->connectionCounter;
+               } catch ( DBConnectionError $e ) {
+                       // ignore; let the DB handle the logging
+               }
+
                if ( $server['serverIndex'] === $this->getWriterIndex() ) {
                        if ( $this->trxRoundId !== false ) {
                                $this->applyTransactionRoundFlags( $conn );
@@ -1338,6 +1329,19 @@ class LoadBalancer implements ILoadBalancer {
 
                $this->lazyLoadReplicationPositions(); // session consistency
 
+               // Log when many connection are made on requests
+               $count = $this->getCurrentConnectionCount();
+               if ( $count >= self::CONN_HELD_WARN_THRESHOLD ) {
+                       $this->perfLogger->warning(
+                               __METHOD__ . ": {connections}+ connections made (master={masterdb})",
+                               [
+                                       'connections' => $count,
+                                       'dbserver' => $conn->getServer(),
+                                       'masterdb' => $conn->getLBInfo( 'clusterMasterHost' )
+                               ]
+                       );
+               }
+
                return $conn;
        }