Page MenuHomePhabricator
Create Task
Maniphest T283280

Special:SecurePoll makes primary database connections on GET requests when getting UI messages or checking for election admins
Open, Needs TriagePublic
Actions

Description

This is causing warnings about inappropriate primary DB connections on GET requests (about 1000 in the last 24 hours, at the time of writing).

UI messages

Typical stack trace:

from /srv/mediawiki/php-1.37.0-wmf.6/includes/libs/rdbms/TransactionProfiler.php(444)
#0 /srv/mediawiki/php-1.37.0-wmf.6/includes/libs/rdbms/TransactionProfiler.php(224): Wikimedia\Rdbms\TransactionProfiler->reportExpectationViolated(string, string, integer)
#1 /srv/mediawiki/php-1.37.0-wmf.6/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1010): Wikimedia\Rdbms\TransactionProfiler->recordConnection(string, string, boolean)
#2 /srv/mediawiki/php-1.37.0-wmf.6/includes/libs/rdbms/loadbalancer/LoadBalancer.php(964): Wikimedia\Rdbms\LoadBalancer->getServerConnection(integer, string, integer)
#3 /srv/mediawiki/php-1.37.0-wmf.6/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1130): Wikimedia\Rdbms\LoadBalancer->getConnection(integer, array, string, integer)
#4 /srv/mediawiki/php-1.37.0-wmf.6/includes/GlobalFunctions.php(2307): Wikimedia\Rdbms\LoadBalancer->getMaintenanceConnectionRef(integer, array, string)
#5 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/DBStore.php(135): wfGetDB(integer)
#6 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/DBStore.php(139): MediaWiki\Extensions\SecurePoll\DBStore->getDB()
#7 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/Entities/Election.php(404): MediaWiki\Extensions\SecurePoll\DBStore->getQuestionInfo(string)
#8 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/Entities/Election.php(166): MediaWiki\Extensions\SecurePoll\Entities\Election->getQuestions()
#9 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/Entities/Entity.php(100): MediaWiki\Extensions\SecurePoll\Entities\Election->getChildren()
#10 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/Entities/Entity.php(119): MediaWiki\Extensions\SecurePoll\Entities\Entity->getDescendants()
#11 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/Entities/Entity.php(167): MediaWiki\Extensions\SecurePoll\Entities\Entity->loadMessages(string)
#12 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/Pages/ListPage.php(43): MediaWiki\Extensions\SecurePoll\Entities\Entity->getMessage(string)
#13 /srv/mediawiki/php-1.37.0-wmf.6/extensions/SecurePoll/includes/SpecialSecurePoll.php(70): MediaWiki\Extensions\SecurePoll\Pages\ListPage->execute(array)
#14 /srv/mediawiki/php-1.37.0-wmf.6/includes/specialpage/SpecialPage.php(646): MediaWiki\Extensions\SecurePoll\SpecialSecurePoll->execute(string)
#15 /srv/mediawiki/php-1.37.0-wmf.6/includes/specialpage/SpecialPageFactory.php(1390): SpecialPage->run(string)
#16 /srv/mediawiki/php-1.37.0-wmf.6/includes/MediaWiki.php(314): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, RequestContext)
#17 /srv/mediawiki/php-1.37.0-wmf.6/includes/MediaWiki.php(917): MediaWiki->performRequest()
#18 /srv/mediawiki/php-1.37.0-wmf.6/includes/MediaWiki.php(551): MediaWiki->main()
#19 /srv/mediawiki/php-1.37.0-wmf.6/index.php(53): MediaWiki->run()
#20 /srv/mediawiki/php-1.37.0-wmf.6/index.php(46): wfIndexMain()
#21 /srv/mediawiki/w/index.php(3): require(string)
#22 {main}

Entity::getMessage calls DBStore::getQuestionInfo, which always queries the primary database:

public function getDB( $index = DB_PRIMARY ) {
	return wfGetDB( $this->forceMaster ? DB_PRIMARY : $index );
}

public function getQuestionInfo( $electionId ) {
	$db = $this->getDB();
	$res = $db->select(
		// etc.
	);
	// etc.
}

Entity::getMessage is used by most subpages of Special:SecurePoll, e.g. to display an election's title. In these cases, it should query the replicas.

