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

Change subject: Improve lag/flag behavior and docs in LoadBalancer
......................................................................


Improve lag/flag behavior and docs in LoadBalancer

* Set DBO_DEFAULT if "flags" is not set. MediaWiki config bootstrapping
  does this, but for outside users, it should still be the default too.
* Make "is static" do more than no-oping DatabaseMysqlBase::masterPosWait().
  It now also treats servers with this flag as having 0 lag at all times.
  This makes it far more useful, as the point of the flag is to mark DBs
  that are archival clones of each other and may not even replicate.
* Add more ILoadBalancer usage documentation.

Change-Id: Ib0b44e42e9f4c57a292d6a6ea17beb454db464bc
---
M includes/libs/rdbms/loadbalancer/ILoadBalancer.php
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
2 files changed, 65 insertions(+), 8 deletions(-)

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



diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php 
b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index 0efd8bc..3b2479f1 100644
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -23,7 +23,51 @@
  */
 
 /**
- * Interface for database load balancing object that manages IDatabase handles
+ * Database cluster connection, tracking, load balancing, and transaction 
manager interface
+ *
+ * A "cluster" is considered to be one master database and zero or more 
replica databases.
+ * Typically, the replica DBs replicate from the master asynchronously. The 
first node in the
+ * "servers" configuration array is always considered the "master". However, 
this class can still
+ * be used when all or some of the "replica" DBs are multi-master peers of the 
master or even
+ * when all the DBs are non-replicating clones of each other holding read-only 
data. Thus, the
+ * role of "master" is in some cases merely nominal.
+ *
+ * By default, each DB server uses DBO_DEFAULT for its 'flags' setting, unless 
explicitly set
+ * otherwise in configuration. DBO_DEFAULT behavior depends on whether 
'cliMode' is set:
+ *   - In CLI mode, the flag has no effect with regards to LoadBalancer.
+ *   - In non-CLI mode, the flag causes implicit transactions to be used; the 
first query on
+ *     a database starts a transaction on that database. The transactions are 
meant to remain
+ *     pending until either commitMasterChanges() or rollbackMasterChanges() 
is called. The
+ *     application must have some point where it calls commitMasterChanges() 
near the end of
+ *     the PHP request.
+ * Every iteration of beginMasterChanges()/commitMasterChanges() is called a 
"transaction round".
+ * Rounds are useful on the master DB connections because they make single-DB 
(and by and large
+ * multi-DB) updates in web requests all-or-nothing. Also, transactions on 
replica DBs are useful
+ * when REPEATABLE-READ or SERIALIZABLE isolation is used because all foriegn 
keys and constraints
+ * hold across separate queries in the DB transaction since the data appears 
within a consistent
+ * point-in-time snapshot.
+ *
+ * The typical caller will use LoadBalancer::getConnection( DB_* ) to yield a 
live database
+ * connection handle. The choice of which DB server to use is based on 
pre-defined loads for
+ * weighted random selection, adjustments thereof by LoadMonitor, and the 
amount of replication
+ * lag on each DB server. Lag checks might cause problems in certain setups, 
so they should be
+ * tuned in the server configuration maps as follows:
+ *   - Master + N Replica(s): set 'max lag' to an appropriate threshold for 
avoiding any database
+ *      lagged by this much or more. If all DBs are this lagged, then the load 
balancer considers
+ *      the cluster to be read-only.
+ *   - Galera Cluster: Seconds_Behind_Master will be 0, so there probably is 
nothing to tune.
+ *      Note that lag is still possible depending on how wsrep-sync-wait is 
set server-side.
+ *   - Read-only archive clones: set 'is static' in the server configuration 
maps. This will
+ *      treat all such DBs as having 0 lag.
+ *   - SQL load balancing proxy: any proxy should handle lag checks on its 
own, so the 'max lag'
+ *     parameter should probably be set to INF in the server configuration 
maps. This will make
+ *     the load balancer ignore whatever it detects as the lag of the logical 
replica is (which
+ *     would probably just randomly bounce around).
+ *
+ * If using a SQL proxy service, it would probably be best to have two proxy 
hosts for the
+ * load balancer to talk to. One would be the 'host' of the master server 
entry and another for
+ * the (logical) replica server entry. The proxy could map the load balancer's 
"replica" DB to
+ * any number of physical replica DBs.
  *
  * @since 1.28
  * @ingroup Database
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php 
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index c07d38f..ba9bb37 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -23,7 +23,7 @@
 use Psr\Log\LoggerInterface;
 
 /**
- * Database load balancing, tracking, and transaction management object
+ * Database connection, tracking, load balancing, and transaction manager for 
a cluster
  *
  * @ingroup Database
  */
@@ -801,14 +801,18 @@
                }
 
                $server['srvCache'] = $this->srvCache;
-               // Set loggers
+               // Set loggers and profilers
                $server['connLogger'] = $this->connLogger;
                $server['queryLogger'] = $this->queryLogger;
+               $server['errorLogger'] = $this->errorLogger;
                $server['profiler'] = $this->profiler;
                $server['trxProfiler'] = $this->trxProfiler;
+               // Use the same agent and PHP mode for all DB handles
                $server['cliMode'] = $this->cliMode;
-               $server['errorLogger'] = $this->errorLogger;
                $server['agent'] = $this->agent;
+               // Use DBO_DEFAULT flags by default for LoadBalancer managed 
databases. Assume that the
+               // application calls LoadBalancer::commitMasterChanges() before 
the PHP script completes.
+               $server['flags'] = isset( $server['flags'] ) ? $server['flags'] 
: DBO_DEFAULT;
 
                // Create a live connection object
                try {
@@ -821,7 +825,7 @@
 
                $db->setLBInfo( $server );
                $db->setLazyMasterHandle(
-                       $this->getLazyConnectionRef( DB_MASTER, [], 
$db->getWikiID() )
+                       $this->getLazyConnectionRef( DB_MASTER, [], 
$db->getDomainID() )
                );
                $db->setTableAliases( $this->tableAliases );
 
@@ -1386,11 +1390,20 @@
 
        public function getLagTimes( $domain = false ) {
                if ( $this->getServerCount() <= 1 ) {
-                       return [ 0 => 0 ]; // no replication = no lag
+                       return [ $this->getWriterIndex() => 0 ]; // no 
replication = no lag
                }
 
-               # Send the request to the load monitor
-               return $this->getLoadMonitor()->getLagTimes( array_keys( 
$this->mServers ), $domain );
+               $knownLagTimes = []; // map of (server index => 0 seconds)
+               $indexesWithLag = [];
+               foreach ( $this->mServers as $i => $server ) {
+                       if ( empty( $server['is static'] ) ) {
+                               $indexesWithLag[] = $i; // DB server might have 
replication lag
+                       } else {
+                               $knownLagTimes[$i] = 0; // DB server is a 
non-replicating and read-only archive
+                       }
+               }
+
+               return $this->getLoadMonitor()->getLagTimes( $indexesWithLag, 
$domain ) + $knownLagTimes;
        }
 
        public function safeGetLag( IDatabase $conn ) {

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0b44e42e9f4c57a292d6a6ea17beb454db464bc
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
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