jenkins-bot has submitted this change and it was merged.

Change subject: Do not auto-reconnect to DBs if named locks where lost
......................................................................


Do not auto-reconnect to DBs if named locks where lost

Otherwise, transactions might be committed without cooperative
locks that were supposed to be present. This is similar to the
logic of not auto-reconnecting if a transaction was started to
avoid committing incomplete changes.

Change-Id: Ia7bc6b188bb5ee53a5bf7c5a30718bc7c4dd0ba9
---
M includes/db/Database.php
M includes/db/DatabaseMysqlBase.php
M includes/db/DatabasePostgres.php
3 files changed, 31 insertions(+), 7 deletions(-)

Approvals:
  Addshore: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/db/Database.php b/includes/db/Database.php
index dc1570e..9315daf 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -155,6 +155,9 @@
         */
        private $mTrxWriteDuration = 0.0;
 
+       /** @var array Map of (name => 1) for locks obtained via lock() */
+       private $mNamedLocksHeld = array();
+
        /** @var IDatabase|null Lazy handle to the master DB this server 
replicates from */
        private $lazyMasterHandle;
 
@@ -871,7 +874,7 @@
                                $msg = __METHOD__ . ": lost connection to 
$server; reconnected";
                                wfDebugLog( 'DBPerformance', "$msg:\n" . 
wfBacktrace( true ) );
 
-                               if ( $hadTrx ) {
+                               if ( $hadTrx || $this->mNamedLocksHeld ) {
                                        # Leave $ret as false and let an error 
be reported.
                                        # Callers may catch the exception and 
continue to use the DB.
                                        $this->reportQueryError( $lastError, 
$lastErrno, $sql, $fname, $tempIgnore );
@@ -3160,10 +3163,14 @@
        }
 
        public function lock( $lockName, $method, $timeout = 5 ) {
+               $this->mNamedLocksHeld[$lockName] = 1;
+
                return true;
        }
 
        public function unlock( $lockName, $method ) {
+               unset( $this->mNamedLocksHeld[$lockName] );
+
                return true;
        }
 
diff --git a/includes/db/DatabaseMysqlBase.php 
b/includes/db/DatabaseMysqlBase.php
index 3a8f737..29106ab 100644
--- a/includes/db/DatabaseMysqlBase.php
+++ b/includes/db/DatabaseMysqlBase.php
@@ -932,12 +932,13 @@
                $row = $this->fetchObject( $result );
 
                if ( $row->lockstatus == 1 ) {
+                       parent::lock( $lockName, $method, $timeout ); // record
                        return true;
-               } else {
-                       wfDebug( __METHOD__ . " failed to acquire lock\n" );
-
-                       return false;
                }
+
+               wfDebug( __METHOD__ . " failed to acquire lock\n" );
+
+               return false;
        }
 
        /**
@@ -952,7 +953,14 @@
                $result = $this->query( "SELECT RELEASE_LOCK($lockName) as 
lockstatus", $method );
                $row = $this->fetchObject( $result );
 
-               return ( $row->lockstatus == 1 );
+               if ( $row->lockstatus == 1 ) {
+                       parent::unlock( $lockName, $method ); // record
+                       return true;
+               }
+
+               wfDebug( __METHOD__ . " failed to release lock\n" );
+
+               return false;
        }
 
        private function makeLockName( $lockName ) {
diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php
index 4d9891e..e84f264 100644
--- a/includes/db/DatabasePostgres.php
+++ b/includes/db/DatabasePostgres.php
@@ -1581,11 +1581,13 @@
                                "SELECT pg_try_advisory_lock($key) AS 
lockstatus", $method );
                        $row = $this->fetchObject( $result );
                        if ( $row->lockstatus === 't' ) {
+                               parent::lock( $lockName, $method, $timeout ); 
// record
                                return true;
                        } else {
                                sleep( 1 );
                        }
                }
+
                wfDebug( __METHOD__ . " failed to acquire lock\n" );
 
                return false;
@@ -1603,7 +1605,14 @@
                $result = $this->query( "SELECT pg_advisory_unlock($key) as 
lockstatus", $method );
                $row = $this->fetchObject( $result );
 
-               return ( $row->lockstatus === 't' );
+               if ( $row->lockstatus === 't' ) {
+                       parent::unlock( $lockName, $method ); // record
+                       return true;
+               }
+
+               wfDebug( __METHOD__ . " failed to release lock\n" );
+
+               return false;
        }
 
        /**

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7bc6b188bb5ee53a5bf7c5a30718bc7c4dd0ba9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Parent5446 <tylerro...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

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

Reply via email to