jenkins-bot has submitted this change and it was merged.
Change subject: Move cutting of index data into index code
......................................................................
Move cutting of index data into index code
Alternative fix to gerrit 112750, more performant but more invasive.
Bug: 61066
Change-Id: I1a575c723790fdfea38d5670e7e50b223cdc856c
---
M includes/Data/BoardHistoryStorage.php
M includes/Data/ObjectManager.php
M includes/Data/RevisionStorage.php
3 files changed, 159 insertions(+), 121 deletions(-)
Approvals:
Bsitu: Looks good to me, but someone else must approve
EBernhardson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Data/BoardHistoryStorage.php
b/includes/Data/BoardHistoryStorage.php
index 4d31793..6a5d5c1 100644
--- a/includes/Data/BoardHistoryStorage.php
+++ b/includes/Data/BoardHistoryStorage.php
@@ -113,18 +113,16 @@
return parent::findMulti( $queries, $options );
}
- public function backingStoreFindMulti( array $queries, array $idxToKey,
array $retval = array() ) {
- $res = $this->storage->findMulti( $queries,
$this->queryOptions() );
+ public function backingStoreFindMulti( array $queries, array $cacheKeys
) {
+ $options = $this->queryOptions();
+ $res = $this->storage->findMulti( $queries, $options );
if ( !$res ) {
return false;
}
$res = reset( $res );
-
- $this->cache->add( current( $idxToKey ),
$this->rowCompactor->compactRows( $res ) );
- $retval[] = $res;
-
- return $retval;
+ $this->cache->add( current( $cacheKeys ),
$this->rowCompactor->compactRows( $res ) );
+ return array( $res );
}
public function onAfterInsert( $object, array $new ) {
diff --git a/includes/Data/ObjectManager.php b/includes/Data/ObjectManager.php
index b2548b8..62fbde5 100644
--- a/includes/Data/ObjectManager.php
+++ b/includes/Data/ObjectManager.php
@@ -310,33 +310,17 @@
} catch ( NoIndexException $e ) {
wfDebugLog( __CLASS__, __FUNCTION__ . ': ' .
$e->getMessage() );
$res = $this->storage->findMulti( $queries,
$this->convertToDbOptions( $options ) );
- $output = array();
-
- foreach( $res as $index => $queryOutput ) {
- $output[$index] = array_map( array( $this,
'load' ), $queryOutput );
- }
-
- return $output;
}
if ( $res === null ) {
return null;
}
- $retval = array();
- foreach ( $res as $i => $rows ) {
- list( $startPos, $limit ) = $this->getOffsetLimit(
$rows, $index, $options );
- $keys = array_keys( $rows );
- for(
- $k = $startPos;
- $k < $startPos + $limit && $k < count( $keys );
- ++$k
- ) {
- $j = $keys[$k];
- $retval[$i][$j] = $this->load( $rows[$j] );
- }
+ $output = array();
+ foreach( $res as $index => $queryOutput ) {
+ $output[$index] = array_map( array( $this, 'load' ),
$queryOutput );
}
- return $retval;
+ return $output;
}
/**
@@ -456,76 +440,6 @@
}
return $this->foundMulti( $queries );
- }
-
- /**
- * @param $rows
- * @param Index $index
- * @param array $options
- * @return array
- */
- protected function getOffsetLimit( $rows, Index $index, array $options
) {
- $limit = isset( $options['limit'] ) ? $options['limit'] :
$index->getLimit();
-
- if ( ! isset( $options['offset-key'] ) ) {
- $offset = isset( $options['offset'] ) ?
$options['offset'] : 0;
- return array( $offset, $limit );
- }
-
- $offsetKey = $options['offset-key'];
- if ( $offsetKey instanceof UUID ) {
- $offsetKey = $offsetKey->getBinary();
- }
-
- $dir = 'fwd';
- if (
- isset( $options['offset-dir'] ) &&
- $options['offset-dir'] === 'rev'
- ) {
- $dir = 'rev';
- }
-
- $offset = $this->getOffsetFromKey( $rows, $offsetKey, $index );
-
- if ( $dir === 'fwd' ) {
- $startPos = $offset + 1;
- } elseif ( $dir === 'rev' ) {
- $startPos = $offset - $limit;
-
- if ( $startPos < 0 ) {
- if (
- isset( $options['offset-elastic'] ) &&
- $options['offset-elastic'] === false
- ) {
- // If non-elastic, then reduce the
number of items shown commeasurately
- $limit += $startPos;
- }
- $startPos = 0;
- }
- } else {
- // bail?
- $startPos = 0;
- }
-
- return array( $startPos, $limit );
- }
-
- protected function getOffsetFromKey( $rows, $offsetKey, Index $index ) {
- $offset = false;
- for( $rowIndex = 0; $rowIndex < count( $rows ); ++$rowIndex ) {
- $row = $rows[$rowIndex];
- $comparisonValue = $index->compareRowToOffset( $row,
$offsetKey );
- if ( $comparisonValue <= 0 ) {
- $offset = $rowIndex;
- break;
- }
- }
-
- if ( $offset === false ) {
- throw new DataModelException( 'Unable to find specified
offset in query results', 'process-data' );
- }
-
- return $offset;
}
public function clear() {
@@ -1185,6 +1099,84 @@
return isset( $this->options['sort'] ) ? $this->options['sort']
: false;
}
+ /**
+ * @param array $rows
+ * @param array $options
+ * @return array [offset, limit]
+ */
+ protected function getOffsetLimit( $rows, $options ) {
+ $limit = isset( $options['limit'] ) ? $options['limit'] :
$this->getLimit();
+
+ if ( !isset( $options['offset-key'] ) ) {
+ $offset = isset( $options['offset'] ) ?
$options['offset'] : 0;
+ return array( $offset, $limit );
+ }
+
+ $offsetKey = $options['offset-key'];
+ if ( $offsetKey instanceof UUID ) {
+ $offsetKey = $offsetKey->getBinary();
+ }
+
+ $dir = 'fwd';
+ if (
+ isset( $options['offset-dir'] ) &&
+ $options['offset-dir'] === 'rev'
+ ) {
+ $dir = 'rev';
+ }
+
+ $offset = $this->getOffsetFromKey( $rows, $offsetKey );
+
+ if ( $dir === 'fwd' ) {
+ $startPos = $offset + 1;
+ } elseif ( $dir === 'rev' ) {
+ $startPos = $offset - $limit;
+
+ if ( $startPos < 0 ) {
+ if (
+ isset( $options['offset-elastic'] ) &&
+ $options['offset-elastic'] === false
+ ) {
+ // If non-elastic, then reduce the
number of items shown commensurately
+ $limit += $startPos;
+ }
+ $startPos = 0;
+ }
+ }
+
+ return array( $startPos, $limit );
+ }
+
+ /**
+ * @param array $rows
+ * @param $offsetKey
+ * @return int
+ * @throws DataModelException
+ */
+ protected function getOffsetFromKey( $rows, $offsetKey ) {
+ $offset = false;
+ for( $rowIndex = 0; $rowIndex < count( $rows ); ++$rowIndex ) {
+ $row = $rows[$rowIndex];
+ $comparisonValue = $this->compareRowToOffset( $row,
$offsetKey );
+ if ( $comparisonValue <= 0 ) {
+ $offset = $rowIndex;
+ break;
+ }
+ }
+
+ if ( $offset === false ) {
+ throw new DataModelException( 'Unable to find specified
offset in query results', 'process-data' );
+ }
+
+ return $offset;
+ }
+
+ /**
+ * @param $row
+ * @param $offset
+ * @return int
+ * @throws DataModelException
+ */
public function compareRowToOffset( $row, $offset ) {
$sortFields = $this->getSort();
$splitOffset = explode( '|', $offset );
@@ -1273,11 +1265,8 @@
// get cache keys for all queries
$cacheKeys = $this->getCacheKeys( $queries );
- $results = $keyToIdx = $keyToQuery = array();
+ $results = $keyToQuery = array();
foreach ( $cacheKeys as $i => $key ) {
- // allow for duplicate queries
- $keyToIdx[$key][] = $i;
-
// These results will be merged into the query results,
and as such need binary
// uuid's as would be received from storage
if ( !isset( $keyToQuery[$key] ) ) {
@@ -1285,21 +1274,65 @@
}
}
- // Retrieve from cache
- $cached = $this->cache->getMulti( $cacheKeys );
- // expand partial results and merge into result set
- foreach ( $this->rowCompactor->expandCacheResult( $cached,
$keyToQuery ) as $key => $rows ) {
- foreach ( $keyToIdx[$key] as $idx ) {
- $results[$idx] = $rows;
- unset( $queries[$idx] );
+ // retrieve from cache (only query duplicate queries once)
+ $cached = $this->cache->getMulti( array_unique( $cacheKeys ) );
+ $cached = $this->filterResults( $cached, $options );
+ $cached = $this->rowCompactor->expandCacheResult( $cached,
$keyToQuery );
+
+ foreach ( $cached as $key => $result ) {
+ $indexes = array_keys( $cacheKeys, $key );
+ foreach ( $indexes as $i ) {
+ // write to complete results array (where the
index of the
+ // result matches the index of the given
queries)
+ $results[$i] = $result;
+
+ // filter out queries that have been resolved
from cache
+ unset( $queries[$i] );
}
}
- // don't need to query backing store
- if ( count( $queries ) === 0 ) {
- return $results;
+
+ // whatever hasn't been found in cache - fetch it from storage
+ if ( $queries ) {
+ $remaining = $this->backingStoreFindMulti( $queries,
$cacheKeys );
+ $remaining = $this->filterResults( $remaining, $options
);
+
+ // keys are exactly the same as they were in $queries,
so we can +
+ // the result arrays without worrying about overwriting
stuff that
+ // has the same key
+ $results += $remaining;
}
- return $this->backingStoreFindMulti( $queries, $cacheKeys,
$results );
+ // now make sure $results are in the same order $queries was
(use
+ // $cacheKeys for references, because $queries has been cleaned
out)
+ $ordered = array();
+ foreach ( $cacheKeys as $i => $key ) {
+ if ( isset( $results[$i] ) ) {
+ $ordered[$i] = $results[$i];
+ }
+ }
+
+ return $ordered;
+ }
+
+ /**
+ * Get rid of unneeded, according to the given $options.
+ *
+ * This is used to strip entries before expanding them;
+ * basically, at that point, we may only have a list of ids, which we
need
+ * to expand (= fetch from cache) - don't want to do this for more than
+ * what is needed
+ *
+ * @param array $results
+ * @param array[optional] $options
+ * @return array
+ */
+ protected function filterResults( array $results, array $options =
array() ) {
+ foreach ( $results as $i => $result ) {
+ list( $offset, $limit ) = $this->getOffsetLimit(
$result, $options );
+ $results[$i] = array_slice( $result, $offset, $limit,
true );
+ }
+
+ return $results;
}
/**
@@ -1401,23 +1434,26 @@
return $idxToKey;
}
- protected function backingStoreFindMulti( array $queries, array
$idxToKey, array $results = array() ) {
+ protected function backingStoreFindMulti( array $queries, array
$cacheKeys ) {
// query backing store
- $stored = $this->storage->findMulti( $queries,
$this->queryOptions() );
+ $options = $this->queryOptions();
+ $stored = $this->storage->findMulti( $queries, $options );
+ $results = array();
+
// map store results to cache key
foreach ( $stored as $idx => $rows ) {
if ( !$rows ) {
// Nothing found, should we cache failures as
well as success?
continue;
}
- foreach( $rows as $row ) {
+ foreach ( $rows as $row ) {
foreach ( $row as $k => $foo ) {
if ( $foo !== null && !is_scalar( $foo
) ) {
throw new DataModelException(
"Received non-scalar row value for '$k' from: " . get_class( $this->storage ),
'process-data' );
}
}
}
- $this->cache->add( $idxToKey[$idx],
$this->rowCompactor->compactRows( $rows ) );
+ $this->cache->add( $cacheKeys[$idx],
$this->rowCompactor->compactRows( $rows ) );
$results[$idx] = $rows;
unset( $queries[$idx] );
}
diff --git a/includes/Data/RevisionStorage.php
b/includes/Data/RevisionStorage.php
index b694f51..5353290 100644
--- a/includes/Data/RevisionStorage.php
+++ b/includes/Data/RevisionStorage.php
@@ -543,7 +543,7 @@
parent::onAfterRemove( $object, $old );
}
- public function backingStoreFindMulti( array $queries, array $idxToKey,
array $retval = array() ) {
+ protected function backingStoreFindMulti( array $queries, array
$cacheKeys ) {
// all queries are for roots( guaranteed by constructor), so
anything that falls
// through and has to be queried from storage will actually
need to be doing a
// special condition either joining against flow_tree_node or
first collecting the
@@ -570,19 +570,23 @@
);
}
- $res = $this->storage->findMulti( $descendantQueries,
$this->queryOptions() );
+ $options = $this->queryOptions();
+ $res = $this->storage->findMulti( $descendantQueries, $options
);
if ( !$res ) {
return false;
}
+
+ $results = array();
+
foreach ( $res as $idx => $rows ) {
- $retval[$idx] = $rows;
- $this->cache->add( $idxToKey[$idx],
$this->rowCompactor->compactRows( $rows ) );
+ $results[$idx] = $rows;
+ $this->cache->add( $cacheKeys[$idx],
$this->rowCompactor->compactRows( $rows ) );
unset( $queries[$idx] );
}
if ( $queries ) {
// Log something about not finding everything?
}
- return $retval;
+ return $results;
}
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/112917
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1a575c723790fdfea38d5670e7e50b223cdc856c
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <mmul...@wikimedia.org>
Gerrit-Reviewer: Bsitu <bs...@wikimedia.org>
Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Matthias Mullie <mmul...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
