Enforce stricter slave lag limits for bot API requests

* All users with the bot right sending requests to
  write APIs will get a read-only error when slave
  lag is high. The lag tolerance is configured via
  $wgAPIMaxLagThreshold.
* Making this lower than 'max lag' in the LBFactory
  config means bots get read-only errors before other
  users, which is sensible.
* The default LoadBalancer 'max lag' is now 10 instead
  of 30, which now matches the timeout used for waiting
  on MASTER_POS_WAIT() for ChronologyProtector. This is
  better for assuring that users see their own changes.

Bug: T95501
Change-Id: I56a7f3538278378c67fa3ef5ff0ba4aa6a9a8c64

diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27
index 13b9ba2..2701593 100644 (file)
--- a/RELEASE-NOTES-1.27
+++ b/RELEASE-NOTES-1.27
@@ -67,6 +67,8 @@ production.
 * Extensions can now return detailed error information via the API when
   preventing user actions using 'getUserPermissionsErrors' and similar hooks
   by using ApiMessage instances instead of strings for the $result value.
+* $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 6032ba3..bf6e245 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 86a8a87..e4ab7a5 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,20 +1147,20 @@ class ApiMain extends ApiBase {
                ) {
                        $this->dieUsageMsg( 'readrequired' );
                }
+
                if ( $module->isWriteMode() ) {
                        if ( !$this->mEnableWrite ) {
                                $this->dieUsageMsg( 'writedisabled' );
                        } elseif ( !$user->isAllowed( 'writeapi' ) ) {
                                $this->dieUsageMsg( 'writerequired' );
-                       } elseif ( wfReadOnly() ) {
-                               $this->dieReadOnly();
-                       }
-                       if ( $this->getRequest()->getHeader( 'Promise-Non-Write-API-Action' ) ) {
+                       } elseif ( $this->getRequest()->getHeader( 'Promise-Non-Write-API-Action' ) ) {
                                $this->dieUsage(
                                        "Promise-Non-Write-API-Action HTTP header cannot be sent to write API modules",
                                        'promised-nonwrite-api'
                                );
                        }
+
+                       $this->checkReadOnly( $module );
                }
 
                // Allow extensions to stop execution for arbitrary reasons.
@@ -1171,6 +1170,42 @@ 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()
+                       && in_array( 'bot', $this->getUser()->getGroups() )
+                       && wfGetLB()->getServerCount() > 1
+               ) {
+                       // 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;