[MediaWiki-CodeReview] [MediaWiki r79518]: New comment added
User Brion VIBBER posted a comment on MediaWiki.r79518. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/79518#c19558 Commit summary: * Modified Article::loadPageData() to use a slave database connection and pageDataFromTitle() instead of pageDataFromId() in the default case, as in Wiki.php (this also saves a query since the ID will be fetched with other fileds) * Removed the loadPageData() call for the initial article in Wiki.php, will be triggered by the isRedirect() call 7 lines below if needed (this was not needed if $target is set by the InitializeArticleMaybeRedirect hook), but kept the second one (same as above, Article::exists() triggers Title::getArticleId() that would use one query to get id and a second one is needed to get the complete page data) * Modified Article::fetchContent() to use common code (loadPageData()) and to only call it if really needed Comment: I'm pretty sure we've gone back and forth on this sort of thing several times over the years... Reading page data from the slave is dangerous as it may be out of date. Reading page data from the master is potentially slow as it slams the master with more activity. In theory what we're supposed to be doing I think is something like 'grab the most basic page/revision row from master' for reliability, followed by being able to load the page text and do rendering existence checks etc etc on the slave. ___ MediaWiki-CodeReview mailing list mediawiki-coderev...@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
[MediaWiki-CodeReview] [MediaWiki r79518]: New comment added
User Aaron Schulz posted a comment on MediaWiki.r79518. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/79518#c19559 Commit summary: * Modified Article::loadPageData() to use a slave database connection and pageDataFromTitle() instead of pageDataFromId() in the default case, as in Wiki.php (this also saves a query since the ID will be fetched with other fileds) * Removed the loadPageData() call for the initial article in Wiki.php, will be triggered by the isRedirect() call 7 lines below if needed (this was not needed if $target is set by the InitializeArticleMaybeRedirect hook), but kept the second one (same as above, Article::exists() triggers Title::getArticleId() that would use one query to get id and a second one is needed to get the complete page data) * Modified Article::fetchContent() to use common code (loadPageData()) and to only call it if really needed Comment: With chronology protection and the session key having the db position, it normally should be fine to use a slave. I suppose if the slave lag gets really bad (as it does on occasion) you want to fall back to the master. It might be nice to have some kind cache key or something that's set if the lag gets really bad; when the key isn't set, the lag is checked every other X hits. I can't see why we have to use the master for a simple page view. Also, I'd like it if the functions let you choose master or slave. ___ MediaWiki-CodeReview mailing list mediawiki-coderev...@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
[MediaWiki-CodeReview] [MediaWiki r79518]: New comment added
User Aaron Schulz posted a comment on MediaWiki.r79518. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/79518#c18072 Commit summary: * Modified Article::loadPageData() to use a slave database connection and pageDataFromTitle() instead of pageDataFromId() in the default case, as in Wiki.php (this also saves a query since the ID will be fetched with other fileds) * Removed the loadPageData() call for the initial article in Wiki.php, will be triggered by the isRedirect() call 7 lines below if needed (this was not needed if $target is set by the InitializeArticleMaybeRedirect hook), but kept the second one (same as above, Article::exists() triggers Title::getArticleId() that would use one query to get id and a second one is needed to get the complete page data) * Modified Article::fetchContent() to use common code (loadPageData()) and to only call it if really needed Comment: I like the idea here, but I worry if something calls an accessor of Article that needs DB_MASTER data. ___ MediaWiki-CodeReview mailing list mediawiki-coderev...@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