Checking for election admins

Typical stack trace:

from /srv/mediawiki/php-1.37.0-wmf.5/includes/libs/rdbms/TransactionProfiler.php(432)
#0 /srv/mediawiki/php-1.37.0-wmf.5/includes/libs/rdbms/TransactionProfiler.php(212): Wikimedia\Rdbms\TransactionProfiler->reportExpectationViolated(string, string, integer)
#1 /srv/mediawiki/php-1.37.0-wmf.5/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1010): Wikimedia\Rdbms\TransactionProfiler->recordConnection(string, string, boolean)
#2 /srv/mediawiki/php-1.37.0-wmf.5/includes/libs/rdbms/loadbalancer/LoadBalancer.php(964): Wikimedia\Rdbms\LoadBalancer->getServerConnection(integer, string, integer)
#3 /srv/mediawiki/php-1.37.0-wmf.5/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1130): Wikimedia\Rdbms\LoadBalancer->getConnection(integer, array, string, integer)
#4 /srv/mediawiki/php-1.37.0-wmf.5/includes/GlobalFunctions.php(2307): Wikimedia\Rdbms\LoadBalancer->getMaintenanceConnectionRef(integer, array, string)
#5 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/DBStore.php(135): wfGetDB(integer)
#6 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/DBStore.php(60): MediaWiki\Extensions\SecurePoll\DBStore->getDB()
#7 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/Entities/Entity.php(132): MediaWiki\Extensions\SecurePoll\DBStore->getProperties(array)
#8 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/Entities/Entity.php(238): MediaWiki\Extensions\SecurePoll\Entities\Entity->loadProperties()
#9 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/Entities/Election.php(361): MediaWiki\Extensions\SecurePoll\Entities\Entity->getProperty(string)
#10 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/Pages/ElectionPager.php(127): MediaWiki\Extensions\SecurePoll\Entities\Election->isAdmin(User)
#11 /srv/mediawiki/php-1.37.0-wmf.5/includes/pager/IndexPager.php(611): MediaWiki\Extensions\SecurePoll\Pages\ElectionPager->formatRow(stdClass)
#12 /srv/mediawiki/php-1.37.0-wmf.5/includes/pager/TablePager.php(94): IndexPager->getBody()
#13 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/Pages/EntryPage.php(38): TablePager->getBodyOutput()
#14 /srv/mediawiki/php-1.37.0-wmf.5/extensions/SecurePoll/includes/SpecialSecurePoll.php(70): MediaWiki\Extensions\SecurePoll\Pages\EntryPage->execute(array)
#15 /srv/mediawiki/php-1.37.0-wmf.5/includes/specialpage/SpecialPage.php(646): MediaWiki\Extensions\SecurePoll\SpecialSecurePoll->execute(string)
#16 /srv/mediawiki/php-1.37.0-wmf.5/includes/specialpage/SpecialPageFactory.php(1396): SpecialPage->run(NULL)
#17 /srv/mediawiki/php-1.37.0-wmf.5/includes/MediaWiki.php(313): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, RequestContext)
#18 /srv/mediawiki/php-1.37.0-wmf.5/includes/MediaWiki.php(916): MediaWiki->performRequest()
#19 /srv/mediawiki/php-1.37.0-wmf.5/includes/MediaWiki.php(550): MediaWiki->main()
#20 /srv/mediawiki/php-1.37.0-wmf.5/index.php(53): MediaWiki->run()
#21 /srv/mediawiki/php-1.37.0-wmf.5/index.php(46): wfIndexMain()
#22 /srv/mediawiki/w/index.php(3): require(string)
#23 {main}

Election::isAdmin calls DBStore::getProperties, which always queries the primary database:

public function getProperties( $ids ) {
	// See DBStore::getDB above
	$db = $this->getDB();
	$res = $db->select(
		// etc.
	);
	//etc.
}

Election::isAdmin is used for permissions checks when displaying forms and links. In these cases, it should query the replicas.

Possible solutions

We might need to allow passing which database to query when calling getQuestionInfo and getProperties.

Interestingly, a similar method getElectionInfo was changed to always query the replicas in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/332952. (...Though it seems unlikely we could do this with getProperties, since admin checks may be performed when admins are actually trying to perform an action.)

Details

