Ori.livneh has uploaded a new change for review.
https://gerrit.wikimedia.org/r/60233
Change subject: (Bug 47482) Update redis in bulk onTransactionIdle
......................................................................
(Bug 47482) Update redis in bulk onTransactionIdle
Change I00107ffc9 in mediawiki/core makes WikiPage::updateCategoryCounts defer
work to a callback function which executes at the end of the current
transaction, if one is open. This means that LinksUpdateComplete is no longer
guaranteed to run after all updates have, in fact, completed, and that
consequently we can no longer rely on it for batch-processing all pending
updates.
This change uses a different mechanism to defer batch processing of redis
updates: it registers a callback using Database->onTransactionIdle.
IMHO the change to when LinksUpdateComplete fires amounts to a bug in core.
I'll file a bug, but I'd rather we not wait for it to be fixed to resolve this
particular issue.
Change-Id: Ib073661c267cbabbe2ab2245224afd12b8784fca
---
M GettingStarted.php
M RedisCategorySync.php
2 files changed, 36 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/GettingStarted
refs/changes/33/60233/1
diff --git a/GettingStarted.php b/GettingStarted.php
index d8a5308..45f9bf1 100644
--- a/GettingStarted.php
+++ b/GettingStarted.php
@@ -164,7 +164,6 @@
$wgHooks[ 'RecentChange_save' ][] = 'GettingStartedHooks::onRecentChange_save';
$wgHooks[ 'CategoryAfterPageAdded' ][] =
'RedisCategorySync::onCategoryAfterPageAdded';
$wgHooks[ 'CategoryAfterPageRemoved' ][] =
'RedisCategorySync::onCategoryAfterPageRemoved';
-$wgHooks[ 'LinksUpdateComplete' ][] =
'RedisCategorySync::onLinksUpdateComplete';
$wgHooks[ 'ListDefinedTags' ][] = 'GettingStartedHooks::onListDefinedTags';
$wgHooks[ 'MakeGlobalVariablesScript' ][] =
'GettingStartedHooks::onMakeGlobalVariablesScript';
$wgHooks[ 'BeforeCreateEchoEvent' ][] =
'GettingStartedHooks::onBeforeCreateEchoEvent';
diff --git a/RedisCategorySync.php b/RedisCategorySync.php
index 18e2bc2..fdd2cf9 100644
--- a/RedisCategorySync.php
+++ b/RedisCategorySync.php
@@ -22,6 +22,10 @@
/** @var array: arrays of [Category, WikiPage] removals to process. **/
public static $removals = array();
+ /** @var bool: whether or not an update callback has been registered.
**/
+ public static $callbackSet = false;
+
+
/**
* Acquire a Redis connection.
* @return Redis|bool Redis client or false.
@@ -87,6 +91,7 @@
public static function onCategoryAfterPageAdded( Category $category,
WikiPage $page ) {
if ( self::isUpdateRelevant( $category, $page ) ) {
self::$additions[] = array( $category, $page );
+ self::setCallback();
}
return true;
}
@@ -98,39 +103,48 @@
public static function onCategoryAfterPageRemoved( Category $category,
WikiPage $page ) {
if ( self::isUpdateRelevant( $category, $page ) ) {
self::$removals[] = array( $category, $page );
+ self::setCallback();
}
return true;
}
/**
- * @param LinksUpdate &$linksUpdate
+ * Register a callback function that will perform all redis updates once
+ * the current transaction completes.
+ * @return bool True if callback was set, false if it was set already.
*/
- public static function onLinksUpdateComplete( $linksUpdate ) {
- if ( !count( self::$additions ) && !count( self::$removals ) ) {
- return true;
+ public static function setCallback() {
+ if ( self::$callbackSet ) {
+ return false;
}
- $conn = self::getClient();
- if ( !$conn ) {
- wfDebugLog( 'GettingStarted', "Unable to acquire redis
connection.\n" );
- return true;
- }
+ $dbw = wfGetDB( DB_MASTER );
+ $dbw->onTransactionIdle( function () {
+ // Any category updates that happen after this will
require an
+ // additional run of the callback.
+ RedisCategorySync::$callbackSet = false;
- try {
- $redis = $conn->multi( Redis::PIPELINE );
- foreach( self::$additions as $addition ) {
- list( $category, $page ) = $addition;
- $redis->sAdd( self::makeCategoryKey( $category
), $page->getId() );
+ $conn = RedisCategorySync::getClient();
+ if ( !$conn ) {
+ wfDebugLog( 'GettingStarted', "Unable to
acquire redis connection.\n" );
+ return;
}
- foreach( self::$removals as $removal ) {
- list( $category, $page ) = $removal;
- $redis->sRem( self::makeCategoryKey( $category
), $page->getId() );
- }
- $redis->exec();
- } catch ( RedisException $e ) {
- wfDebugLog( 'GettingStarted', 'Redis exception: ' .
$e->getMessage() . "\n" );
- }
+ try {
+ $redis = $conn->multi( Redis::PIPELINE );
+ while ( RedisCategorySync::$additions ) {
+ list( $category, $page ) = array_pop(
RedisCategorySync::$additions );
+ $redis->sAdd(
RedisCategorySync::makeCategoryKey( $category ), $page->getId() );
+ }
+ while ( RedisCategorySync::$removals ) {
+ list( $category, $page ) = array_pop(
RedisCategorySync::$removals );
+ $redis->sRem(
RedisCategorySync::makeCategoryKey( $category ), $page->getId() );
+ }
+ $redis->exec();
+ } catch ( RedisException $e ) {
+ wfDebugLog( 'GettingStarted', 'Redis exception:
' . $e->getMessage() . "\n" );
+ }
+ } );
return true;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/60233
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib073661c267cbabbe2ab2245224afd12b8784fca
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/GettingStarted
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
