Chad has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/107927


Change subject: Split cirrusSearchLinksUpdate into two jobs
......................................................................

Split cirrusSearchLinksUpdate into two jobs

We were doing this based on a 'primary' parameter, but this didn't
give us the control we wanted. Splitting it into two jobs will allow
more to be done at the same time, as well as separate code paths
that really were separate.

Change-Id: I7e3f4d5cb9479005a1d406efa1ffcdb1dc93e749
---
M CirrusSearch.php
M includes/Hooks.php
M includes/LinksUpdateJob.php
A includes/LinksUpdateSecondaryJob.php
4 files changed, 76 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch 
refs/changes/27/107927/1

diff --git a/CirrusSearch.php b/CirrusSearch.php
index a042325..fcb37ce 100644
--- a/CirrusSearch.php
+++ b/CirrusSearch.php
@@ -210,6 +210,7 @@
 $wgAutoloadClasses['CirrusSearch\DeletePagesJob'] = $includes . 
'DeletePagesJob.php';
 $wgAutoloadClasses['CirrusSearch\ElasticsearchIntermediary'] = $includes . 
'ElasticsearchIntermediary.php';
 $wgAutoloadClasses['CirrusSearch\LinksUpdateJob'] = $includes . 
'LinksUpdateJob.php';
+$wgAutoloadClasses['CirrusSearch\LinksUpdateSecondaryJob'] = $includes . 
'LinksUpdateSecondaryJob.php';
 $wgAutoloadClasses['CirrusSearch\FullTextResultsType'] = $includes . 
'ResultsType.php';
 $wgAutoloadClasses['CirrusSearch\Job'] = $includes . 'Job.php';
 $wgAutoloadClasses['CirrusSearch\MappingConfigBuilder'] = $includes . 
'MappingConfigBuilder.php';
@@ -251,5 +252,6 @@
 $wgJobClasses[ 'cirrusSearchDeletePages' ] = 'CirrusSearch\DeletePagesJob';
 $wgJobClasses[ 'cirrusSearchLinksUpdate' ] = 'CirrusSearch\LinksUpdateJob';
 $wgJobClasses[ 'cirrusSearchLinksUpdatePrioritized' ] = 
'CirrusSearch\LinksUpdateJob';
+$wgJobClasses[ 'cirrusSearchLinksUpdateSecondary' ] = 
'CirrusSearch\LinksUpdateSecondaryJob';
 $wgJobClasses[ 'cirrusSearchOtherIndex' ] = 'CirrusSearch\OtherIndexJob';
 $wgJobClasses[ 'cirrusSearchUpdatePages' ] = 'CirrusSearch\UpdatePagesJob';
diff --git a/includes/Hooks.php b/includes/Hooks.php
index 0e51d0b..58c15b1 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -171,7 +171,6 @@
                                $linksUpdate->getAddedLinks(), 
$wgCirrusSearchLinkedArticlesToUpdate ),
                        'removedLinks' => self::prepareTitlesForLinksUpdate(
                                $linksUpdate->getRemovedLinks(), 
$wgCirrusSearchUnlinkedArticlesToUpdate ),
-                       'primary' => true,
                );
                // Prioritize jobs that are triggered from a web process.  This 
should prioritize
                // single page update jobs over those triggered by template 
changes.
diff --git a/includes/LinksUpdateJob.php b/includes/LinksUpdateJob.php
index 2290ca5..3e3a334 100644
--- a/includes/LinksUpdateJob.php
+++ b/includes/LinksUpdateJob.php
@@ -2,7 +2,6 @@
 
 namespace CirrusSearch;
 use \JobQueueGroup;
