Manybubbles has uploaded a new change for review.
https://gerrit.wikimedia.org/r/97002
Change subject: WIP:Count links from Elasticsearch instead of db
......................................................................
WIP:Count links from Elasticsearch instead of db
Use a multisearch to get both the count of incoming links and incoming
links to redirects to a page. Elasticsearch will parallelize the whole
counting process which is nice. Unfortunately counting with Elasticsearch
take longer than counting with the DB for pages with fewer links. I'm not
sure why but I assume it has to do with inter node communication times.
It isn't a big loss as the counts still stay under 10ms mostly. Anyway,
this isn't a win all around but it should be a bit win for scalability.
Another thing: this commit change the from scratch index building procedure.
Instead of sending the documents to Elasticsearch fully formed Cirrus will
have to send them without links information and then run a second pass to
fill that information in. This is because Cirrus uses information populated
in the first pass to perform the second pass.
This is a WIP because:
1. It won't merge with I5a717c254a6797436d5bebea460459dad09f6b89 cleanly.
2. It has refresh issues that I worked around with a well placed
sleep( 5 ). The problem is that we trigger a page link recount right
after indexing a page link add or remove to Elasticsearch. Since
Elasticsearch kicks off async refreshes every second or so it could take
a few seconds for a change to become visible. This is probably fine in
production because pages popular enough to matter will pick up the change
in the next edit. It makes tests fail (probably) spuriously, though.
3. It needs README changes.
4. It needs comment changes to reflect new parameters.
Bug: 56798
Change-Id: Ide705f1e77c3076dc3c1a324689dd1cf46633855
---
M includes/CirrusSearchMappingConfigBuilder.php
M includes/CirrusSearchSearcher.php
M includes/CirrusSearchUpdatePagesJob.php
M includes/CirrusSearchUpdater.php
M maintenance/forceSearchIndex.php
M maintenance/updateOneSearchIndexConfig.php
6 files changed, 150 insertions(+), 100 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch
refs/changes/02/97002/1
diff --git a/includes/CirrusSearchMappingConfigBuilder.php
b/includes/CirrusSearchMappingConfigBuilder.php
index fb97ab9..cf7e1b2 100644
--- a/includes/CirrusSearchMappingConfigBuilder.php
+++ b/includes/CirrusSearchMappingConfigBuilder.php
@@ -57,7 +57,7 @@
'title' => $this->buildStringField( 'title',
$titleExtraAnalyzers ),
'text' => $this->buildStringField( 'text',
$textExtraAnalyzers ),
'category' =>
$this->buildLowercaseKeywordField(),
- 'outgoing_link' =>
$this->buildLowercaseKeywordField(),
+ 'outgoing_link' => $this->buildKeywordField(),
'heading' => $this->buildStringField( 'heading'
),
'text_bytes' => $this->buildLongField(),
'text_words' => $this->buildLongField(),
@@ -67,8 +67,6 @@
'title' =>
$this->buildStringField( 'title', array( 'suggest' ) ),
)
),
- 'links' => $this->buildLongField(),
#Deprecated
- 'redirect_links' => $this->buildLongField(),
#Deprecated
'incoming_links' => $this->buildLongField(),
'incoming_redirect_links' =>
$this->buildLongField(),
),
@@ -123,6 +121,18 @@
}
/**
+ * Create a string field that does no analyzing whatsoever.
+ * @return array definition of the field
+ */
+ private function buildKeywordField() {
+ return array(
+ 'type' => 'string',
+ 'analyzer' => 'keyword',
+ 'include_in_all' => false,
+ );
+ }
+
+ /**
* Create a long field.
* @return array definition of the field
*/
diff --git a/includes/CirrusSearchSearcher.php
b/includes/CirrusSearchSearcher.php
index 8381a87..63d17d9 100644
--- a/includes/CirrusSearchSearcher.php
+++ b/includes/CirrusSearchSearcher.php
@@ -708,7 +708,7 @@
*/
private function boostQuery( $query = null ) {
// MVEL code for incoming links boost
- $scoreBoostMvel = " * log10(doc['links'].value +
doc['redirect_links'].value + 2)";
+ $scoreBoostMvel = " * log10(doc['incoming_links'].value +
doc['incoming_redirect_links'].value + 2)";
// MVEL code for last update time decay
$lastUpdateDecayMvel = '';
if ( $this->preferRecentDecayPortion > 0 &&
$this->preferRecentHalfLife > 0 ) {
diff --git a/includes/CirrusSearchUpdatePagesJob.php
b/includes/CirrusSearchUpdatePagesJob.php
index 356d365..226bfaf 100644
--- a/includes/CirrusSearchUpdatePagesJob.php
+++ b/includes/CirrusSearchUpdatePagesJob.php
@@ -19,17 +19,20 @@
* http://www.gnu.org/copyleft/gpl.html
*/
class CirrusSearchUpdatePagesJob extends Job {
- public static function build( $revisions, $checkFreshness ) {
- // Strip $revisions down to page ids so we don't put a ton of
stuff in the job queue.
- $pageIds = array();
- foreach ( $revisions as $rev ) {
- $pageIds[] = $rev[ 'id' ];
+ public static function build( $pages, $checkFreshness, $updateOnSkip,
$skipParse, $skipLinks ) {
+ // Strip $pages down to PrefixedDBKeys so we don't put a ton of
stuff in the job queue.
+ $pageDBKeys = array();
+ foreach ( $pages as $page ) {
+ $pageDBKeys[] = $page->getTitle()->getPrefixedDBkey();
}
// We don't have a "title" for this job so we use the Main Page
because it exists.
return new CirrusSearchUpdatePagesJob( Title::newMainPage(),
array(
- 'pageIds' => $pageIds,
+ 'pageDBKeys' => $pageDBKeys,
'checkFreshness' => $checkFreshness,
+ 'updateOnSkip' => $updateOnSkip,
+ 'skipParse' => $skipParse,
+ 'skipLinks' => $skipLinks,
) );
}
@@ -40,10 +43,11 @@
public function run() {
// Reload pages from pageIds to throw into the updater
$pageData = array();
- foreach ( $this->params[ 'pageIds' ] as $pageId ) {
- $pageData[] = array( 'page' => WikiPage::newFromID(
$pageId ) );
+ foreach ( $this->params[ 'pageDBKeys' ] as $pageDBKey ) {
+ $pageData[] = WikiPage::factory( Title::newFromDBKey(
$pageDBKey ) );
}
// Now invoke the updater!
- CirrusSearchUpdater::updatePages( $pageData, $this->params[
'checkFreshness' ] );
+ CirrusSearchUpdater::updatePages( $pageData, $this->params[
'checkFreshness' ], null, null,
+ $this->params[ 'updateOnSkip' ], $this->params[
'skipParse' ], $this->params[ 'skipLinks' ] );
}
}
diff --git a/includes/CirrusSearchUpdater.php b/includes/CirrusSearchUpdater.php
index 48c63a1..bfa70c4 100644
--- a/includes/CirrusSearchUpdater.php
+++ b/includes/CirrusSearchUpdater.php
@@ -60,9 +60,8 @@
wfDebugLog( 'CirrusSearch', "Updating search index for
$title which is a redirect to " . $target->getText() );
self::updateFromTitle( $target );
} else {
- self::updatePages( array( array(
- 'page' => $page,
- ) ), false, $wgCirrusSearchShardTimeout,
$wgCirrusSearchClientSideUpdateTimeout );
+ self::updatePages( array( $page ), false,
$wgCirrusSearchShardTimeout,
+ $wgCirrusSearchClientSideUpdateTimeout, false,
false, false );
}
}
@@ -119,13 +118,7 @@
/**
* This updates pages in elasticsearch.
*
- * @param array $pageData An array of pages. The format is as follows:
- * array(
- * array(
- * 'page' => page,
- * 'skip-parse' => true, # Don't parse the page, just update link
counts. Optional.
- * )
- * )
+ * @param $pages array(WikiPage) pages to update
* @param boolean $checkFreshness Should we check if Elasticsearch
already has
* up to date copy of the document before sending it?
* @param null|string $shardTimeout How long should elaticsearch wait
for an offline
@@ -135,8 +128,8 @@
* @param null|int $clientSideTimeout timeout in seconds to update
pages or null if using
* the Elastica default which is 300 seconds.
*/
- public static function updatePages( $pageData, $checkFreshness = false,
$shardTimeout = null,
- $clientSideTimeout = null) {
+ public static function updatePages( $pages, $checkFreshness,
$shardTimeout, $clientSideTimeout,
+ $updateOnSkip, $skipParse, $skipLinks) {
wfProfileIn( __METHOD__ );
if ( $clientSideTimeout !== null ) {
@@ -144,11 +137,11 @@
}
$contentDocuments = array();
$generalDocuments = array();
- foreach ( $pageData as $page ) {
+ foreach ( $pages as $page ) {
if ( $checkFreshness && self::isFresh( $page ) ) {
continue;
}
- $document = self::buildDocumentforRevision( $page );
+ $document = self::buildDocumentForPage( $page,
$updateOnSkip, $skipParse, $skipLinks );
if ( $document === null ) {
continue;
}
@@ -185,7 +178,6 @@
}
private static function isFresh( $page ) {
- $page = $page[ 'page' ];
$searcher = new CirrusSearchSearcher( 0, 0, array(
$page->getTitle()->getNamespace() ) );
$get = $searcher->get( $page->getTitle()->getArticleId(),
array( 'timestamp ') );
if ( !$get->isOk() ) {
@@ -267,12 +259,10 @@
wfProfileOut( __METHOD__ );
}
- private static function buildDocumentforRevision( $page ) {
+ private static function buildDocumentForPage( $page, $updateOnSkip,
$skipParse, $skipLinks ) {
global $wgCirrusSearchIndexedRedirects;
wfProfileIn( __METHOD__ );
- $skipParse = isset( $page[ 'skip-parse' ] ) && $page[
'skip-parse' ];
- $page = $page[ 'page' ];
$title = $page->getTitle();
if ( !$page->exists() ) {
wfLogWarning( 'Attempted to build a document for a page
that doesn\'t exist. This should be caught ' .
@@ -286,14 +276,16 @@
'timestamp' => wfTimestamp( TS_ISO_8601,
$page->getTimestamp() ),
) );
- if ( $skipParse ) {
+ if ( $updateOnSkip && ( $skipParse || $skipLinks ) ) {
// These are sent as updates so if the document isn't
already in the index this is
- // ignored. This is prferable to sending regular index
requests because those ignore
+ // ignored. This is preferable to sending regular
index requests because those ignore
// doc_as_upsert. Without that this feature just
trashes the search index by removing
// the text from entries.
$doc->setDocAsUpsert( true );
$doc->setOpType( 'update' );
- } else {
+ }
+
+ if ( !$skipParse ) {
$doc->setOpType( 'index' );
$parserOutput = $page->getParserOutput( new
ParserOptions(), $page->getRevision()->getId() );
$text = Sanitizer::stripAllTags( SearchEngine::create(
'CirrusSearch' )
@@ -328,38 +320,94 @@
foreach ( $parserOutput->getLinks() as $linkedNamespace
=> $namespaceLinks ) {
foreach ( $namespaceLinks as $linkedDbKey =>
$ignored ) {
$linked = Title::makeTitle(
$linkedNamespace, $linkedDbKey );
+ wfDebugLog( 'CirrusSearch', "$title
links to $linked" );
$outgoingLinks[] =
$linked->getPrefixedDBKey();
}
}
$doc->add( 'outgoing_link', $outgoingLinks );
}
- $incomingLinks = self::countLinksToTitle( $title );
- $doc->add( 'links', $incomingLinks );
#Deprecated
- $doc->add( 'incoming_links', $incomingLinks );
-
- // Handle redirects to this page
- $redirectTitles = $title->getLinksTo( array( 'limit' =>
$wgCirrusSearchIndexedRedirects ), 'redirect', 'rd' );
- $redirects = array();
- $redirectLinks = 0;
- foreach ( $redirectTitles as $redirect ) {
- // If the redirect is in main or the same namespace as
the article the index it
- if ( $redirect->getNamespace() === NS_MAIN &&
$redirect->getNamespace() === $title->getNamespace()) {
- $redirects[] = array(
- 'namespace' =>
$redirect->getNamespace(),
- 'title' => $redirect->getText()
- );
+ if ( !$skipLinks ) {
+ // Handle redirects to this page
+ $redirectTitles = $title->getLinksTo( array( 'limit' =>
$wgCirrusSearchIndexedRedirects ), 'redirect', 'rd' );
+ $redirects = array();
+ $redirectPrefixedDBKeys = array();
+ foreach ( $redirectTitles as $redirect ) {
+ // If the redirect is in main or the same
namespace as the article the index it
+ if ( $redirect->getNamespace() === NS_MAIN &&
$redirect->getNamespace() === $title->getNamespace()) {
+ $redirects[] = array(
+ 'namespace' =>
$redirect->getNamespace(),
+ 'title' => $redirect->getText()
+ );
+ $redirectPrefixedDBKeys[] =
$redirect->getPrefixedDBKey();
+ }
}
- // Count links to redirects
- // Note that we don't count redirect to redirects here
because that seems a bit much.
- $redirectLinks += self::countLinksToTitle( $redirect );
+ $doc->add( 'redirect', $redirects );
+
+ // Count links
+ $linkCountMultiSearch = new \Elastica\Multi\Search(
CirrusSearchConnection::getClient() );
+ $linkCountMultiSearch->addSearch( self::buildCount(
+ new \Elastica\Filter\Term( array(
'outgoing_link' => $title->getPrefixedDBKey() ) ) ) );
+ if ( count( $redirectPrefixedDBKeys ) ) {
+ $linkCountMultiSearch->addSearch(
self::buildCount(
+ new \Elastica\Filter\Terms(
'outgoing_link', $redirectPrefixedDBKeys ) ) );
+ }
+
+ sleep( 5 );
+ $linkCountWork = new PoolCounterWorkViaCallback(
'CirrusSearch-Search', "_elasticsearch", array(
+ 'doWork' => function() use ( $title, $doc,
$linkCountMultiSearch ) {
+ try {
+ $start = microtime();
+ $result =
$linkCountMultiSearch->search();
+ $took = round( ( microtime() -
$start ) * 1000 );
+ wfDebugLog( 'CirrusSearch',
"Counted links to $title in $took millis." );
+ return Status::newGood( $result
);
+ } catch (
\Elastica\Exception\ExceptionInterface $e ) {
+ $id = $doc->getId();
+ wfLogWarning( "Search backend
error during link count for $id. Error message is: " .
+ $e->getMessage() );
+ wfDebugLog(
'CirrusSearchChangeFailed', "Links: $id" );
+ return Status::newFatal(
'cirrussearch-backend-error' );
+ }
+ },
+ 'error' => function( $status ) use ( $doc ) {
+ $status = $status->getErrorsArray();
+ $id = $doc->getId();
+ wfLogWarning( "Pool error performing a
link count against Elasticsearch for $id: " .
+ $status[ 0 ][ 0 ] );
+ wfDebugLog( 'CirrusSearchChangeFailed',
"Links: $id" );
+ return $status;
+ }
+ ) );
+ $linkCountWork = $linkCountWork->execute();
+ if ( $linkCountWork->isGood() ) {
+ // If it isn't good we've already logged enough
information to track it down
+ $linkCountMultiSearchResult =
$linkCountWork->getValue();
+ // Incoming links is the sum of the number of
linked pages which we count in Elasticsearch
+ // and the number of incoming redirects of
which we have a handy list so we count that here.
+ $doc->add( 'incoming_links',
$linkCountMultiSearchResult[ 0 ]->getTotalHits() +
+ count( $redirects ) );
+ // If a page doesn't have any redirects then we
won't have asked to count links to them.
+ // If so, we default to 0. Otherwise, read the
count.
+ $doc->add( 'incoming_redirect_links', isset(
$linkCountMultiSearchResult[ 1 ] ) ?
+ $linkCountMultiSearchResult[ 1
]->getTotalHits() : 0 );
+ }
}
- $doc->add( 'redirect', $redirects );
- $doc->add( 'redirect_links', $redirectLinks );
#Deprecated
- $doc->add( 'incoming_redirect_links', $redirectLinks );
wfProfileOut( __METHOD__ );
return $doc;
+ }
+
+ private static function buildCount( $filter ) {
+ $type = CirrusSearchConnection::getPageType();
+ $search = new \Elastica\Search( $type->getIndex()->getClient()
);
+ $search->addIndex( $type->getIndex() );
+ $search->addType( $type );
+ $search->setOption( \Elastica\Search::OPTION_SEARCH_TYPE,
+ \Elastica\Search::OPTION_SEARCH_TYPE_COUNT );
+ $matchAll = new \Elastica\Query\MatchAll();
+ $search->setQuery( new \Elastica\Query\Filtered( $matchAll,
$filter ) );
+ return $search;
}
private static function getIgnoredHeadings() {
@@ -376,37 +424,6 @@
}
}
return self::$ignoredHeadings;
- }
-
- /**
- * Count the links to $title directly in the slave db.
- * @param $title a title
- * @return an integer count
- */
- private static function countLinksToTitle( $title ) {
- global $wgMemc, $wgCirrusSearchLinkCountCacheTime;
- $key = wfMemcKey( 'cirrus', 'linkcounts',
$title->getPrefixedDBKey() );
- $count = $wgMemc->get( $key );
- if ( !is_int( $count ) ) {
- $dbr = wfGetDB( DB_SLAVE );
- $count = $dbr->selectField(
- array( 'pagelinks' ),
- 'COUNT(*)',
- array(
- "pl_namespace" =>
$title->getNamespace(),
- "pl_title" => $title->getDBkey() ),
- __METHOD__
- );
- // Looks like $count can come back as a string....
- if ( is_string( $count ) ) {
- $count = (int)$count;
- }
- if ( is_int( $count ) &&
$wgCirrusSearchLinkCountCacheTime > 0 ) {
- $wgMemc->set( $key, $count,
$wgCirrusSearchLinkCountCacheTime );
- }
- }
-
- return $count ? $count : 0;
}
/**
@@ -448,13 +465,11 @@
continue;
}
// Note that we don't add this page to the list of
updated pages because this update isn't
- // a full update (just link counts.)
- $pages[] = array(
- 'page' => $page,
- 'skip-parse' => true, // Just update link
counts
- );
+ // a full update (just link counts).
+ $pages[] = $page;
}
- self::updatePages( $pages, false, $wgCirrusSearchShardTimeout,
$wgCirrusSearchClientSideUpdateTimeout );
+ self::updatePages( $pages, false, $wgCirrusSearchShardTimeout,
$wgCirrusSearchClientSideUpdateTimeout,
+ true, true, false );
}
/**
@@ -494,7 +509,7 @@
}
wfDebugLog( 'CirrusSearch', "Sending $idCount deletes to the
index." );
$work = new PoolCounterWorkViaCallback( 'CirrusSearch-Update',
"_elasticsearch", array(
- 'doWork' => function() use ( $indexType, $ids ) {
+ 'doWork' => function() use ( $ids ) {
try {
$start = microtime();
CirrusSearchConnection::getPageType(
CirrusSearchConnection::CONTENT_INDEX_TYPE )
diff --git a/maintenance/forceSearchIndex.php b/maintenance/forceSearchIndex.php
index dd0f654..d805a8e 100644
--- a/maintenance/forceSearchIndex.php
+++ b/maintenance/forceSearchIndex.php
@@ -36,6 +36,9 @@
var $limit;
var $forceUpdate;
var $queue;
+ var $indexOnSkip;
+ var $skipParse;
+ var $skipLinks;
public function __construct() {
parent::__construct();
@@ -55,9 +58,18 @@
$this->addOption( 'forceUpdate', 'Blindly upload pages to
Elasticsearch whether or not it already has an up ' .
'to date copy. Not used with --deletes.' );
$this->addOption( 'queue', 'Rather than perform the indexes in
process add them to the job queue. Ignored for delete.' );
+ $this->addOption( 'indexOnSkip', 'When skipping either parsing
or links send the document as an index. ' .
+ 'This replaces the contents of the index for that entry
with the entry built from a skipped process.' .
+ 'Without this if the entry does not exist then it will
be skipped enirely. Only set this when running ' .
+ 'the first pass of building the index. Otherwise,
don\'t tempt fate by indexing half complete documents.' );
+ $this->addOption( 'skipParse', 'Skip parsing the page. This is
realy only good for running the second half ' .
+ 'of the two phase index build.' );
+ $this->addOption( 'skipLinks', 'Skip looking for links to the
page (counting and finding redirects). Use ' .
+ 'this with --indexOnSkip for the first half of the two
phase index build.' );
}
public function execute() {
+ global $wgPoolCounterConf;
wfProfileIn( __METHOD__ );
// Make sure we don't flood the pool counter
@@ -79,6 +91,9 @@
}
$this->forceUpdate = $this->getOption( 'forceUpdate' );
$this->queue = $this->getOption( 'queue' );
+ $this->indexOnSkip = $this->getOption( 'indexOnSkip' );
+ $this->skipParse = $this->getOption( 'skipParse' );
+ $this->skipLinks = $this->getOption( 'skipLinks' );
if ( $this->indexUpdates ) {
if ( $this->queue ) {
@@ -124,16 +139,21 @@
}
$minId = $last[ 'id' ];
- // Strip entries without a page. We can't do
anything with them.
- $updates = array_filter( $updates, function
($rev) {
- return $rev[ 'page' ] !== null;
- } );
- // Update size with the actual number of
updated documents.
+ // Strip updates down to just pages
+ $pages = array();
+ foreach ( $updates as $update ) {
+ if ( isset( $update[ 'page' ] ) ) {
+ $pages[] = $update[ 'page' ];
+ }
+ }
if ( $this->queue ) {
JobQueueGroup::singleton()->push(
-
CirrusSearchUpdatePagesJob::build( $updates, !$this->forceUpdate ) );
+
CirrusSearchUpdatePagesJob::build( $pages, !$this->forceUpdate,
+ !$this->indexOnSkip,
$this->skipParse, $this->skipLinks ) );
} else {
- $size =
CirrusSearchUpdater::updatePages( $updates, !$this->forceUpdate );
+ // Update size with the actual number
of updated documents.
+ $size =
CirrusSearchUpdater::updatePages( $pages, !$this->forceUpdate,
+ null, null,
!$this->indexOnSkip, $this->skipParse, $this->skipLinks );
}
} else {
$idsToDelete = array();
diff --git a/maintenance/updateOneSearchIndexConfig.php
b/maintenance/updateOneSearchIndexConfig.php
index 82690f1..9485531 100644
--- a/maintenance/updateOneSearchIndexConfig.php
+++ b/maintenance/updateOneSearchIndexConfig.php
@@ -109,6 +109,7 @@
}
public function execute() {
+ global $wgPoolCounterConf;
// Make sure we don't flood the pool counter
unset( $wgPoolCounterConf['CirrusSearch-Update'] );
unset( $wgPoolCounterConf['CirrusSearch-Search'] );
--
To view, visit https://gerrit.wikimedia.org/r/97002
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide705f1e77c3076dc3c1a324689dd1cf46633855
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: Manybubbles <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
