Hello Aaron Schulz, Krinkle, jenkins-bot,
I'd like you to do a code review. Please visit
https://gerrit.wikimedia.org/r/344105
to review the following change.
Change subject: Revert "Make MessageCache use the immutable text cache during
cache rebuilds"
......................................................................
Revert "Make MessageCache use the immutable text cache during cache rebuilds"
The change fails on beta update.php with:
Fatal error: Call to undefined method ExternalStore::decompressRevisionText()
in /srv/mediawiki-staging/php-master/includes/Revision.php on line 1290
Guess ExternalStore::decompressRevisionText does not exist.
This reverts commit c2bbb51c20822b91cac4d416e0af103491bf4e7f.
Bug: T158084
Change-Id: Ifb4959a765ac7b0ea05ff176c0faee9b3a7b0c1d
---
M includes/Revision.php
M includes/cache/MessageCache.php
2 files changed, 13 insertions(+), 39 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/05/344105/1
diff --git a/includes/Revision.php b/includes/Revision.php
index dca2e1b..4b9435a 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -1240,9 +1240,8 @@
/**
* Get revision text associated with an old or archive row
- *
- * Both the flags and the text field must be included. Including the
old_id
- * field will activate cache usage as long as the $wiki parameter is
not set.
+ * $row is usually an object from wfFetchRow(), both the flags and the
text
+ * field must be included.
*
* @param stdClass $row The text data
* @param string $prefix Table prefix (default 'old_')
@@ -1253,6 +1252,8 @@
* @return string|false Text the text requested or false on failure
*/
public static function getRevisionText( $row, $prefix = 'old_', $wiki =
false ) {
+
+ # Get data
$textField = $prefix . 'text';
$flagsField = $prefix . 'flags';
@@ -1268,36 +1269,20 @@
return false;
}
- // Use external methods for external objects, text in table is
URL-only then
+ # Use external methods for external objects, text in table is
URL-only then
if ( in_array( 'external', $flags ) ) {
$url = $text;
$parts = explode( '://', $url, 2 );
if ( count( $parts ) == 1 || $parts[1] == '' ) {
return false;
}
-
- if ( isset( $row->old_id ) && $wiki === false ) {
- // Make use of the wiki-local revision text
cache
- $cache =
MediaWikiServices::getInstance()->getMainWANObjectCache();
- $text = $cache->getWithSetCallback(
- $cache->makeKey( 'revisiontext',
'textid', $row->old_id ),
- self::getCacheTTL( $cache ),
- function () use ( $url, $wiki ) {
- // No negative caching per
Revision::loadText()
- return
ExternalStore::fetchFromURL( $url, [ 'wiki' => $wiki ] );
- },
- [ 'pcGroup' => self::TEXT_CACHE_GROUP,
'pcTTL' => $cache::TTL_PROC_LONG ]
- );
- } else {
- $text = ExternalStore::fetchFromURL( $url, [
'wiki' => $wiki ] );
- }
+ $text = ExternalStore::fetchFromURL( $url, [ 'wiki' =>
$wiki ] );
}
// If the text was fetched without an error, convert it
if ( $text !== false ) {
$text = self::decompressRevisionText( $text, $flags );
}
-
return $text;
}
@@ -1574,14 +1559,15 @@
}
/**
- * Get the text cache TTL
+ * Lazy-load the revision's text.
+ * Currently hardcoded to the 'text' table storage engine.
*
- * @param WANObjectCache $cache
- * @return integer
+ * @return string|bool The revision's text, or false on failure
*/
- private static function getCacheTTL( WANObjectCache $cache ) {
+ private function loadText() {
global $wgRevisionCacheExpiry;
+ $cache = ObjectCache::getMainWANInstance();
if ( $cache->getQoS( $cache::ATTR_EMULATION ) <=
$cache::QOS_EMULATION_SQL ) {
// Do not cache RDBMs blobs in...the RDBMs store
$ttl = $cache::TTL_UNCACHEABLE;
@@ -1589,22 +1575,10 @@
$ttl = $wgRevisionCacheExpiry ?:
$cache::TTL_UNCACHEABLE;
}
- return $ttl;
- }
-
- /**
- * Lazy-load the revision's text.
- * Currently hardcoded to the 'text' table storage engine.
- *
- * @return string|bool The revision's text, or false on failure
- */
- private function loadText() {
- $cache = ObjectCache::getMainWANInstance();
-
// No negative caching; negative hits on text rows may be due
to corrupted replica DBs
return $cache->getWithSetCallback(
$cache->makeKey( 'revisiontext', 'textid',
$this->getTextId() ),
- self::getCacheTTL( $cache ),
+ $ttl,
function () {
return $this->fetchText();
},
diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php
index 7cd489a..4facc20 100644
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@ -508,7 +508,7 @@
$res = $dbr->select(
[ 'page', 'revision', 'text' ],
- [ 'page_title', 'old_id', 'old_text', 'old_flags' ],
+ [ 'page_title', 'old_text', 'old_flags' ],
$smallConds,
__METHOD__ . "($code)-small"
);
--
To view, visit https://gerrit.wikimedia.org/r/344105
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb4959a765ac7b0ea05ff176c0faee9b3a7b0c1d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Hashar <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