-use \Title;
 
 /**
  * Performs the appropriate updates to Elasticsearch after a LinksUpdate is
@@ -43,48 +42,19 @@
 
        protected function doJob() {
                $updater = new Updater();
-               if ( $this->params[ 'primary' ] ) {
-                       $updater->updateFromTitle( $this->title );
-                       if ( count( $this->params[ 'addedLinks' ] ) > 0 ||
-                                       count( $this->params[ 'removedLinks' ] 
) > 0 ) {
-                               $params = array(
-                                       'addedLinks' => $this->params[ 
'addedLinks' ],
-                                       'removedLinks' => $this->params[ 
'removedLinks' ],
-                                       'primary' => false,
-                               );
-                               if ( $this->isPrioritized() ) {
-                                       $params[ 'prioritize' ] = true;
-                               }
-                               $next = new LinksUpdateJob( $this->title, 
$params );
-                               JobQueueGroup::singleton()->push( $next );
+               $updater->updateFromTitle( $this->title );
+               if ( count( $this->params[ 'addedLinks' ] ) > 0 ||
+                               count( $this->params[ 'removedLinks' ] ) > 0 ) {
+                       $params = array(
+                               'addedLinks' => $this->params[ 'addedLinks' ],
+                               'removedLinks' => $this->params[ 'removedLinks' 
],
+                       );
+                       if ( $this->isPrioritized() ) {
+                               $params[ 'prioritize' ] = true;
                        }
-               } else {
-                       // Load the titles and filter out any that no longer 
exist.
-                       $updater->updateLinkedArticles(
-                               $this->loadTitles( $this->params[ 'addedLinks' 
] ),
-                               $this->loadTitles( $this->params[ 
'removedLinks' ] ) );
+                       $next = new LinksUpdateSecondaryJob( $this->title, 
$params );
+                       JobQueueGroup::singleton()->push( $next );
                }
-       }
-
-       /**
-        * Convert a serialized title to a title ready to be passed to 
updateLinkedArticles.
-        * @param Title|string $title Either a Title or a string to be loaded.
-        * @return array(Title) loaded titles
-        */
-       private function loadTitles( $titles ) {
-               $result = array();
-               foreach ( $titles as $title ) {
-                       // TODO remove support for Title objects when the 
queues have drained of them
-                       if ( $title instanceof Title ) {
-                               $result[] = $title;
-                       } else {
-                               $title = Title::newFromDBKey( $title );
-                               if ( $title ) {
-                                       $result[] = $title;
-                               }
-                       }
-               }
-               return $result;
        }
 
        /**
diff --git a/includes/LinksUpdateSecondaryJob.php 
b/includes/LinksUpdateSecondaryJob.php
new file mode 100644
index 0000000..d501bb2
--- /dev/null
+++ b/includes/LinksUpdateSecondaryJob.php
@@ -0,0 +1,63 @@
+<?php
+
+namespace CirrusSearch;
+use \Title;
+
+/**
+ * Performs the appropriate updates to Elasticsearch after a LinksUpdate is
+ * completed.  The page itself is updated first then a second copy of this job
+ * is queued to update linked articles if any links change.  The job can be
+ * 'prioritized' via the 'prioritize' parameter which will switch it to a
+ * different queue then the non-prioritized jobs.  Prioritized jobs will never
+ * be deduplicated with non-prioritized jobs which is good because we can't
+ * control which job is removed during deduplication.  In our case it'd only be
+ * ok to remove the non-prioritized version.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+class LinksUpdateSecondaryJob extends Job {
+       public function __construct( $title, $params, $id = 0 ) {
+               parent::__construct( $title, $params, $id );
+       }
+
+       protected function doJob() {
+               // Load the titles and filter out any that no longer exist.
+               $updater->updateLinkedArticles(
+                       $this->loadTitles( $this->params[ 'addedLinks' ] ),
+                       $this->loadTitles( $this->params[ 'removedLinks' ] ) );
+       }
+
+       /**
+        * Convert a serialized title to a title ready to be passed to 
updateLinkedArticles.
+        * @param Title|string $title Either a Title or a string to be loaded.
+        * @return array(Title) loaded titles
+        */
+       private function loadTitles( $titles ) {
+               $result = array();
+               foreach ( $titles as $title ) {
+                       // TODO remove support for Title objects when the 
queues have drained of them
+                       if ( $title instanceof Title ) {
+                               $result[] = $title;
+                       } else {
+                               $title = Title::newFromDBKey( $title );
+                               if ( $title ) {
+                                       $result[] = $title;
+                               }
+                       }
+               }
+               return $result;
+       }
+}

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e3f4d5cb9479005a1d406efa1ffcdb1dc93e749
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: Chad <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to