Merge "Enforce stricter slave lag limits for bot API requests"

diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27
index b3add69..d08e398 100644 (file)
--- a/RELEASE-NOTES-1.27
+++ b/RELEASE-NOTES-1.27
@@ -63,6 +63,8 @@ production.
   setting in the relevant section of $wgLBFactoryConf.
 * User::newSystemUser() may be used to simplify the creation of passwordless
   "system" users for logged actions from scripts and extensions.
+* $wgAPIMaxLagThreshold was added to limit bot changes when databases lag
+  becomes too high.
 
 ==== External libraries ====
 

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index c8da21e..f0049f5 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -7325,6 +7325,13 @@ $wgAPIMaxResultSize = 8388608;
  */
 $wgAPIMaxUncachedDiffs = 1;
 
+/**
+ * Maximum amount of DB lag on a majority of DB slaves to tolerate
+ * before forcing bots to retry any write requests via API errors.
+ * This should be lower than the 'max lag' value in $wgLBFactoryConf.
+ */
+$wgAPIMaxLagThreshold = 7;
+
 /**
  * Log file or URL (TCP or UDP) to log API requests to, or false to disable
  * API request logging

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index c641c95..f60f5df 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -1002,7 +1002,6 @@ class ApiMain extends ApiBase {
         */
        protected function checkMaxLag( $module, $params ) {
                if ( $module->shouldCheckMaxlag() && isset( $params['maxlag'] ) ) {
-                       // Check for maxlag
                        $maxLag = $params['maxlag'];
                        list( $host, $lag ) = wfGetLB()->getMaxLag();
                        if ( $lag > $maxLag ) {
@@ -1148,6 +1147,7 @@ class ApiMain extends ApiBase {
                ) {
                        $this->dieUsageMsg( 'readrequired' );
                }
+
                if ( $module->isWriteMode() ) {
                        if ( !$this->mEnableWrite ) {
                                $this->dieUsageMsg( 'writedisabled' );
@@ -1155,9 +1155,7 @@ class ApiMain extends ApiBase {
                        if ( !$user->isAllowed( 'writeapi' ) ) {
                                $this->dieUsageMsg( 'writerequired' );
                        }
-                       if ( wfReadOnly() ) {
-                               $this->dieReadOnly();
-                       }
+                       $this->checkReadOnly( $module );
                }
 
                // Allow extensions to stop execution for arbitrary reasons.
@@ -1167,6 +1165,39 @@ class ApiMain extends ApiBase {
                }
        }
 
+       /**
+        * Check if the DB is read-only for this user
+        * @param ApiBase $module An Api module
+        */
+       protected function checkReadOnly( $module ) {
+               if ( wfReadOnly() ) {
+                       $this->dieReadOnly();
+               }
+
+               if ( $module->isWriteMode() && $this->getUser()->isAllowed( 'bot' ) ) {
+                       // Figure out how many servers have passed the lag threshold
+                       $numLagged = 0;
+                       $lagLimit = $this->getConfig()->get( 'APIMaxLagThreshold' );
+                       foreach ( wfGetLB()->getLagTimes() as $lag ) {
+                               if ( $lag > $lagLimit ) {
+                                       ++$numLagged;
+                               }
+                       }
+                       // If a majority of slaves are too lagged then disallow writes
+                       $slaveCount = wfGetLB()->getServerCount() - 1;
+                       if ( $numLagged >= ceil( $slaveCount / 2 ) ) {
+                               $parsed = $this->parseMsg( array( 'readonlytext' ) );
+                               $this->dieUsage(
+                                       $parsed['info'],
+                                       $parsed['code'],
+                                       /* http error */
+                                       0,
+                                       array( 'readonlyreason' => "Waiting for $numLagged lagged database(s)" )
+                               );
+                       }
+               }
+       }
+
        /**
         * Check asserts of the user's rights
         * @param array $params

diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index bc9465b..a5a5c37 100644 (file)
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -68,7 +68,9 @@ class LoadBalancer {
        /** @var integer Warn when this many connection are held */
        const CONN_HELD_WARN_THRESHOLD = 10;
        /** @var integer Default 'max lag' when unspecified */
-       const MAX_LAG = 30;
+       const MAX_LAG = 10;
+       /** @var integer Max time to wait for a slave to catch up (e.g. ChronologyProtector) */
+       const POS_WAIT_TIMEOUT = 10;
 
        /**
         * @param array $params Array with keys:
@@ -82,7 +84,7 @@ class LoadBalancer {
                        throw new MWException( __CLASS__ . ': missing servers parameter' );
                }
                $this->mServers = $params['servers'];
-               $this->mWaitTimeout = 10;
+               $this->mWaitTimeout = self::POS_WAIT_TIMEOUT;
 
                $this->mReadIndex = -1;
                $this->mWriteIndex = -1;