LoadBalancerTest: Clean up transaction handling for sqlite

We need to make sure a DBO_TRX transaction was started before doing the
CREATE TABLE, because CREATE TABLE itself won't start one and sqlite
breaks if schema changes are done on one handle while another is open.

Also, incidentally, have the handles in these LoadBalancerTests log to
the standard channel. And clean up the auto-rollback of DBO_TRX
transactions to use ->rollback() instead of ->doRollback() plus
incorrect manual setting of trxStatus.

Bug: T191863
Change-Id: Ib422ef89e7eba21281e6ea98def9f98ae762b9fe

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index f681795..1779880 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1181,8 +1181,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        } else {
                                                # Nothing prior was there to lose from the transaction,
                                                # so just roll it back.
-                                               $this->doRollback( __METHOD__ . " ($fname)" );
-                                               $this->trxStatus = self::STATUS_TRX_OK;
+                                               $this->rollback( __METHOD__ . " ($fname)", self::FLUSHING_INTERNAL );
                                        }
                                        $this->trxStatusIgnoredCause = null;
                                } else {

diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php
index cb29975..88cf0e0 100644 (file)
--- a/tests/phpunit/includes/db/LoadBalancerTest.php
+++ b/tests/phpunit/includes/db/LoadBalancerTest.php
@@ -51,6 +51,7 @@ class LoadBalancerTest extends MediaWikiTestCase {
 
                $lb = new LoadBalancer( [
                        'servers' => $servers,
+                       'queryLogger' => MediaWiki\Logger\LoggerFactory::getInstance( 'DBQuery' ),
                        'localDomain' => new DatabaseDomain( $wgDBname, null, $this->dbPrefix() )
                ] );
 
@@ -120,6 +121,7 @@ class LoadBalancerTest extends MediaWikiTestCase {
                $lb = new LoadBalancer( [
                        'servers' => $servers,
                        'localDomain' => new DatabaseDomain( $wgDBname, null, $this->dbPrefix() ),
+                       'queryLogger' => MediaWiki\Logger\LoggerFactory::getInstance( 'DBQuery' ),
                        'loadMonitorClass' => LoadMonitorNull::class
                ] );
 
@@ -175,8 +177,6 @@ class LoadBalancerTest extends MediaWikiTestCase {
                                // re-throw original error, to preserve stack trace
                                throw $ex;
                        }
-               } finally {
-                       $db->rollback( __METHOD__, 'flush' );
                }
        }
 
@@ -184,6 +184,14 @@ class LoadBalancerTest extends MediaWikiTestCase {
                $table = $db->tableName( 'some_table' );
                try {
                        $db->dropTable( 'some_table' ); // clear for sanity
+
+                       // Trigger DBO_TRX to create a transaction so the flush below will
+                       // roll everything here back in sqlite. But don't actually do the
+                       // code below inside an atomic section becaue MySQL and Oracle
+                       // auto-commit transactions for DDL statements like CREATE TABLE.
+                       $db->startAtomic( __METHOD__ );
+                       $db->endAtomic( __METHOD__ );
+
                        // Use only basic SQL and trivial types for these queries for compatibility
                        $this->assertNotSame(
                                false,
@@ -195,12 +203,10 @@ class LoadBalancerTest extends MediaWikiTestCase {
                                $db->query( "DELETE FROM $table WHERE id=57634126", __METHOD__ ),
                                "delete query"
                        );
-                       $this->assertNotSame(
-                               false,
-                               $db->query( "DROP TABLE $table", __METHOD__ ),
-                               "table dropped"
-                       );
                } finally {
+                       // Drop the table to clean up, ignoring any error.
+                       $db->query( "DROP TABLE $table", __METHOD__, true );
+                       // Rollback the DBO_TRX transaction for sqlite's benefit.
                        $db->rollback( __METHOD__, 'flush' );
                }
        }