rdbms: avoid lag estimates in getLagFromPtHeartbeat ruined by snapshots

Bug: T190960
Change-Id: I57dd8d3d0ca96d6fb2f9e83f062f29b1d53224dd

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 00d9b0b..d736477 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -3682,7 +3682,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @return array|null ('lag': seconds or false on error, 'since': UNIX timestamp of BEGIN)
         * @since 1.27
         */
         * @return array|null ('lag': seconds or false on error, 'since': UNIX timestamp of BEGIN)
         * @since 1.27
         */

-       protected function getTransactionLagStatus() {
+       final protected function getTransactionLagStatus() {

                return $this->trxLevel
                        ? [ 'lag' => $this->trxReplicaLag, 'since' => $this->trxTimestamp() ]
                        : null;
                return $this->trxLevel
                        ? [ 'lag' => $this->trxReplicaLag, 'since' => $this->trxTimestamp() ]
                        : null;

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 315c9eb..bc4beba 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -763,6 +763,17 @@ abstract class DatabaseMysqlBase extends Database {
        protected function getLagFromPtHeartbeat() {
                $options = $this->lagDetectionOptions;
 
        protected function getLagFromPtHeartbeat() {
                $options = $this->lagDetectionOptions;
 

+               if ( $this->trxLevel ) {
+                       // Avoid returning higher and higher lag value due to snapshot age
+                       // given that the isolation level will typically be REPEATABLE-READ
+                       $this->queryLogger->warning(
+                               "Using cached lag value for {db_server} due to active transaction",
+                               $this->getLogContext( [ 'method' => __METHOD__ ] )
+                       );
+
+                       return $this->getTransactionLagStatus()['lag'];
+               }
+

                if ( isset( $options['conds'] ) ) {
                        // Best method for multi-DC setups: use logical channel names
                        $data = $this->getHeartbeatData( $options['conds'] );
                if ( isset( $options['conds'] ) ) {
                        // Best method for multi-DC setups: use logical channel names
                        $data = $this->getHeartbeatData( $options['conds'] );

diff --git a/includes/libs/rdbms/loadmonitor/LoadMonitor.php b/includes/libs/rdbms/loadmonitor/LoadMonitor.php
index 50c878d..3372839 100644 (file)
--- a/includes/libs/rdbms/loadmonitor/LoadMonitor.php
+++ b/includes/libs/rdbms/loadmonitor/LoadMonitor.php
@@ -157,7 +157,9 @@ class LoadMonitor implements ILoadMonitor {
                        }
 
                        $conn = $this->parent->getAnyOpenConnection( $i );
                        }
 
                        $conn = $this->parent->getAnyOpenConnection( $i );

-                       if ( $conn ) {
+                       if ( $conn && !$conn->trxLevel() ) {
+                               # Handles with open transactions are avoided since they might be subject
+                               # to REPEATABLE-READ snapshots, which could affect the lag estimate query.

                                $close = false; // already open
                        } else {
                                $conn = $this->parent->openConnection( $i, '' );
                                $close = false; // already open
                        } else {
                                $conn = $this->parent->openConnection( $i, '' );