jenkins-bot has submitted this change and it was merged. Change subject: Account for DB lag when refreshing cached ChoiceData ......................................................................
Account for DB lag when refreshing cached ChoiceData This fixes an apparent race condition in which ObjectCache-cached ChoiceData is invalidated following a change in campaign or banner settings, then a client request has a cache miss and re-sets the cache using data form a laggy DB slave, causing stale ChoiceData to be cached and served. For details about the solution, see the inline documentation for WANObjectCache::getWithSetCallback(), in MediaWiki core. Bug: T144393 Change-Id: Id4e46b653554d39ac5721c4327774ccfce63b734 --- M includes/ChoiceDataProvider.php 1 file changed, 21 insertions(+), 4 deletions(-) Approvals: Ejegg: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/ChoiceDataProvider.php b/includes/ChoiceDataProvider.php index 0d765fb..715e450 100644 --- a/includes/ChoiceDataProvider.php +++ b/includes/ChoiceDataProvider.php @@ -42,10 +42,26 @@ $choices = $cache->getWithSetCallback( $dataKey, self::CACHE_TTL, - function () use ( $project, $language ) { - return self::fetchChoices( $project, $language ); + function ( $oldValue, &$ttl, array &$setOpts ) + use ( $project, $language ) { + + $dbr = CNDatabase::getDb( DB_SLAVE ); + + // Account for slave lag to prevent a race condition when + // campaigns are updated, the cache is invalidated, and + // a client queries a yet-unsynced slave DB. Also, gracefully + // fail if we're running an old version of core (<1.27). + if ( method_exists( 'Database', 'getCacheSetOptions') ) { + $setOpts += Database::getCacheSetOptions( $dbr ); + } + + return self::fetchChoices( $project, $language, $dbr ); }, [ + // We don't bother with the lockTSE option because the only + // potentially high-volume requests that would ask for this + // object are heavily cached by Varnish, for all users. (Those + // requests are for load.php.) 'checkKeys' => [ $checkKey ], 'pcTTL' => $cache::TTL_PROC_LONG, ] @@ -59,11 +75,12 @@ } ); } - private static function fetchChoices( $project, $language ) { + private static function fetchChoices( $project, $language, + IDatabase $dbr ) { + // For speed, we'll do our own queries instead of using methods in // Campaign and Banner. - $dbr = CNDatabase::getDb( DB_SLAVE ); $cache = ObjectCache::getMainWANInstance(); // Set up conditions -- To view, visit https://gerrit.wikimedia.org/r/308126 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Id4e46b653554d39ac5721c4327774ccfce63b734 Gerrit-PatchSet: 7 Gerrit-Project: mediawiki/extensions/CentralNotice Gerrit-Branch: master Gerrit-Owner: AndyRussG <andrew.green...@gmail.com> Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org> Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com> Gerrit-Reviewer: Awight <awi...@wikimedia.org> Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org> Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org> Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org> Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits