PageProps doesn't cache result when it is false
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Esanders
	Dec 8 2021, 5:09 PM

Project Tags

Referenced Files

None

Subscribers

Description

In T297132#7555212, @matmarex wrote:

I think this is actually a bug in the core PageProps class, which we are using. In PageProps::getProperties(), there's a ton of code that does caching, but only when the property is set – nothing is cached if the property we're querying for isn't present on the page. This seems unexpected to me.

Details

Related Changes in Gerrit:

Subject	Author	Repo	Branch	Lines +/-
Page: Fix absence caching when combined with multiple properties	Krinkle	mediawiki/core	wmf/1.47.0-wmf.18	+35 -4
Page: Fix absence caching when combined with multiple properties	Krinkle	mediawiki/core	master	+35 -4
Page: Clean up tests of PageProps service class	Krinkle	mediawiki/core	master	+67 -125

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Open	None	T432486 FY2026-2027 ST6.4 Logged-in user performance (tracking)
Open	Krinkle	T432488 [Hypothesis] ST6.4.2 Increase MediaWiki backend capacity for logged-in pageviews (tracking) FY2026-2027
Open	None	T302623 2022-2026: Improve Backend Pageview Timing
		Restricted Task
Open	None	T347123 Reduce database queries on parsercached logged-out page views (Sep 2023)
Resolved	Krinkle	T297300 PageProps doesn't cache result when it is false

Event Timeline

Esanders created this task.Dec 8 2021, 5:09 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 8 2021, 5:09 PM

When this is done we should revert https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/744889 (or at lest remove the caching part of it, and keep the utility function).

LSobanski subscribed.Dec 13 2021, 9:57 AM

• matmarex mentioned this in T347123: Reduce database queries on parsercached logged-out page views (Sep 2023).Sep 23 2023, 4:01 AM

Krinkle added a project: Wikimedia-Performance-recommendation.Sep 23 2023, 11:47 PM

Pppery edited projects, added: MediaWiki-Page-derived-data; removed: MediaWiki-General.Aug 5 2024, 1:05 AM

Fixed in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1256284 + https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/1257710 (T347123).

Krinkle added parent tasks: T432488: [Hypothesis] ST6.4.2 Increase MediaWiki backend capacity for logged-in pageviews (tracking) FY2026-2027, T347123: Reduce database queries on parsercached logged-out page views (Sep 2023).Jul 27 2026, 12:57 PM

Krinkle mentioned this in T432488: [Hypothesis] ST6.4.2 Increase MediaWiki backend capacity for logged-in pageviews (tracking) FY2026-2027.Aug 13 2026, 5:45 AM

In T347123#12229338, @Krinkle wrote:

@Krinkle wrote in the task description in September 2023:

Capturing via verbose Logstash […] yields 19 queries, roughly in order: […]

[…] the same scenario now has returns 23 queries […]

MediaWiki\Page\PageProps::getProperties [0.638ms] [0 rows] db1169: SELECT pp_page,pp_propname,pp_value FROM page_props WHERE pp_page = 54249587 AND pp_propname IN ('newsectionlink','nonewsectionlink','notalk','archivedtalk')

MediaWiki\Page\PageProps::getProperties [0.543ms] [0 rows] db1169: SELECT pp_page,pp_propname,pp_value FROM page_props WHERE pp_page = 54249587 AND pp_propname IN ('newsectionlink','nonewsectionlink','notalk','archivedtalk')

MediaWiki\Page\PageProps::getProperties [0.81ms] [0 rows] db1169: SELECT pp_page,pp_propname,pp_value FROM page_props WHERE pp_page = 54249587 AND pp_propname IN ('newsectionlink','nonewsectionlink','notalk','archivedtalk')

MediaWiki\Page\PageProps::getProperties [0.581ms] [0 rows] db1169: SELECT pp_page,pp_propname,pp_value FROM page_props WHERE pp_page = 54249587 AND pp_propname IN ('newsectionlink','nonewsectionlink','notalk','archivedtalk')

What: DiscussionTools extension, determining whether this is a talk page, via isAvailableForTitle() from DiscussionTools\PageHooks::onOutputPageBeforeHTML.

Unexpected and regressed. Pre-existing and already analyzed in-depth above. This seems avoidable because ParserCache provides this information for free without any DB queries. This information can't change between edits. One query would be an improvement though, it currently makes 4! This was supposed to be fixed as part of T297300: PageProps doesn't cache result when it is false. It's now worse than before. It used to make 3 queries (one prop each), now it makes 4 queries (each for the same 4 properties).