Merge "Avoid unnecessary WaitConditionLoop delays in ChronologyProtector"

[lhc/web/wiklou.git] / includes / libs / rdbms / ChronologyProtector.php

diff --git a/includes/libs/rdbms/ChronologyProtector.php b/includes/libs/rdbms/ChronologyProtector.php
index 88e276f..8b21ede 100644 (file)
--- a/includes/libs/rdbms/ChronologyProtector.php
+++ b/includes/libs/rdbms/ChronologyProtector.php
@@ -63,6 +63,8 @@ class ChronologyProtector implements LoggerAwareInterface {
 
        /** @var int Seconds to store positions */
        const POSITION_TTL = 60;
+       /** @var int Seconds to store position write index cookies (safely less than POSITION_TTL) */
+       const POSITION_COOKIE_TTL = 60;
        /** @var int Max time to wait for positions to appear */
        const POS_STORE_WAIT_TIMEOUT = 5;
 
@@ -75,7 +77,7 @@ class ChronologyProtector implements LoggerAwareInterface {
        public function __construct( BagOStuff $store, array $client, $posIndex = null ) {
                $this->store = $store;
                $this->clientId = md5( $client['ip'] . "\n" . $client['agent'] );
-               $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v1' );
+               $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v2' );
                $this->waitForPosIndex = $posIndex;
                $this->logger = new NullLogger();
        }
@@ -190,13 +192,14 @@ class ChronologyProtector implements LoggerAwareInterface {
                        implode( ', ', array_keys( $this->shutdownPositions ) ) . "\n"
                );
 
-               // CP-protected writes should overwhemingly go to the master datacenter, so get DC-local
-               // lock to merge the values. Use a DC-local get() and a synchronous all-DC set(). This
-               // makes it possible for the BagOStuff class to write in parallel to all DCs with one RTT.
+               // CP-protected writes should overwhelmingly go to the master datacenter, so use a
+               // DC-local lock to merge the values. Use a DC-local get() and a synchronous all-DC
+               // set(). This makes it possible for the BagOStuff class to write in parallel to all
+               // DCs with one RTT. The use of WRITE_SYNC avoids needing READ_LATEST for the get().
                if ( $store->lock( $this->key, 3 ) ) {
                        if ( $workCallback ) {
-                               // Let the store run the work before blocking on a replication sync barrier. By the
-                               // time it's done with the work, the barrier should be fast if replication caught up.
+                               // Let the store run the work before blocking on a replication sync barrier.
+                               // If replication caught up while the work finished, the barrier will be fast.
                                $store->addBusyCallback( $workCallback );
                        }
                        $ok = $store->set(
@@ -212,10 +215,10 @@ class ChronologyProtector implements LoggerAwareInterface {
                        $store->unlock( $this->key );
                } else {
                        $ok = false;
-                       $cpIndex = null; // nothing saved
                }
 
                if ( !$ok ) {
+                       $cpIndex = null; // nothing saved
                        $bouncedPositions = $this->shutdownPositions;
                        // Raced out too many times or stash is down
                        $this->logger->warning( __METHOD__ . ": failed to save master pos for " .
@@ -269,14 +272,16 @@ class ChronologyProtector implements LoggerAwareInterface {
                        // already be expired and thus treated as non-existing, maintaining correctness.
                        if ( $this->waitForPosIndex > 0 ) {
                                $data = null;
+                               $indexReached = null; // highest index reached in the position store
                                $loop = new WaitConditionLoop(
-                                       function () use ( &$data ) {
+                                       function () use ( &$data, &$indexReached ) {
                                                $data = $this->store->get( $this->key );
                                                if ( !is_array( $data ) ) {
                                                        return WaitConditionLoop::CONDITION_CONTINUE; // not found yet
                                                } elseif ( !isset( $data['writeIndex'] ) ) {
                                                        return WaitConditionLoop::CONDITION_REACHED; // b/c
                                                }
+                                               $indexReached = max( $data['writeIndex'], $indexReached );
 
                                                return ( $data['writeIndex'] >= $this->waitForPosIndex )
                                                        ? WaitConditionLoop::CONDITION_REACHED
@@ -288,11 +293,22 @@ class ChronologyProtector implements LoggerAwareInterface {
                                $waitedMs = $loop->getLastWaitTime() * 1e3;
 
                                if ( $result == $loop::CONDITION_REACHED ) {
-                                       $msg = "expected and found pos index {$this->waitForPosIndex} ({$waitedMs}ms)";
-                                       $this->logger->debug( $msg );
+                                       $this->logger->debug(
+                                               __METHOD__ . ": expected and found position index.",
+                                               [
+                                                       'cpPosIndex' => $this->waitForPosIndex,
+                                                       'waitTimeMs' => $waitedMs
+                                               ]
+                                       );
                                } else {
-                                       $msg = "expected but missed pos index {$this->waitForPosIndex} ({$waitedMs}ms)";
-                                       $this->logger->info( $msg );
+                                       $this->logger->warning(
+                                               __METHOD__ . ": expected but failed to find position index.",
+                                               [
+                                                       'cpPosIndex' => $this->waitForPosIndex,
+                                                       'indexReached' => $indexReached,
+                                                       'waitTimeMs' => $waitedMs
+                                               ]
+                                       );
                                }
                        } else {
                                $data = $this->store->get( $this->key );