rdbms: rename and clarify getTransactionLagStatus method regarding begin()

Make sure any value from the last transaction is cleared and not used.

For sanity, make getLagFromPtHeartbeat() use this regardless of the age of
the transaction unless it returns null.

Change-Id: I52df6147f99736ad1a389ae70d347ae968e50c7f

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 699fee7..8739cbf 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -3393,6 +3393,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxWriteCallers = [];
                // First SELECT after BEGIN will establish the snapshot in REPEATABLE-READ.
                // Get an estimate of the replication lag before any such queries.
+               $this->trxReplicaLag = null; // clear cached value first
                $this->trxReplicaLag = $this->getApproximateLagStatus()['lag'];
                // T147697: make explicitTrxActive() return true until begin() finishes. This way, no
                // caller will think its OK to muck around with the transaction just because startAtomic()
@@ -3666,7 +3667,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function getSessionLagStatus() {
-               return $this->getTransactionLagStatus() ?: $this->getApproximateLagStatus();
+               return $this->getRecordedTransactionLagStatus() ?: $this->getApproximateLagStatus();
        }
 
        /**
@@ -3677,11 +3678,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * is this lag plus transaction duration. If they don't, it is still
         * safe to be pessimistic. This returns null if there is no transaction.
         *
+        * This returns null if the lag status for this transaction was not yet recorded.
+        *
         * @return array|null ('lag': seconds or false on error, 'since': UNIX timestamp of BEGIN)
         * @since 1.27
         */
-       final protected function getTransactionLagStatus() {
-               return $this->trxLevel
+       final protected function getRecordedTransactionLagStatus() {
+               return ( $this->trxLevel && $this->trxReplicaLag !== null )
                        ? [ 'lag' => $this->trxReplicaLag, 'since' => $this->trxTimestamp() ]
                        : null;
        }

diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 0a63bdc..86ec641 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -766,18 +766,20 @@ abstract class DatabaseMysqlBase extends Database {
        protected function getLagFromPtHeartbeat() {
                $options = $this->lagDetectionOptions;
 
-               $staleness = $this->trxLevel
-                       ? microtime( true ) - $this->trxTimestamp()
-                       : 0;
-               if ( $staleness > self::LAG_STALE_WARN_THRESHOLD ) {
-                       // 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__ ] )
-                       );
+               $currentTrxInfo = $this->getRecordedTransactionLagStatus();
+               if ( $currentTrxInfo ) {
+                       // There is an active transaction and the initial lag was already queried
+                       $staleness = microtime( true ) - $currentTrxInfo['since'];
+                       if ( $staleness > self::LAG_STALE_WARN_THRESHOLD ) {
+                               // 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__, 'age' => $staleness ] )
+                               );
+                       }
 
-                       return $this->getTransactionLagStatus()['lag'];
+                       return $currentTrxInfo['lag'];
                }
 
                if ( isset( $options['conds'] ) ) {