SubjectRepoBranchLines +/-
DBStore: Send reads to DB_REPLICA by defaultmediawiki/extensions/SecurePollmaster+6 -6
Use a replica connection in DBStore::getQuestionInfo()mediawiki/extensions/SecurePollmaster+1 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

Tchanders created this task.May 20 2021, 7:15 PM
Restricted Application added a project: Platform Engineering. · View Herald TranscriptMay 20 2021, 7:15 PM
Tchanders moved this task from Untriaged to Production-error on the Anti-Harassment board.May 23 2021, 4:10 PM
• AMooney removed a project: Platform Engineering.May 25 2021, 3:08 PM
Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.Jun 28 2021, 6:32 PM
gerritbot added a comment.Aug 2 2021, 8:42 PM

Change 709533 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/extensions/SecurePoll@master] Use a replica connection in DBStore::getQuestionInfo()

https://gerrit.wikimedia.org/r/709533

gerritbot added a project: Patch-For-Review.Aug 2 2021, 8:42 PM
Zabe subscribed.Aug 2 2021, 8:55 PM
gerritbot added a comment.Aug 2 2021, 9:56 PM

Change 709533 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Use a replica connection in DBStore::getQuestionInfo()

https://gerrit.wikimedia.org/r/709533

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.17; 2021-08-02).Aug 2 2021, 10:00 PM
Maintenance_bot removed a project: Patch-For-Review.Aug 2 2021, 10:10 PM
• Niharika edited projects, added Anti-Harassment (The Letter Song); removed Anti-Harassment.Aug 3 2021, 12:13 AM
STran moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to QA/Testing 🐞 on the Anti-Harassment (The Letter Song) board.Aug 4 2021, 7:04 PM
dom_walden subscribed.Aug 10 2021, 2:34 PM

@Zabe Further to your comment https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/709533/1#message-dd40e9955064e53f6d6eaaea03533aaebed5d6e3, will this patch actually change anything? Are you planning a follow up patch? Thanks.

ARamirez_WMF moved this task from QA/Testing 🐞 to Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) on the Anti-Harassment (The Letter Song) board.Aug 11 2021, 4:10 PM
Zabe added a comment.Aug 12 2021, 12:39 PM
In T283280#7272252, @dom_walden wrote:

@Zabe Further to your comment https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/709533/1#message-dd40e9955064e53f6d6eaaea03533aaebed5d6e3, will this patch actually change anything? Are you planning a follow up patch? Thanks.

I am not very familiar with the extension and it was set to force a primary connection in T209804, so we need to make sure it doesn't break again. Maybe if I find some time I can take another look.

Reedy subscribed.Aug 12 2021, 12:59 PM
In T283280#7278691, @Zabe wrote:
In T283280#7272252, @dom_walden wrote:

@Zabe Further to your comment https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/709533/1#message-dd40e9955064e53f6d6eaaea03533aaebed5d6e3, will this patch actually change anything? Are you planning a follow up patch? Thanks.

I am not very familiar with the extension and it was set to force a primary connection in T209804, so we need to make sure it doesn't break again. Maybe if I find some time I can take another look.

