Remove "masterPos" stuff from RefreshLinksJob
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 9 Jun 2016 11:49:33 +0000 (04:49 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 9 Jun 2016 11:57:25 +0000 (04:57 -0700)
Just do a single slave lag wait check when branching the base job.
Any remnant/leaf jobs after than do not have to do anything special.

This should also improve de-duplication and reduce commonswiki
errors like "Could not acquire lock on page #42482792" due to
insane pages.

Change-Id: I40f9c6e0e905bd8149bb364c33a0642628cb1423

includes/jobqueue/jobs/RefreshLinksJob.php

index 9711496..8870569 100644 (file)
@@ -40,13 +40,13 @@ class RefreshLinksJob extends Job {
        const PARSE_THRESHOLD_SEC = 1.0;
        /** @var integer Lag safety margin when comparing root job times to last-refresh times */
        const CLOCK_FUDGE = 10;
+       /** @var integer How many seconds to wait for slaves to catch up */
+       const LAG_WAIT_TIMEOUT = 15;
 
        function __construct( Title $title, array $params ) {
                parent::__construct( 'refreshLinks', $title, $params );
                // Avoid the overhead of de-duplication when it would be pointless
                $this->removeDuplicates = (
-                       // Master positions won't match
-                       !isset( $params['masterPos'] ) &&
                        // Ranges rarely will line up
                        !isset( $params['range'] ) &&
                        // Multiple pages per job make matches unlikely
@@ -83,20 +83,22 @@ class RefreshLinksJob extends Job {
 
                // Job to update all (or a range of) backlink pages for a page
                if ( !empty( $this->params['recursive'] ) ) {
+                       // When the base job branches, wait for the slaves to catch up to the master.
+                       // From then on, we know that any template changes at the time the base job was
+                       // enqueued will be reflected in backlink page parses when the leaf jobs run.
+                       if ( !isset( $params['range'] ) ) {
+                               try {
+                                       wfGetLBFactory()->waitForReplication( [
+                                               'wiki'    => wfWikiID(),
+                                               'timeout' => self::LAG_WAIT_TIMEOUT
+                                       ] );
+                               } catch ( DBReplicationWaitError $e ) { // only try so hard
+                                       $stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
+                                       $stats->increment( 'refreshlinks.lag_wait_failed' );
+                               }
+                       }
                        // Carry over information for de-duplication
                        $extraParams = $this->getRootJobParams();
-                       // Avoid slave lag when fetching templates.
-                       // When the outermost job is run, we know that the caller that enqueued it must have
-                       // committed the relevant changes to the DB by now. At that point, record the master
-                       // position and pass it along as the job recursively breaks into smaller range jobs.
-                       // Hopefully, when leaf jobs are popped, the slaves will have reached that position.
-                       if ( isset( $this->params['masterPos'] ) ) {
-                               $extraParams['masterPos'] = $this->params['masterPos'];
-                       } elseif ( wfGetLB()->getServerCount() > 1 ) {
-                               $extraParams['masterPos'] = wfGetLB()->getMasterPos();
-                       } else {
-                               $extraParams['masterPos'] = false;
-                       }
                        $extraParams['triggeredRecursive'] = true;
                        // Convert this into no more than $wgUpdateRowsPerJob RefreshLinks per-title
                        // jobs and possibly a recursive RefreshLinks job for the rest of the backlinks
@@ -109,29 +111,18 @@ class RefreshLinksJob extends Job {
                        JobQueueGroup::singleton()->push( $jobs );
                // Job to update link tables for a set of titles
                } elseif ( isset( $this->params['pages'] ) ) {
-                       $this->waitForMasterPosition();
                        foreach ( $this->params['pages'] as $pageId => $nsAndKey ) {
                                list( $ns, $dbKey ) = $nsAndKey;
                                $this->runForTitle( Title::makeTitleSafe( $ns, $dbKey ) );
                        }
                // Job to update link tables for a given title
                } else {
-                       $this->waitForMasterPosition();
                        $this->runForTitle( $this->title );
                }
 
                return true;
        }
 
-       protected function waitForMasterPosition() {
-               if ( !empty( $this->params['masterPos'] ) && wfGetLB()->getServerCount() > 1 ) {
-                       // Wait for the current/next slave DB handle to catch up to the master.
-                       // This way, we get the correct page_latest for templates or files that just
-                       // changed milliseconds ago, having triggered this job to begin with.
-                       wfGetLB()->waitFor( $this->params['masterPos'] );
-               }
-       }
-
        /**
         * @param Title $title
         * @return bool