Make LoadBalancer::waitFor() and friends more robust via try/finally
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 12 Apr 2017 23:48:40 +0000 (16:48 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 13 Apr 2017 04:19:47 +0000 (04:19 +0000)
Change-Id: I50742752bb94ca17997138b440b911849bcbadf3

includes/libs/rdbms/loadbalancer/LoadBalancer.php

index d268f83..ffa4f78 100644 (file)
@@ -430,60 +430,65 @@ class LoadBalancer implements ILoadBalancer {
 
        public function waitFor( $pos ) {
                $oldPos = $this->mWaitForPos;
-               $this->mWaitForPos = $pos;
-
-               // If a generic reader connection was already established, then wait now
-               $i = $this->mReadIndex;
-               if ( $i > 0 ) {
-                       if ( !$this->doWait( $i ) ) {
-                               $this->laggedReplicaMode = true;
+               try {
+                       $this->mWaitForPos = $pos;
+                       // If a generic reader connection was already established, then wait now
+                       $i = $this->mReadIndex;
+                       if ( $i > 0 ) {
+                               if ( !$this->doWait( $i ) ) {
+                                       $this->laggedReplicaMode = true;
+                               }
                        }
+               } finally {
+                       // Restore the older position if it was higher
+                       $this->setWaitForPositionIfHigher( $oldPos );
                }
-
-               // Restore the older position if it was higher
-               $this->setWaitForPositionIfHigher( $oldPos );
        }
 
        public function waitForOne( $pos, $timeout = null ) {
                $oldPos = $this->mWaitForPos;
-               $this->mWaitForPos = $pos;
+               try {
+                       $this->mWaitForPos = $pos;
 
-               $i = $this->mReadIndex;
-               if ( $i <= 0 ) {
-                       // Pick a generic replica DB if there isn't one yet
-                       $readLoads = $this->mLoads;
-                       unset( $readLoads[$this->getWriterIndex()] ); // replica DBs only
-                       $readLoads = array_filter( $readLoads ); // with non-zero load
-                       $i = ArrayUtils::pickRandom( $readLoads );
-               }
+                       $i = $this->mReadIndex;
+                       if ( $i <= 0 ) {
+                               // Pick a generic replica DB if there isn't one yet
+                               $readLoads = $this->mLoads;
+                               unset( $readLoads[$this->getWriterIndex()] ); // replica DBs only
+                               $readLoads = array_filter( $readLoads ); // with non-zero load
+                               $i = ArrayUtils::pickRandom( $readLoads );
+                       }
 
-               if ( $i > 0 ) {
-                       $ok = $this->doWait( $i, true, $timeout );
-               } else {
-                       $ok = true; // no applicable loads
+                       if ( $i > 0 ) {
+                               $ok = $this->doWait( $i, true, $timeout );
+                       } else {
+                               $ok = true; // no applicable loads
+                       }
+               } finally {
+                       // Restore the older position if it was higher
+                       $this->setWaitForPositionIfHigher( $oldPos );
                }
 
-               // Restore the older position if it was higher
-               $this->setWaitForPositionIfHigher( $oldPos );
-
                return $ok;
        }
 
        public function waitForAll( $pos, $timeout = null ) {
                $oldPos = $this->mWaitForPos;
-               $this->mWaitForPos = $pos;
-               $serverCount = count( $this->mServers );
+               try {
+                       $this->mWaitForPos = $pos;
+                       $serverCount = count( $this->mServers );
 
-               $ok = true;
-               for ( $i = 1; $i < $serverCount; $i++ ) {
-                       if ( $this->mLoads[$i] > 0 ) {
-                               $ok = $this->doWait( $i, true, $timeout ) && $ok;
+                       $ok = true;
+                       for ( $i = 1; $i < $serverCount; $i++ ) {
+                               if ( $this->mLoads[$i] > 0 ) {
+                                       $ok = $this->doWait( $i, true, $timeout ) && $ok;
+                               }
                        }
+               } finally {
+                       // Restore the older position if it was higher
+                       $this->setWaitForPositionIfHigher( $oldPos );
                }
 
-               // Restore the older position if it was higher
-               $this->setWaitForPositionIfHigher( $oldPos );
-
                return $ok;
        }