jenkins-bot has submitted this change and it was merged. Change subject: Stop reading from DB_MASTER ......................................................................
Stop reading from DB_MASTER The reason we read from DB_MASTER is that we persist just about all data to cache as well, and we wanted to make sure it is current (we can't backfill cache from a lagging DB slave...) The easier way to accomplish this was to remove the cache backfill. After this patch: data that falls out of cache is gone from cache forever, it won't be backfilled anymore. Long-term, we either want to reinstate backfilling cache (using WANObjectCache), or get rid of the cache layer altogether. But we'll first figure out the impact of removing the cache layer, for which this is the first step (without this, lowering cache TTL would make data fall out of cache sooner = more to backfill = more reads from DB_MASTER) Next steps: * Wait a bit & see impact of stuff that falls out of cache and doesn't get backfilled * Start lowering $wgFlowCacheTime to let data fall out of cache earlier & get even more DB queries. Keep monitoring. * Set $wgFlowUseMemcache = false; Keep monitoring. * Completely remove cache layers from codebase. Optimize queries. I also considered replacing BagOStuff::merge calls with WANObjectCache::delete. However, we rely on BufferedBagOStuff, which lets us deffer cache writes and commit them all at once. This would require a bunch of refactoring that I suggest not doing unless our cache proves effective (see above steps about how to measure impact of eliminating it) Slow queries can be monitored at: https://tendril.wikimedia.org/report/slow_queries?host=family%3Adb1029 Bug: T94028 Change-Id: I4198fef645eae73ac7d6fcf3533f4acfcbb06c54 --- M includes/Data/Index/FeatureIndex.php M includes/Data/Storage/BasicDbStorage.php M includes/Data/Storage/RevisionStorage.php M includes/Data/Storage/TopicListLastUpdatedStorage.php 4 files changed, 5 insertions(+), 33 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/Data/Index/FeatureIndex.php b/includes/Data/Index/FeatureIndex.php index 52a3600..9c02504 100644 --- a/includes/Data/Index/FeatureIndex.php +++ b/includes/Data/Index/FeatureIndex.php @@ -411,34 +411,6 @@ $fromStorage = array(); if ( $storageQueries ) { $fromStorage = $this->backingStoreFindMulti( $storageQueries ); - - // store the data we've just retrieved to cache - foreach ( $fromStorage as $index => $rows ) { - // backing store returns data that may not be valid to cache (e.g. - // if we couldn't retrieve content from ExternalStore, we shouldn't - // cache that result) - $rows = array_filter( $rows, array( $this->storage, 'validate' ) ); - if ( $rows !== $fromStorage[$index] ) { - continue; - } - - // normalize rows fetched from DB: schema may be different than the - // real data (e.g. nullable columns no longer used or in preparation - // for changes) - $rows = array_map( array( $this->mapper, 'normalizeRow' ), $rows ); - - $compacted = $this->rowCompactor->compactRows( $rows ); - $callback = function( \BagOStuff $cache, $key, $value ) use ( $compacted ) { - if ( $value !== false ) { - // somehow, the data was already cached in the meantime - return false; - } - - return $compacted; - }; - - $this->cache->merge( $cacheKeys[$index], $callback ); - } } $results = $fromStorage; diff --git a/includes/Data/Storage/BasicDbStorage.php b/includes/Data/Storage/BasicDbStorage.php index 9c6be5f..2825220 100644 --- a/includes/Data/Storage/BasicDbStorage.php +++ b/includes/Data/Storage/BasicDbStorage.php @@ -135,7 +135,7 @@ throw new \MWException( "Validation error in database options" ); } - $res = $this->dbFactory->getDB( DB_MASTER )->select( + $res = $this->dbFactory->getDB( DB_SLAVE )->select( $this->table, '*', $attributes, diff --git a/includes/Data/Storage/RevisionStorage.php b/includes/Data/Storage/RevisionStorage.php index 609ceee..9125e76 100644 --- a/includes/Data/Storage/RevisionStorage.php +++ b/includes/Data/Storage/RevisionStorage.php @@ -115,7 +115,7 @@ } protected function findInternal( array $attributes, array $options = array() ) { - $dbr = $this->dbFactory->getDB( DB_MASTER ); + $dbr = $this->dbFactory->getDB( DB_SLAVE ); if ( ! $this->validateOptions( $options ) ) { throw new MWException( "Validation error in database options" ); @@ -237,7 +237,7 @@ $duplicator->add( $query, $idx ); } - $dbr = $this->dbFactory->getDB( DB_MASTER ); + $dbr = $this->dbFactory->getDB( DB_SLAVE ); $res = $dbr->select( array( 'flow_revision' ), array( 'rev_id' => "MAX( 'rev_id' )" ), @@ -272,7 +272,7 @@ // SELECT * from flow_revision // JOIN flow_tree_revision ON tree_rev_id = rev_id // WHERE rev_id IN (...) - $dbr = $this->dbFactory->getDB( DB_MASTER ); + $dbr = $this->dbFactory->getDB( DB_SLAVE ); $tables = array( 'flow_revision' ); $joins = array(); diff --git a/includes/Data/Storage/TopicListLastUpdatedStorage.php b/includes/Data/Storage/TopicListLastUpdatedStorage.php index 51e4b03..5cca579 100644 --- a/includes/Data/Storage/TopicListLastUpdatedStorage.php +++ b/includes/Data/Storage/TopicListLastUpdatedStorage.php @@ -21,7 +21,7 @@ throw new \MWException( "Validation error in database options" ); } - $res = $this->dbFactory->getDB( DB_MASTER )->select( + $res = $this->dbFactory->getDB( DB_SLAVE )->select( array( $this->table, 'flow_workflow' ), 'topic_list_id, topic_id, workflow_last_update_timestamp', array_merge( $attributes, array( 'topic_id = workflow_id' ) ), -- To view, visit https://gerrit.wikimedia.org/r/247575 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4198fef645eae73ac7d6fcf3533f4acfcbb06c54 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: master Gerrit-Owner: Matthias Mullie <[email protected]> Gerrit-Reviewer: Catrope <[email protected]> Gerrit-Reviewer: Mattflaschen <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
