jenkins-bot has submitted this change and it was merged.
Change subject: Better error handling for memc and some comments
......................................................................
Better error handling for memc and some comments
Change-Id: Ibdd49677c7eac6b8d601ea1d317c0a4d61e42943
---
M includes/Data/MultiDimArray.php
M includes/Data/ObjectManager.php
M includes/Data/RevisionStorage.php
M includes/Model/AbstractRevision.php
M includes/Repository/MultiGetList.php
M includes/Repository/TreeRepository.php
6 files changed, 128 insertions(+), 28 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Data/MultiDimArray.php b/includes/Data/MultiDimArray.php
index 3a89928..9ed77db 100644
--- a/includes/Data/MultiDimArray.php
+++ b/includes/Data/MultiDimArray.php
@@ -2,6 +2,29 @@
namespace Flow\Data;
+/**
+ * This object can be used to easily set keys in a multi-dimensional array.
+ *
+ * Usage:
+ *
+ * $arr = new Flow\Data\MultiDimArray;
+ * $arr[array(1,2,3)] = 4;
+ * $arr[array(2,3,4)] = 5;
+ * var_export( $arr->all() );
+ *
+ * array (
+ * 1 => array (
+ * 2 => array (
+ * 3 => 4,
+ * ),
+ * ),
+ * 2 => array (
+ * 3 => array (
+ * 4 => 5,
+ * ),
+ * ),
+ * )
+ */
class MultiDimArray implements \ArrayAccess {
protected $data = array();
diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php
index 8ac2e5b..dd19e56 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -76,36 +76,75 @@
function fromStorageRow( array $row, $object = null );
}
-// An Index is just a store that receives updates via handler.
-// backing store's can be passed via constructor
+/**
+ * Indexes store one or more values bucketed by exact key/value combinations.
+ */
interface Index extends LifecycleHandler {
- /// Indexes accept no query options
+ /**
+ * Find data models matching the provided equality condition.
+ *
+ * @param array $keys A map of k,v pairs to find via equality condition
+ * @return array|false Cached subset of data model rows matching the
+ * equality conditions provided in $keys.
+ */
function find( array $keys );
- /// Indexes accept no query options
+ /**
+ * Batch together multiple calls to self::find with minimal network
round trips.
+ *
+ * @param array $queries An array of arrays in the form of $keys
parameter of self::find
+ * @return array|false Array of arrays in same order as $queries
representing batched result set.
+ */
function findMulti( array $queries );
- // Maximum number of items in a single index value
+ /**
+ * @return integer Maximum number of items in a single index value
+ */
function getLimit();
- // Can the index locate a result for this keys and options pair
+ /**
+ * Query options are not supported at the query level, the index always
+ * returns the same value for the same key/value combination.
Depending on what
+ * the query stores it may contain the answers to various options,
which will require
+ * post-processing by the caller.
+ *
+ * @return boolean Can the index locate a result for this keys and
options pair
+ */
function canAnswer( array $keys, array $options );
}
-// Compact rows before writing to memcache, expand when receiving back
-// Still returns arrays, just removes unneccessary values
+/**
+ * Compact rows before writing to memcache, expand when receiving back
+ * Still returns arrays, just removes unneccessary values
+ */
interface Compactor {
- // Return only the values in $row that will be written to the cache
+ /**
+ * @param array $row A data model row to strip unnecessary data from
+ * @return array Only the values in $row that will be written to the
cache
+ */
public function compactRow( array $row );
- // perform self::compactRow against an array of rows
+
+ /**
+ * @param array $rows Multiple data model rows to strip unnecesssary
data from
+ * @return array The provided rows now containing only the values the
will be written to cache
+ */
public function compactRows( array $rows );
- // Repopulate BagOStuff::multiGet results with any values removed in
self::compactRow
+
+ /**
+ * Repopulate BagOStuff::multiGet results with any values removed in
self::compactRow
+ *
+ * @param array $cached The multi-dimensional array results of
BagOStuff::multiGet
+ * @param array $keyToQuery An array mapping memcache-key to the values
used to generate that cache key
+ * @return array The cached content from memcache along with any data
stripped in self::compactRow
+ */
public function expandCacheResult( array $cached, array $keyToQuery );
}
-// A little glue code so you dont need to use the individual manager for each
class
-// Can be made more specific once the interfaces settle down
+/**
+ * A little glue code to allow passing arround and manipulating multiple
+ * ObjectManagers more convenient.
+ */
class ManagerGroup {
public function __construct( Container $container, array $classMap ) {
$this->container = $container;
@@ -653,7 +692,7 @@
foreach ( $res as $row ) {
$result[] = (array) $row;
}
- wfDebugLog( __CLASS__, __METHOD__ . ': ' . print_r( $result,
true ) );
+ // wfDebugLog( __CLASS__, __METHOD__ . ': ' . print_r( $result,
true ) );
return $result;
}
@@ -1292,7 +1331,12 @@
}
if ( $keys ) {
$flipped = array_flip( $keys );
- foreach ( parent::getMulti( $keys ) as $key => $value )
{
+ $res = parent::getMulti( $keys );
+ if ( $res === false ) {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ':
Failure requesting data from memcache : ' . implode( ',', $keys ) );
+ return $found;
+ }
+ foreach ( $res as $key => $value ) {
$this->internal[$key] = $found[$key] = $value;
unset( $keys[$flipped[$key]] );
}
diff --git a/includes/Data/RevisionStorage.php
b/includes/Data/RevisionStorage.php
index 715cf31..f7a326a 100644
--- a/includes/Data/RevisionStorage.php
+++ b/includes/Data/RevisionStorage.php
@@ -488,6 +488,10 @@
$roots[] = UUID::create( $features['topic_root'] );
}
$nodeList = $this->treeRepository->fetchSubtreeNodeList( $roots
);
+ if ( $nodeList === false ) {
+ // We can't return the existing $retval, that false
data would be cached.
+ return false;
+ }
$descendantQueries = array();
foreach ( $queries as $idx => $features ) {
diff --git a/includes/Model/AbstractRevision.php
b/includes/Model/AbstractRevision.php
index d11cfdb..4130052 100644
--- a/includes/Model/AbstractRevision.php
+++ b/includes/Model/AbstractRevision.php
@@ -13,11 +13,18 @@
const MODERATED_CENSORED = 'censor';
+ /**
+ * Possible moderation states of a revision. These must be ordered from
+ * least restrictive to most restrictive permission.
+ */
static protected $perms = array(
self::MODERATED_NONE => array(
+ // The name of the permission checked with
User::isAllowed
'perm' => null,
+ // This is the bit of text rendered instead of the post
creator
'usertext' => null,
- 'content' => null
+ // This is the bit of text rendered instead of the
content
+ 'content' => null,
),
self::MODERATED_HIDDEN => array(
'perm' => 'flow-hide',
@@ -227,11 +234,12 @@
}
public function getUserText( $user = null ) {
- if ( $this->isAllowed( $user ) ) {
- return $this->getUserTextRaw();
- } else {
+ // The text of *this* revision is only stripped when fully
moderated
+ if ( $this->isCensored() ) {
// Messages: flow-post-hidden, flow-post-deleted,
flow-post-censored
return wfMessage(
self::$perms[$this->moderationState]['usertext'] );
+ } else {
+ return $this->getUserTextRaw();
}
}
@@ -287,6 +295,10 @@
return $this->moderationState !== self::MODERATED_NONE;
}
+ public function isCensored() {
+ return $this->moderationState === self::MODERATED_CENSORED;
+ }
+
public function getModerationTimestamp() {
return $this->moderationTimestamp;
}
diff --git a/includes/Repository/MultiGetList.php
b/includes/Repository/MultiGetList.php
index 61b860c..1e33f63 100644
--- a/includes/Repository/MultiGetList.php
+++ b/includes/Repository/MultiGetList.php
@@ -32,14 +32,20 @@
return array();
}
$result = array();
- foreach ( $this->cache->getMulti( array_keys( $cacheKeys ) ) as
$key => $value ) {
- if ( $cacheKeys[$key] instanceof UUID ) {
- $idx = $cacheKeys[$key]->getBinary();
- } else {
- $idx = $cacheKeys[$key];
+ $multiRes = $this->cache->getMulti( array_keys( $cacheKeys ) );
+ if ( $multiRes === false ) {
+ // Falls through to query only backend
+ wfDebugLog( __CLASS__, __FUNCTION__ . ': Failure
querying memcache' );
+ } else {
+ foreach ( $multiRes as $key => $value ) {
+ if ( $cacheKeys[$key] instanceof UUID ) {
+ $idx = $cacheKeys[$key]->getBinary();
+ } else {
+ $idx = $cacheKeys[$key];
+ }
+ $result[$idx] = $value;
+ unset( $cacheKeys[$key] );
}
- $result[$idx] = $value;
- unset( $cacheKeys[$key] );
}
if ( count( $cacheKeys ) === 0 ) {
return $result;
@@ -57,7 +63,11 @@
$invCacheKeys[$id] = $cacheKey;
}
foreach ( $res as $id => $row ) {
- $this->cache->set( $invCacheKeys[$id], $row );
+ // If we failed contacting memcache a moment ago dont
bother trying to
+ // push values either.
+ if ( $multiRes !== false ) {
+ $this->cache->set( $invCacheKeys[$id], $row );
+ }
$result[$id] = $row;
}
diff --git a/includes/Repository/TreeRepository.php
b/includes/Repository/TreeRepository.php
index 105233f..2d6e98a 100644
--- a/includes/Repository/TreeRepository.php
+++ b/includes/Repository/TreeRepository.php
@@ -215,7 +215,10 @@
$roots,
array( $this, 'fetchSubtreeNodeListFromDb' )
);
-
+ if ( $res === false ) {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ': Failure
fetching node list from cache' );
+ return false;
+ }
// $idx is a binary UUID
$retval = array();
foreach ( $res as $idx => $val ) {
@@ -233,6 +236,10 @@
),
__METHOD__
);
+ if ( $res === false ) {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ': Failure
fetching node list from database' );
+ return false;
+ }
if ( !$res ) {
return array();
}
--
To view, visit https://gerrit.wikimedia.org/r/83030
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibdd49677c7eac6b8d601ea1d317c0a4d61e42943
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson (WMF) <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Bsitu <bs...@wikimedia.org>
Gerrit-Reviewer: EBernhardson (WMF) <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Matthias Mullie <mmul...@wikimedia.org>
Gerrit-Reviewer: Werdna <agarr...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