DBStore is actually intantiated elsewhere in a slightly abstracted way. So when called that way, (and hasn't been through recordElectionToNamespace before in the same session), that will result in it being implicitly false (from Context.php):

	/**
	 * Get the Store instance
	 * @return Store
	 */
	public function getStore() {
		if ( !isset( $this->store ) ) {
			$this->store = new $this->storeClass;
		}

		return $this->store;
	}

So, in theory, you patch should have actually helped in at least some situations. Would need the logs to be checked since .17 to see if/how much it has helped.

Zabe added a comment.Aug 12 2021, 4:46 PM
In T283280#7278729, @Reedy wrote:
In T283280#7278691, @Zabe wrote:
In T283280#7272252, @dom_walden wrote:

@Zabe Further to your comment https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/709533/1#message-dd40e9955064e53f6d6eaaea03533aaebed5d6e3, will this patch actually change anything? Are you planning a follow up patch? Thanks.

I am not very familiar with the extension and it was set to force a primary connection in T209804, so we need to make sure it doesn't break again. Maybe if I find some time I can take another look.

DBStore is actually intantiated elsewhere in a slightly abstracted way. So when called that way, (and hasn't been through recordElectionToNamespace before in the same session), that will result in it being implicitly false (from Context.php):

	/**
	 * Get the Store instance
	 * @return Store
	 */
	public function getStore() {
		if ( !isset( $this->store ) ) {
			$this->store = new $this->storeClass;
		}

		return $this->store;
	}

So, in theory, you patch should have actually helped in at least some situations. Would need the logs to be checked since .17 to see if/how much it has helped.

Thanks, I hadn't spotted that instantiation.

ARamirez_WMF moved this task from The Letter Song to Cards ready for development on the Anti-Harassment board.Aug 12 2021, 5:17 PM
ARamirez_WMF edited projects, added Anti-Harassment; removed Anti-Harassment (The Letter Song).
gerritbot added a comment.Aug 12 2021, 9:10 PM

Change 712605 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/SecurePoll@master] DBStore: Send reads to DB_REPLICA by default

https://gerrit.wikimedia.org/r/712605

gerritbot added a project: Patch-For-Review.Aug 12 2021, 9:10 PM
Reedy mentioned this in T288783: SecurePoll doing DB writes to securepoll_log on HTTP GET [M].Aug 12 2021, 9:16 PM
Reedy mentioned this in T288784: SecurePoll doing DB writes to securepoll_voters on HTTP GET [Timebox: 8h].Aug 12 2021, 9:20 PM

^ That sends all the reads done inside DBStore to DB_REPLICA unless the force primary override is in place.

https://logstash.wikimedia.org/goto/de2b2fa834a039b1ad27833c89262fe2

Screenshot 2021-08-12 at 22.11.30.png (908×1 px, 184 KB)

gerritbot added a comment.Aug 16 2021, 11:21 PM

Change 712605 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] DBStore: Send reads to DB_REPLICA by default

https://gerrit.wikimedia.org/r/712605

ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.19; 2021-08-16); removed MW-1.37-notes (1.37.0-wmf.17; 2021-08-02).Aug 17 2021, 12:00 AM
Maintenance_bot removed a project: Patch-For-Review.Aug 17 2021, 12:10 AM
• Niharika subscribed.Aug 23 2021, 9:53 PM

Is this task good to close?

Reedy added a comment.Aug 24 2021, 2:19 PM
In T283280#7280288, @Reedy wrote:

^ That sends all the reads done inside DBStore to DB_REPLICA unless the force primary override is in place.

https://logstash.wikimedia.org/goto/de2b2fa834a039b1ad27833c89262fe2

Screenshot 2021-08-12 at 22.11.30.png (908×1 px, 184 KB)

^ Using that link, there's definitely more entries, but that's expected as it's being used more.

Most of the log entries now seem to be T288784: SecurePoll doing DB writes to securepoll_voters on HTTP GET [Timebox: 8h]

Screenshot 2021-08-24 at 15.16.06.png (854×1 px, 166 KB)

So... I think the two patches here have definitely helped. And the two forked tasks (T288783: SecurePoll doing DB writes to securepoll_log on HTTP GET [M] being the other) cover the remaining related cases

ARamirez_WMF moved this task to Product/Tech backlog on the Anti-Harassment board.Aug 26 2021, 6:30 PM
Krinkle removed a project: MediaWiki-libs-Rdbms.Oct 16 2021, 1:35 AM
ARamirez_WMF subscribed.Oct 27 2021, 6:01 PM

In the past, AHT stepped in to support Trust & Safety with the time-sensitive matter of the Board Elections by providing help with SecurePoll. Unfortunately, at this time we can no longer support nor maintain SecurePoll. Per the Foundation leadership’s instructions, AHT is dedicating all of our time and energy to other critical efforts.

ARamirez_WMF removed a project: Anti-Harassment.Oct 27 2021, 6:02 PM
Krinkle edited projects, added Sustainability (MediaWiki-MultiDC); removed MW-1.37-notes (1.37.0-wmf.19; 2021-08-16).May 13 2022, 7:43 PM
Krinkle moved this task from Later to Current: Other on the Sustainability (MediaWiki-MultiDC) board.
Krinkle edited projects, added Wikimedia-Performance-recommendation; removed Performance-Team (Radar).Aug 18 2023, 8:41 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL